Bug 1453360 - Store the compositor animation ids to delete until the epoch is rendered. r?nical draft
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 27 Apr 2018 10:32:31 -0400
changeset 789062 d4bba643e47f60f4e9d90c6416af3fad692a39e5
parent 789061 d81960b717b6af8f73dbe886d52d633649aa0107
push id108158
push userkgupta@mozilla.com
push dateFri, 27 Apr 2018 14:32:53 +0000
reviewersnical
bugs1453360
milestone61.0a1
Bug 1453360 - Store the compositor animation ids to delete until the epoch is rendered. r?nical With async scene building, we might get the message to delete certain compositor animation ids while we are still building and rendering scenes that have those compositor animations. This patch associates those ids with the epoch at which they are safe to delete, and only does the deletion once we have rendered that epoch. MozReview-Commit-ID: Jetfcdtwt1q
gfx/layers/ipc/CompositorBridgeParent.cpp
gfx/layers/wr/WebRenderBridgeParent.cpp
gfx/layers/wr/WebRenderBridgeParent.h
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -2111,16 +2111,17 @@ CompositorBridgeParent::NotifyPipelineRe
     mAsyncImageManager->PipelineRendered(aPipelineId, aEpoch);
   }
 
   if (!mWrBridge) {
     return;
   }
 
   if (mWrBridge->PipelineId() == aPipelineId) {
+    mWrBridge->RemoveEpochDataPriorTo(aEpoch);
 
     if (!mPaused) {
       TransactionId transactionId = mWrBridge->FlushTransactionIdsForEpoch(aEpoch, aCompositeEnd);
       Unused << SendDidComposite(LayersId{0}, transactionId, aCompositeStart, aCompositeEnd);
 
       nsTArray<ImageCompositeNotificationInfo> notifications;
       mWrBridge->ExtractImageCompositeNotifications(&notifications);
       if (!notifications.IsEmpty()) {
@@ -2131,16 +2132,18 @@ CompositorBridgeParent::NotifyPipelineRe
   }
 
   MonitorAutoLock lock(*sIndirectLayerTreesLock);
   ForEachIndirectLayerTree([&] (LayerTreeState* lts, const LayersId& aLayersId) -> void {
     if (lts->mCrossProcessParent &&
         lts->mWrBridge &&
         lts->mWrBridge->PipelineId() == aPipelineId) {
 
+      lts->mWrBridge->RemoveEpochDataPriorTo(aEpoch);
+
       if (!mPaused) {
         CrossProcessCompositorBridgeParent* cpcp = lts->mCrossProcessParent;
         TransactionId transactionId = lts->mWrBridge->FlushTransactionIdsForEpoch(aEpoch, aCompositeEnd);
         Unused << cpcp->SendDidComposite(aLayersId, transactionId, aCompositeStart, aCompositeEnd);
       }
     }
   });
 }
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -473,25 +473,37 @@ WebRenderBridgeParent::RecvUpdateResourc
 
 mozilla::ipc::IPCResult
 WebRenderBridgeParent::RecvDeleteCompositorAnimations(InfallibleTArray<uint64_t>&& aIds)
 {
   if (mDestroyed) {
     return IPC_OK();
   }
 
-  for (uint32_t i = 0; i < aIds.Length(); i++) {
-    if (mActiveAnimations.erase(aIds[i]) > 0) {
-      mAnimStorage->ClearById(aIds[i]);
-    } else {
-      NS_ERROR("Tried to delete invalid animation");
+  // Once mWrEpoch has been rendered, we can delete these compositor animations
+  mCompositorAnimationsToDelete.push(CompositorAnimationIdsForEpoch(mWrEpoch, Move(aIds)));
+  return IPC_OK();
+}
+
+void
+WebRenderBridgeParent::RemoveEpochDataPriorTo(const wr::Epoch& aRenderedEpoch)
+{
+  while (!mCompositorAnimationsToDelete.empty()) {
+    if (mCompositorAnimationsToDelete.front().mEpoch.mHandle > aRenderedEpoch.mHandle) {
+      break;
     }
+    for (uint64_t id : mCompositorAnimationsToDelete.front().mIds) {
+      if (mActiveAnimations.erase(id) > 0) {
+        mAnimStorage->ClearById(id);
+      } else {
+        NS_ERROR("Tried to delete invalid animation");
+      }
+    }
+    mCompositorAnimationsToDelete.pop();
   }
-
-  return IPC_OK();
 }
 
 CompositorBridgeParent*
 WebRenderBridgeParent::GetRootCompositorBridgeParent() const
 {
   if (!mCompositorBridge) {
     return nullptr;
   }
@@ -992,16 +1004,17 @@ WebRenderBridgeParent::RecvClearCachedRe
   mApi->SendTransaction(txn);
   // Schedule generate frame to clean up Pipeline
   ScheduleGenerateFrame();
   // Remove animations.
   for (std::unordered_set<uint64_t>::iterator iter = mActiveAnimations.begin(); iter != mActiveAnimations.end(); iter++) {
     mAnimStorage->ClearById(*iter);
   }
   mActiveAnimations.clear();
+  std::queue<CompositorAnimationIdsForEpoch>().swap(mCompositorAnimationsToDelete); // clear queue
   return IPC_OK();
 }
 
 void
 WebRenderBridgeParent::UpdateWebRender(CompositorVsyncScheduler* aScheduler,
                                        wr::WebRenderAPI* aApi,
                                        AsyncImagePipelineManager* aImageMgr,
                                        CompositorAnimationStorage* aAnimStorage)
@@ -1456,16 +1469,17 @@ WebRenderBridgeParent::ClearResources()
   txn.RemovePipeline(mPipelineId);
 
   mApi->SendTransaction(txn);
 
   for (std::unordered_set<uint64_t>::iterator iter = mActiveAnimations.begin(); iter != mActiveAnimations.end(); iter++) {
     mAnimStorage->ClearById(*iter);
   }
   mActiveAnimations.clear();
+  std::queue<CompositorAnimationIdsForEpoch>().swap(mCompositorAnimationsToDelete); // clear queue
 
   if (mWidget) {
     mCompositorScheduler->Destroy();
   }
   mAnimStorage = nullptr;
   mCompositorScheduler = nullptr;
   mAsyncImageManager = nullptr;
   mApi = nullptr;
--- a/gfx/layers/wr/WebRenderBridgeParent.h
+++ b/gfx/layers/wr/WebRenderBridgeParent.h
@@ -178,16 +178,18 @@ public:
    */
   void ScheduleGenerateFrame();
 
   void UpdateWebRender(CompositorVsyncScheduler* aScheduler,
                        wr::WebRenderAPI* aApi,
                        AsyncImagePipelineManager* aImageMgr,
                        CompositorAnimationStorage* aAnimStorage);
 
+  void RemoveEpochDataPriorTo(const wr::Epoch& aRenderedEpoch);
+
 private:
   explicit WebRenderBridgeParent(const wr::PipelineId& aPipelineId);
   virtual ~WebRenderBridgeParent();
 
   void UpdateAPZFocusState(const FocusTarget& aFocus);
   void UpdateAPZScrollData(const wr::Epoch& aEpoch, WebRenderScrollData&& aData);
 
   bool UpdateResources(const nsTArray<OpUpdateResource>& aResourceUpdates,
@@ -234,16 +236,27 @@ private:
       , mFwdTime(aFwdTime)
     {}
     wr::Epoch mEpoch;
     TransactionId mId;
     TimeStamp mTxnStartTime;
     TimeStamp mFwdTime;
   };
 
+  struct CompositorAnimationIdsForEpoch {
+    CompositorAnimationIdsForEpoch(const wr::Epoch& aEpoch, InfallibleTArray<uint64_t>&& aIds)
+      : mEpoch(aEpoch)
+      , mIds(Move(aIds))
+    {
+    }
+
+    wr::Epoch mEpoch;
+    InfallibleTArray<uint64_t> mIds;
+  };
+
   CompositorBridgeParentBase* MOZ_NON_OWNING_REF mCompositorBridge;
   wr::PipelineId mPipelineId;
   RefPtr<widget::CompositorWidget> mWidget;
   RefPtr<wr::WebRenderAPI> mApi;
   RefPtr<AsyncImagePipelineManager> mAsyncImageManager;
   RefPtr<CompositorVsyncScheduler> mCompositorScheduler;
   RefPtr<CompositorAnimationStorage> mAnimStorage;
   // mActiveAnimations is used to avoid leaking animations when WebRenderBridgeParent is
@@ -257,16 +270,17 @@ private:
   // These fields keep track of the latest layer observer epoch values in the child and the
   // parent. mChildLayerObserverEpoch is the latest epoch value received from the child.
   // mParentLayerObserverEpoch is the latest epoch value that we have told TabParent about
   // (via ObserveLayerUpdate).
   uint64_t mChildLayerObserverEpoch;
   uint64_t mParentLayerObserverEpoch;
 
   std::queue<PendingTransactionId> mPendingTransactionIds;
+  std::queue<CompositorAnimationIdsForEpoch> mCompositorAnimationsToDelete;
   wr::Epoch mWrEpoch;
   wr::IdNamespace mIdNamespace;
 
   bool mPaused;
   bool mDestroyed;
   bool mForceRendering;
   bool mReceivedDisplayList;
 };