Bug 1564308 - Always reflow all row groups and rows if we're doing visibility:collapse adjustments. r=dholbert
☠☠ backed out by b3964512ee41 ☠ ☠
authorL. David Baron <dbaron@dbaron.org>
Tue, 09 Jul 2019 23:03:40 +0000
changeset 482035 514565e608cba4d1c309dfd146c8d01746d62651
parent 482034 cb68677410a5c802e860eb4e40cad65ee1117750
child 482036 ab282375f89b90defa1fab455b00618a25bfd848
push id113647
push useraciure@mozilla.com
push dateWed, 10 Jul 2019 09:46:39 +0000
treeherdermozilla-inbound@f3a387c13e2c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1564308
milestone70.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 1564308 - Always reflow all row groups and rows if we're doing visibility:collapse adjustments. r=dholbert Differential Revision: https://phabricator.services.mozilla.com/D37467
layout/tables/crashtests/crashtests.list
layout/tables/nsTableFrame.cpp
layout/tables/nsTableRowGroupFrame.cpp
layout/tables/reftests/reftest.list
--- a/layout/tables/crashtests/crashtests.list
+++ b/layout/tables/crashtests/crashtests.list
@@ -92,17 +92,17 @@ load 391901-1.html
 load 392132-1.xhtml
 load 397448-1.html
 load 398157-1.xhtml
 load 399209-1.xhtml
 load 403249-1.html
 load 403579-1.html
 load 404301-1.xhtml
 load 408753-1.xhtml
-load 410426-1.html
+asserts(2) load 410426-1.html # hitting a width of NS_UNCONSTRAINEDSIZE
 load 410428-1.xhtml
 load 411582.xhtml
 load 413091.xhtml
 load 413180-1.html
 load 416845-1.xhtml
 load 416845-2.xhtml
 load 416845-3.html
 load 420242-1.xhtml
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -1764,17 +1764,18 @@ void nsTableFrame::Reflow(nsPresContext*
   bool mayAdjustXForAllChildren = false;
 
   // Reflow the entire table (pass 2 and possibly pass 3). This phase is
   // necessary during a constrained initial reflow and other reflows which
   // require either a strategy init or balance. This isn't done during an
   // unconstrained reflow, because it will occur later when the parent reflows
   // with a constrained isize.
   if (NS_SUBTREE_DIRTY(this) || aReflowInput.ShouldReflowAllKids() ||
-      IsGeometryDirty() || isPaginated || aReflowInput.IsBResize()) {
+      IsGeometryDirty() || isPaginated || aReflowInput.IsBResize() ||
+      NeedToCollapse()) {
     if (aReflowInput.ComputedBSize() != NS_UNCONSTRAINEDSIZE ||
         // Also check IsBResize(), to handle the first Reflow preceding a
         // special bsize Reflow, when we've already had a special bsize
         // Reflow (where ComputedBSize() would not be
         // NS_UNCONSTRAINEDSIZE, but without a style change in between).
         aReflowInput.IsBResize()) {
       // XXX Eventually, we should modify DistributeBSizeToRows to use
       // nsTableRowFrame::GetInitialBSize instead of nsIFrame::BSize().
@@ -1903,18 +1904,23 @@ void nsTableFrame::Reflow(nsPresContext*
   // until the fixupKidPositions loop just above.
   for (nsIFrame* kid : mFrames) {
     ConsiderChildOverflow(aDesiredSize.mOverflowAreas, kid);
   }
 
   LogicalMargin borderPadding = GetChildAreaOffset(wm, &aReflowInput);
   SetColumnDimensions(aDesiredSize.BSize(wm), wm, borderPadding,
                       aDesiredSize.PhysicalSize());
-  if (NeedToCollapse() &&
-      (NS_UNCONSTRAINEDSIZE != aReflowInput.AvailableISize())) {
+  NS_ASSERTION(NS_UNCONSTRAINEDSIZE != aReflowInput.AvailableISize(),
+               "reflow branch removed unconstrained available isizes");
+  if (NeedToCollapse()) {
+    // This code and the code it depends on assumes that all row groups
+    // and rows have just been reflowed (i.e., it makes adjustments to
+    // their rects that are not idempotent).  Thus the reflow code
+    // checks NeedToCollapse() to ensure this is true.
     AdjustForCollapsingRowsCols(aDesiredSize, wm, borderPadding);
   }
 
   // If there are any relatively-positioned table parts, we need to reflow their
   // absolutely-positioned descendants now that their dimensions are final.
   FixupPositionedTableParts(aPresContext, aDesiredSize, aReflowInput);
 
   // make sure the table overflow area does include the table rect.
@@ -2899,17 +2905,18 @@ void nsTableFrame::ReflowChildren(TableR
   // within the table structure.
   if (presContext->IsPaginated()) {
     SetGeometryDirty();
   }
 
   aOverflowAreas.Clear();
 
   bool reflowAllKids = aReflowInput.reflowInput.ShouldReflowAllKids() ||
-                       mBits.mResizedColumns || IsGeometryDirty();
+                       mBits.mResizedColumns || IsGeometryDirty() ||
+                       NeedToCollapse();
 
   RowGroupArray rowGroups;
   nsTableRowGroupFrame *thead, *tfoot;
   OrderRowGroups(rowGroups, &thead, &tfoot);
   bool pageBreak = false;
   nscoord footerHeight = 0;
 
   // Determine the repeatablility of headers and footers, and also the desired
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@@ -343,17 +343,18 @@ void nsTableRowGroupFrame::ReflowChildre
   // XXXldb Should we really be checking IsPaginated(),
   // or should we *only* check available block-size?
   // (Think about multi-column layout!)
   bool isPaginated = aPresContext->IsPaginated() &&
                      NS_UNCONSTRAINEDSIZE != aReflowInput.availSize.BSize(wm);
 
   bool haveRow = false;
   bool reflowAllKids = aReflowInput.reflowInput.ShouldReflowAllKids() ||
-                       tableFrame->IsGeometryDirty();
+                       tableFrame->IsGeometryDirty() ||
+                       tableFrame->NeedToCollapse();
 
   // in vertical-rl mode, we always need the row bsizes in order to
   // get the necessary containerSize for placing our kids
   bool needToCalcRowBSizes = reflowAllKids || wm.IsVerticalRL();
 
   nsSize containerSize =
       aReflowInput.reflowInput.ComputedSizeAsContainerIfConstrained();
 
--- a/layout/tables/reftests/reftest.list
+++ b/layout/tables/reftests/reftest.list
@@ -2,12 +2,12 @@
 == 1220621-1a.html 1220621-1-ref.html
 == 1220621-1b.html 1220621-1-ref.html
 == 1220621-1c.html 1220621-1-ref.html
 == 1220621-1d.html 1220621-1-ref.html
 == 1220621-1e.html 1220621-1-ref.html
 == 1220621-1f.html 1220621-1-ref.html
 == 1220621-2a.html 1220621-2-ref.html
 == 1220621-2b.html 1220621-2-ref.html
-fails == 1564308.html 1564308-ref.html
+== 1564308.html 1564308-ref.html
 == dynamic-text-overflow-table-cell.html dynamic-text-overflow-table-cell-ref.html
 != dynamic-text-overflow-table-cell.html dynamic-text-overflow-table-cell-notref.html
 == dynamic-text-indent-table-cell.html dynamic-text-indent-table-cell-ref.html