Bug 1727016. nsDisplayMasksAndClipPaths::CreateWebRenderCommands should only instruct WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList to create an override for the current ASR if it created a StackingContextHelper. r=miko

When we call WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList one of the first things it does is call mClipManager.BeginList(aSc) which checks the StackingContextHelper to see if it needs to push an override for the scrollid of the current ASR. The problem is that sometimes we don't create a new StackingContextHelper, so we are pushing an override for a different ASR but for the same StackingContextHelper we've already pushed an override for.

In this case nsDisplayMasksAndClipPaths::CreateWebRenderCommands sometimes creates a StackingContextHelper, and sometimes it does not: https://searchfox.org/mozilla-central/rev/1e7f7235cf822e79cd79ba9e200329ede3d37925/layout/painting/nsDisplayList.cpp#8117 when it does not create a StackingContextHelper (in this case because the clip-path was not found) we experience the bug.

I audited other callers of WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList and fixed the ones that also do not create a StackingContextHelper.

Differential Revision: https://phabricator.services.mozilla.com/D130255

layout/painting/nsDisplayList.cpp

@@ -2653,7 +2653,8 @@ bool nsDisplayContainer::CreateWebRenderCommands(
     const StackingContextHelper& aSc, RenderRootStateManager* aManager,
     nsDisplayListBuilder* aDisplayListBuilder) {
   aManager->CommandBuilder().CreateWebRenderCommandsFromDisplayList(
-      GetChildren(), this, aDisplayListBuilder, aSc, aBuilder, aResources);
+      GetChildren(), this, aDisplayListBuilder, aSc, aBuilder, aResources,
+      false);
   return true;
 }
 
@@ -4607,12 +4608,13 @@ nsRect nsDisplayWrapList::GetComponentAlphaBounds(
   return mListPtr->GetComponentAlphaBounds(aBuilder);
 }
 
-bool nsDisplayWrapList::CreateWebRenderCommands(
+bool nsDisplayWrapList::CreateWebRenderCommandsNewClipListOption(
     wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources,
     const StackingContextHelper& aSc, RenderRootStateManager* aManager,
-    nsDisplayListBuilder* aDisplayListBuilder) {
+    nsDisplayListBuilder* aDisplayListBuilder, bool aNewClipList) {
   aManager->CommandBuilder().CreateWebRenderCommandsFromDisplayList(
-      GetChildren(), this, aDisplayListBuilder, aSc, aBuilder, aResources);
+      GetChildren(), this, aDisplayListBuilder, aSc, aBuilder, aResources,
+      aNewClipList);
   return true;
 }
 
