diff --git a/layout/base/FramePropertyTable.cpp b/layout/base/FramePropertyTable.cpp index bc824432fdce..b84a88c6b016 100644 --- a/layout/base/FramePropertyTable.cpp +++ b/layout/base/FramePropertyTable.cpp @@ -22,6 +22,7 @@ FramePropertyTable::SetInternal( if (mLastFrame != aFrame || !mLastEntry) { mLastFrame = aFrame; mLastEntry = mEntries.PutEntry(aFrame); + aFrame->AddStateBits(NS_FRAME_HAS_PROPERTIES); } Entry* entry = mLastEntry; @@ -63,7 +64,8 @@ FramePropertyTable::SetInternal( void* FramePropertyTable::GetInternal( - const nsIFrame* aFrame, UntypedDescriptor aProperty, bool* aFoundResult) + const nsIFrame* aFrame, UntypedDescriptor aProperty, bool aSkipBitCheck, + bool* aFoundResult) { NS_ASSERTION(aFrame, "Null frame?"); NS_ASSERTION(aProperty, "Null property?"); @@ -72,6 +74,10 @@ FramePropertyTable::GetInternal( *aFoundResult = false; } + if (!aSkipBitCheck && !(aFrame->GetStateBits() & NS_FRAME_HAS_PROPERTIES)) { + return nullptr; + } + // We can end up here during parallel style traversal, in which case the main // thread is blocked. Reading from the cache is fine on any thread, but we // only want to write to it in the main-thread case. @@ -82,6 +88,8 @@ FramePropertyTable::GetInternal( mLastEntry = entry; } + MOZ_ASSERT(entry || aSkipBitCheck, + "NS_FRAME_HAS_PROPERTIES bit should match whether entry exists"); if (!entry) return nullptr; @@ -111,7 +119,8 @@ FramePropertyTable::GetInternal( void* FramePropertyTable::RemoveInternal( - nsIFrame* aFrame, UntypedDescriptor aProperty, bool* aFoundResult) + nsIFrame* aFrame, UntypedDescriptor aProperty, bool aSkipBitCheck, + bool* aFoundResult) { MOZ_ASSERT(NS_IsMainThread()); NS_ASSERTION(aFrame, "Null frame?"); @@ -121,11 +130,17 @@ FramePropertyTable::RemoveInternal( *aFoundResult = false; } + if (!aSkipBitCheck && !(aFrame->GetStateBits() & NS_FRAME_HAS_PROPERTIES)) { + return nullptr; + } + if (mLastFrame != aFrame) { mLastFrame = aFrame; mLastEntry = mEntries.GetEntry(aFrame); } Entry* entry = mLastEntry; + MOZ_ASSERT(entry || aSkipBitCheck, + "NS_FRAME_HAS_PROPERTIES bit should match whether entry exists"); if (!entry) return nullptr; @@ -136,6 +151,7 @@ FramePropertyTable::RemoveInternal( // Here it's ok to use RemoveEntry() -- which may resize mEntries -- // because we null mLastEntry at the same time. mEntries.RemoveEntry(entry); + aFrame->RemoveStateBits(NS_FRAME_HAS_PROPERTIES); mLastEntry = nullptr; if (aFoundResult) { *aFoundResult = true; @@ -176,14 +192,14 @@ FramePropertyTable::RemoveInternal( void FramePropertyTable::DeleteInternal( - nsIFrame* aFrame, UntypedDescriptor aProperty) + nsIFrame* aFrame, UntypedDescriptor aProperty, bool aSkipBitCheck) { MOZ_ASSERT(NS_IsMainThread()); NS_ASSERTION(aFrame, "Null frame?"); NS_ASSERTION(aProperty, "Null property?"); bool found; - void* v = RemoveInternal(aFrame, aProperty, &found); + void* v = RemoveInternal(aFrame, aProperty, aSkipBitCheck, &found); if (found) { PropertyValue pv(aProperty, v); pv.DestroyValueFor(aFrame); @@ -210,7 +226,13 @@ FramePropertyTable::DeleteAllFor(nsIFrame* aFrame) { NS_ASSERTION(aFrame, "Null frame?"); + if (!(aFrame->GetStateBits() & NS_FRAME_HAS_PROPERTIES)) { + return; + } + Entry* entry = mEntries.GetEntry(aFrame); + MOZ_ASSERT(entry, + "NS_FRAME_HAS_PROPERTIES bit should match whether entry exists"); if (!entry) return; @@ -226,6 +248,8 @@ FramePropertyTable::DeleteAllFor(nsIFrame* aFrame) // mLastEntry points into mEntries, so we use RawRemoveEntry() which will not // resize mEntries. 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 2c832fc0e66b..5a9a3672f22a 100644 --- a/layout/base/FramePropertyTable.h +++ b/layout/base/FramePropertyTable.h @@ -188,12 +188,25 @@ public: * * - Calling Has() before Set() in cases where we don't want to overwrite * an existing value for the frame property. + * + * The HasSkippingBitCheck 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. */ template bool Has(const nsIFrame* aFrame, Descriptor aProperty) { bool foundResult = false; - mozilla::Unused << GetInternal(aFrame, aProperty, &foundResult); + mozilla::Unused << GetInternal(aFrame, aProperty, false, &foundResult); + return foundResult; + } + + template + bool HasSkippingBitCheck(const nsIFrame* aFrame, Descriptor aProperty) + { + bool foundResult = false; + mozilla::Unused << GetInternal(aFrame, aProperty, true, &foundResult); return foundResult; } @@ -212,7 +225,7 @@ public: PropertyType Get(const nsIFrame* aFrame, Descriptor aProperty, bool* aFoundResult = nullptr) { - void* ptr = GetInternal(aFrame, aProperty, aFoundResult); + void* ptr = GetInternal(aFrame, aProperty, false, aFoundResult); return ReinterpretHelper::FromPointer(ptr); } /** @@ -231,7 +244,7 @@ public: PropertyType Remove(nsIFrame* aFrame, Descriptor aProperty, bool* aFoundResult = nullptr) { - void* ptr = RemoveInternal(aFrame, aProperty, aFoundResult); + void* ptr = RemoveInternal(aFrame, aProperty, false, aFoundResult); return ReinterpretHelper::FromPointer(ptr); } /** @@ -239,11 +252,23 @@ public: * 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. */ template void Delete(nsIFrame* aFrame, Descriptor aProperty) { - DeleteInternal(aFrame, aProperty); + DeleteInternal(aFrame, aProperty, false); + } + + template + void DeleteSkippingBitCheck(nsIFrame* aFrame, Descriptor aProperty) + { + DeleteInternal(aFrame, aProperty, true); } /** * Remove and destroy all property values for a frame. This requires one @@ -262,12 +287,13 @@ protected: void* aValue); void* GetInternal(const nsIFrame* aFrame, UntypedDescriptor aProperty, - bool* aFoundResult); + bool aSkipBitCheck, bool* aFoundResult); void* RemoveInternal(nsIFrame* aFrame, UntypedDescriptor aProperty, - bool* aFoundResult); + bool aSkipBitCheck, bool* aFoundResult); - void DeleteInternal(nsIFrame* aFrame, UntypedDescriptor aProperty); + void DeleteInternal(nsIFrame* aFrame, UntypedDescriptor aProperty, + bool aSkipBitCheck); template struct ReinterpretHelper diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index 85c4088bd809..3b59fbc61794 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -1396,7 +1396,8 @@ RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList) break; } - const nsStyleChangeData& data = aChangeList[i]; + nsStyleChangeData& mutable_data = aChangeList[i]; + const nsStyleChangeData& data = mutable_data; nsIFrame* frame = data.mFrame; nsIContent* content = data.mContent; nsChangeHint hint = data.mHint; @@ -1407,7 +1408,11 @@ 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. + mutable_data.mFrame = nullptr; continue; } @@ -1475,6 +1480,11 @@ 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. + mutable_data.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 @@ -1664,7 +1674,7 @@ RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList) // is not in fact in a consistent state. for (const nsStyleChangeData& data : aChangeList) { if (data.mFrame) { - propTable->Delete(data.mFrame, ChangeListProperty()); + propTable->DeleteSkippingBitCheck(data.mFrame, ChangeListProperty()); } #ifdef DEBUG diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index e28b09d0066d..03bb9c420280 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -9982,8 +9982,9 @@ nsFrame::BoxReflow(nsBoxLayoutState& aState, parentReflowInput(aPresContext, parentFrame, aRenderingContext, LogicalSize(parentWM, parentSize), ReflowInput::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 8306239e7551..c0a955598d3f 100644 --- a/layout/generic/nsFrameStateBits.h +++ b/layout/generic/nsFrameStateBits.h @@ -270,6 +270,9 @@ FRAME_STATE_BIT(Generic, 54, NS_FRAME_HAS_LAYER_ACTIVITY_PROPERTY) // a stylo tree traversal. FRAME_STATE_BIT(Generic, 55, NS_FRAME_OWNS_ANON_BOXES) +// Frame has properties in the nsIFrame::Properties() hash. +FRAME_STATE_BIT(Generic, 56, NS_FRAME_HAS_PROPERTIES) + // Set for all descendants of MathML sub/supscript elements (other than the // base frame) to indicate that the SSTY font feature should be used. FRAME_STATE_BIT(Generic, 58, NS_FRAME_MATHML_SCRIPT_DESCENDANT)