Bug 1376896 - Fix AnimatedValue leak at CompositorAnimationStorage during OMTA r=kats
authorsotaro <sotaro.ikeda.g@gmail.com>
Thu, 06 Jul 2017 12:06:41 +0900
changeset 367556 fc07a79051918f888c67f456f85da6b1c55ef103
parent 367555 39e61f5e8fc5a66c7b038f897e600a59eb0e75ab
child 367557 800cae1681846f50fa6dc51baf218d17c1422975
push id32137
push usercbook@mozilla.com
push dateThu, 06 Jul 2017 09:18:21 +0000
treeherdermozilla-central@018b3829d0a7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1376896
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1376896 - Fix AnimatedValue leak at CompositorAnimationStorage during OMTA r=kats
gfx/layers/AnimationHelper.h
gfx/layers/ipc/CompositorBridgeParent.cpp
gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp
gfx/layers/ipc/LayerTransactionParent.cpp
gfx/layers/ipc/LayerTransactionParent.h
--- a/gfx/layers/AnimationHelper.h
+++ b/gfx/layers/AnimationHelper.h
@@ -160,17 +160,17 @@ public:
 
   /**
    * Clear AnimatedValues and Animations data
    */
   void Clear();
 
   void ClearById(const uint64_t& aId);
 private:
