closes #375: M1353187 (partial)

This commit is contained in:
Cameron Kaiser 2019-11-18 21:51:47 -08:00
parent 916d37b9ae
commit 45c95c3dad
11 changed files with 128 additions and 42 deletions

View File

@ -19,6 +19,7 @@ FramePropertyTable::Set(nsIFrame* aFrame, const FramePropertyDescriptor* aProper
if (mLastFrame != aFrame || !mLastEntry) {
mLastFrame = aFrame;
mLastEntry = mEntries.PutEntry(aFrame);
aFrame->AddStateBits(NS_FRAME_HAS_PROPERTIES);
}
Entry* entry = mLastEntry;
@ -61,6 +62,7 @@ FramePropertyTable::Set(nsIFrame* aFrame, const FramePropertyDescriptor* aProper
void*
FramePropertyTable::Get(const nsIFrame* aFrame,
const FramePropertyDescriptor* aProperty,
bool aSkipBitCheck,
bool* aFoundResult)
{
NS_ASSERTION(aFrame, "Null frame?");
@ -70,11 +72,19 @@ FramePropertyTable::Get(const nsIFrame* aFrame,
*aFoundResult = false;
}
if (!aSkipBitCheck && !(aFrame->GetStateBits() & NS_FRAME_HAS_PROPERTIES)) {
return nullptr;
}
if (mLastFrame != aFrame) {
mLastFrame = const_cast<nsIFrame*>(aFrame);
mLastEntry = mEntries.GetEntry(mLastFrame);
}
Entry* entry = mLastEntry;
// If this assertion fires, see bug 1353187 (TenFourFox issue 375).
MOZ_ASSERT(entry || aSkipBitCheck,
"NS_FRAME_HAS_PROPERTIES bit should match whether entry exists");
if (!entry)
return nullptr;
@ -103,7 +113,9 @@ FramePropertyTable::Get(const nsIFrame* aFrame,
}
void*
FramePropertyTable::Remove(nsIFrame* aFrame, const FramePropertyDescriptor* aProperty,
FramePropertyTable::Remove(nsIFrame* aFrame,
const FramePropertyDescriptor* aProperty,
bool aSkipBitCheck,
bool* aFoundResult)
{
NS_ASSERTION(aFrame, "Null frame?");
@ -113,11 +125,19 @@ FramePropertyTable::Remove(nsIFrame* aFrame, const FramePropertyDescriptor* aPro
*aFoundResult = false;
}
if (!aSkipBitCheck && !(aFrame->GetStateBits() & NS_FRAME_HAS_PROPERTIES)) {
return nullptr;
}
if (mLastFrame != aFrame) {
mLastFrame = aFrame;
mLastEntry = mEntries.GetEntry(aFrame);
}
Entry* entry = mLastEntry;
// If this assertion fires, see bug 1353187 (TenFourFox issue 375).
MOZ_ASSERT(entry || aSkipBitCheck,
"NS_FRAME_HAS_PROPERTIES bit should match whether entry exists");
if (!entry)
return nullptr;
@ -125,6 +145,7 @@ FramePropertyTable::Remove(nsIFrame* aFrame, const FramePropertyDescriptor* aPro
// There's only one entry and it's the one we want
void* value = entry->mProp.mValue;
mEntries.RawRemoveEntry(entry);
aFrame->RemoveStateBits(NS_FRAME_HAS_PROPERTIES);
mLastEntry = nullptr;
if (aFoundResult) {
*aFoundResult = true;
@ -164,13 +185,15 @@ FramePropertyTable::Remove(nsIFrame* aFrame, const FramePropertyDescriptor* aPro
}
void
FramePropertyTable::Delete(nsIFrame* aFrame, const FramePropertyDescriptor* aProperty)
FramePropertyTable::Delete(nsIFrame* aFrame,
const FramePropertyDescriptor* aProperty,
bool aSkipBitCheck)
{
NS_ASSERTION(aFrame, "Null frame?");
NS_ASSERTION(aProperty, "Null property?");
bool found;
void* v = Remove(aFrame, aProperty, &found);
void* v = Remove(aFrame, aProperty, aSkipBitCheck, &found);
if (found) {
PropertyValue pv(aProperty, v);
pv.DestroyValueFor(aFrame);
@ -197,7 +220,14 @@ FramePropertyTable::DeleteAllFor(nsIFrame* aFrame)
{
NS_ASSERTION(aFrame, "Null frame?");
if (!(aFrame->GetStateBits() & NS_FRAME_HAS_PROPERTIES)) {
return;
}
Entry* entry = mEntries.GetEntry(aFrame);
// If this assertion fires, see bug 1353187 (TenFourFox issue 375).
MOZ_ASSERT(entry,
"NS_FRAME_HAS_PROPERTIES bit should match whether entry exists");
if (!entry)
return;
@ -210,6 +240,8 @@ FramePropertyTable::DeleteAllFor(nsIFrame* aFrame)
DeleteAllForEntry(entry);
mEntries.RawRemoveEntry(entry);
// Don't bother unsetting NS_FRAME_HAS_PROPERTIES, since aFrame is going away
}
void

View File

@ -92,6 +92,7 @@ public:
* 'property value is null'.
*/
void* Get(const nsIFrame* aFrame, const FramePropertyDescriptor* aProperty,
bool aSkipBitCheck,
bool* aFoundResult = nullptr);
/**
* Remove a property value for a frame. This requires one hashtable
@ -105,14 +106,30 @@ public:
* 'property value is null'.
*/
void* Remove(nsIFrame* aFrame, const FramePropertyDescriptor* aProperty,
bool aSkipBitCheck,
bool* aFoundResult = nullptr);
/**
* Remove and destroy a property value for a frame. This requires one
* hashtable lookup (using the frame as the key) and a linear search
* through the properties of that frame. If the frame has no such
* property, nothing happens.
*
* The DeleteSkippingBitCheck variant doesn't test
* NS_FRAME_HAS_PROPERTIES on aFrame, so it is safe to call after
* aFrame has been destroyed as long as, since that destruction
* happened, it isn't possible for a new frame to have been created
* and the same property added.
*/
void Delete(nsIFrame* aFrame, const FramePropertyDescriptor* aProperty);
void Delete(nsIFrame* aFrame, const FramePropertyDescriptor* aProperty,
bool aSkipBitCheck);
// TenFourFox issue 375, from M1353187.
void DeleteSkippingBitCheck(nsIFrame* aFrame,
const FramePropertyDescriptor* aProperty)
{
Delete(aFrame, aProperty, true /* aSkipBitCheck */);
}
/**
* Remove and destroy all property values for a frame. This requires one
* hashtable lookup (using the frame as the key).
@ -129,7 +146,16 @@ public:
bool Has(const nsIFrame* aFrame, const FramePropertyDescriptor* aProperty)
{
bool foundResult = false;
(void)Get(aFrame, aProperty, &foundResult);
(void)Get(aFrame, aProperty, false /* aSkipBitCheck */, &foundResult);
return foundResult;
}
// TenFourFox issue 375, from M1353187.
bool HasSkippingBitCheck(const nsIFrame* aFrame,
const FramePropertyDescriptor* aProperty)
{
bool foundResult = false;
(void)Get(aFrame, aProperty, true /* aSkipBitCheck */, &foundResult);
return foundResult;
}
@ -237,22 +263,22 @@ public:
void* Get(const FramePropertyDescriptor* aProperty,
bool* aFoundResult = nullptr) const
{
return mTable->Get(mFrame, aProperty, aFoundResult);
return mTable->Get(mFrame, aProperty, false /* aSkipBitCheck */, aFoundResult);
}
void* Remove(const FramePropertyDescriptor* aProperty,
bool* aFoundResult = nullptr) const
{
return mTable->Remove(mFrame, aProperty, aFoundResult);
return mTable->Remove(mFrame, aProperty, false /* aSkipBitCheck */, aFoundResult);
}
void Delete(const FramePropertyDescriptor* aProperty)
{
mTable->Delete(mFrame, aProperty);
mTable->Delete(mFrame, aProperty, false /* aSkipBitCheck */);
}
// TenFourFox issue 493
bool Has(const FramePropertyDescriptor* aProperty)
{
bool foundResult;
(void)mTable->Get(mFrame, aProperty, &foundResult);
(void)mTable->Get(mFrame, aProperty, false /* aSkipBitCheck */, &foundResult);
return foundResult;
}

View File

@ -130,7 +130,7 @@ GetNextBlockInInlineSibling(FramePropertyTable* aPropTable, nsIFrame* aFrame)
}
return static_cast<nsIFrame*>
(aPropTable->Get(aFrame, nsIFrame::IBSplitSibling()));
(aPropTable->Get(aFrame, nsIFrame::IBSplitSibling(), false /* aSkipBitCheck */));
}
static nsIFrame*
@ -757,7 +757,13 @@ RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList)
"Reflow hint bits set without actually asking for a reflow");
// skip any frame that has been destroyed due to a ripple effect
if (frame && !propTable->Get(frame, ChangeListProperty())) {
if (frame && !propTable->HasSkippingBitCheck(frame, ChangeListProperty())) {
// Null out the pointer since the frame was already destroyed.
// This is important so we don't try to delete its
// ChangeListProperty() below. (marked as YYYY)
nsStyleChangeData* changeData;
aChangeList.ChangeAt(index, &changeData);
changeData->mFrame = nullptr;
continue;
}
@ -806,6 +812,13 @@ RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList)
}
}
if (hint & nsChangeHint_ReconstructFrame) {
// We're about to destroy data.mFrame, so null out the pointer.
// This is important so we don't try to delete its
// ChangeListProperty() below. (marked as YYYY)
nsStyleChangeData* changeData;
aChangeList.ChangeAt(index, &changeData);
changeData->mFrame = nullptr;
// If we ever start passing true here, be careful of restyles
// that involve a reframe and animations. In particular, if the
// restyle we're processing here is an animation restyle, but
@ -980,8 +993,8 @@ RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList)
while (0 <= --index) {
const nsStyleChangeData* changeData;
aChangeList.ChangeAt(index, &changeData);
if (changeData->mFrame) {
propTable->Delete(changeData->mFrame, ChangeListProperty());
if (changeData->mFrame) { // YYYY see above
propTable->DeleteSkippingBitCheck(changeData->mFrame, ChangeListProperty());
}
#ifdef DEBUG

View File

@ -822,7 +822,8 @@ void nsDisplayListBuilder::MarkOutOfFlowFrameForDisplay(nsIFrame* aDirtyFrame,
static void UnmarkFrameForDisplay(nsIFrame* aFrame) {
nsPresContext* presContext = aFrame->PresContext();
presContext->PropertyTable()->
Delete(aFrame, nsDisplayListBuilder::OutOfFlowDisplayDataProperty());
Delete(aFrame, nsDisplayListBuilder::OutOfFlowDisplayDataProperty(),
false /* aSkipBitCheck */);
for (nsIFrame* f = aFrame; f;
f = nsLayoutUtils::GetParentOrPlaceholderFor(f)) {

View File

@ -1995,7 +1995,8 @@ PresShell::NotifyDestroyingFrame(nsIFrame* aFrame)
// the DisplayItemData destructor will use the destroyed frame when it
// tries to remove it from the (array) value of this property.
mPresContext->PropertyTable()->
Delete(aFrame, FrameLayerBuilder::LayerManagerDataProperty());
Delete(aFrame, FrameLayerBuilder::LayerManagerDataProperty(),
false /* aSkipBitCheck */);
}
}

View File

@ -169,12 +169,12 @@ nsContainerFrame::SafelyDestroyFrameListProp(nsIFrame* aDestructRoot,
// delete the property -- that's why we fetch the property again before
// removing each frame rather than fetching it once and iterating the list.
while (nsFrameList* frameList =
static_cast<nsFrameList*>(aPropTable->Get(this, aProp))) {
static_cast<nsFrameList*>(aPropTable->Get(this, aProp, false /* aSkipBitCheck */))) {
nsIFrame* frame = frameList->RemoveFirstChild();
if (MOZ_LIKELY(frame)) {
frame->DestroyFrom(aDestructRoot);
} else {
aPropTable->Remove(this, aProp);
aPropTable->Remove(this, aProp, false /* aSkipBitCheck */);
frameList->Delete(aPresShell);
return;
}
@ -201,8 +201,8 @@ nsContainerFrame::DestroyFrom(nsIFrame* aDestructRoot)
SafelyDestroyFrameListProp(aDestructRoot, shell, props, OverflowProperty());
MOZ_ASSERT(IsFrameOfType(nsIFrame::eCanContainOverflowContainers) ||
!(props->Get(this, nsContainerFrame::OverflowContainersProperty()) ||
props->Get(this, nsContainerFrame::ExcessOverflowContainersProperty())),
!(props->Get(this, nsContainerFrame::OverflowContainersProperty(), false /* aSkipBitCheck */) ||
props->Get(this, nsContainerFrame::ExcessOverflowContainersProperty(), false /* aSkipBitCheck */)),
"this type of frame should't have overflow containers");
SafelyDestroyFrameListProp(aDestructRoot, shell, props,
@ -248,7 +248,7 @@ static void AppendIfNonempty(const nsIFrame* aFrame,
nsIFrame::ChildListID aListID)
{
nsFrameList* list = static_cast<nsFrameList*>(
aPropTable->Get(aFrame, aProperty));
aPropTable->Get(aFrame, aProperty, false /* aSkipBitCheck */));
if (list) {
list->AppendIfNonempty(aLists, aListID);
}
@ -1337,11 +1337,11 @@ static bool
TryRemoveFrame(nsIFrame* aFrame, FramePropertyTable* aPropTable,
const FramePropertyDescriptor* aProp, nsIFrame* aChildToRemove)
{
nsFrameList* list = static_cast<nsFrameList*>(aPropTable->Get(aFrame, aProp));
nsFrameList* list = static_cast<nsFrameList*>(aPropTable->Get(aFrame, aProp, false /* aSkipBitCheck */));
if (list && list->StartRemoveFrame(aChildToRemove)) {
// aChildToRemove *may* have been removed from this list.
if (list->IsEmpty()) {
aPropTable->Remove(aFrame, aProp);
aPropTable->Remove(aFrame, aProp, false /* aSkipBitCheck */);
list->Delete(aFrame->PresContext()->PresShell());
}
return true;
@ -1359,10 +1359,10 @@ nsContainerFrame::StealFrame(nsIFrame* aChild,
if (!list || !list->ContainsFrame(aChild)) {
FramePropertyTable* propTable = PresContext()->PropertyTable();
list = static_cast<nsFrameList*>(
propTable->Get(this, OverflowContainersProperty()));
propTable->Get(this, OverflowContainersProperty(), false /* aSkipBitCheck */));
if (!list || !list->ContainsFrame(aChild)) {
list = static_cast<nsFrameList*>(
propTable->Get(this, ExcessOverflowContainersProperty()));
propTable->Get(this, ExcessOverflowContainersProperty(), false /* aSkipBitCheck */));
MOZ_ASSERT(list && list->ContainsFrame(aChild), "aChild isn't our child"
" or on a frame list not supported by StealFrame");
}
@ -1534,14 +1534,14 @@ nsFrameList*
nsContainerFrame::GetPropTableFrames(const FramePropertyDescriptor* aProperty) const
{
FramePropertyTable* propTable = PresContext()->PropertyTable();
return static_cast<nsFrameList*>(propTable->Get(this, aProperty));
return static_cast<nsFrameList*>(propTable->Get(this, aProperty, false /* aSkipBitCheck */));
}
nsFrameList*
nsContainerFrame::RemovePropTableFrames(const FramePropertyDescriptor* aProperty)
{
FramePropertyTable* propTable = PresContext()->PropertyTable();
return static_cast<nsFrameList*>(propTable->Remove(this, aProperty));
return static_cast<nsFrameList*>(propTable->Remove(this, aProperty, false /* aSkipBitCheck */));
}
void
@ -1957,10 +1957,12 @@ nsOverflowContinuationTracker::EndFinish(nsIFrame* aChild)
nsPresContext* pc = aChild->PresContext();
FramePropertyTable* propTable = pc->PropertyTable();
nsFrameList* eoc = static_cast<nsFrameList*>(propTable->Get(mParent,
nsContainerFrame::ExcessOverflowContainersProperty()));
nsContainerFrame::ExcessOverflowContainersProperty(),
false /* aSkipBitCheck */));
if (eoc != mOverflowContList) {
nsFrameList* oc = static_cast<nsFrameList*>(propTable->Get(mParent,
nsContainerFrame::OverflowContainersProperty()));
nsContainerFrame::OverflowContainersProperty(),
false /* aSkipBitCheck */));
if (oc != mOverflowContList) {
// mOverflowContList was deleted
mPrevOverflowCont = nullptr;

View File

@ -8914,8 +8914,9 @@ nsFrame::BoxReflow(nsBoxLayoutState& aState,
parentReflowState(aPresContext, parentFrame, aRenderingContext,
LogicalSize(parentWM, parentSize),
nsHTMLReflowState::DUMMY_PARENT_REFLOW_STATE);
parentFrame->RemoveStateBits(~nsFrameState(0));
parentFrame->AddStateBits(savedState);
const nsFrameState bitsToLeaveUntouched = NS_FRAME_HAS_PROPERTIES;
parentFrame->RemoveStateBits(~bitsToLeaveUntouched);
parentFrame->AddStateBits(savedState & ~bitsToLeaveUntouched);
// This may not do very much useful, but it's probably worth trying.
if (parentSize.width != NS_INTRINSICSIZE)

View File

@ -255,13 +255,17 @@ FRAME_STATE_BIT(Generic, 53, NS_FRAME_IS_NONDISPLAY)
FRAME_STATE_BIT(Generic, 54, NS_FRAME_HAS_LAYER_ACTIVITY_PROPERTY)
// The display list of the frame can be handled by the shortcut for
// COMMON CASE. This is bug 1342009, but it uses bit 57. Since we might
// implement NS_FRAME_HAS_PROPERTIES (56) in the near future, we will use bit
// COMMON CASE. This is bug 1342009, but it uses bit 57. Since we have now
// implemented NS_FRAME_HAS_PROPERTIES (56) in TenFourFox, we will use bit
// 55. XXX: Dump bit 57 and make it all work out -- we'd need the backout
// bits from bug 1250244 and then just not implement the replacement API.
// However, that's only worth doing if we're really short on bits later.
FRAME_STATE_BIT(Generic, 55, NS_FRAME_SIMPLE_DISPLAYLIST)
// Frame has properties in the nsIFrame::Properties() hash.
// See bug 1353187 (TenFourFox issue 375).
FRAME_STATE_BIT(Generic, 56, NS_FRAME_HAS_PROPERTIES)
// Frame has VR content, and needs VR display items created
FRAME_STATE_BIT(Generic, 57, NS_FRAME_HAS_VR_CONTENT)

View File

@ -4213,9 +4213,12 @@ nsContinuingTextFrame::Init(nsIContent* aContent,
FramePropertyTable *propTable = PresContext()->PropertyTable();
// Get all the properties from the prev-in-flow first to take
// advantage of the propTable's cache and simplify the assertion below
void* embeddingLevel = propTable->Get(aPrevInFlow, EmbeddingLevelProperty());
void* baseLevel = propTable->Get(aPrevInFlow, BaseLevelProperty());
void* paragraphDepth = propTable->Get(aPrevInFlow, ParagraphDepthProperty());
void* embeddingLevel = propTable->Get(aPrevInFlow, EmbeddingLevelProperty(),
false /* aSkipBitCheck */);
void* baseLevel = propTable->Get(aPrevInFlow, BaseLevelProperty(), false /*
aSkipBitCheck */);
void* paragraphDepth = propTable->Get(aPrevInFlow, ParagraphDepthProperty(),
false /* aSkipBitCheck */);
propTable->Set(this, EmbeddingLevelProperty(), embeddingLevel);
propTable->Set(this, BaseLevelProperty(), baseLevel);
propTable->Set(this, ParagraphDepthProperty(), paragraphDepth);
@ -4227,9 +4230,12 @@ nsContinuingTextFrame::Init(nsIContent* aContent,
while (nextContinuation &&
nextContinuation->GetContentOffset() < mContentOffset) {
NS_ASSERTION(
embeddingLevel == propTable->Get(nextContinuation, EmbeddingLevelProperty()) &&
baseLevel == propTable->Get(nextContinuation, BaseLevelProperty()) &&
paragraphDepth == propTable->Get(nextContinuation, ParagraphDepthProperty()),
embeddingLevel == propTable->Get(nextContinuation,
EmbeddingLevelProperty(), false /* aSkipBitCheck */) &&
baseLevel == propTable->Get(nextContinuation, BaseLevelProperty(),
false /* aSkipBitCheck */) &&
paragraphDepth == propTable->Get(nextContinuation,
ParagraphDepthProperty(), false /* aSkipBitCheck */),
"stealing text from different type of BIDI continuation");
nextContinuation->mContentOffset = mContentOffset;
nextContinuation = static_cast<nsTextFrame*>(nextContinuation->GetNextContinuation());

View File

@ -180,7 +180,7 @@ nsMathMLContainerFrame::ClearSavedChildMetrics()
nsIFrame* childFrame = mFrames.FirstChild();
FramePropertyTable* props = PresContext()->PropertyTable();
while (childFrame) {
props->Delete(childFrame, HTMLReflowMetricsProperty());
props->Delete(childFrame, HTMLReflowMetricsProperty(), false /* aSkipBitCheck */);
childFrame = childFrame->GetNextSibling();
}
}

View File

@ -765,7 +765,7 @@ nsMathMLmtableOuterFrame::AttributeChanged(int32_t aNameSpaceID,
aAttribute == nsGkAtoms::columnlines_) {
// clear any cached property list for this table
presContext->PropertyTable()->
Delete(tableFrame, AttributeToProperty(aAttribute));
Delete(tableFrame, AttributeToProperty(aAttribute), false /* aSkipBitCheck */);
// Reparse the new attribute on the table.
ParseFrameAttribute(tableFrame, aAttribute, true);
} else {
@ -1117,7 +1117,7 @@ nsMathMLmtrFrame::AttributeChanged(int32_t aNameSpaceID,
return NS_OK;
}
presContext->PropertyTable()->Delete(this, AttributeToProperty(aAttribute));
presContext->PropertyTable()->Delete(this, AttributeToProperty(aAttribute), false /* aSkipBitCheck */);
bool allowMultiValues = (aAttribute == nsGkAtoms::columnalign_);
@ -1175,7 +1175,7 @@ nsMathMLmtdFrame::AttributeChanged(int32_t aNameSpaceID,
aAttribute == nsGkAtoms::columnalign_) {
nsPresContext* presContext = PresContext();
presContext->PropertyTable()->Delete(this, AttributeToProperty(aAttribute));
presContext->PropertyTable()->Delete(this, AttributeToProperty(aAttribute), false /* aSkipBitCheck */);
// Reparse the attribute.
ParseFrameAttribute(this, aAttribute, false);