Add assertions that the frame tree is safe to destroy (i.e., doesn't contain any first-in-flows or other things we should never destroy) when we call DeleteNextInFlowChild. (Bug 619021) r=roc a2.0=blocking
authorL. David Baron <dbaron@dbaron.org>
Tue, 11 Jan 2011 17:09:22 -0800
changeset 60341 dfa73f7b1acf32ed01c5fdad939be4a365ae0773
parent 60340 37d585cbcb75b2bd9e77145fa75f376b37402356
child 60342 67cfc95b4b9096008e98e90489ae649d287d61d7
push id17944
push userdbaron@mozilla.com
push dateWed, 12 Jan 2011 01:09:43 +0000
treeherdermozilla-central@e2f7319148ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs619021
milestone2.0b10pre
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
Add assertions that the frame tree is safe to destroy (i.e., doesn't contain any first-in-flows or other things we should never destroy) when we call DeleteNextInFlowChild. (Bug 619021) r=roc a2.0=blocking
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
layout/generic/nsBlockFrame.cpp
layout/generic/nsContainerFrame.cpp
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -3806,16 +3806,59 @@ nsLayoutUtils::AssertNoDuplicateContinua
     for (nsIFrame *c = f; (c = c->GetNextInFlow());) {
       NS_ASSERTION(c->GetParent() != aContainer ||
                    !aFrameList.ContainsFrame(c),
                    "Two continuations of the same frame in the same "
                    "frame list");
     }
   }
 }
+
+// Is one of aFrame's ancestors a letter frame?
+static bool
+IsInLetterFrame(nsIFrame *aFrame)
+{
+  for (nsIFrame *f = aFrame->GetParent(); f; f = f->GetParent()) {
+    if (f->GetType() == nsGkAtoms::letterFrame) {
+      return true;
+    }
+  }
+  return false;
+}
+
+/* static */ void
+nsLayoutUtils::AssertTreeOnlyEmptyNextInFlows(nsIFrame *aSubtreeRoot)
+{
+  NS_ASSERTION(aSubtreeRoot->GetPrevInFlow(),
+               "frame tree not empty, but caller reported complete status");
+
+  // Also assert that text frames map no text.
+  PRInt32 start, end;
+  nsresult rv = aSubtreeRoot->GetOffsets(start, end);
+  NS_ASSERTION(NS_SUCCEEDED(rv), "GetOffsets failed");
+  // In some cases involving :first-letter, we'll partially unlink a
+  // continuation in the middle of a continuation chain from its
+  // previous and next continuations before destroying it, presumably so
+  // that we don't also destroy the later continuations.  Once we've
+  // done this, GetOffsets returns incorrect values.
+  // For examples, see list of tests in
+  // https://bugzilla.mozilla.org/show_bug.cgi?id=619021#c29
+  NS_ASSERTION(start == end || IsInLetterFrame(aSubtreeRoot),
+               "frame tree not empty, but caller reported complete status");
+
+  PRInt32 listIndex = 0;
+  nsIAtom* childList = nsnull;
+  do {
+    for (nsIFrame* child = aSubtreeRoot->GetFirstChild(childList); child;
+         child = child->GetNextSibling()) {
+      nsLayoutUtils::AssertTreeOnlyEmptyNextInFlows(child);
+    }
+    childList = aSubtreeRoot->GetAdditionalChildListName(listIndex++);
+  } while (childList);
+}
 #endif
 
 nsSetAttrRunnable::nsSetAttrRunnable(nsIContent* aContent, nsIAtom* aAttrName,
                                      const nsAString& aValue)
   : mContent(aContent),
     mAttrName(aAttrName),
     mValue(aValue)
 {
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -1268,16 +1268,23 @@ public:
   /**
    * Assert that there are no duplicate continuations of the same frame
    * within aFrameList.  Optimize the tests by assuming that all frames
    * in aFrameList have parent aContainer.
    */
   static void
   AssertNoDuplicateContinuations(nsIFrame* aContainer,
                                  const nsFrameList& aFrameList);
+
+  /**
+   * Assert that the frame tree rooted at |aSubtreeRoot| is empty, i.e.,
+   * that it contains no first-in-flows.
+   */
+  static void
+  AssertTreeOnlyEmptyNextInFlows(nsIFrame *aSubtreeRoot);
 #endif
 };
 
 class nsSetAttrRunnable : public nsRunnable
 {
 public:
   nsSetAttrRunnable(nsIContent* aContent, nsIAtom* aAttrName,
                     const nsAString& aValue);
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -5637,16 +5637,21 @@ nsBlockFrame::DeleteNextInFlowChild(nsPr
   NS_PRECONDITION(aNextInFlow->GetPrevInFlow(), "bad next-in-flow");
 
   if (aNextInFlow->GetStateBits() &
       (NS_FRAME_OUT_OF_FLOW | NS_FRAME_IS_OVERFLOW_CONTAINER)) {
     nsContainerFrame::DeleteNextInFlowChild(aPresContext,
         aNextInFlow, aDeletingEmptyFrames);
   }
   else {
+#ifdef DEBUG
+    if (aDeletingEmptyFrames) {
+      nsLayoutUtils::AssertTreeOnlyEmptyNextInFlows(aNextInFlow);
+    }
+#endif
     DoRemoveFrame(aNextInFlow,
         aDeletingEmptyFrames ? FRAMES_ARE_EMPTY : 0);
   }
 }
 
 ////////////////////////////////////////////////////////////////////////
 // Float support
 
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@@ -1135,16 +1135,22 @@ nsContainerFrame::DeleteNextInFlowChild(
 
   // Take the next-in-flow out of the parent's child list
 #ifdef DEBUG
   nsresult rv =
 #endif
     StealFrame(aPresContext, aNextInFlow);
   NS_ASSERTION(NS_SUCCEEDED(rv), "StealFrame failure");
 
+#ifdef DEBUG
+  if (aDeletingEmptyFrames) {
+    nsLayoutUtils::AssertTreeOnlyEmptyNextInFlows(aNextInFlow);
+  }
+#endif
+
   // Delete the next-in-flow frame and its descendants. This will also
   // remove it from its next-in-flow/prev-in-flow chain.
   aNextInFlow->Destroy();
 
   NS_POSTCONDITION(!prevInFlow->GetNextInFlow(), "non null next-in-flow");
 }
 
 /**