Bug 1369037 - Make the assertions about NS_FRAME_PART_OF_IBSPLIT stricter and update the associated frame properties in nsContainerFrame::DestroyFrom instead of nsFrame::DestroyFrom. r=jfkthame
authorMats Palmgren <mats@mozilla.com>
Wed, 31 May 2017 21:29:49 +0200
changeset 361679 62c2ff7599a301ec3e3cfee0e16325ab2afa0eea
parent 361678 bd52d785f0d482d6a106b5d5603c3c334a521b2e
child 361680 d8c7a5c7cb77f8fc3527a4b020291b920a7afda2
push id43854
push userryanvm@gmail.com
push dateThu, 01 Jun 2017 00:50:28 +0000
treeherderautoland@5ecd65c9136b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame
bugs1369037
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1369037 - Make the assertions about NS_FRAME_PART_OF_IBSPLIT stricter and update the associated frame properties in nsContainerFrame::DestroyFrom instead of nsFrame::DestroyFrom. r=jfkthame MozReview-Commit-ID: G8NQ70xzkQU
layout/base/nsCSSFrameConstructor.cpp
layout/generic/nsContainerFrame.cpp
layout/generic/nsFrame.cpp
layout/generic/nsFrameStateBits.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -507,17 +507,21 @@ ReparentFrames(nsCSSFrameConstructor* aF
 //
 // When inline frames get weird and have block frames in them, we
 // annotate them to help us respond to incremental content changes
 // more easily.
 
 static inline bool
 IsFramePartOfIBSplit(nsIFrame* aFrame)
 {
-  return (aFrame->GetStateBits() & NS_FRAME_PART_OF_IBSPLIT) != 0;
+  bool result = (aFrame->GetStateBits() & NS_FRAME_PART_OF_IBSPLIT) != 0;
+  MOZ_ASSERT(!result || static_cast<nsBlockFrame*>(do_QueryFrame(aFrame)) ||
+                        static_cast<nsInlineFrame*>(do_QueryFrame(aFrame)),
+             "only block/inline frames can have NS_FRAME_PART_OF_IBSPLIT");
+  return result;
 }
 
 static nsContainerFrame* GetIBSplitSibling(nsIFrame* aFrame)
 {
   NS_PRECONDITION(IsFramePartOfIBSplit(aFrame), "Shouldn't call this");
 
   // We only store the "ib-split sibling" annotation with the first
   // frame in the continuation chain. Walk back to find that frame now.
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@@ -217,16 +217,42 @@ nsContainerFrame::DestroyFrom(nsIFrame* 
     GetView()->SetFrame(nullptr);
   }
 
   DestroyAbsoluteFrames(aDestructRoot);
 
   // Destroy frames on the principal child list.
   mFrames.DestroyFramesFrom(aDestructRoot);
 
+  // If we have any IB split siblings, clear their references to us.
+  if (HasAnyStateBits(NS_FRAME_PART_OF_IBSPLIT)) {
+    // Delete previous sibling's reference to me.
+    nsIFrame* prevSib = GetProperty(nsIFrame::IBSplitPrevSibling());
+    if (prevSib) {
+      NS_WARNING_ASSERTION(
+        this == prevSib->GetProperty(nsIFrame::IBSplitSibling()),
+        "IB sibling chain is inconsistent");
+      prevSib->DeleteProperty(nsIFrame::IBSplitSibling());
+    }
+
+    // Delete next sibling's reference to me.
+    nsIFrame* nextSib = GetProperty(nsIFrame::IBSplitSibling());
+    if (nextSib) {
+      NS_WARNING_ASSERTION(
+        this == nextSib->GetProperty(nsIFrame::IBSplitPrevSibling()),
+        "IB sibling chain is inconsistent");
+      nextSib->DeleteProperty(nsIFrame::IBSplitPrevSibling());
+    }
+
+#ifdef DEBUG
+    // This is just so we can assert it's not set in nsFrame::DestroyFrom.
+    RemoveStateBits(NS_FRAME_PART_OF_IBSPLIT);
+#endif
+  }
+
   // Destroy frames on the auxiliary frame lists and delete the lists.
   nsPresContext* pc = PresContext();
   nsIPresShell* shell = pc->PresShell();
   SafelyDestroyFrameListProp(aDestructRoot, shell, OverflowProperty());
 
   MOZ_ASSERT(IsFrameOfType(nsIFrame::eCanContainOverflowContainers) ||
              !(GetProperty(nsContainerFrame::OverflowContainersProperty()) ||
                GetProperty(nsContainerFrame::ExcessOverflowContainersProperty())),
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -707,16 +707,18 @@ void
 nsFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
   NS_ASSERTION(!nsContentUtils::IsSafeToRunScript(),
     "destroy called on frame while scripts not blocked");
   NS_ASSERTION(!GetNextSibling() && !GetPrevSibling(),
                "Frames should be removed before destruction.");
   NS_ASSERTION(aDestructRoot, "Must specify destruct root");
   MOZ_ASSERT(!HasAbsolutelyPositionedChildren());
+  MOZ_ASSERT(!HasAnyStateBits(NS_FRAME_PART_OF_IBSPLIT),
+             "NS_FRAME_PART_OF_IBSPLIT set on non-nsContainerFrame?");
 
   nsSVGEffects::InvalidateDirectRenderingObservers(this);
 
   if (StyleDisplay()->mPosition == NS_STYLE_POSITION_STICKY) {
     StickyScrollContainer* ssc =
       StickyScrollContainer::GetStickyScrollContainerForFrame(this);
     if (ssc) {
       ssc->RemoveFrame(this);
@@ -733,38 +735,16 @@ nsFrame::DestroyFrom(nsIFrame* aDestruct
                  nsLayoutUtils::IsProperAncestorFrame(aDestructRoot, placeholder),
                  "Placeholder relationship should have been torn down already; "
                  "this might mean we have a stray placeholder in the tree.");
     if (placeholder) {
       placeholder->SetOutOfFlowFrame(nullptr);
     }
   }
 
-  // If we have any IB split siblings, clear their references to us.
-  // (Note: This has to happen before we clear our Properties() table.)
-  if (mState & NS_FRAME_PART_OF_IBSPLIT) {
-    // Delete previous sibling's reference to me.
-    nsIFrame* prevSib = GetProperty(nsIFrame::IBSplitPrevSibling());
-    if (prevSib) {
-      NS_WARNING_ASSERTION(
-        this == prevSib->GetProperty(nsIFrame::IBSplitSibling()),
-        "IB sibling chain is inconsistent");
-      prevSib->DeleteProperty(nsIFrame::IBSplitSibling());
-    }
-
-    // Delete next sibling's reference to me.
-    nsIFrame* nextSib = GetProperty(nsIFrame::IBSplitSibling());
-    if (nextSib) {
-      NS_WARNING_ASSERTION(
-        this == nextSib->GetProperty(nsIFrame::IBSplitPrevSibling()),
-        "IB sibling chain is inconsistent");
-      nextSib->DeleteProperty(nsIFrame::IBSplitPrevSibling());
-    }
-  }
-
   bool isPrimaryFrame = (mContent && mContent->GetPrimaryFrame() == this);
   if (isPrimaryFrame) {
     // This needs to happen before we clear our Properties() table.
     ActiveLayerTracker::TransferActivityToContent(this, mContent);
 
     // Unfortunately, we need to do this for all frames being reframed
     // and not only those whose current style involves CSS transitions,
     // because what matters is whether the new style (not the old)
--- a/layout/generic/nsFrameStateBits.h
+++ b/layout/generic/nsFrameStateBits.h
@@ -135,17 +135,17 @@ FRAME_STATE_BIT(Generic, 12, NS_FRAME_HA
 FRAME_STATE_BIT(Generic, 13, NS_FRAME_HAS_VIEW)
 
 // If this bit is set, the frame was created from anonymous content.
 FRAME_STATE_BIT(Generic, 14, NS_FRAME_INDEPENDENT_SELECTION)
 
 // If this bit is set, the frame is part of the mangled frame hierarchy
 // that results when an inline has been split because of a nested block.
 // See the comments in nsCSSFrameConstructor::ConstructInline for
-// more details.
+// more details.  (this is only set on nsBlockFrame/nsInlineFrame frames)
 FRAME_STATE_BIT(Generic, 15, NS_FRAME_PART_OF_IBSPLIT)
 
 // If this bit is set, then transforms (e.g. CSS or SVG transforms) are allowed
 // to affect the frame, and a transform may currently be in affect. If this bit
 // is not set, then any transforms on the frame will be ignored.
 // This is used primarily in GetTransformMatrix to optimize for the
 // common case.
 FRAME_STATE_BIT(Generic, 16, NS_FRAME_MAY_BE_TRANSFORMED)