Bug 1677929 - Store APZScroll composition payloads based on composition vsync ID, not based on epoch. r=kats

The epoch doesn't change during pure-APZ scrolling, so we were picking up
payloads from a future composite. As a result, we were computing incorrect
latencies, when the actual latencies were in fact one frame higher.

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

gfx/layers/apz/src/APZCTreeManager.cpp

@@ -712,16 +712,17 @@ void APZCTreeManager::SampleForWebRender(
   AssertOnSamplerThread();
   MutexAutoLock lock(mMapLock);
 
+  RefPtr<WebRenderBridgeParent> wrBridgeParent;
+  RefPtr<CompositorController> controller;
+  CompositorBridgeParent::CallWithIndirectShadowTree(
+      mRootLayersId, [&](LayerTreeState& aState) -> void {
+        controller = aState.GetCompositorController();
+        wrBridgeParent = aState.mWrBridge;
+      });
+
   bool activeAnimations = AdvanceAnimationsInternal(lock, aSampleTime);
-  if (activeAnimations) {
-    RefPtr<CompositorController> controller;
-    CompositorBridgeParent::CallWithIndirectShadowTree(
-        mRootLayersId, [&](LayerTreeState& aState) -> void {
-          controller = aState.GetCompositorController();
-        });
-    if (controller) {
-      controller->ScheduleRenderOnCompositorThread();
-    }
+  if (activeAnimations && controller) {
+    controller->ScheduleRenderOnCompositorThread();
   }
 
   nsTArray<wr::WrTransformProperty> transforms;
@@ -740,23 +741,8 @@ void APZCTreeManager::SampleForWebRender(
             .mTranslation;
 
     if (Maybe<CompositionPayload> payload = apzc->NotifyScrollSampling()) {
-      RefPtr<WebRenderBridgeParent> wrBridgeParent;
-      LayersId layersId = apzc->GetGuid().mLayersId;
-      CompositorBridgeParent::CallWithIndirectShadowTree(
-          layersId, [&](LayerTreeState& aState) -> void {
-            wrBridgeParent = aState.mWrBridge;
-          });
-
-      if (wrBridgeParent) {
-        wr::PipelineId pipelineId = wr::AsPipelineId(layersId);
-        for (size_t i = 0; i < aEpochsBeingRendered->Length(); i++) {
-          if ((*aEpochsBeingRendered)[i].pipeline_id == pipelineId) {
-            auto& epoch = (*aEpochsBeingRendered)[i].epoch;
-            wrBridgeParent->AddPendingScrollPayload(
-                *payload, std::make_pair(pipelineId, epoch));
-            break;
-          }
-        }
+      if (wrBridgeParent && aVsyncId) {
+        wrBridgeParent->AddPendingScrollPayload(*payload, *aVsyncId);
       }
     }
 

gfx/layers/ipc/CompositorBridgeParent.cpp

@@ -2336,6 +2336,12 @@ void CompositorBridgeParent::NotifyDidRender(const VsyncId& aCompositeStartId,
     mIsForcedFirstPaint = false;
   }
 
+  nsTArray<CompositionPayload> payload =
+      mWrBridge->TakePendingScrollPayload(aCompositeStartId);
+  if (!payload.IsEmpty()) {
+    RecordCompositionPayloadsPresented(aCompositeEnd, payload);
+  }
+
   nsTArray<ImageCompositeNotificationInfo> notifications;
   mWrBridge->ExtractImageCompositeNotifications(&notifications);
   if (!notifications.IsEmpty()) {
@@ -2371,13 +2377,6 @@ void CompositorBridgeParent::NotifyPipelineRendered(
 
   wrBridge->RemoveEpochDataPriorTo(aEpoch);
 
-  std::pair<wr::PipelineId, wr::Epoch> key(aPipelineId, aEpoch);
-  nsTArray<CompositionPayload> payload =
-      wrBridge->TakePendingScrollPayload(key);
-  if (!payload.IsEmpty()) {
-    RecordCompositionPayloadsPresented(aCompositeEnd, payload);
-  }
-
   nsTArray<FrameStats> stats;
 
   RefPtr<UiCompositorControllerParent> uiController =

gfx/layers/wr/WebRenderBridgeParent.cpp

@@ -893,23 +893,22 @@ WebRenderBridgeParent::GetCollectedFrames() {
 }
 
 void WebRenderBridgeParent::AddPendingScrollPayload(
-    CompositionPayload& aPayload,
-    const std::pair<wr::PipelineId, wr::Epoch>& aKey) {
+    CompositionPayload& aPayload, const VsyncId& aCompositeStartId) {
   auto pendingScrollPayloads = mPendingScrollPayloads.Lock();
   nsTArray<CompositionPayload>* payloads =
-      pendingScrollPayloads->LookupOrAdd(aKey);
+      pendingScrollPayloads->LookupOrAdd(aCompositeStartId.mId);
 
   payloads->AppendElement(aPayload);
 }
 
 nsTArray<CompositionPayload> WebRenderBridgeParent::TakePendingScrollPayload(
-    const std::pair<wr::PipelineId, wr::Epoch>& aKey) {
+    const VsyncId& aCompositeStartId) {
   auto pendingScrollPayloads = mPendingScrollPayloads.Lock();
   nsTArray<CompositionPayload> payload;
   if (nsTArray<CompositionPayload>* storedPayload =
-          pendingScrollPayloads->Get(aKey)) {
+          pendingScrollPayloads->Get(aCompositeStartId.mId)) {
     payload.AppendElements(std::move(*storedPayload));
-    pendingScrollPayloads->Remove(aKey);
+    pendingScrollPayloads->Remove(aCompositeStartId.mId);
   }
   return payload;
 }

gfx/layers/wr/WebRenderBridgeParent.h

@@ -52,42 +52,6 @@ class WebRenderBridgeParentRef;
 class WebRenderImageHost;
 struct WrAnimations;
 
-class PipelineIdAndEpochHashEntry : public PLDHashEntryHdr {
- public:
-  typedef const std::pair<wr::PipelineId, wr::Epoch>& KeyType;
-  typedef const std::pair<wr::PipelineId, wr::Epoch>* KeyTypePointer;
-  enum { ALLOW_MEMMOVE = true };
-
-  explicit PipelineIdAndEpochHashEntry(wr::PipelineId aPipelineId,
-                                       wr::Epoch aEpoch)
-      : mValue(aPipelineId, aEpoch) {}
-
-  PipelineIdAndEpochHashEntry(PipelineIdAndEpochHashEntry&& aOther) = default;
-
-  explicit PipelineIdAndEpochHashEntry(KeyTypePointer aKey)
-      : mValue(aKey->first, aKey->second) {}
-
-  ~PipelineIdAndEpochHashEntry() {}
-
-  KeyType GetKey() const { return mValue; }
-
-  bool KeyEquals(KeyTypePointer aKey) const {
-    return mValue.first.mHandle == aKey->first.mHandle &&
-           mValue.first.mNamespace == aKey->first.mNamespace &&
-           mValue.second.mHandle == aKey->second.mHandle;
-  };
-
-  static KeyTypePointer KeyToPointer(KeyType aKey) { return &aKey; }
-
-  static PLDHashNumber HashKey(KeyTypePointer aKey) {
-    return mozilla::HashGeneric(aKey->first.mHandle, aKey->first.mNamespace,
-                                aKey->second.mHandle);
-  }
-
- private:
-  std::pair<wr::PipelineId, wr::Epoch> mValue;
-};
-
 struct CompositorAnimationIdsForEpoch {
   CompositorAnimationIdsForEpoch(const wr::Epoch& aEpoch,
                                  nsTArray<uint64_t>&& aIds)
@@ -325,12 +289,11 @@ class WebRenderBridgeParent final : public PWebRenderBridgeParent,
   RefPtr<wr::WebRenderAPI::GetCollectedFramesPromise> GetCollectedFrames();
 
   void DisableNativeCompositor();
-  void AddPendingScrollPayload(
-      CompositionPayload& aPayload,
-      const std::pair<wr::PipelineId, wr::Epoch>& aKey);
+  void AddPendingScrollPayload(CompositionPayload& aPayload,
+                               const VsyncId& aCompositeStartId);
 
   nsTArray<CompositionPayload> TakePendingScrollPayload(
-      const std::pair<wr::PipelineId, wr::Epoch>& aKey);
+      const VsyncId& aCompositeStartId);
 
   RefPtr<WebRenderBridgeParentRef> GetWebRenderBridgeParentRef();
 
@@ -525,8 +488,7 @@ class WebRenderBridgeParent final : public PWebRenderBridgeParent,
   bool mSkippedComposite;
   bool mDisablingNativeCompositor;
   // These payloads are being used for SCROLL_PRESENT_LATENCY telemetry
-  DataMutex<nsClassHashtable<PipelineIdAndEpochHashEntry,
-                             nsTArray<CompositionPayload>>>
+  DataMutex<nsClassHashtable<nsUint64HashKey, nsTArray<CompositionPayload>>>
       mPendingScrollPayloads;
 };