Bug 840818 - Simplify PullFrameFrom when the last frame was removed from an overflow list (avoiding an assertion). r=bz a=bajaj
authorMats Palmgren <matspal@gmail.com>
Mon, 11 Mar 2013 15:34:13 -0500
changeset 132348 aa3e16175fe93147415e67a984c5a46bfad8e27f
parent 132347 41a072aa81e53d2b1c64a43ac9286e61f72e584e
child 132349 3fc3f4c0701f0303200e433b73169bb39481c7a4
push id2323
push userbbajaj@mozilla.com
push dateMon, 01 Apr 2013 19:47:02 +0000
treeherdermozilla-beta@7712be144d91 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, bajaj
bugs840818
milestone21.0a2
Bug 840818 - Simplify PullFrameFrom when the last frame was removed from an overflow list (avoiding an assertion). r=bz a=bajaj
layout/generic/crashtests/840818.html
layout/generic/crashtests/crashtests.list
layout/generic/nsBlockFrame.cpp
layout/generic/nsBlockFrame.h
layout/reftests/columns/reftest.list
layout/reftests/pagination/reftest.list
new file mode 100644
--- /dev/null
+++ b/layout/generic/crashtests/840818.html
@@ -0,0 +1,8 @@
+<html>
+<head>
+<meta charset="UTF-8">
+</head>
+<body style="font-family: monospace;">
+<div style="-moz-column-count: 2;"><div style="-moz-column-count: 2; width: 9ch;">πŸ˜ŽδΈ­ζ–‡<span>; </span><span>!</span></div></div>
+</body>
+</html>
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@@ -423,11 +423,12 @@ test-pref(layout.css.flexbox.enabled,tru
 test-pref(layout.css.flexbox.enabled,true) load 737313-3.html
 load 762764-1.html
 load 786740-1.html
 asserts(12) test-pref(layout.css.flexbox.enabled,true) load 798020-1.html
 test-pref(layout.css.flexbox.enabled,true) load 798235-1.html
 test-pref(layout.css.flexbox.enabled,true) load 799207-1.html
 asserts(12) test-pref(layout.css.flexbox.enabled,true) load 799207-2.html
 test-pref(layout.css.flexbox.enabled,true) load 804089-1.xhtml
-asserts(0-2) load 810726.html # bug 840818 and bug 847130 
+load 840818.html
+load 810726.html
 load 842132-1.html
 load 847130.xhtml
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -2512,52 +2512,54 @@ nsBlockFrame::ReflowLine(nsBlockReflowSt
 }
 
 nsIFrame*
 nsBlockFrame::PullFrame(nsBlockReflowState& aState,
                         line_iterator       aLine)
 {
   // First check our remaining lines.
   if (end_lines() != aLine.next()) {
-    return PullFrameFrom(aState, aLine, this, false, mFrames, aLine.next());
+    return PullFrameFrom(aState, aLine, this, false, mFrames, mLines,
+                         aLine.next());
   }
 
   NS_ASSERTION(!GetOverflowLines(),
     "Our overflow lines should have been removed at the start of reflow");
 
   // Try each next-in-flow.
   nsBlockFrame* nextInFlow = aState.mNextInFlow;
   while (nextInFlow) {
     // first normal lines, then overflow lines
     if (!nextInFlow->mLines.empty()) {
       return PullFrameFrom(aState, aLine, nextInFlow, false,
-                           nextInFlow->mFrames,
+                           nextInFlow->mFrames, nextInFlow->mLines,
                            nextInFlow->mLines.begin());
     }
 
     FrameLines* overflowLines = nextInFlow->GetOverflowLines();
     if (overflowLines) {
       return PullFrameFrom(aState, aLine, nextInFlow, true,
-                           overflowLines->mFrames,
+                           overflowLines->mFrames, overflowLines->mLines,
                            overflowLines->mLines.begin());
     }
 
     nextInFlow = static_cast<nsBlockFrame*>(nextInFlow->GetNextInFlow());
     aState.mNextInFlow = nextInFlow;
   }
 
   return nullptr;
 }
 
 nsIFrame*
 nsBlockFrame::PullFrameFrom(nsBlockReflowState&  aState,
                             nsLineBox*           aLine,
                             nsBlockFrame*        aFromContainer,
                             bool                 aFromOverflowLine,
                             nsFrameList&         aFromFrameList,
+                            nsLineList&          aFromLineList,
                             nsLineList::iterator aFromLine)
 {
   nsLineBox* fromLine = aFromLine;
   NS_ABORT_IF_FALSE(fromLine, "bad line to pull from");
   NS_ABORT_IF_FALSE(fromLine->GetChildCount(), "empty line");
   NS_ABORT_IF_FALSE(aLine->GetChildCount(), "empty line");
 
   NS_ASSERTION(fromLine->IsBlock() == fromLine->mFirstChild->IsBlockOutside(),
@@ -2602,38 +2604,28 @@ nsBlockFrame::PullFrameFrom(nsBlockReflo
   aLine->NoteFrameAdded(frame);
   fromLine->NoteFrameRemoved(frame);
 
   if (fromLine->GetChildCount() > 0) {
     // Mark line dirty now that we pulled a child
     fromLine->MarkDirty();
     fromLine->mFirstChild = newFirstChild;
   } else {
-    // Free up the fromLine now that it's empty
+    // Free up the fromLine now that it's empty.
     // Its bounds might need to be redrawn, though.
-    FrameLines* overflowLines =
-      aFromOverflowLine ? aFromContainer->RemoveOverflowLines() : nullptr;
-    nsLineList* fromLineList =
-      aFromOverflowLine ? &overflowLines->mLines : &aFromContainer->mLines;
-    if (aFromLine.next() != fromLineList->end())
+    if (aFromLine.next() != aFromLineList.end()) {
       aFromLine.next()->MarkPreviousMarginDirty();
-
-    fromLineList->erase(aFromLine);
+    }
+    aFromLineList.erase(aFromLine);
     // aFromLine is now invalid
     aFromContainer->FreeLineBox(fromLine);
 
-    // Put any remaining overflow lines back.
-    if (aFromOverflowLine) {
-      if (!fromLineList->empty()) {
-        aFromContainer->SetOverflowLines(overflowLines);
-      } else {
-        delete overflowLines;
-        // Now any iterators into fromLineList are invalid (but
-        // aFromLine already was invalidated above)
-      }
+    // Destroy the property if we pulled the last frame.
+    if (aFromOverflowLine && aFromFrameList.IsEmpty()) {
+      aFromContainer->DestroyOverflowLines();
     }
   }
 
 #ifdef DEBUG
   VerifyLines(true);
   VerifyOverflowSituation();
 #endif
 
--- a/layout/generic/nsBlockFrame.h
+++ b/layout/generic/nsBlockFrame.h
@@ -668,16 +668,17 @@ protected:
    *
    * @return the pulled frame or nullptr
    */
   nsIFrame* PullFrameFrom(nsBlockReflowState&  aState,
                           nsLineBox*           aLine,
                           nsBlockFrame*        aFromContainer,
                           bool                 aFromOverflowLine,
                           nsFrameList&         aFromFrameList,
+                          nsLineList&          aFromLineList,
                           nsLineList::iterator aFromLine);
 
   /**
    * Push the line after aLineBefore to the overflow line list.
    * @param aLineBefore a line in 'mLines' (or begin_lines() when
    *        pushing the first line)
    */
   void PushLines(nsBlockReflowState& aState,
--- a/layout/reftests/columns/reftest.list
+++ b/layout/reftests/columns/reftest.list
@@ -3,18 +3,18 @@
 == pref-width-1b.html pref-width-1-ref.html
 == pref-width-1c.html pref-width-1-ref.html
 == min-width-1a.html min-width-1-ref.html
 == min-width-1b.html min-width-1-ref.html
 == min-width-1c.html min-width-1-ref.html
 == column-balancing-overflow-000.html column-balancing-overflow-000.ref.html
 == column-balancing-overflow-001.html column-balancing-overflow-000.ref.html
 == column-balancing-overflow-002.html column-balancing-overflow-002.ref.html
-asserts(0-1) == column-balancing-overflow-003.html column-balancing-overflow-003.ref.html # bug 840818 and bug 847130 
-asserts(0-1) == column-balancing-overflow-004.html column-balancing-overflow-004.ref.html # bug 840818 and bug 847130 
+== column-balancing-overflow-003.html column-balancing-overflow-003.ref.html
+== column-balancing-overflow-004.html column-balancing-overflow-004.ref.html
 == column-balancing-overflow-005.html column-balancing-overflow-005.ref.html
 == column-balancing-000.html column-balancing-000.ref.html
 == column-balancing-001.html column-balancing-000.ref.html
 == column-balancing-002.html column-balancing-002.ref.html
 == column-balancing-003.html column-balancing-000.ref.html
 == column-balancing-004.html column-balancing-004.ref.html
 == column-box-alignment-rtl.html column-box-alignment-rtl-ref.html
 HTTP(..) == columnfill-balance.html columnfill-balance-ref.html
--- a/layout/reftests/pagination/reftest.list
+++ b/layout/reftests/pagination/reftest.list
@@ -54,9 +54,9 @@ skip-if(B2G) == table-caption-splitafter
 skip-if(B2G) == table-caption-splitaftercaption-4.html table-caption-splitaftercaption-4-ref.html # bug 773482
 skip-if(B2G) == table-caption-splitaftercaption-5.html table-caption-splitaftercaption-5-ref.html # bug 773482
 skip-if(B2G) == table-caption-splitaftercaption-6.html table-caption-splitaftercaption-6-ref.html # bug 773482
 skip-if(B2G) == table-caption-splitaftercaption-7.html table-caption-splitaftercaption-7-ref.html # bug 773482
 # == table-caption-splitaftercaption-8.html blank.html # bug 672654
 # == table-caption-splitaftercaption-9.html blank.html # bug 672654
 # == table-caption-splitaftercaption-10.html blank.html # bug 672654
 # == table-caption-splitaftercaption-11.html blank.html # bug 672654
-asserts(0-2) == column-balancing-break-inside-avoid-2.html column-balancing-break-inside-avoid-2-ref.html # bug 840818 and bug 847130 
+== column-balancing-break-inside-avoid-2.html column-balancing-break-inside-avoid-2-ref.html