Bug 1280762 - ReadUnlock the TextureHost when its IPDL actor dies. r=sotaro a=lhenry
authorNicolas Silva <nsilva@mozilla.com>
Tue, 21 Jun 2016 18:06:34 +0200
changeset 339819 3a78197e37b6ffaa41aa426f4a7d8479c061febc
parent 339818 a2db4c13a2b9ac395db681b7f16e5abb528f0663
child 339820 a8e8182cf0001db745f02afed94fd60275054ccd
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro, lhenry
bugs1280762
milestone49.0a2
Bug 1280762 - ReadUnlock the TextureHost when its IPDL actor dies. r=sotaro a=lhenry
gfx/layers/Compositor.cpp
gfx/layers/Compositor.h
gfx/layers/basic/BasicCompositor.h
gfx/layers/composite/TextureHost.cpp
gfx/layers/composite/TextureHost.h
gfx/layers/d3d11/CompositorD3D11.h
gfx/layers/d3d9/CompositorD3D9.h
gfx/layers/opengl/CompositorOGL.cpp
--- a/gfx/layers/Compositor.cpp
+++ b/gfx/layers/Compositor.cpp
@@ -33,29 +33,44 @@ Compositor::Compositor(widget::Composito
   , mPixelsFilled(0)
   , mScreenRotation(ROTATION_0)
   , mWidget(aWidget)
 {
 }
 
 Compositor::~Compositor()
 {
-  for (auto& lock : mUnlockAfterComposition) {
-    lock->ReadUnlock();
+  ReadUnlockTextures();
+}
+
+void
+Compositor::Destroy()
+{
+  ReadUnlockTextures();
+}
+
+void
+Compositor::EndFrame()
+{
+  ReadUnlockTextures();
+}
+
+void
+Compositor::ReadUnlockTextures()
+{
+  for (auto& texture : mUnlockAfterComposition) {
+    texture->ReadUnlock();
   }
   mUnlockAfterComposition.Clear();
 }
 
 void
-Compositor::EndFrame()
+Compositor::UnlockAfterComposition(TextureHost* aTexture)
 {
-  for (auto& lock : mUnlockAfterComposition) {
-    lock->ReadUnlock();
-  }
-  mUnlockAfterComposition.Clear();
+  mUnlockAfterComposition.AppendElement(aTexture);
 }
 
 /* static */ void
 Compositor::AssertOnCompositorThread()
 {
   MOZ_ASSERT(!CompositorThreadHolder::Loop() ||
              CompositorThreadHolder::Loop() == MessageLoop::current(),
              "Can only call this from the compositor thread!");
--- a/gfx/layers/Compositor.h
+++ b/gfx/layers/Compositor.h
@@ -195,17 +195,17 @@ public:
                       CompositorBridgeParent* aParent = nullptr);
 
   virtual already_AddRefed<DataTextureSource> CreateDataTextureSource(TextureFlags aFlags = TextureFlags::NO_FLAGS) = 0;
 
   virtual already_AddRefed<DataTextureSource>
   CreateDataTextureSourceAround(gfx::DataSourceSurface* aSurface) { return nullptr; }
 
   virtual bool Initialize() = 0;
-  virtual void Destroy() = 0;
+  virtual void Destroy();
 
   virtual void DetachWidget() { mWidget = nullptr; }
 
   /**
    * Return true if the effect type is supported.
    *
    * By default Compositor implementations should support all effects but in
    * some rare cases it is not possible to support an effect efficiently.
@@ -534,30 +534,30 @@ public:
   }
 
   /// Most compositor backends operate asynchronously under the hood. This
   /// means that when a layer stops using a texture it is often desirable to
   /// wait for the end of the next composition before releasing the texture's
   /// ReadLock.
   /// This function provides a convenient way to do this delayed unlocking, if
   /// the texture itself requires it.
-  void UnlockAfterComposition(already_AddRefed<TextureReadLock> aLock)
-  {
-    mUnlockAfterComposition.AppendElement(aLock);
-  }
+  void UnlockAfterComposition(TextureHost* aTexture);
 
 protected:
   void DrawDiagnosticsInternal(DiagnosticFlags aFlags,
                                const gfx::Rect& aVisibleRect,
                                const gfx::IntRect& aClipRect,
                                const gfx::Matrix4x4& transform,
                                uint32_t aFlashCounter);
 
   bool ShouldDrawDiagnostics(DiagnosticFlags);
 
+  // Should be called at the end of each composition.
+  void ReadUnlockTextures();
+
   /**
    * Given a layer rect, clip, and transform, compute the area of the backdrop that
    * needs to be copied for mix-blending. The output transform translates from 0..1
    * space into the backdrop rect space.
    *
    * The transformed layer quad is also optionally returned - this is the same as
    * the result rect, before rounding.
    */
@@ -566,17 +566,17 @@ protected:
     const gfx::IntRect& aClipRect,
     const gfx::Matrix4x4& aTransform,
     gfx::Matrix4x4* aOutTransform,
     gfx::Rect* aOutLayerQuad = nullptr);
 
   /**
    * An array of locks that will need to be unlocked after the next composition.
    */
-  nsTArray<RefPtr<TextureReadLock>> mUnlockAfterComposition;
+  nsTArray<RefPtr<TextureHost>> mUnlockAfterComposition;
 
   /**
    * Render time for the current composition.
    */
   TimeStamp mCompositionTime;
   /**
    * When nonnull, during rendering, some compositable indicated that it will
    * change its rendering at this time. In order not to miss it, we composite
--- a/gfx/layers/basic/BasicCompositor.h
+++ b/gfx/layers/basic/BasicCompositor.h
@@ -48,18 +48,16 @@ protected:
   virtual ~BasicCompositor();
 
 public:
 
   virtual BasicCompositor* AsBasicCompositor() override { return this; }
 
   virtual bool Initialize() override;
 
-  virtual void Destroy() override {}
-
   virtual void DetachWidget() override;
 
   virtual TextureFactoryIdentifier GetTextureFactoryIdentifier() override;
 
   virtual already_AddRefed<CompositingRenderTarget>
   CreateRenderTarget(const gfx::IntRect &aRect, SurfaceInitMode aInit) override;
 
   virtual already_AddRefed<CompositingRenderTarget>
--- a/gfx/layers/composite/TextureHost.cpp
+++ b/gfx/layers/composite/TextureHost.cpp
@@ -334,17 +334,17 @@ TextureHost::UnbindTextureSource()
   if (mReadLock) {
     auto compositor = GetCompositor();
     // This TextureHost is not used anymore. Since most compositor backends are
     // working asynchronously under the hood a compositor could still be using
     // this texture, so it is generally best to wait until the end of the next
     // composition before calling ReadUnlock. We ask the compositor to take care
     // of that for us.
     if (compositor) {
-      compositor->UnlockAfterComposition(mReadLock.forget());
+      compositor->UnlockAfterComposition(this);
     } else {
       // GetCompositor returned null which means no compositor can be using this
       // texture. We can ReadUnlock right away.
       ReadUnlock();
     }
   }
 }
 
@@ -1031,16 +1031,20 @@ TextureParent::Init(const SurfaceDescrip
 
 void
 TextureParent::Destroy()
 {
   if (!mTextureHost) {
     return;
   }
 
+  // ReadUnlock here to make sure the ReadLock's shmem does not outlive the
+  // protocol that created it.
+  mTextureHost->ReadUnlock();
+
   if (mTextureHost->GetFlags() & TextureFlags::RECYCLE) {
     RECYCLE_LOG("clear recycling for tile %p\n", this);
     mTextureHost->ClearRecycleCallback();
   }
   if (mTextureHost->GetFlags() & TextureFlags::DEALLOCATE_CLIENT) {
     mTextureHost->ForgetSharedData();
   }
 
--- a/gfx/layers/composite/TextureHost.h
+++ b/gfx/layers/composite/TextureHost.h
@@ -617,16 +617,17 @@ protected:
 
   virtual void UpdatedInternal(const nsIntRegion *Region) {}
 
   PTextureParent* mActor;
   RefPtr<TextureReadLock> mReadLock;
   TextureFlags mFlags;
   int mCompositableCount;
 
+  friend class Compositor;
   friend class TextureParent;
 };
 
 /**
  * TextureHost that wraps a random access buffer such as a Shmem or some raw
  * memory.
  *
  * This TextureHost is backend-independent and the backend-specific bits are
--- a/gfx/layers/d3d11/CompositorD3D11.h
+++ b/gfx/layers/d3d11/CompositorD3D11.h
@@ -43,17 +43,16 @@ class CompositorD3D11 : public Composito
 {
 public:
   CompositorD3D11(CompositorBridgeParent* aParent, widget::CompositorWidgetProxy* aWidget);
   ~CompositorD3D11();
 
   virtual CompositorD3D11* AsCompositorD3D11() override { return this; }
 
   virtual bool Initialize() override;
-  virtual void Destroy() override {}
 
   virtual TextureFactoryIdentifier
     GetTextureFactoryIdentifier() override;
 
   virtual already_AddRefed<DataTextureSource>
     CreateDataTextureSource(TextureFlags aFlags = TextureFlags::NO_FLAGS) override;
 
   virtual bool CanUseCanvasLayerForSize(const gfx::IntSize& aSize) override;
--- a/gfx/layers/d3d9/CompositorD3D9.h
+++ b/gfx/layers/d3d9/CompositorD3D9.h
@@ -22,17 +22,16 @@ class CompositorD3D9 : public Compositor
 {
 public:
   CompositorD3D9(CompositorBridgeParent* aParent, widget::CompositorWidgetProxy* aWidget);
   ~CompositorD3D9();
 
   virtual CompositorD3D9* AsCompositorD3D9() override { return this; }
 
   virtual bool Initialize() override;
-  virtual void Destroy() override {}
 
   virtual TextureFactoryIdentifier
     GetTextureFactoryIdentifier() override;
 
   virtual bool CanUseCanvasLayerForSize(const gfx::IntSize &aSize) override;
   virtual int32_t GetMaxTextureSize() const final;
 
   virtual void MakeCurrent(MakeCurrentFlags aFlags = 0) override {}
--- a/gfx/layers/opengl/CompositorOGL.cpp
+++ b/gfx/layers/opengl/CompositorOGL.cpp
@@ -152,16 +152,18 @@ CompositorOGL::CreateContext()
 #endif
 
   return context.forget();
 }
 
 void
 CompositorOGL::Destroy()
 {
+  Compositor::Destroy();
+
   if (mTexturePool) {
     mTexturePool->Clear();
     mTexturePool = nullptr;
   }
 
   if (!mDestroyed) {
     mDestroyed = true;
     CleanupResources();