From 3859e5d739601145717f45b52a28d52a9e471742 Mon Sep 17 00:00:00 2001
From: Matt Woodrow <mwoodrow@mozilla.com>
Date: Tue, 26 Mar 2019 02:14:50 +0000
Subject: [PATCH] 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

--HG--
extra : moz-landing-system : lando
---
 layout/generic/nsFrame.cpp                    | 24 +++++++++------
 layout/reftests/display-list/reftest.list     |  1 +
 ...l-style-change-stacking-context-3-ref.html | 20 +++++++++++++
 ...ed-dl-style-change-stacking-context-3.html | 29 +++++++++++++++++++
 4 files changed, 65 insertions(+), 9 deletions(-)
 create mode 100644 layout/reftests/display-list/retained-dl-style-change-stacking-context-3-ref.html
 create mode 100644 layout/reftests/display-list/retained-dl-style-change-stacking-context-3.html

diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp
index 0dbb04729a69..f7c0d1ab16f4 100644
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -2964,15 +2964,21 @@ void nsIFrame::BuildDisplayListForStackingContext(
   }
 
   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;
+      }
     }
   }
 
diff --git a/layout/reftests/display-list/reftest.list b/layout/reftests/display-list/reftest.list
index be4b7cc1b554..cd1614a8c8b6 100644
--- a/layout/reftests/display-list/reftest.list
+++ b/layout/reftests/display-list/reftest.list
@@ -3,6 +3,7 @@ skip-if(!retainedDisplayList) == retained-dl-frame-deleted-1.html retained-dl-st
 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
diff --git a/layout/reftests/display-list/retained-dl-style-change-stacking-context-3-ref.html b/layout/reftests/display-list/retained-dl-style-change-stacking-context-3-ref.html
new file mode 100644
index 000000000000..0da0ec14f10d
--- /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>
diff --git a/layout/reftests/display-list/retained-dl-style-change-stacking-context-3.html b/layout/reftests/display-list/retained-dl-style-change-stacking-context-3.html
new file mode 100644
index 000000000000..faef00a1851a
--- /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>