Bug 1366502 - Update the thread model for RegisterExternalImage(), UnregisterExternalImage() and GetRenderTexture() call. v2. r=sotaro
☠☠ backed out by b6e861dcd4c2 ☠ ☠
authorJerryShih <hshih@mozilla.com>
Tue, 06 Jun 2017 19:18:40 +0800
changeset 413042 265e39153027942550b4b2769b57d0bf04bfdcbf
parent 413041 cf598918bb1b06c2e7f2b7a268d31f3188a6174b
child 413043 7f98b7f60e5885589af43fa130c90c1b9ffe65c5
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro
bugs1366502
milestone55.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 1366502 - Update the thread model for RegisterExternalImage(), UnregisterExternalImage() and GetRenderTexture() call. v2. r=sotaro If we call UnregisterExternalImage() at non-render-thread and decrease the RenderTextureHost's ref-count to zero, the RenderTextureHost will be released in non-render-thread. That will cause some problems if we use some thread-specific functions in ~RenderTextureHost(). This patch uses a message loop in UnregisterExternalImage() to resolve this problem. MozReview-Commit-ID: CDazxGkE1cK
gfx/layers/wr/WebRenderTextureHost.cpp
gfx/webrender_bindings/RenderThread.cpp
gfx/webrender_bindings/RenderThread.h
--- a/gfx/layers/wr/WebRenderTextureHost.cpp
+++ b/gfx/layers/wr/WebRenderTextureHost.cpp
@@ -62,17 +62,17 @@ WebRenderTextureHost::CreateRenderTextur
       mIsWrappingNativeHandle = true;
       break;
     }
 #endif
     default:
       gfxCriticalError() << "No WR implement for texture type:" << aDesc.type();
   }
 
-  wr::RenderThread::Get()->RegisterExternalImage(wr::AsUint64(mExternalImageId), texture);
+  wr::RenderThread::Get()->RegisterExternalImage(wr::AsUint64(mExternalImageId), texture.forget());
 }
 
 bool
 WebRenderTextureHost::Lock()
 {
   MOZ_ASSERT_UNREACHABLE("unexpected to be called");
   return false;
 }
--- a/gfx/webrender_bindings/RenderThread.cpp
+++ b/gfx/webrender_bindings/RenderThread.cpp
@@ -270,34 +270,59 @@ RenderThread::DecPendingFrameCount(wr::W
   if (oldCount <= 0) {
     return;
   }
   // Update pending frame count.
   mPendingFrameCounts.Put(AsUint64(aWindowId), oldCount - 1);
 }
 
 void
-RenderThread::RegisterExternalImage(uint64_t aExternalImageId, RenderTextureHost* aTexture)
+RenderThread::RegisterExternalImage(uint64_t aExternalImageId, already_AddRefed<RenderTextureHost> aTexture)
 {
   MutexAutoLock lock(mRenderTextureMapLock);
-  MOZ_ASSERT(!mRenderTextures.Get(aExternalImageId));
-  mRenderTextures.Put(aExternalImageId, aTexture);
+
+  MOZ_ASSERT(!mRenderTextures.Get(aExternalImageId).get());
+  RefPtr<RenderTextureHost> texture(aTexture);
+  mRenderTextures.Put(aExternalImageId, Move(texture));
 }
 
 void
 RenderThread::UnregisterExternalImage(uint64_t aExternalImageId)
 {
   MutexAutoLock lock(mRenderTextureMapLock);
   MOZ_ASSERT(mRenderTextures.Get(aExternalImageId).get());
-  mRenderTextures.Remove(aExternalImageId);
+  if (!IsInRenderThread()) {
+    // The RenderTextureHost should be released in render thread. So, post the
+    // deletion task here.
+    // The shmem and raw buffer are owned by compositor ipc channel. It's
+    // possible that RenderTextureHost is still exist after the shmem/raw buffer
+    // deletion. Then the buffer in RenderTextureHost becomes invalid. It's fine
+    // for this situation. Gecko will only release the buffer if WR doesn't need
+    // it. So, no one will access the invalid buffer in RenderTextureHost.
+    RefPtr<RenderTextureHost> texture = mRenderTextures.Get(aExternalImageId);
+    mRenderTextures.Remove(aExternalImageId);
+    Loop()->PostTask(NewRunnableMethod<RefPtr<RenderTextureHost>>(
+      this, &RenderThread::DeferredRenderTextureHostDestroy, Move(texture)
+    ));
+  } else {
+    mRenderTextures.Remove(aExternalImageId);
+  }
+}
+
+void
+RenderThread::DeferredRenderTextureHostDestroy(RefPtr<RenderTextureHost>)
+{
+  // Do nothing. Just decrease the ref-count of RenderTextureHost.
 }
 
 RenderTextureHost*
 RenderThread::GetRenderTexture(WrExternalImageId aExternalImageId)
 {
+  MOZ_ASSERT(IsInRenderThread());
+
   MutexAutoLock lock(mRenderTextureMapLock);
   MOZ_ASSERT(mRenderTextures.Get(aExternalImageId.mHandle).get());
   return mRenderTextures.Get(aExternalImageId.mHandle).get();
 }
 
 } // namespace wr
 } // namespace mozilla
 
--- a/gfx/webrender_bindings/RenderThread.h
+++ b/gfx/webrender_bindings/RenderThread.h
@@ -100,32 +100,37 @@ public:
   void RunEvent(wr::WindowId aWindowId, UniquePtr<RendererEvent> aCallBack);
 
   /// Can only be called from the render thread.
   void UpdateAndRender(wr::WindowId aWindowId);
 
   void Pause(wr::WindowId aWindowId);
   bool Resume(wr::WindowId aWindowId);
 
-  void RegisterExternalImage(uint64_t aExternalImageId, RenderTextureHost* aTexture);
+  /// Can be called from any thread.
+  void RegisterExternalImage(uint64_t aExternalImageId, already_AddRefed<RenderTextureHost> aTexture);
 
+  /// Can be called from any thread.
   void UnregisterExternalImage(uint64_t aExternalImageId);
 
+  /// Can only be called from the render thread.
   RenderTextureHost* GetRenderTexture(WrExternalImageId aExternalImageId);
 
   /// Can be called from any thread.
   uint32_t GetPendingFrameCount(wr::WindowId aWindowId);
   /// Can be called from any thread.
   void IncPendingFrameCount(wr::WindowId aWindowId);
   /// Can be called from any thread.
   void DecPendingFrameCount(wr::WindowId aWindowId);
 
 private:
   explicit RenderThread(base::Thread* aThread);
 
+  void DeferredRenderTextureHostDestroy(RefPtr<RenderTextureHost> aTexture);
+
   ~RenderThread();
 
   base::Thread* const mThread;
 
   std::map<wr::WindowId, UniquePtr<RendererOGL>> mRenderers;
 
   Mutex mPendingFrameCountMapLock;
   nsDataHashtable<nsUint64HashKey, uint32_t> mPendingFrameCounts;