Bug 1453801 - Part 2. Ensure shared surfaces are properly released from render texture cache. r=sotaro

gfx/layers/SourceSurfaceSharedData.h

@@ -40,6 +40,7 @@ public:
 
   SourceSurfaceSharedDataWrapper()
     : mStride(0)
+    , mConsumers(0)
     , mFormat(SurfaceFormat::UNKNOWN)
   { }
 
@@ -72,16 +73,6 @@ public:
     return false;
   }
 
-  bool Map(MapType, MappedSurface *aMappedSurface) override
-  {
-    aMappedSurface->mData = GetData();
-    aMappedSurface->mStride = mStride;
-    return true;
-  }
-
-  void Unmap() override
-  { }
-
   bool AddConsumer()
   {
     return ++mConsumers == 1;

gfx/layers/ipc/SharedSurfacesParent.cpp

@@ -88,6 +88,7 @@ SharedSurfacesParent::Release(const wr::ExternalImageId& aId)
   }
 
   if (surface->RemoveConsumer()) {
+    wr::RenderThread::Get()->UnregisterExternalImage(id);
     sInstance->mSurfaces.Remove(id);
   }
 
@@ -124,6 +125,7 @@ SharedSurfacesParent::AddSameProcess(const wr::ExternalImageId& aId,
         new wr::RenderSharedSurfaceTextureHost(surface);
       wr::RenderThread::Get()->RegisterExternalImage(id, texture.forget());
 
+      surface->AddConsumer();
       sInstance->mSurfaces.Put(id, surface);
     });
 
@@ -140,7 +142,7 @@ SharedSurfacesParent::RemoveSameProcess(const wr::ExternalImageId& aId)
   RefPtr<Runnable> task = NS_NewRunnableFunction(
     "layers::SharedSurfacesParent::RemoveSameProcess",
     [id]() -> void {
-      Remove(id);
+      Release(id);
     });
 
   CompositorThreadHolder::Loop()->PostTask(task.forget());
@@ -156,7 +158,8 @@ SharedSurfacesParent::DestroyProcess(base::ProcessId aPid)
   // Note that the destruction of a parent may not be cheap if it still has a
   // lot of surfaces still bound that require unmapping.
   for (auto i = sInstance->mSurfaces.Iter(); !i.Done(); i.Next()) {
-    if (i.Data()->GetCreatorPid() == aPid) {
+    SourceSurfaceSharedDataWrapper* surface = i.Data();
+    if (surface->GetCreatorPid() == aPid && surface->RemoveConsumer()) {
       wr::RenderThread::Get()->UnregisterExternalImage(i.Key());
       i.Remove();
     }
@@ -190,6 +193,7 @@ SharedSurfacesParent::Add(const wr::ExternalImageId& aId,
     new wr::RenderSharedSurfaceTextureHost(surface);
   wr::RenderThread::Get()->RegisterExternalImage(id, texture.forget());
 
+  surface->AddConsumer();
   sInstance->mSurfaces.Put(id, surface.forget());
 }
 

gfx/layers/wr/AsyncImagePipelineManager.cpp

@@ -10,6 +10,7 @@
 #include "gfxEnv.h"
 #include "mozilla/gfx/gfxVars.h"
 #include "mozilla/layers/CompositorThread.h"
+#include "mozilla/layers/SharedSurfacesParent.h"
 #include "mozilla/layers/WebRenderImageHost.h"
 #include "mozilla/layers/WebRenderTextureHost.h"
 #include "mozilla/webrender/WebRenderAPI.h"
@@ -382,6 +383,24 @@ AsyncImagePipelineManager::HoldExternalImage(const wr::PipelineId& aPipelineId,
   holder->mTextureHosts.push(ForwardingTextureHost(aEpoch, aTexture));
 }
 
+void
+AsyncImagePipelineManager::HoldExternalImage(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch, const wr::ExternalImageId& aImageId)
+{
+  if (mDestroyed) {
+    SharedSurfacesParent::Release(aImageId);
+    return;
+  }
+
+  PipelineTexturesHolder* holder = mPipelineTexturesHolders.Get(wr::AsUint64(aPipelineId));
+  MOZ_ASSERT(holder);
+  if (!holder) {
+    SharedSurfacesParent::Release(aImageId);
+    return;
+  }
+
+  holder->mExternalImages.push(ForwardingExternalImage(aEpoch, aImageId));
+}
+
 void
 AsyncImagePipelineManager::PipelineRendered(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch)
 {
@@ -397,6 +416,15 @@ AsyncImagePipelineManager::PipelineRendered(const wr::PipelineId& aPipelineId, c
       }
       holder->mTextureHosts.pop();
     }
+    while (!holder->mExternalImages.empty()) {
+      if (aEpoch <= holder->mExternalImages.front().mEpoch) {
+        break;
+      }
+      DebugOnly<bool> released =
+        SharedSurfacesParent::Release(holder->mExternalImages.front().mImageId);
+      MOZ_ASSERT(released);
+      holder->mExternalImages.pop();
+    }
   }
 }
 

