Bug 1504699 - Part 5. Stop sharing code between SharedUserData and SharedSurfacesAnimation. r=nical
authorAndrew Osmond <aosmond@mozilla.com>
Wed, 03 Oct 2018 08:19:14 -0400
changeset 506741 a687664414491ef27cbca5060c83443814fb0693
parent 506740 0c50877a2132bc8bcbcdee8ddb8dce13913c1c80
child 506742 ecddcae1b26630a30263749186c5250030f72dfb
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1504699
milestone65.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 1504699 - Part 5. Stop sharing code between SharedUserData and SharedSurfacesAnimation. r=nical Originally it made sense to share the code, but now the latter has become too specialized to reuse it. Fork it off here and update it later parts in this series. Differential Revision: https://phabricator.services.mozilla.com/D10901
gfx/layers/ImageContainer.cpp
gfx/layers/ImageContainer.h
gfx/layers/ipc/SharedSurfacesChild.cpp
gfx/layers/ipc/SharedSurfacesChild.h
--- a/gfx/layers/ImageContainer.cpp
+++ b/gfx/layers/ImageContainer.cpp
@@ -240,16 +240,19 @@ ImageContainer::~ImageContainer()
   if (mNotifyCompositeListener) {
     mNotifyCompositeListener->ClearImageContainer();
   }
   if (mAsyncContainerHandle) {
     if (RefPtr<ImageBridgeChild> imageBridge = ImageBridgeChild::GetSingleton()) {
       imageBridge->ForgetImageContainer(mAsyncContainerHandle);
     }
   }
