diff --git a/layout/base/FramePropertyTable.cpp b/layout/base/FramePropertyTable.cpp index 165df7935..4ff6a96ff 100644 --- a/layout/base/FramePropertyTable.cpp +++ b/layout/base/FramePropertyTable.cpp @@ -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(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 diff --git a/layout/base/FramePropertyTable.h b/layout/base/FramePropertyTable.h index 188f5304f..c0fc3211f 100644 --- a/layout/base/FramePropertyTable.h +++ b/layout/base/FramePropertyTable.h @@ -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; } diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index 1b472c806..281c95386 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -130,7 +130,7 @@ GetNextBlockInInlineSibling(FramePropertyTable* aPropTable, nsIFrame* aFrame) } return static_cast - (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 diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index 4390ab92e..e22643671 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -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)) { diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index 25e8df119..ee68dba20 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -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 */); } } diff --git a/layout/generic/nsContainerFrame.cpp b/layout/generic/nsContainerFrame.cpp index c481cd67b..7a4cd52ad 100644 --- a/layout/generic/nsContainerFrame.cpp +++ b/layout/generic/nsContainerFrame.cpp @@ -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(aPropTable->Get(this, aProp))) { + static_cast(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( - 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(aPropTable->Get(aFrame, aProp)); + nsFrameList* list = static_cast(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( - propTable->Get(this, OverflowContainersProperty())); + propTable->Get(this, OverflowContainersProperty(), false /* aSkipBitCheck */)); if (!list || !list->ContainsFrame(aChild)) { list = static_cast( - 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(propTable->Get(this, aProperty)); + return static_cast(propTable->Get(this, aProperty, false /* aSkipBitCheck */)); } nsFrameList* nsContainerFrame::RemovePropTableFrames(const FramePropertyDescriptor* aProperty) { FramePropertyTable* propTable = PresContext()->PropertyTable(); - return static_cast(propTable->Remove(this, aProperty)); + return static_cast(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(propTable->Get(mParent, - nsContainerFrame::ExcessOverflowContainersProperty())); + nsContainerFrame::ExcessOverflowContainersProperty(), + false /* aSkipBitCheck */)); if (eoc != mOverflowContList) { nsFrameList* oc = static_cast(propTable->Get(mParent, - nsContainerFrame::OverflowContainersProperty())); + nsContainerFrame::OverflowContainersProperty(), + false /* aSkipBitCheck */)); if (oc != mOverflowContList) { // mOverflowContList was deleted mPrevOverflowCont = nullptr; diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 7ccfe6683..4a51286e1 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -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) diff --git a/layout/generic/nsFrameStateBits.h b/layout/generic/nsFrameStateBits.h index 422ad0da2..126933c9b 100644 --- a/layout/generic/nsFrameStateBits.h +++ b/layout/generic/nsFrameStateBits.h @@ -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) diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index f01cd5d9f..09bfa9548 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -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(nextContinuation->GetNextContinuation()); diff --git a/layout/mathml/nsMathMLContainerFrame.cpp b/layout/mathml/nsMathMLContainerFrame.cpp index 7a4c7fa13..fd61d03fa 100644 --- a/layout/mathml/nsMathMLContainerFrame.cpp +++ b/layout/mathml/nsMathMLContainerFrame.cpp @@ -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(); } } diff --git a/layout/mathml/nsMathMLmtableFrame.cpp b/layout/mathml/nsMathMLmtableFrame.cpp index 78e6c4859..c1e39a9ab 100644 --- a/layout/mathml/nsMathMLmtableFrame.cpp +++ b/layout/mathml/nsMathMLmtableFrame.cpp @@ -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);