gfx/layers/wr/AsyncImagePipelineManager.h

@@ -48,6 +48,7 @@ public:
   void RemovePipeline(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch);
 
   void HoldExternalImage(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch, WebRenderTextureHost* aTexture);
+  void HoldExternalImage(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch, const wr::ExternalImageId& aImageId);
   void PipelineRendered(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch);
   void PipelineRemoved(const wr::PipelineId& aPipelineId);
 
@@ -117,9 +118,19 @@ private:
     CompositableTextureHostRef mTexture;
   };
 
+  struct ForwardingExternalImage {
+    ForwardingExternalImage(const wr::Epoch& aEpoch, const wr::ExternalImageId& aImageId)
+      : mEpoch(aEpoch)
+      , mImageId(aImageId)
+    {}
+    wr::Epoch mEpoch;
+    wr::ExternalImageId mImageId;
+  };
+
   struct PipelineTexturesHolder {
     // Holds forwarding WebRenderTextureHosts.
     std::queue<ForwardingTextureHost> mTextureHosts;
+    std::queue<ForwardingExternalImage> mExternalImages;
     Maybe<wr::Epoch> mDestroyedEpoch;
   };
 

gfx/layers/wr/WebRenderBridgeParent.cpp

@@ -384,8 +384,16 @@ WebRenderBridgeParent::AddExternalImage(wr::ExternalImageId aExtId, wr::ImageKey
     return true;
   }
 
-  RefPtr<DataSourceSurface> dSurf = SharedSurfacesParent::Get(aExtId);
+  RefPtr<DataSourceSurface> dSurf = SharedSurfacesParent::Acquire(aExtId);
   if (dSurf) {
+    bool inserted = mSharedSurfaceIds.EnsureInserted(wr::AsUint64(aExtId));
+    if (!inserted) {
+      // We already have a mapping for this image, so decrement the ownership
+      // counter just increased unnecessarily. This can happen when an image is
+      // slow to decode and we need to invalidate it by updating its image key.
+      SharedSurfacesParent::Release(aExtId);
+    }
+
     if (!gfxEnv::EnableWebRenderRecording()) {
       wr::ImageDescriptor descriptor(dSurf->GetSize(), dSurf->Stride(),
                                      dSurf->GetFormat());
@@ -943,13 +951,19 @@ WebRenderBridgeParent::RemoveExternalImageId(const ExternalImageId& aImageId)
     return;
   }
 
-  WebRenderImageHost* wrHost = mExternalImageIds.Get(wr::AsUint64(aImageId)).get();
+  uint64_t imageId = wr::AsUint64(aImageId);
+  if (mSharedSurfaceIds.EnsureRemoved(imageId)) {
+    mAsyncImageManager->HoldExternalImage(mPipelineId, mWrEpoch, aImageId);
+    return;
+  }
+
+  WebRenderImageHost* wrHost = mExternalImageIds.Get(imageId).get();
   if (!wrHost) {
     return;
   }
 
   wrHost->ClearWrBridge();
-  mExternalImageIds.Remove(wr::AsUint64(aImageId));
+  mExternalImageIds.Remove(imageId);
 
   return;
 }
@@ -1432,6 +1446,11 @@ WebRenderBridgeParent::ClearResources()
     mAsyncImageManager->RemoveAsyncImagePipeline(pipelineId, txn);
   }
   mAsyncCompositables.Clear();
+  for (auto iter = mSharedSurfaceIds.Iter(); !iter.Done(); iter.Next()) {
+    wr::ExternalImageId id = wr::ToExternalImageId(iter.Get()->GetKey());
+    mAsyncImageManager->HoldExternalImage(mPipelineId, mWrEpoch, id);
+  }
+  mSharedSurfaceIds.Clear();
 
   mAsyncImageManager->RemovePipeline(mPipelineId, wrEpoch);
   txn.RemovePipeline(mPipelineId);

gfx/layers/wr/WebRenderBridgeParent.h

@@ -251,6 +251,7 @@ private:
   std::unordered_set<uint64_t> mActiveAnimations;
   nsDataHashtable<nsUint64HashKey, RefPtr<WebRenderImageHost>> mAsyncCompositables;
   nsDataHashtable<nsUint64HashKey, RefPtr<WebRenderImageHost>> mExternalImageIds;
+  nsTHashtable<nsUint64HashKey> mSharedSurfaceIds;
 
   TimeStamp mPreviousFrameTimeStamp;
   // These fields keep track of the latest layer observer epoch values in the child and the