Bug 1535507 - Assume that we have an empty display list building override rect for frames that support it, even if an explicit one isn't present. r=miko
☠☠ backed out by 1df7ae29b121 ☠ ☠
authorMatt Woodrow <mwoodrow@mozilla.com>
Tue, 19 Mar 2019 22:24:36 +0000
changeset 465129 458563d6a69e9874abd2fafb8889da378a59678b
parent 465128 48726c586c4ffbf25505d117fea5c56172a6e8ce
child 465130 13e9a230eb255fb95dd9e0d40db978717afa6a05
push id35732
push useropoprus@mozilla.com
push dateWed, 20 Mar 2019 10:52:37 +0000
treeherdermozilla-central@708979f9c3f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiko
bugs1535507
milestone68.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 1535507 - Assume that we have an empty display list building override rect for frames that support it, even if an explicit one isn't present. r=miko If the frame supports it (stacking context + containing block for fixed), and a descendant was modified, we would have created an override dirty region with just the area of that descendant. In the case where no descendants are modified, we should use an empty rect, rather than the area inherited from our parent. This fixes the case where we forcibly build position:fixed frames (since they might async scroll differently to the rest of the page), but we only need to build the container item, not the whole frame subtree within it. Added a test that shows us building the non-intersecting position:fixed, but not items within it. Differential Revision: https://phabricator.services.mozilla.com/D23610
layout/generic/nsFrame.cpp
layout/reftests/display-list/reftest.list
layout/reftests/display-list/retained-dl-style-change-stacking-context-3-ref.html
layout/reftests/display-list/retained-dl-style-change-stacking-context-3.html
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2959,25 +2959,31 @@ void nsIFrame::BuildDisplayListForStacki
     // Restict the building area to the overflow rect for these frames, since
     // RetainedDisplayListBuilder uses it to know if the size of the stacking
     // context changed.
     visibleRect.IntersectRect(visibleRect, GetVisualOverflowRect());
     dirtyRect.IntersectRect(dirtyRect, GetVisualOverflowRect());
   }
 
   bool hasOverrideDirtyRect = false;
-  // If we have an override dirty region, and neither us nor our ancestors are
-  // modified, then use it.
-  if (HasOverrideDirtyRegion() && !aBuilder->InInvalidSubtree() &&
-      !IsFrameModified()) {
-    nsDisplayListBuilder::DisplayListBuildingData* data =
-        GetProperty(nsDisplayListBuilder::DisplayListBuildingRect());
-    if (data) {
-      dirtyRect = data->mDirtyRect.Intersect(visibleRect);
-      hasOverrideDirtyRect = true;
+  // If we're doing a partial build, we're not invalid and we're capable
+  // of having an override building rect (stacking context and fixed pos
+  // containing block), then we should assume we have one.
+  // Either we have an explicit one, or nothing in our subtree changed and
+  // we have an implicit empty rect.
+  if (aBuilder->IsPartialUpdate() && !aBuilder->InInvalidSubtree() &&
+      !IsFrameModified() && IsFixedPosContainingBlock()) {
+    dirtyRect = nsRect();
+    if (HasOverrideDirtyRegion()) {
+      nsDisplayListBuilder::DisplayListBuildingData* data =
+          GetProperty(nsDisplayListBuilder::DisplayListBuildingRect());
+      if (data) {
+        dirtyRect = data->mDirtyRect.Intersect(visibleRect);
+        hasOverrideDirtyRect = true;
+      }
     }
   }
 
   bool usingFilter = StyleEffects()->HasFilters();
   bool usingMask = nsSVGIntegrationUtils::UsingMaskOrClipPathForFrame(this);
   bool usingSVGEffects = usingFilter || usingMask;
 
   nsRect visibleRectOutsideSVGEffects = visibleRect;
--- a/layout/reftests/display-list/reftest.list
+++ b/layout/reftests/display-list/reftest.list
@@ -1,13 +1,14 @@
 skip-if(!retainedDisplayList) == retained-dl-style-change-1.html retained-dl-style-change-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-frame-deleted-1.html retained-dl-style-change-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-frame-created-1.html retained-dl-style-change-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-style-change-stacking-context-1.html retained-dl-style-change-stacking-context-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-style-change-stacking-context-2.html retained-dl-style-change-stacking-context-2-ref.html
+skip-if(!retainedDisplayList) == retained-dl-style-change-stacking-context-3.html retained-dl-style-change-stacking-context-3-ref.html
 skip-if(!retainedDisplayList||!asyncPan) == retained-dl-async-scrolled-1.html retained-dl-async-scrolled-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-remove-for-ancestor-change-1.html retained-dl-remove-for-ancestor-change-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-scroll-out-of-view-1.html retained-dl-scroll-out-of-view-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-displayport-1.html retained-dl-displayport-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-prerender-transform-1.html retained-dl-prerender-transform-1-ref.html
 skip-if(!retainedDisplayList) == retained-dl-animation-on-pseudo.html retained-dl-animation-on-pseudo-ref.html
 skip-if(!retainedDisplayList) == retained-dl-opacity-animation-on-ib-split.html retained-dl-opacity-animation-on-ib-split-ref.html
 == retained-dl-wrap-list.html retained-dl-wrap-list-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/retained-dl-style-change-stacking-context-3-ref.html
@@ -0,0 +1,20 @@
+<html>
+<head>
+<style>
+  body {
+    margin: 0px;
+  }
+  div {
+    width:100px;
+    height:100px;
+    display: inline-block;
+    position:absolute;
+    background-color: green;
+  }
+</style>
+</head>
+<body>
+  <div></div>
+  <div style="top: 110px;"></div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/display-list/retained-dl-style-change-stacking-context-3.html
@@ -0,0 +1,29 @@
+<html class="reftest-wait">
+<head>
+<style>
+  body {
+    margin: 0px;
+  }
+  div {
+    width:100px;
+    height:100px;
+    display: inline-block;
+    position:absolute;
+  }
+</style>
+</head>
+<body>
+  <div style="position:fixed;" class="reftest-display-list">
+    <div style="background-color:green;" class="reftest-no-display-list">
+  </div>
+  <div id="second" style="background-color:red; top: 110px;"></div>
+</body>
+<script>
+function doTest() {
+  document.getElementById("second").style.backgroundColor = "green";
+  document.documentElement.removeAttribute("class");
+}
+
+window.addEventListener("MozReftestInvalidate", doTest);
+</script>
+</html>