Bug 1128354 - Don't optimize away a flex item's second reflow, if it has percent-height children. r=mats, a=sledru
authorDaniel Holbert <dholbert@cs.stanford.edu>
Thu, 19 Mar 2015 23:00:05 -0700
changeset 257895 0936a835649d0f2332a4e7864c2f3a7f34662027
parent 257894 14e163e39e344e477203d756b353eff4149dc26c
child 257896 d885f561788f38c68c0a9e03d91b43c282430147
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats, sledru
bugs1128354
milestone38.0a2
Bug 1128354 - Don't optimize away a flex item's second reflow, if it has percent-height children. r=mats, a=sledru
layout/generic/nsFlexContainerFrame.cpp
layout/reftests/bugs/1128354-1-ref.html
layout/reftests/bugs/1128354-1.html
layout/reftests/bugs/reftest.list
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -3690,21 +3690,29 @@ nsFlexContainerFrame::DoFlexLayout(nsPre
       // Check if we actually need to reflow the item -- if we already reflowed
       // it with the right size, we can just reposition it as-needed.
       bool itemNeedsReflow = true; // (Start out assuming the worst.)
       if (item->HadMeasuringReflow()) {
         // We've already reflowed the child once. Was the size we gave it in
         // that reflow the same as its final (post-flexing/stretching) size?
         if (finalFlexedPhysicalSize ==
             item->Frame()->GetContentRectRelativeToSelf().Size()) {
-          // It has the correct size --> no need to reflow! Just make sure it's
-          // at the right position.
-          itemNeedsReflow = false;
-          MoveFlexItemToFinalPosition(aReflowState, *item, framePos,
-                                      containerWidth);
+          // Even if our size hasn't changed, some of our descendants might
+          // care that our height is now considered "definite" (whereas it
+          // wasn't in our previous "measuring" reflow), if they have a
+          // relative height.
+          if (!(item->Frame()->GetStateBits() &
+                NS_FRAME_CONTAINS_RELATIVE_HEIGHT)) {
+            // Item has the correct size (and its children don't care that
+            // it's now "definite"). Let's just make sure it's at the right
+            // position.
+            itemNeedsReflow = false;
+            MoveFlexItemToFinalPosition(aReflowState, *item, framePos,
+                                        containerWidth);
+          }
         }
       }
       if (itemNeedsReflow) {
         ReflowFlexItem(aPresContext, aAxisTracker, aReflowState,
                        *item, framePos, containerWidth);
       }
 
       // If this is our first child and we haven't established a baseline for
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1128354-1-ref.html
@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <style>
+      .flexVert {
+        display: flex;
+        flex-direction: column;
+      }
+
+      .flexIntermediateHoriz {
+        display: flex;
+      }
+
+      .flexInnerHoriz {
+        display: flex;
+        background: pink;
+      }
+
+      .spacer {
+        background: lightblue;
+        height: 200px;
+        width: 50px;
+      }
+    </style>
+  </head>
+  <body>
+    <div class="flexVert">
+      <div class="flexIntermediateHoriz">
+        <div class="flexInnerHoriz">text</div>
+        <div class="spacer"></div>
+      </div>
+    </div>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/1128354-1.html
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <style>
+      .flexVert {
+        display: flex;
+        flex-direction: column;
+      }
+
+      .flexIntermediateHoriz {
+        display: flex;
+      }
+
+      .flexInnerHoriz {
+        display: flex;
+        height: 100%; /* If you delete this line, then pink area is stretched
+                         to have its height match blue area. */
+        background: pink;
+      }
+
+      .spacer {
+        background: lightblue;
+        height: 200px;
+        width: 50px;
+      }
+    </style>
+  </head>
+  <body>
+    <div class="flexVert">
+      <div class="flexIntermediateHoriz">
+        <div class="flexInnerHoriz">text</div>
+        <div class="spacer"></div>
+      </div>
+    </div>
+  </body>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1860,10 +1860,11 @@ fuzzy-if(d2d,36,304) HTTP(..) == 1116480
 == 1120431-1.html 1120431-1-ref.html
 == 1120431-2.html 1120431-2-ref.html
 == 1121748-1.html 1121748-1-ref.html
 == 1121748-2.html 1121748-2-ref.html
 == 1127107-1a-nowrap.html 1127107-1-ref.html
 == 1127107-1b-pre.html 1127107-1-ref.html
 == 1127107-2-capitalize.html 1127107-2-capitalize-ref.html
 == 1127679-1a-inline-flex-relpos.html 1127679-1b-inline-flex-relpos.html
+== 1128354-1.html 1128354-1-ref.html
 == 1130231-1-button-padding-rtl.html 1130231-1-button-padding-rtl-ref.html
 == 1130231-2-button-padding-rtl.html 1130231-2-button-padding-rtl-ref.html