Bug 1489770 - Don't convert InlineBreak::Before reflow status to Incomplete unless we know the child frame is splittable. r=dholbert a=pascalc
authorMats Palmgren <mats@mozilla.com>
Tue, 11 Sep 2018 20:07:28 +0200
changeset 492509 2d928d5aead4300a7d7f247686a82fc39d724b61
parent 492508 2c477d1661b232efdeb5e0a24ae2b296c4093c39
child 492510 f86b30a8ec01c2f6605af20dbeb4acaceb3ee585
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, pascalc
bugs1489770
milestone63.0
Bug 1489770 - Don't convert InlineBreak::Before reflow status to Incomplete unless we know the child frame is splittable. r=dholbert a=pascalc
layout/generic/crashtests/1489770.html
layout/generic/crashtests/1490032.html
layout/generic/crashtests/crashtests.list
layout/generic/nsGridContainerFrame.cpp
new file mode 100644
--- /dev/null
+++ b/layout/generic/crashtests/1489770.html
@@ -0,0 +1,23 @@
+<style>
+#a {
+  grid-auto-rows: min-content;
+  display: grid;
+}
+:not(html) {
+  margin-bottom: -1vmin;
+  column-width: 0;
+}
+</style>
+<script>
+function go() {
+  a.appendChild(b);
+  document.documentElement.style.display = "none"
+  document.documentElement.getBoundingClientRect()
+  document.documentElement.style.display = ""
+}
+</script>
+<a id="a">A</a>
+<ul>
+<li>
+<audio onloadstart="go()" src="">
+<keygen id="b">
new file mode 100644
--- /dev/null
+++ b/layout/generic/crashtests/1490032.html
@@ -0,0 +1,20 @@
+<html>
+<head>
+  <title>AddressSanitizer: SEGV /builds/worker/workspace/build/src/obj-firefox/dist/include/nsTArray.h in Length</title>
+	<style class="">
+		@namespace math url(http://www.w3.org/1998/Math/MathML);
+		*>* {
+			display: grid;
+			max-block-size: calc(3*25px + 50%);
+			-webkit-align-self: start;
+			grid-template-rows: repeat(auto-fill, minmax(1ch, min-content)) minmax(min-content, 0);
+		}
+
+		*,
+		HTML {
+			-moz-columns: 2 10px;
+			page-break-inside: avoid !important;
+			
+  </style>
+</head>
+</html>
\ No newline at end of file
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@@ -701,10 +701,12 @@ load 1460158-3.html
 load 1461039.html
 load 1461979-1.html
 load 1467239.html
 load 1472403.html
 load 1474768.html
 load 1478178.html
 load 1483972.html
 load 1486457.html
+load 1489770.html
+load 1490032.html
 load 1489287.html
 load 1489863.html
--- a/layout/generic/nsGridContainerFrame.cpp
+++ b/layout/generic/nsGridContainerFrame.cpp
@@ -5472,23 +5472,23 @@ nsGridContainerFrame::ReflowRowsInFragme
           maxRowSize -= sz.mPosition;
           // ...and only if there is space for it to grow.
           rowCanGrow = maxRowSize > sz.mBase;
         }
       }
     }
 
     // aFragmentainer.mIsTopOfPage is propagated to the child reflow state.
-    // When it's false the child can request BREAK_BEFORE.  We intentionally
-    // set it to false when the row is growable (as determined in CSS Grid
-    // Fragmentation) and there is a non-zero space between it and the
+    // When it's false the child may request InlineBreak::Before.  We set it
+    // to false when the row is growable (as determined in the CSS Grid
+    // Fragmentation spec) and there is a non-zero space between it and the
     // fragmentainer end (that can be used to grow it).  If the child reports
     // a forced break in this case, we grow this row to fill the fragment and
     // restart the loop.  We also restart the loop with |aEndRow = row|
-    // (but without growing any row) for a BREAK_BEFORE child if it spans
+    // (but without growing any row) for a InlineBreak::Before child if it spans
     // beyond the last row in this fragment.  This is to avoid fragmenting it.
     // We only restart the loop once.
     aFragmentainer.mIsTopOfPage = isRowTopOfPage && !rowCanGrow;
     nsReflowStatus childStatus;
     // Pass along how much to stretch this fragment, in case it's needed.
     nscoord bSize =
       aState.mRows.GridLineEdge(std::min(aEndRow, info->mArea.mRows.mEnd),
                                 GridLineSide::eBeforeGridGap) -
@@ -5498,17 +5498,17 @@ nsGridContainerFrame::ReflowRowsInFragme
                       aState, aContentArea, aDesiredSize, childStatus);
     MOZ_ASSERT(childStatus.IsInlineBreakBefore() ||
                !childStatus.IsFullyComplete() ||
                !child->GetNextInFlow(),
                "fully-complete reflow should destroy any NIFs");
 
     if (childStatus.IsInlineBreakBefore()) {
       MOZ_ASSERT(!child->GetPrevInFlow(),
-                 "continuations should never report BREAK_BEFORE status");
+                 "continuations should never report InlineBreak::Before status");
       MOZ_ASSERT(!aFragmentainer.mIsTopOfPage,
                  "got IsInlineBreakBefore() at top of page");
       if (!didGrowRow) {
         if (rowCanGrow) {
           // Grow this row and restart with the next row as |aEndRow|.
           aState.mRows.ResizeRow(row, maxRowSize);
           if (aState.mSharedGridData) {
             aState.mSharedGridData->mRows.ResizeRow(row, maxRowSize);
@@ -5539,26 +5539,33 @@ nsGridContainerFrame::ReflowRowsInFragme
           aBSize = aState.mRows.GridLineEdge(aEndRow, GridLineSide::eBeforeGridGap);
           i = -1;  // i == 0 after the next loop increment
           isRowTopOfPage = isStartRowTopOfPage;
           overflowIncompleteItems.Clear();
           incompleteItems.Clear();
           aStatus.SetIncomplete();
           continue;
         }
-        NS_ERROR("got BREAK_BEFORE at top-of-page");
+        NS_ERROR("got InlineBreak::Before at top-of-page");
         childStatus.Reset();
       } else {
-        NS_ERROR("got BREAK_BEFORE again after growing the row?");
-        childStatus.SetIncomplete();
+        // We got InlineBreak::Before again after growing the row - this can happen
+        // if the child isn't splittable, e.g. some form controls.
+        childStatus.Reset();
+        if (child->GetNextInFlow()) {
+          // The child already has a fragment, so we know it's splittable.
+          childStatus.SetIncomplete();
+        } // else, report that it's complete
       }
     } else if (childStatus.IsInlineBreakAfter()) {
       MOZ_ASSERT_UNREACHABLE("unexpected child reflow status");
     }
 
+    MOZ_ASSERT(!childStatus.IsInlineBreakBefore(),
+               "should've handled InlineBreak::Before above");
     if (childStatus.IsIncomplete()) {
       incompleteItems.PutEntry(child);
     } else if (!childStatus.IsFullyComplete()) {
       overflowIncompleteItems.PutEntry(child);
     }
   }
 
   // Record a break before |aEndRow|.