Bug 1449326: Account for min- / max- block size changes too in the flex caching code. r=dholbert a=lizzard
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 17 Apr 2018 13:30:04 +0200
changeset 463491 bb01c965834f75b4558353870c69d5dd0c20d4eb
parent 463490 37c3ffc007e5980779ebf06a02247916b895062f
child 463492 dc7d484e8d3dbbc21ab500aa1282bcfcf1ca89a1
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, lizzard
bugs1449326
milestone60.0
Bug 1449326: Account for min- / max- block size changes too in the flex caching code. r=dholbert a=lizzard We may be reflowing the same kid with different (definite / non-definite) containing block sizes, which changes the min block size in: https://searchfox.org/mozilla-central/rev/f65d7528e34ef1a7665b4a1a7b7cdb1388fcd3aa/layout/generic/ReflowInput.cpp#3077 In which case we may need to reflow in order to properly align the flex item. MozReview-Commit-ID: 27aS2UrgXjs
layout/generic/nsFlexContainerFrame.cpp
layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-001-ref.html
layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-001.html
layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-002.html
layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-003.html
layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-004.html
layout/reftests/w3c-css/submitted/flexbox/reftest.list
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -1658,36 +1658,55 @@ nsFlexContainerFrame::
  *
  * Caching it prevents us from doing exponential reflows in cases of deeply
  * nested flex and scroll frames.
  *
  * We store them in the frame property table for simplicity.
  */
 class nsFlexContainerFrame::CachedMeasuringReflowResult
 {
-  // Members that are part of the cache key:
-  const LogicalSize mAvailableSize;
-  const nscoord mComputedBSize;
-
-  // Members that are part of the cache value:
+  struct Key
+  {
+    const LogicalSize mAvailableSize;
+    const nscoord mComputedBSize;
+    const nscoord mComputedMinBSize;
+    const nscoord mComputedMaxBSize;
+
+    explicit Key(const ReflowInput& aRI)
+      : mAvailableSize(aRI.AvailableSize())
+      , mComputedBSize(aRI.ComputedBSize())
+      , mComputedMinBSize(aRI.ComputedMinBSize())
+      , mComputedMaxBSize(aRI.ComputedMaxBSize())
+    { }
+
+    bool operator==(const Key& aOther) const
+    {
+      return mAvailableSize == aOther.mAvailableSize &&
+        mComputedBSize == aOther.mComputedBSize &&
+        mComputedMinBSize == aOther.mComputedMinBSize &&
+        mComputedMaxBSize == aOther.mComputedMaxBSize;
+    }
+  };
+
+  const Key mKey;
+
   const nscoord mBSize;
   const nscoord mAscent;
 
 public:
   CachedMeasuringReflowResult(const ReflowInput& aReflowInput,
                               const ReflowOutput& aDesiredSize)
-    : mAvailableSize(aReflowInput.AvailableSize())
-    , mComputedBSize(aReflowInput.ComputedBSize())
+    : mKey(aReflowInput)
     , mBSize(aDesiredSize.BSize(aReflowInput.GetWritingMode()))
     , mAscent(aDesiredSize.BlockStartAscent())
-  {}
-
-  bool IsValidFor(const ReflowInput& aReflowInput) const {
-    return mAvailableSize == aReflowInput.AvailableSize() &&
-      mComputedBSize == aReflowInput.ComputedBSize();
+  { }
+
+  bool IsValidFor(const ReflowInput& aReflowInput) const
+  {
+    return mKey == Key(aReflowInput);
   }
 
   nscoord BSize() const { return mBSize; }
 
   nscoord Ascent() const { return mAscent; }
 };
 
 NS_DECLARE_FRAME_PROPERTY_DELETABLE(CachedFlexMeasuringReflow,
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-001-ref.html
@@ -0,0 +1,13 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test Reference</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<style>
+div {
+  width: 100px;
+  height: 100px;
+  background: green;
+}
+</style>
+<p>Test passes if you see a green 100px x 100px square, and no red</p>
+<div></div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-001.html
@@ -0,0 +1,30 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: nested flex containers with height established by 'min-height'</title>
+<link rel="match" href="flexbox-definite-sizes-001-ref.html">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#definite-sizes">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1449326">
+<style>
+div {
+  display: flex;
+}
+
+.item {
+  width: 100px;
+  background: red;
+  align-items: center;
+}
+
+.item span {
+  min-height: 100%;
+  width: 100%;
+  background: green;
+}
+</style>
+<p>Test passes if you see a green 100px x 100px square, and no red</p>
+<div style="min-height: 100px;">
+  <div class="item">
+    <span></span>
+  </div>
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-002.html
@@ -0,0 +1,31 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: nested flex containers with height established by 'min-height'</title>
+<link rel="match" href="flexbox-definite-sizes-001-ref.html">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#definite-sizes">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1449326">
+<style>
+div {
+  display: flex;
+}
+
+.item {
+  width: 100px;
+  background: red;
+  align-items: center;
+  min-height: 100px;
+}
+
+.item span {
+  min-height: 100%;
+  width: 100%;
+  background: green;
+}
+</style>
+<p>Test passes if you see a green 100px x 100px square, and no red</p>
+<div>
+  <div class="item">
+    <span></span>
+  </div>
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-003.html
@@ -0,0 +1,38 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: nested flex containers with definite max-height</title>
+<link rel="match" href="flexbox-definite-sizes-001-ref.html">
+<link rel="author" href="mailto:dholbert@mozilla.com" title="Daniel Holbert">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#definite-sizes">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1449326">
+<style>
+body { overflow: hidden }
+
+.outerFlex {
+  display: flex;
+  width: 100px;
+  /* Implicit "align-items:stretch" */
+}
+
+.innerFlex {
+  display: flex;
+  width: 100px;
+  background: red;
+
+  /* This reveals if we miscalculate the height of our flex item: */
+  align-items: flex-end;
+}
+
+.block {
+  width: 100px;
+  max-height: 100%;
+  background-color: green;
+}
+</style>
+<p>Test passes if you see a green 100px x 100px square, and no red</p>
+<div class="outerFlex" style="max-height: 100px">
+  <div class="innerFlex">
+    <div class="block"><div style="height:9999px"></div></div>
+  </div>
+</div>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/w3c-css/submitted/flexbox/flexbox-definite-sizes-004.html
@@ -0,0 +1,38 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: nested flex containers with definite max-height</title>
+<link rel="match" href="flexbox-definite-sizes-001-ref.html">
+<link rel="author" href="mailto:dholbert@mozilla.com" title="Daniel Holbert">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://drafts.csswg.org/css-flexbox/#definite-sizes">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1449326">
+<style>
+body { overflow: hidden }
+
+.outerFlex {
+  display: flex;
+  width: 100px;
+  /* Implicit "align-items:stretch" */
+}
+
+.innerFlex {
+  display: flex;
+  width: 100px;
+  background: red;
+
+  /* This reveals if we miscalculate the height of our flex item: */
+  align-items: flex-end;
+}
+
+.block {
+  width: 100px;
+  max-height: 100%;
+  background-color: green;
+}
+</style>
+<p>Test passes if you see a green 100px x 100px square, and no red</p>
+<div class="outerFlex">
+  <div class="innerFlex" style="max-height: 100px">
+    <div class="block"><div style="height:9999px"></div></div>
+  </div>
+</div>
--- a/layout/reftests/w3c-css/submitted/flexbox/reftest.list
+++ b/layout/reftests/w3c-css/submitted/flexbox/reftest.list
@@ -118,16 +118,22 @@ fuzzy-if(Android,158,32) == flexbox-alig
 == flexbox-intrinsic-ratio-003v.html flexbox-intrinsic-ratio-003-ref.html
 == flexbox-intrinsic-ratio-004.html flexbox-intrinsic-ratio-004-ref.html
 == flexbox-intrinsic-ratio-004v.html flexbox-intrinsic-ratio-004-ref.html
 == flexbox-intrinsic-ratio-005.html flexbox-intrinsic-ratio-005-ref.html
 == flexbox-intrinsic-ratio-005v.html flexbox-intrinsic-ratio-005-ref.html
 == flexbox-intrinsic-ratio-006.html flexbox-intrinsic-ratio-006-ref.html
 == flexbox-intrinsic-ratio-006v.html flexbox-intrinsic-ratio-006-ref.html
 
+# Test for definite and indefinite sizes.
+== flexbox-definite-sizes-001.html flexbox-definite-sizes-001-ref.html
+== flexbox-definite-sizes-002.html flexbox-definite-sizes-001-ref.html
+== flexbox-definite-sizes-003.html flexbox-definite-sizes-001-ref.html
+== flexbox-definite-sizes-004.html flexbox-definite-sizes-001-ref.html
+
 # Tests for flex items as (pseudo) stacking contexts
 == flexbox-items-as-stacking-contexts-001.xhtml flexbox-items-as-stacking-contexts-001-ref.xhtml
 == flexbox-items-as-stacking-contexts-002.html flexbox-items-as-stacking-contexts-002-ref.html
 == flexbox-items-as-stacking-contexts-003.html flexbox-items-as-stacking-contexts-003-ref.html
 
 # Tests for main-axis alignment (jusify-content property)
 == flexbox-justify-content-horiz-001a.xhtml flexbox-justify-content-horiz-001-ref.xhtml
 == flexbox-justify-content-horiz-001b.xhtml flexbox-justify-content-horiz-001-ref.xhtml