Bug 847130 - Do DestroyOverflowLines() if the last line was removed instead of always RemoveOverflowLines() and SetOverflowLines() when there's lines remaining. r=bzbarsky a=bajaj
authorMats Palmgren <matspal@gmail.com>
Mon, 11 Mar 2013 14:47:48 -0500
changeset 132347 41a072aa81e53d2b1c64a43ac9286e61f72e584e
parent 132346 02c22d594266211229574ee6c391b0dd7d423302
child 132348 aa3e16175fe93147415e67a984c5a46bfad8e27f
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)
reviewersbzbarsky, bajaj
bugs847130
milestone21.0a2
Bug 847130 - Do DestroyOverflowLines() if the last line was removed instead of always RemoveOverflowLines() and SetOverflowLines() when there's lines remaining. r=bzbarsky a=bajaj
layout/generic/crashtests/847130.xhtml
layout/generic/crashtests/crashtests.list
layout/generic/nsBlockFrame.cpp
new file mode 100644
--- /dev/null
+++ b/layout/generic/crashtests/847130.xhtml
@@ -0,0 +1,15 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+<script>
+
+function boom()
+{
+  document.getElementById("x").appendChild(document.createElementNS("http://www.w3.org/1999/xhtml", "span"));
+}
+
+</script>
+</head>
+<body onload="boom();">
+<div style="-moz-column-count: 15;"><div style="-moz-column-count: 15;" id="x"><td style="display: block; height: 2.5em;"></td></div></div>
+</body>
+</html>
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@@ -425,8 +425,9 @@ 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 842132-1.html
+load 847130.xhtml
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -5531,23 +5531,19 @@ nsBlockFrame::DoRemoveFrame(nsIFrame* aD
         // cases...
 #ifdef NOISY_BLOCK_INVALIDATE
         nsRect visOverflow(cur->GetVisualOverflowArea());
         printf("%p invalidate 10 (%d, %d, %d, %d)\n",
                this, visOverflow.x, visOverflow.y,
                visOverflow.width, visOverflow.height);
 #endif
       } else {
-        // XXX update searchingOverflowList directly, remove only when empty
-        FrameLines* overflowLines = RemoveOverflowLines();
         line = overflowLines->mLines.erase(line);
-        if (!overflowLines->mLines.empty()) {
-          SetOverflowLines(overflowLines);
-        } else {
-          delete overflowLines;
+        if (overflowLines->mLines.empty()) {
+          DestroyOverflowLines();
           // We just invalidated our iterators.  Since we were in
           // the overflow lines list, which is now empty, set them
           // so we're at the end of the regular line list.
           line_start = mLines.begin();
           line_end = mLines.end();
           line = line_end;
         }
       }
@@ -5672,24 +5668,20 @@ nsBlockFrame::StealFrame(nsPresContext* 
         // Register removal with the line boxes
         line->NoteFrameRemoved(frame);
         if (line->GetChildCount() > 0) {
            line->MarkDirty();
         } else {
           // Remove the line box
           nsLineBox* lineBox = line;
           if (searchingOverflowList) {
-            // Erase line, but avoid making the overflow line list empty
-            // XXX update overflowLines directly, remove only when empty
-            RemoveOverflowLines();
+            // Erase the line, destroy the property if it was the last one.
             line = overflowLines->mLines.erase(line);
-            if (!overflowLines->mLines.empty()) {
-              SetOverflowLines(overflowLines);
-            } else {
-              delete overflowLines;
+            if (overflowLines->mLines.empty()) {
+              DestroyOverflowLines();
               // We just invalidated our iterators.  Since we were in
               // the overflow lines list, which is now empty, set them
               // so we're at the end of the regular line list.
               line_start = mLines.begin();
               line_end = mLines.end();
               line = line_end;
             }
           } else {