@@ -8138,8 +8140,8 @@ bool nsDisplayMasksAndClipPaths::CreateWebRenderCommands(
   aBuilder.SetInheritedOpacity(1.0f);
   const DisplayItemClipChain* oldClipChain = aBuilder.GetInheritedClipChain();
   aBuilder.SetInheritedClipChain(nullptr);
-  nsDisplayEffectsBase::CreateWebRenderCommands(aBuilder, aResources, *sc,
-                                                aManager, aDisplayListBuilder);
+  CreateWebRenderCommandsNewClipListOption(aBuilder, aResources, *sc, aManager,
+                                           aDisplayListBuilder, layer.isSome());
   aBuilder.SetInheritedOpacity(oldOpacity);
   aBuilder.SetInheritedClipChain(oldClipChain);
 
@@ -8423,8 +8425,8 @@ bool nsDisplaySVGWrapper::CreateWebRenderCommands(
     wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources,
     const StackingContextHelper& aSc, RenderRootStateManager* aManager,
     nsDisplayListBuilder* aDisplayListBuilder) {
-  return nsDisplayWrapList::CreateWebRenderCommands(
-      aBuilder, aResources, aSc, aManager, aDisplayListBuilder);
+  return CreateWebRenderCommandsNewClipListOption(
+      aBuilder, aResources, aSc, aManager, aDisplayListBuilder, false);
 }
 
 nsDisplayForeignObject::nsDisplayForeignObject(nsDisplayListBuilder* aBuilder,
@@ -8450,8 +8452,8 @@ bool nsDisplayForeignObject::CreateWebRenderCommands(
     nsDisplayListBuilder* aDisplayListBuilder) {
   AutoRestore<bool> restoreDoGrouping(aManager->CommandBuilder().mDoGrouping);
   aManager->CommandBuilder().mDoGrouping = false;
-  return nsDisplayWrapList::CreateWebRenderCommands(
-      aBuilder, aResources, aSc, aManager, aDisplayListBuilder);
+  return CreateWebRenderCommandsNewClipListOption(
+      aBuilder, aResources, aSc, aManager, aDisplayListBuilder, false);
 }
 
 void nsDisplayLink::Paint(nsDisplayListBuilder* aBuilder, gfxContext* aCtx) {

layout/painting/nsDisplayList.h

@@ -4838,7 +4838,18 @@ class nsDisplayWrapList : public nsPaintedDisplayItem {
       wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources,
       const StackingContextHelper& aSc,
       layers::RenderRootStateManager* aManager,
-      nsDisplayListBuilder* aDisplayListBuilder) override;
+      nsDisplayListBuilder* aDisplayListBuilder) override {
+    return CreateWebRenderCommandsNewClipListOption(
+        aBuilder, aResources, aSc, aManager, aDisplayListBuilder, true);
+  }
+
+  // Same as the above but with the option to pass the aNewClipList argument to
+  // WebRenderCommandBuilder::CreateWebRenderCommandsFromDisplayList.
+  bool CreateWebRenderCommandsNewClipListOption(
+      wr::DisplayListBuilder& aBuilder, wr::IpcResourceUpdateQueue& aResources,
+      const StackingContextHelper& aSc,
+      layers::RenderRootStateManager* aManager,
+      nsDisplayListBuilder* aDisplayListBuilder, bool aNewClipList);
 
   const ActiveScrolledRoot* GetFrameActiveScrolledRoot() {
     return mFrameActiveScrolledRoot;

layout/reftests/bugs/1727016-1-ref.html

@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html reftest-async-scroll reftest-async-scroll-y="100">
+   <head>
+    <style>
+
+.mask--6 .mask__clip-path{
+    /* clip-path:url(#notfound) */
+}
+
+.mask--6 .mask__container{
+    padding-top:117%
+}
+.mask__container{
+    position:relative
+}
+.mask__clip-path{
+    background:blue;
+    position:absolute;
+    width: 100px;
+    height: 200px;
+}
+    </style>
+
+   </head>
+   <body>
+
+  <div style="height: 1vh;"></div>
+
+  <div class="mask--6" style="max-width: 500px;">
+     <div class="mask__container">
+        <i class="mask__clip-path"></i>
+     </div>
+  </div>
+
+  <div style="height: 4000vh;"></div>

layout/reftests/bugs/1727016-1.html

@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<html reftest-async-scroll reftest-async-scroll-y="100">
+   <head>
+    <style>
+
+.mask--6 .mask__clip-path{
+    clip-path:url(#notfound)
+}
+
+.mask--6 .mask__container{
+    padding-top:117%
+}
+.mask__container{
+    position:relative
+}
+.mask__clip-path{
+    background:blue;
+    position:absolute;
+    width: 100px;
+    height: 200px;
+}
+    </style>
+
+   </head>
+   <body>
+
+  <div style="height: 1vh;"></div>
+
+  <div class="mask--6" style="max-width: 500px;">
+     <div class="mask__container">
+        <i class="mask__clip-path"></i>
+     </div>
+  </div>
+
+  <div style="height: 4000vh;"></div>

layout/reftests/bugs/reftest.list

@@ -2103,5 +2103,6 @@ fuzzy(0-2,0-96600) == 1648282-1b.html 1648282-1-ref.html
 skip-if(!appleSilicon) != 1721223-1.html 1721223-1-notref.html # Big Sur required for updated system font; OSX value is clamped to 10.15
 skip-if(Android) == 1727172-1.xhtml 1727172-1-ref.html
 == 1726663-1.html 1726663-1-ref.html
+== 1727016-1.html 1727016-1-ref.html
 != 1730314-1.html 1730314-1-ref.html
 fuzzy(0-3,0-3) fuzzy-if(Android,0-3,0-1901) fuzzy-if(winWidget,0-154,0-118) == 1738700-1.html 1738700-1-ref.html