Bug 1675709. Make sure to only call NotifyApzTransaction on scroll frames after the paint transaction is complete in the webrender case. r=mattwoodrow

In bug 1674935 we encountered a situation where NotifyApzTransaction was called more than once on a scroll frame in one paint transaction, clearing the scroll updates before they should have been. In that bug I moved the NotifyApzTransaction calls to happen at the end of one transaction for the non-wr code path., but I left the wr code path alone to reduce risk becase we didn't have proof the same could happen with wr. In this bug I will change how the wr code path works to ensure that this problem cannot happen.

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

gfx/layers/wr/WebRenderCommandBuilder.cpp

@@ -1552,7 +1552,7 @@ void WebRenderCommandBuilder::BuildWebRenderCommands(
   AUTO_PROFILER_LABEL_CATEGORY_PAIR(GRAPHICS_WRDisplayList);
 
   StackingContextHelper sc;
-  aScrollData = WebRenderScrollData(mManager);
+  aScrollData = WebRenderScrollData(mManager, aDisplayListBuilder);
   MOZ_ASSERT(mLayerScrollData.empty());
   mClipManager.BeginBuild(mManager, aBuilder);
   mBuilderDumpIndex = 0;
@@ -1926,8 +1926,6 @@ bool BuildLayer(nsDisplayItem* aItem, BlobItemData* aData,
   RefPtr<Layer> root = aItem->AsPaintedDisplayItem()->BuildLayer(
       aDisplayListBuilder, blm, param);
 
-  aDisplayListBuilder->NotifyAndClearScrollFrames();
-
   if (root) {
     blm->SetRoot(root);
     layerBuilder->WillEndTransaction();
@@ -1970,8 +1968,6 @@ static bool PaintByLayer(nsDisplayItem* aItem,
   RefPtr<Layer> root = aItem->AsPaintedDisplayItem()->BuildLayer(
       aDisplayListBuilder, aManager, param);
 
-  aDisplayListBuilder->NotifyAndClearScrollFrames();
-
   if (root) {
     aManager->SetRoot(root);
     layerBuilder->WillEndTransaction();

gfx/layers/wr/WebRenderLayerManager.cpp

@@ -358,6 +358,8 @@ void WebRenderLayerManager::EndTransactionWithoutLayer(
         builder, resourceUpdates, aDisplayList, aDisplayListBuilder,
         mScrollData, std::move(aFilters));
 
+    aDisplayListBuilder->NotifyAndClearScrollFrames();
+
     builderDumpIndex = mWebRenderCommandBuilder.GetBuilderDumpIndex();
     containsSVGGroup = mWebRenderCommandBuilder.GetContainsSVGGroup();
   } else {

gfx/layers/wr/WebRenderScrollData.cpp

@@ -63,7 +63,7 @@ void WebRenderLayerScrollData::Initialize(
       Maybe<ScrollMetadata> metadata =
           asr->mScrollableFrame->ComputeScrollMetadata(
               aOwner.GetManager(), aItem->ReferenceFrame(), Nothing(), nullptr);
-      asr->mScrollableFrame->NotifyApzTransaction();
+      aOwner.GetBuilder()->AddScrollFrameToNotify(asr->mScrollableFrame);
       if (metadata) {
         MOZ_ASSERT(metadata->GetMetrics().GetScrollId() == scrollId);
         mScrollIds.AppendElement(aOwner.AddMetadata(metadata.ref()));
@@ -163,13 +163,21 @@ void WebRenderLayerScrollData::Dump(std::ostream& aOut,
 WebRenderScrollData::WebRenderScrollData()
     : mManager(nullptr), mIsFirstPaint(false), mPaintSequenceNumber(0) {}
 
-WebRenderScrollData::WebRenderScrollData(WebRenderLayerManager* aManager)
+WebRenderScrollData::WebRenderScrollData(WebRenderLayerManager* aManager,
-    : mManager(aManager), mIsFirstPaint(false), mPaintSequenceNumber(0) {}
+                                         nsDisplayListBuilder* aBuilder)
+    : mManager(aManager),
+      mBuilder(aBuilder),
+      mIsFirstPaint(false),
+      mPaintSequenceNumber(0) {}
 
 WebRenderLayerManager* WebRenderScrollData::GetManager() const {
   return mManager;
 }
 
+nsDisplayListBuilder* WebRenderScrollData::GetBuilder() const {
+  return mBuilder;
+}
+
 size_t WebRenderScrollData::AddMetadata(const ScrollMetadata& aMetadata) {
   ScrollableLayerGuid::ViewID scrollId = aMetadata.GetMetrics().GetScrollId();
   auto p = mScrollIdMap.lookupForAdd(scrollId);

gfx/layers/wr/WebRenderScrollData.h

@@ -24,6 +24,7 @@
 #include "mozilla/Maybe.h"
 #include "nsTArrayForwardDeclare.h"
 
+class nsDisplayListBuilder;
 class nsDisplayItem;
 
 namespace mozilla {
@@ -211,10 +212,13 @@ class WebRenderLayerScrollData final {
 class WebRenderScrollData final {
  public:
   WebRenderScrollData();
-  explicit WebRenderScrollData(WebRenderLayerManager* aManager);
+  explicit WebRenderScrollData(WebRenderLayerManager* aManager,
+                               nsDisplayListBuilder* aBuilder);
 
   WebRenderLayerManager* GetManager() const;
 
+  nsDisplayListBuilder* GetBuilder() const;
+
   // Add the given ScrollMetadata if it doesn't already exist. Return an index
   // that can be used to look up the metadata later.
   size_t AddMetadata(const ScrollMetadata& aMetadata);
@@ -259,6 +263,11 @@ class WebRenderScrollData final {
   // outlive |this|.
   WebRenderLayerManager* MOZ_NON_OWNING_REF mManager;
 
+  // Pointer to the display list builder; if this is non-null, it will always be
+  // valid, because the nsDisplayListBuilder that created the layer manager will
+  // outlive |this|.
+  nsDisplayListBuilder* MOZ_NON_OWNING_REF mBuilder;
+
   // Internal data structure used to maintain uniqueness of mScrollMetadatas.
   // This is not serialized/deserialized over IPC, but it is rebuilt on the
   // parent side when mScrollMetadatas is deserialized. So it should always be

layout/painting/FrameLayerBuilder.cpp

@@ -5044,8 +5044,8 @@ void ContainerState::ProcessDisplayItems(nsDisplayList* aList) {
         nsDisplayScrollInfoLayer* scrollItem =
             static_cast<nsDisplayScrollInfoLayer*>(item);
         newLayerEntry->mOpaqueForAnimatedGeometryRootParent = false;
-        newLayerEntry->mBaseScrollMetadata =
+        newLayerEntry->mBaseScrollMetadata = scrollItem->ComputeScrollMetadata(
-            scrollItem->ComputeScrollMetadata(ownLayer->Manager(), mParameters);
+            mBuilder, ownLayer->Manager(), mParameters);
       }
 
       /**

layout/painting/nsDisplayList.cpp

@@ -7177,7 +7177,7 @@ LayerState nsDisplayScrollInfoLayer::GetLayerState(
 }
 
 UniquePtr<ScrollMetadata> nsDisplayScrollInfoLayer::ComputeScrollMetadata(
-    LayerManager* aLayerManager,
+    nsDisplayListBuilder* aBuilder, LayerManager* aLayerManager,
     const ContainerLayerParameters& aContainerParameters) {
   ScrollMetadata metadata = nsLayoutUtils::ComputeScrollMetadata(
       mScrolledFrame, mScrollFrame, mScrollFrame->GetContent(),
@@ -7186,7 +7186,7 @@ UniquePtr<ScrollMetadata> nsDisplayScrollInfoLayer::ComputeScrollMetadata(
   metadata.GetMetrics().SetIsScrollInfoLayer(true);
   nsIScrollableFrame* scrollableFrame = mScrollFrame->GetScrollTargetFrame();
   if (scrollableFrame) {
-    scrollableFrame->NotifyApzTransaction();
+    aBuilder->AddScrollFrameToNotify(scrollableFrame);
   }
 
   return UniquePtr<ScrollMetadata>(new ScrollMetadata(metadata));
@@ -7196,8 +7196,8 @@ bool nsDisplayScrollInfoLayer::UpdateScrollData(
     mozilla::layers::WebRenderScrollData* aData,
     mozilla::layers::WebRenderLayerScrollData* aLayerData) {
   if (aLayerData) {
-    UniquePtr<ScrollMetadata> metadata =
+    UniquePtr<ScrollMetadata> metadata = ComputeScrollMetadata(
-        ComputeScrollMetadata(aData->GetManager(), ContainerLayerParameters());
+        aData->GetBuilder(), aData->GetManager(), ContainerLayerParameters());
     MOZ_ASSERT(aData);
     MOZ_ASSERT(metadata);
     aLayerData->AppendScrollMetadata(*aData, *metadata);

layout/painting/nsDisplayList.h

@@ -6308,7 +6308,7 @@ class nsDisplayScrollInfoLayer : public nsDisplayWrapList {
 
   void WriteDebugInfo(std::stringstream& aStream) override;
   mozilla::UniquePtr<ScrollMetadata> ComputeScrollMetadata(
-      LayerManager* aLayerManager,
+      nsDisplayListBuilder* aBuilder, LayerManager* aLayerManager,
       const ContainerLayerParameters& aContainerParameters);
   bool UpdateScrollData(
       mozilla::layers::WebRenderScrollData* aData,