-  ~CompositorAnimationStorage() { Clear(); };
+  ~CompositorAnimationStorage() { };
 
 private:
   AnimatedValueTable mAnimatedValues;
   AnimationsTable mAnimations;
 };
 
 class AnimationHelper
 {
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -1510,24 +1510,24 @@ CompositorBridgeParent::AllocPLayerTrans
                                                      const uint64_t& aId)
 {
   MOZ_ASSERT(aId == 0);
 
   InitializeLayerManager(aBackendHints);
 
   if (!mLayerManager) {
     NS_WARNING("Failed to initialise Compositor");
-    LayerTransactionParent* p = new LayerTransactionParent(nullptr, this, 0);
+    LayerTransactionParent* p = new LayerTransactionParent(/* aManager */ nullptr, this, /* aAnimStorage */ nullptr, 0);
     p->AddIPDLReference();
     return p;
   }
 
   mCompositionManager = new AsyncCompositionManager(this, mLayerManager);
 
-  LayerTransactionParent* p = new LayerTransactionParent(mLayerManager, this, 0);
+  LayerTransactionParent* p = new LayerTransactionParent(mLayerManager, this, GetAnimationStorage(0), 0);
   p->AddIPDLReference();
   return p;
 }
 
 bool
 CompositorBridgeParent::DeallocPLayerTransactionParent(PLayerTransactionParent* actor)
 {
   static_cast<LayerTransactionParent*>(actor)->ReleaseIPDLReference();
@@ -1639,17 +1639,17 @@ CompositorBridgeParent::RecvAdoptChild(c
   APZCTreeManagerParent* parent;
   {
     MonitorAutoLock lock(*sIndirectLayerTreesLock);
     // We currently don't support adopting children from one compositor to
     // another if the two compositors don't have the same options.
     MOZ_ASSERT(sIndirectLayerTrees[child].mParent->mOptions == mOptions);
     NotifyChildCreated(child);
     if (sIndirectLayerTrees[child].mLayerTree) {
-      sIndirectLayerTrees[child].mLayerTree->SetLayerManager(mLayerManager);
+      sIndirectLayerTrees[child].mLayerTree->SetLayerManager(mLayerManager, GetAnimationStorage(0));
       // Trigger composition to handle a case that mLayerTree was not composited yet
       // by previous CompositorBridgeParent, since nsRefreshDriver might wait composition complete.
       ScheduleComposition();
     }
     if (mWrBridge && sIndirectLayerTrees[child].mWrBridge) {
       sIndirectLayerTrees[child].mWrBridge->UpdateWebRender(mWrBridge->CompositorScheduler(),
                                                             mWrBridge->GetWebRenderAPI(),
                                                             mWrBridge->CompositableHolder(),
--- a/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CrossProcessCompositorBridgeParent.cpp
@@ -83,24 +83,25 @@ CrossProcessCompositorBridgeParent::Allo
   LayerTreeMap::iterator itr = sIndirectLayerTrees.find(aId);
   if (sIndirectLayerTrees.end() != itr) {
     state = &itr->second;
   }
 
   if (state && state->mLayerManager) {
     state->mCrossProcessParent = this;
     HostLayerManager* lm = state->mLayerManager;
-    LayerTransactionParent* p = new LayerTransactionParent(lm, this, aId);
+    CompositorAnimationStorage* animStorage = state->mParent ? state->mParent->GetAnimationStorage(0) : nullptr;
+    LayerTransactionParent* p = new LayerTransactionParent(lm, this, animStorage, aId);
     p->AddIPDLReference();
     sIndirectLayerTrees[aId].mLayerTree = p;
     return p;
   }
 
   NS_WARNING("Created child without a matching parent?");
-  LayerTransactionParent* p = new LayerTransactionParent(nullptr, this, aId);
+  LayerTransactionParent* p = new LayerTransactionParent(/* aManager */ nullptr, this, /* aAnimStorage */ nullptr, aId);
   p->AddIPDLReference();
   return p;
 }
 
 bool
 CrossProcessCompositorBridgeParent::DeallocPLayerTransactionParent(PLayerTransactionParent* aLayers)
 {
   LayerTransactionParent* slp = static_cast<LayerTransactionParent*>(aLayers);
--- a/gfx/layers/ipc/LayerTransactionParent.cpp
+++ b/gfx/layers/ipc/LayerTransactionParent.cpp
@@ -46,40 +46,50 @@ using mozilla::layout::RenderFrameParent
 
 namespace mozilla {
 namespace layers {
 
 //--------------------------------------------------
 // LayerTransactionParent
 LayerTransactionParent::LayerTransactionParent(HostLayerManager* aManager,
                                                CompositorBridgeParentBase* aBridge,
+                                               CompositorAnimationStorage* aAnimStorage,
                                                uint64_t aId)
   : mLayerManager(aManager)
   , mCompositorBridge(aBridge)
+  , mAnimStorage(aAnimStorage)
   , mId(aId)
   , mChildEpoch(0)
   , mParentEpoch(0)
   , mPendingTransaction(0)
   , mDestroyed(false)
   , mIPCOpen(false)
 {
 }
 
 LayerTransactionParent::~LayerTransactionParent()
 {
 }
 
 void
-LayerTransactionParent::SetLayerManager(HostLayerManager* aLayerManager)
+LayerTransactionParent::SetLayerManager(HostLayerManager* aLayerManager, CompositorAnimationStorage* aAnimStorage)
 {
+  if (mDestroyed) {
+    return;
+  }
   mLayerManager = aLayerManager;
   for (auto iter = mLayerMap.Iter(); !iter.Done(); iter.Next()) {
     auto layer = iter.Data();
+    if (mAnimStorage &&
+        layer->GetCompositorAnimationsId()) {
+      mAnimStorage->ClearById(layer->GetCompositorAnimationsId());
+    }
     layer->AsHostLayer()->SetLayerManager(aLayerManager);
   }
+  mAnimStorage = aAnimStorage;
 }
 
 mozilla::ipc::IPCResult
 LayerTransactionParent::RecvShutdown()
 {
   Destroy();
   IProtocol* mgr = Manager();
   if (!Send__delete__(this)) {
@@ -96,17 +106,26 @@ LayerTransactionParent::RecvShutdownSync
 
 void
 LayerTransactionParent::Destroy()
 {
   if (mDestroyed) {
     return;
   }
   mDestroyed = true;
+  if (mAnimStorage) {
+    for (auto iter = mLayerMap.Iter(); !iter.Done(); iter.Next()) {
+      auto layer = iter.Data();
+      if (layer->GetCompositorAnimationsId()) {
+        mAnimStorage->ClearById(layer->GetCompositorAnimationsId());
+      }
+    }
+  }
   mCompositables.clear();
+  mAnimStorage = nullptr;
 }
 
 class MOZ_STACK_CLASS AutoLayerTransactionParentAsyncMessageSender
 {
 public:
   explicit AutoLayerTransactionParentAsyncMessageSender(LayerTransactionParent* aLayerTransaction,
                                                         const InfallibleTArray<OpDestroy>* aDestroyActors = nullptr)
     : mLayerTransaction(aLayerTransaction)
@@ -530,24 +549,20 @@ LayerTransactionParent::SetLayerAttribut
   if (LayerHandle maskLayer = common.maskLayer()) {
     layer->SetMaskLayer(AsLayer(maskLayer));
   } else {
     layer->SetMaskLayer(nullptr);
   }
   layer->SetCompositorAnimations(common.compositorAnimations());
   // Clean up the Animations by id in the CompositorAnimationStorage
   // if there are no active animations on the layer
-  if (layer->GetCompositorAnimationsId() &&
+  if (mAnimStorage &&
+      layer->GetCompositorAnimationsId() &&
       layer->GetAnimations().IsEmpty()) {
-    CompositorAnimationStorage* storage =
-      mCompositorBridge->GetAnimationStorage(GetId());
-
-    if (storage) {
-      storage->ClearById(layer->GetCompositorAnimationsId());
-    }
+    mAnimStorage->ClearById(layer->GetCompositorAnimationsId());
   }
   if (common.scrollMetadata() != layer->GetAllScrollMetadata()) {
     UpdateHitTestingTree(layer, "scroll metadata changed");
     layer->SetScrollMetadata(common.scrollMetadata());
   }
   layer->SetDisplayListLog(common.displayListLog().get());
 
   // The updated invalid region is added to the existing one, since we can
@@ -722,24 +737,21 @@ LayerTransactionParent::RecvGetAnimation
 {
   *aHasAnimationOpacity = false;
   if (mDestroyed || !layer_manager() || layer_manager()->IsDestroyed()) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   mCompositorBridge->ApplyAsyncProperties(this);
 
-  CompositorAnimationStorage* storage =
-    mCompositorBridge->GetAnimationStorage(GetId());
-
-  if (!storage) {
+  if (!mAnimStorage) {
     return IPC_FAIL_NO_REASON(this);
   }
 
-  Maybe<float> opacity = storage->GetAnimationOpacity(aCompositorAnimationsId);
+  Maybe<float> opacity = mAnimStorage->GetAnimationOpacity(aCompositorAnimationsId);
   if (opacity) {
     *aOpacity = *opacity;
     *aHasAnimationOpacity = true;
   }
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
@@ -751,24 +763,21 @@ LayerTransactionParent::RecvGetAnimation
   }
 
   // Make sure we apply the latest animation style or else we can end up with
   // a race between when we temporarily clear the animation transform (in
   // CompositorBridgeParent::SetShadowProperties) and when animation recalculates
   // the value.
   mCompositorBridge->ApplyAsyncProperties(this);
 
-  CompositorAnimationStorage* storage =
-    mCompositorBridge->GetAnimationStorage(GetId());
-
-  if (!storage) {
+  if (!mAnimStorage) {
     return IPC_FAIL_NO_REASON(this);
   }
 
-  Maybe<Matrix4x4> transform = storage->GetAnimationTransform(aCompositorAnimationsId);
+  Maybe<Matrix4x4> transform = mAnimStorage->GetAnimationTransform(aCompositorAnimationsId);
   if (transform) {
     *aTransform = *transform;
   } else {
     *aTransform = mozilla::void_t();
   }
   return IPC_OK();
 }
 
@@ -1003,16 +1012,20 @@ LayerTransactionParent::RecvNewComposita
 
 mozilla::ipc::IPCResult
 LayerTransactionParent::RecvReleaseLayer(const LayerHandle& aHandle)
 {
   RefPtr<Layer> layer;
   if (!aHandle || !mLayerMap.Remove(aHandle.Value(), getter_AddRefs(layer))) {
     return IPC_FAIL_NO_REASON(this);
   }
+  if (mAnimStorage &&
+      layer->GetCompositorAnimationsId()) {
+    mAnimStorage->ClearById(layer->GetCompositorAnimationsId());
+  }
   layer->Disconnect();
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 LayerTransactionParent::RecvReleaseCompositable(const CompositableHandle& aHandle)
 {
   ReleaseCompositable(aHandle);
--- a/gfx/layers/ipc/LayerTransactionParent.h
+++ b/gfx/layers/ipc/LayerTransactionParent.h
@@ -28,42 +28,44 @@ class RenderFrameParent;
 } // namespace layout
 
 namespace layers {
 
 class Layer;
 class HostLayerManager;
 class ShadowLayerParent;
 class CompositableParent;
+class CompositorAnimationStorage;
 class CompositorBridgeParentBase;
 
 class LayerTransactionParent final : public PLayerTransactionParent,
                                      public CompositableParentManager,
                                      public ShmemAllocator
 {
   typedef mozilla::layout::RenderFrameParent RenderFrameParent;
   typedef InfallibleTArray<Edit> EditArray;
   typedef InfallibleTArray<OpDestroy> OpDestroyArray;
   typedef InfallibleTArray<PluginWindowData> PluginsArray;
   typedef InfallibleTArray<ReadLockInit> ReadLockArray;
 
 public:
   LayerTransactionParent(HostLayerManager* aManager,
                          CompositorBridgeParentBase* aBridge,
+                         CompositorAnimationStorage* aAnimStorage,
                          uint64_t aId);
 
 protected:
   ~LayerTransactionParent();
 
 public:
   void Destroy();
 
   HostLayerManager* layer_manager() const { return mLayerManager; }
 
-  void SetLayerManager(HostLayerManager* aLayerManager);
+  void SetLayerManager(HostLayerManager* aLayerManager, CompositorAnimationStorage* aAnimStorage);
 
   uint64_t GetId() const { return mId; }
   Layer* GetRoot() const { return mRoot; }
 
   uint64_t GetChildEpoch() const { return mChildEpoch; }
   bool ShouldParentObserveEpoch();
 
   virtual ShmemAllocator* AsShmemAllocator() override { return this; }
@@ -169,16 +171,17 @@ private:
   // testing tree changes are made.
   void UpdateHitTestingTree(Layer* aLayer, const char* aWhy) {
     mUpdateHitTestingTree = true;
   }
 
 private:
   RefPtr<HostLayerManager> mLayerManager;
   CompositorBridgeParentBase* mCompositorBridge;
+  RefPtr<CompositorAnimationStorage> mAnimStorage;
 
   // Hold the root because it might be grafted under various
   // containers in the "real" layer tree
   RefPtr<Layer> mRoot;
 
   // Mapping from LayerHandles to Layers.
   nsRefPtrHashtable<nsUint64HashKey, Layer> mLayerMap;