Bug 1379458 - The ScrollParentId should match the ASR tree. r=botond,tnikkel,mstange

Make the ScrollMetadata's mScrollParentId match what is found in the active
scroll root tree.

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

layout/base/nsLayoutUtils.cpp

@@ -3318,6 +3318,9 @@ void nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame,
         autoRecording;
 
     ViewID id = ScrollableLayerGuid::NULL_SCROLL_ID;
+    nsDisplayListBuilder::AutoCurrentActiveScrolledRootSetter asrSetter(
+        builder);
+
     if (presShell->GetDocument() &&
         presShell->GetDocument()->IsRootDisplayDocument() &&
         !presShell->GetRootScrollFrame()) {
@@ -3350,7 +3353,7 @@ void nsLayoutUtils::PaintFrame(gfxContext* aRenderingContext, nsIFrame* aFrame,
       }
     }
 
-    nsDisplayListBuilder::AutoCurrentScrollParentIdSetter idSetter(builder, id);
+    asrSetter.SetCurrentScrollParentId(id);
 
     builder->SetVisibleRect(visibleRect);
     builder->SetIsBuilding(true);

layout/generic/nsGfxScrollFrame.cpp

@@ -4166,18 +4166,8 @@ void ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder* aBuilder,
   }
 
   {
-    // Note that setting the current scroll parent id here means that positioned
-    // children of this scroll info layer will pick up the scroll info layer as
-    // their scroll handoff parent. This is intentional because that is what
-    // happens for positioned children of scroll layers, and we want to maintain
-    // consistent behaviour between scroll layers and scroll info layers.
-    nsDisplayListBuilder::AutoCurrentScrollParentIdSetter idSetter(
-        aBuilder,
-        couldBuildLayer && mScrolledFrame->GetContent()
-            ? nsLayoutUtils::FindOrCreateIDFor(mScrolledFrame->GetContent())
-            : aBuilder->GetCurrentScrollParentId());
-
     DisplayListClipState::AutoSaveRestore clipState(aBuilder);
+
     // If we're building an async zoom container, clip the contents inside
     // to the layout viewport (scrollPortClip). The composition bounds clip
     // (clipRect) will be applied to the zoom container itself in
@@ -4310,13 +4300,13 @@ void ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder* aBuilder,
     if (aBuilder->IsPaintingToWindow()) {
       mIsParentToActiveScrollFrames =
           ShouldActivateAllScrollFrames()
-              ? idSetter.GetContainsNonMinimalDisplayPort()
-              : idSetter.ShouldForceLayerForScrollParent();
+              ? asrSetter.GetContainsNonMinimalDisplayPort()
+              : asrSetter.ShouldForceLayerForScrollParent();
     }
-    if (idSetter.ShouldForceLayerForScrollParent()) {
+    if (asrSetter.ShouldForceLayerForScrollParent()) {
       // Note that forcing layerization of scroll parents follows the scroll
       // handoff chain which is subject to the out-of-flow-frames caveat noted
-      // above (where the idSetter variable is created).
+      // above (where the asrSetter variable is created).
       MOZ_ASSERT(couldBuildLayer && mScrolledFrame->GetContent() &&
                  aBuilder->IsPaintingToWindow());
       if (!mWillBuildScrollableLayer) {

layout/generic/nsSubDocumentFrame.cpp

@@ -464,14 +464,6 @@ void nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                                            visible, dirty);
 
     if (subdocRootFrame) {
-      nsIFrame* rootScrollFrame = presShell->GetRootScrollFrame();
-      nsDisplayListBuilder::AutoCurrentScrollParentIdSetter idSetter(
-          aBuilder,
-          ignoreViewportScrolling && rootScrollFrame &&
-                  rootScrollFrame->GetContent()
-              ? nsLayoutUtils::FindOrCreateIDFor(rootScrollFrame->GetContent())
-              : aBuilder->GetCurrentScrollParentId());
-
       bool hasDocumentLevelListenersForApzAwareEvents =
           gfxPlatform::AsyncPanZoomEnabled() &&
           nsLayoutUtils::HasDocumentLevelListenersForApzAwareEvents(presShell);

layout/painting/nsDisplayList.cpp

@@ -394,6 +394,10 @@ void nsDisplayListBuilder::AutoCurrentActiveScrolledRootSetter::
   // Set the builder's mCurrentActiveScrolledRoot.
   mBuilder->mCurrentActiveScrolledRoot = aActiveScrolledRoot;
 
+  // Update the current scroll parent id to match the new
+  // active scrolled root.
+  UpdateCurrentScrollParentId();
+
   // We also need to adjust the builder's mCurrentContainerASR.
   // mCurrentContainerASR needs to be an ASR that all the container's
   // contents have finite bounds with respect to. If aActiveScrolledRoot

layout/painting/nsDisplayList.h

@@ -1101,25 +1101,41 @@ class nsDisplayListBuilder {
   };
 
   /**
-   * A helper class to temporarily set the value of mCurrentScrollParentId.
+   * Used to update the current active scrolled root on the display list
+   * builder, and to create new active scrolled roots.
    */
-  class AutoCurrentScrollParentIdSetter {
+  class AutoCurrentActiveScrolledRootSetter {
    public:
-    AutoCurrentScrollParentIdSetter(nsDisplayListBuilder* aBuilder,
-                                    ViewID aScrollId)
+    explicit AutoCurrentActiveScrolledRootSetter(nsDisplayListBuilder* aBuilder)
         : mBuilder(aBuilder),
-          mOldValue(aBuilder->mCurrentScrollParentId),
+          mSavedActiveScrolledRoot(aBuilder->mCurrentActiveScrolledRoot),
+          mContentClipASR(aBuilder->ClipState().GetContentClipASR()),
+          mDescendantsStartIndex(aBuilder->mActiveScrolledRoots.Length()),
+          mUsed(false),
+          mOldScrollParentId(aBuilder->mCurrentScrollParentId),
           mOldForceLayer(aBuilder->mForceLayerForScrollParent),
           mOldContainsNonMinimalDisplayPort(
-              mBuilder->mContainsNonMinimalDisplayPort) {
-      // If this AutoCurrentScrollParentIdSetter has the same scrollId as the
-      // previous one on the stack, then that means the scrollframe that
+              mBuilder->mContainsNonMinimalDisplayPort),
+          mCanBeScrollParent(true) {
+      UpdateCurrentScrollParentId();
+    }
+
+    void SetCurrentScrollParentId(ViewID aScrollId) {
+      // If this AutoCurrentActiveScrolledRootSetter has the same aScrollId as
+      // the previous one on the stack, then that means the scrollframe that
       // created this isn't actually scrollable and cannot participate in
       // scroll handoff. We set mCanBeScrollParent to false to indicate this.
-      mCanBeScrollParent = (mOldValue != aScrollId);
-      aBuilder->mCurrentScrollParentId = aScrollId;
-      aBuilder->mForceLayerForScrollParent = false;
-      aBuilder->mContainsNonMinimalDisplayPort = false;
+      mCanBeScrollParent = (mOldScrollParentId != aScrollId);
+      mBuilder->mCurrentScrollParentId = aScrollId;
+      mBuilder->mForceLayerForScrollParent = false;
+      mBuilder->mContainsNonMinimalDisplayPort = false;
+    }
+
+    void UpdateCurrentScrollParentId() {
+      ViewID scrollId = mBuilder->mCurrentActiveScrolledRoot
+                            ? mBuilder->mCurrentActiveScrolledRoot->GetViewId()
+                            : layers::ScrollableLayerGuid::NULL_SCROLL_ID;
+      SetCurrentScrollParentId(scrollId);
     }
 
     bool ShouldForceLayerForScrollParent() const {
@@ -1134,8 +1150,9 @@ class nsDisplayListBuilder {
       return mCanBeScrollParent && mBuilder->mContainsNonMinimalDisplayPort;
     }
 
-    ~AutoCurrentScrollParentIdSetter() {
-      mBuilder->mCurrentScrollParentId = mOldValue;
+    ~AutoCurrentActiveScrolledRootSetter() {
+      mBuilder->mCurrentActiveScrolledRoot = mSavedActiveScrolledRoot;
+      mBuilder->mCurrentScrollParentId = mOldScrollParentId;
       if (mCanBeScrollParent) {
         // If this flag is set, caller code is responsible for having dealt
         // with the current value of mBuilder->mForceLayerForScrollParent, so
@@ -1151,31 +1168,6 @@ class nsDisplayListBuilder {
           mOldContainsNonMinimalDisplayPort;
     }
 
-   private:
-    nsDisplayListBuilder* mBuilder;
-    ViewID mOldValue;
-    bool mOldForceLayer;
-    bool mOldContainsNonMinimalDisplayPort;
-    bool mCanBeScrollParent;
-  };
-
-  /**
-   * Used to update the current active scrolled root on the display list
-   * builder, and to create new active scrolled roots.
-   */
-  class AutoCurrentActiveScrolledRootSetter {
-   public:
-    explicit AutoCurrentActiveScrolledRootSetter(nsDisplayListBuilder* aBuilder)
-        : mBuilder(aBuilder),
-          mSavedActiveScrolledRoot(aBuilder->mCurrentActiveScrolledRoot),
-          mContentClipASR(aBuilder->ClipState().GetContentClipASR()),
-          mDescendantsStartIndex(aBuilder->mActiveScrolledRoots.Length()),
-          mUsed(false) {}
-
-    ~AutoCurrentActiveScrolledRootSetter() {
-      mBuilder->mCurrentActiveScrolledRoot = mSavedActiveScrolledRoot;
-    }
-
     void SetCurrentActiveScrolledRoot(
         const ActiveScrolledRoot* aActiveScrolledRoot);
 
@@ -1185,6 +1177,7 @@ class nsDisplayListBuilder {
           mBuilder->mCurrentActiveScrolledRoot, aScrollableFrame);
       mBuilder->mCurrentActiveScrolledRoot = asr;
       mUsed = true;
+      UpdateCurrentScrollParentId();
     }
 
     void InsertScrollFrame(nsIScrollableFrame* aScrollableFrame);
@@ -1215,6 +1208,10 @@ class nsDisplayListBuilder {
      * class.
      */
     bool mUsed;
+    ViewID mOldScrollParentId;
+    bool mOldForceLayer;
+    bool mOldContainsNonMinimalDisplayPort;
+    bool mCanBeScrollParent;
   };
 
   /**