Bug 1576355 - Don't try to optimize position style changes for rel.pos. grid items. r=dholbert
authorMats Palmgren <mats@mozilla.com>
Mon, 26 Aug 2019 20:00:35 +0000
changeset 553696 e7511b57c48e74f369ab71162adc6699a2f9ee99
parent 553695 a902d015e909abb13de811988b58490111786cdb
child 553697 fdd1a25b68e1d43d41a77a3aaad0b330df51c36f
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1576355
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 1576355 - Don't try to optimize position style changes for rel.pos. grid items. r=dholbert Differential Revision: https://phabricator.services.mozilla.com/D43387
layout/base/RestyleManager.cpp
layout/generic/nsIFrame.h
layout/generic/nsIFrameInlines.h
testing/web-platform/tests/css/css-grid/grid-items/grid-item-rel-pos-001-ref.html
testing/web-platform/tests/css/css-grid/grid-items/grid-item-rel-pos-001.html
testing/web-platform/tests/css/css-grid/grid-items/grid-item-rel-pos-002-ref.html
testing/web-platform/tests/css/css-grid/grid-items/grid-item-rel-pos-002.html
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -717,16 +717,21 @@ static bool RecomputePosition(nsIFrame* 
   if (aFrame->DescendantMayDependOnItsStaticPosition()) {
     return false;
   }
 
   aFrame->SchedulePaint();
 
   // For relative positioning, we can simply update the frame rect
   if (display->IsRelativelyPositionedStyle()) {
+    if (aFrame->IsGridItem()) {
+      // A grid item's CB is its grid area, not the parent frame content area
+      // as is assumed below.
+      return false;
+    }
     // Move the frame
     if (display->mPosition == NS_STYLE_POSITION_STICKY) {
       // Update sticky positioning for an entire element at once, starting with
       // the first continuation or ib-split sibling.
       // It's rare that the frame we already have isn't already the first
       // continuation or ib-split sibling, but it can happen when styles differ
       // across continuations such as ::first-line or ::first-letter, and in
       // those cases we will generally (but maybe not always) do the work twice.
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -3940,16 +3940,20 @@ class nsIFrame : public nsQueryFrame {
   virtual void FindCloserFrameForSelection(
       const nsPoint& aPoint, FrameWithDistance* aCurrentBestFrame);
 
   /**
    * Is this a flex item? (i.e. a non-abs-pos child of a flex container)
    */
   inline bool IsFlexItem() const;
   /**
+   * Is this a grid item? (i.e. a non-abs-pos child of a grid container)
+   */
+  inline bool IsGridItem() const;
+  /**
    * Is this a flex or grid item? (i.e. a non-abs-pos child of a flex/grid
    * container)
    */
   inline bool IsFlexOrGridItem() const;
   inline bool IsFlexOrGridContainer() const;
 
   /**
    * @return true if this frame is used as a table caption.
--- a/layout/generic/nsIFrameInlines.h
+++ b/layout/generic/nsIFrameInlines.h
@@ -14,16 +14,21 @@
 #include "nsCSSAnonBoxes.h"
 #include "nsFrameManager.h"
 
 bool nsIFrame::IsFlexItem() const {
   return GetParent() && GetParent()->IsFlexContainerFrame() &&
          !(GetStateBits() & NS_FRAME_OUT_OF_FLOW);
 }
 
+bool nsIFrame::IsGridItem() const {
+  return GetParent() && GetParent()->IsGridContainerFrame() &&
+         !(GetStateBits() & NS_FRAME_OUT_OF_FLOW);
+}
+
 bool nsIFrame::IsFlexOrGridContainer() const {
   return IsFlexContainerFrame() || IsGridContainerFrame();
 }
 
 bool nsIFrame::IsFlexOrGridItem() const {
   return !(GetStateBits() & NS_FRAME_OUT_OF_FLOW) && GetParent() &&
          GetParent()->IsFlexOrGridContainer();
 }
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-grid/grid-items/grid-item-rel-pos-001-ref.html
@@ -0,0 +1,30 @@
+<!DOCTYPE HTML>
+<meta charset="utf-8">
+<title>Reference: Rel.pos. grid item with style change.</title>
+<link rel="author" title="James0x57" href="mailto:James0x57@gmail.com">
+<link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+<style>
+.grid {
+  margin: 40px;
+  display: grid;
+  grid: auto / repeat(3,100px);
+  grid-gap: 20px;
+}
+span, even {
+  position: relative;
+  min-height: 20px;
+  background: grey;
+  left: 0px;
+}
+.offset even {
+  left: 50%;
+}
+
+</style>
+
+
+<div class="grid offset">
+  <span></span><span></span><span></span>
+  <even></even><even></even><even></even>
+  <span></span><span></span><span></span>
+</div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-grid/grid-items/grid-item-rel-pos-001.html
@@ -0,0 +1,42 @@
+<!DOCTYPE HTML>
+<meta charset="utf-8">
+<title>CSS Grid Layout Test: Rel.pos. grid item with style change.</title>
+<link rel="author" title="James0x57" href="mailto:James0x57@gmail.com">
+<link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+<link rel="help" href="http://www.w3.org/TR/css-grid-1/#grid-items" title="Grid Items">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1576355">
+<link rel="match" href="grid-item-rel-pos-001-ref.html">
+<meta name="assert" content="Checks that the rel.pos. grid items are positioned correctly after a 'left' style change.">
+<style>
+.grid {
+  margin: 40px;
+  display: grid;
+  grid: auto / repeat(3,100px);
+  grid-gap: 20px;
+}
+span, even {
+  position: relative;
+  min-height: 20px;
+  background: grey;
+  left: 0px;
+}
+.offset even {
+  left: 50%;
+}
+
+</style>
+
+
+<div class="grid offset">
+  <span></span><span></span><span></span>
+  <even></even><even></even><even></even>
+  <span></span><span></span><span></span>
+</div>
+
+<script>
+  document.body.offsetHeight;
+  let grid = document.querySelector('.grid');
+  grid.classList.remove('offset')
+  document.body.offsetHeight;
+  grid.classList.add('offset')
+</script>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-grid/grid-items/grid-item-rel-pos-002-ref.html
@@ -0,0 +1,31 @@
+<!DOCTYPE HTML>
+<meta charset="utf-8">
+<title>Reference: Rel.pos. centered grid item with style change.</title>
+<link rel="author" title="James0x57" href="mailto:James0x57@gmail.com">
+<link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+<style>
+.grid {
+  margin: 40px;
+  display: grid;
+  grid: auto / repeat(3,100px);
+  grid-gap: 20px;
+}
+span, even {
+  position: relative;
+  min-height: 20px;
+  background: grey;
+  left: 0px;
+  width: 30px;
+  justify-self: center;
+}
+.offset even {
+  left: 20%;
+}
+
+</style>
+
+<div class="grid offset">
+  <span></span><span></span><span></span>
+  <even></even><even></even><even></even>
+  <span></span><span></span><span></span>
+</div>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-grid/grid-items/grid-item-rel-pos-002.html
@@ -0,0 +1,43 @@
+<!DOCTYPE HTML>
+<meta charset="utf-8">
+<title>CSS Grid Layout Test: Rel.pos. centered grid item with style change.</title>
+<link rel="author" title="James0x57" href="mailto:James0x57@gmail.com">
+<link rel="author" title="Mats Palmgren" href="mailto:mats@mozilla.com">
+<link rel="help" href="http://www.w3.org/TR/css-grid-1/#grid-items" title="Grid Items">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1576355">
+<link rel="match" href="grid-item-rel-pos-002-ref.html">
+<meta name="assert" content="Checks that the rel.pos. grid items are positioned correctly after a 'left' style change.">
+<style>
+.grid {
+  margin: 40px;
+  display: grid;
+  grid: auto / repeat(3,100px);
+  grid-gap: 20px;
+}
+span, even {
+  position: relative;
+  min-height: 20px;
+  background: grey;
+  left: 0px;
+  width: 30px;
+  justify-self: center;
+}
+.offset even {
+  left: 20%;
+}
+
+</style>
+
+<div class="grid offset">
+  <span></span><span></span><span></span>
+  <even></even><even></even><even></even>
+  <span></span><span></span><span></span>
+</div>
+
+<script>
+  document.body.offsetHeight;
+  let grid = document.querySelector('.grid');
+  grid.classList.remove('offset')
+  document.body.offsetHeight;
+  grid.classList.add('offset')
+</script>