+  if (mSharedAnimation) {
+    mSharedAnimation->Destroy();
+  }
 }
 
 RefPtr<PlanarYCbCrImage>
 ImageContainer::CreatePlanarYCbCrImage()
 {
   RecursiveMutexAutoLock lock(mRecursiveMutex);
   EnsureImageClient();
   if (mImageClient && mImageClient->AsImageClientSingle()) {
--- a/gfx/layers/ImageContainer.h
+++ b/gfx/layers/ImageContainer.h
@@ -704,17 +704,17 @@ private:
   // sucessfully created with ENABLE_ASYNC, or points to null otherwise.
   // 'unsuccessful' in this case only means that the ImageClient could not
   // be created, most likely because off-main-thread compositing is not enabled.
   // In this case the ImageContainer is perfectly usable, but it will forward
   // frames to the compositor through transactions in the main thread rather than
   // asynchronusly using the ImageBridge IPDL protocol.
   RefPtr<ImageClient> mImageClient;
 
-  nsAutoPtr<SharedSurfacesAnimation> mSharedAnimation;
+  RefPtr<SharedSurfacesAnimation> mSharedAnimation;
 
   bool mIsAsync;
   CompositableHandle mAsyncContainerHandle;
 
   // ProducerID for last current image(s)
   ProducerID mCurrentProducerID;
 
   RefPtr<ImageContainerListener> mNotifyCompositeListener;
--- a/gfx/layers/ipc/SharedSurfacesChild.cpp
+++ b/gfx/layers/ipc/SharedSurfacesChild.cpp
@@ -470,41 +470,65 @@ SharedSurfacesChild::UpdateAnimation(Ima
 
   SharedSurfacesAnimation* anim =
     aContainer->EnsureSharedSurfacesAnimation();
   MOZ_ASSERT(anim);
 
   return anim->SetCurrentFrame(aSurface, sharedSurface, aDirtyRect);
 }
 
+SharedSurfacesAnimation::~SharedSurfacesAnimation()
+{
+  MOZ_ASSERT(mKeys.IsEmpty());
+}
+
+void
+SharedSurfacesAnimation::Destroy()
+{
+  if (!NS_IsMainThread()) {
+    nsCOMPtr<nsIRunnable> task =
+      NewRunnableMethod("SharedSurfacesAnimation::Destroy",
+                        this, &SharedSurfacesAnimation::Destroy);
+    SystemGroup::Dispatch(TaskCategory::Other, task.forget());
+    return;
+  }
+
+  if (mKeys.IsEmpty()) {
+    return;
+  }
+
+  for (const auto& entry : mKeys) {
+    MOZ_ASSERT(!entry.mManager->IsDestroyed());
+    entry.mManager->AddImageKeyForDiscard(entry.mImageKey);
+  }
+
+  mKeys.Clear();
+}
+
 nsresult
 SharedSurfacesAnimation::SetCurrentFrame(SourceSurface* aParentSurface,
                                          SourceSurfaceSharedData* aSurface,
                                          const gfx::IntRect& aDirtyRect)
 {
   MOZ_ASSERT(aSurface);
 
-  SharedUserData* data = nullptr;
+  SharedSurfacesChild::SharedUserData* data = nullptr;
   nsresult rv = SharedSurfacesChild::ShareInternal(aSurface, &data);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   MOZ_ASSERT(data);
   mId = data->Id();
 
   auto i = mKeys.Length();
   while (i > 0) {
     --i;
-    SharedSurfacesChild::ImageKeyData& entry = mKeys[i];
-    if (entry.mManager->IsDestroyed()) {
-      mKeys.RemoveElementAt(i);
-      continue;
-    }
-
+    ImageKeyData& entry = mKeys[i];
+    MOZ_ASSERT(!entry.mManager->IsDestroyed());
     entry.MergeDirtyRect(Some(aDirtyRect));
 
     Maybe<IntRect> dirtyRect = entry.TakeDirtyRect();
     if (dirtyRect) {
       auto& resourceUpdates = entry.mManager->AsyncResourceUpdates();
       resourceUpdates.UpdateExternalImage(mId, entry.mImageKey,
                                           ViewAs<ImagePixel>(dirtyRect.ref()));
     }
@@ -515,31 +539,73 @@ SharedSurfacesAnimation::SetCurrentFrame
 
 nsresult
 SharedSurfacesAnimation::UpdateKey(SourceSurface* aParentSurface,
                                    SourceSurfaceSharedData* aSurface,
                                    WebRenderLayerManager* aManager,
                                    wr::IpcResourceUpdateQueue& aResources,
                                    wr::ImageKey& aKey)
 {
-  SharedUserData* data = nullptr;
+  SharedSurfacesChild::SharedUserData* data = nullptr;
   nsresult rv = SharedSurfacesChild::ShareInternal(aSurface, &data);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   MOZ_ASSERT(data);
   if (mId.mHandle != data->Id().mHandle) {
     mKeys.Clear();
     mId = data->Id();
   }
 
-  aKey = SharedSurfacesChild::SharedUserData::UpdateKey(aManager,
-                                                        aResources,
-                                                        Nothing());
+  // We iterate through all of the items to ensure we clean up the old
+  // WebRenderLayerManager references. Most of the time there will be few
+  // entries and this should not be particularly expensive compared to the
+  // cost of duplicating image keys. In an ideal world, we would generate a
+  // single key for the surface, and it would be usable on all of the
+  // renderer instances. For now, we must allocate a key for each WR bridge.
+  wr::ImageKey key;
+  bool found = false;
+  auto i = mKeys.Length();
+  while (i > 0) {
+    --i;
+    ImageKeyData& entry = mKeys[i];
+    MOZ_ASSERT(!entry.mManager->IsDestroyed());
+    if (entry.mManager == aManager) {
+      WebRenderBridgeChild* wrBridge = aManager->WrBridge();
+      MOZ_ASSERT(wrBridge);
+
+      // Even if the manager is the same, its underlying WebRenderBridgeChild
+      // can change state. If our namespace differs, then our old key has
+      // already been discarded.
+      bool ownsKey = wrBridge->GetNamespace() == entry.mImageKey.mNamespace;
+      if (!ownsKey) {
+        entry.mImageKey = wrBridge->GetNextImageKey();
+        aResources.AddExternalImage(mId, entry.mImageKey);
+      } else {
+        Maybe<IntRect> dirtyRect = entry.TakeDirtyRect();
+        if (dirtyRect) {
+          aResources.UpdateExternalImage(mId, entry.mImageKey,
+                                         ViewAs<ImagePixel>(dirtyRect.ref()));
+        }
+      }
+
+      aKey = entry.mImageKey;
+      found = true;
+      break;
+    }
+  }
+
+  if (!found) {
+    aKey = aManager->WrBridge()->GetNextImageKey();
+    ImageKeyData data(aManager, aKey);
+    mKeys.AppendElement(std::move(data));
+    aResources.AddExternalImage(mId, aKey);
+  }
+
   return NS_OK;
 }
 
 void
 SharedSurfacesAnimation::ReleasePreviousFrame(WebRenderLayerManager* aManager,
                                               const wr::ExternalImageId& aId)
 {
 }
--- a/gfx/layers/ipc/SharedSurfacesChild.h
+++ b/gfx/layers/ipc/SharedSurfacesChild.h
@@ -92,17 +92,17 @@ public:
                                   const gfx::IntRect& aDirtyRect);
 
 private:
   SharedSurfacesChild() = delete;
   ~SharedSurfacesChild() = delete;
 
   friend class SharedSurfacesAnimation;
 
-  class ImageKeyData final {
+  class ImageKeyData {
   public:
     ImageKeyData(WebRenderLayerManager* aManager,
                  const wr::ImageKey& aImageKey);
     ~ImageKeyData();
 
     ImageKeyData(ImageKeyData&& aOther);
     ImageKeyData& operator=(ImageKeyData&& aOther);
     ImageKeyData(const ImageKeyData&) = delete;
@@ -115,17 +115,17 @@ private:
       return std::move(mDirtyRect);
     }
 
     RefPtr<WebRenderLayerManager> mManager;
     Maybe<gfx::IntRect> mDirtyRect;
     wr::ImageKey mImageKey;
   };
 
-  class SharedUserData {
+  class SharedUserData final {
   public:
     SharedUserData()
       : mShared(false)
     { }
 
     explicit SharedUserData(const wr::ExternalImageId& aId)
       : mId(aId)
       , mShared(false)
@@ -183,22 +183,26 @@ private:
 
   static gfx::UserDataKey sSharedKey;
 };
 
 /**
  * This helper class owns a single ImageKey which will map to different external
  * image IDs representing different frames in an animation.
  */
-class SharedSurfacesAnimation final : private SharedSurfacesChild::SharedUserData
+class SharedSurfacesAnimation final
 {
 public:
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SharedSurfacesAnimation)
+
   SharedSurfacesAnimation()
   { }
 
+  void Destroy();
+
   /**
    * Set the animation to display the given frame.
    * @param aParentSurface The owning surface of aSurface. This may be the same
    *                       or it may be a wrapper surface such as
    *                       RecyclingSourceSurface.
    * @param aSurface    The current frame.
    * @param aDirtyRect  Dirty rect representing the change between the new frame
    *                    and the previous frame. We will request only the delta
@@ -230,14 +234,22 @@ public:
   void ReleasePreviousFrame(WebRenderLayerManager* aManager,
                             const wr::ExternalImageId& aId);
 
   /**
    * Destroy any state information bound for the given layer manager. Any
    * image keys are already invalid.
    */
   void Invalidate(WebRenderLayerManager* aManager);
+
+private:
+  ~SharedSurfacesAnimation();
+
+  typedef SharedSurfacesChild::ImageKeyData ImageKeyData;
+
+  AutoTArray<ImageKeyData, 1> mKeys;
+  wr::ExternalImageId mId;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif