Bug 1385101 Part 2 Hold TextureClients alive during async painting. r…

…=mattwoodrow,mchang

gfx/layers/AtomicRefCountedWithFinalize.h

@@ -175,6 +175,11 @@ class AtomicRefCountedWithFinalize
       return mRefCount < 0;
     }
 
+    bool HasOneRef() const
+    {
+      return mRefCount == 1;
+    }
+
 private:
     RecycleCallback mRecycleCallback;
     void *mClosure;

gfx/layers/PaintThread.cpp

@@ -24,20 +24,17 @@ PlatformThreadId PaintThread::sThreadId;
 
 // RAII make sure we clean up and restore our draw targets
 // when we paint async.
-struct AutoCapturedPaintSetup {
-  AutoCapturedPaintSetup(DrawTarget* aTarget,
-                         DrawTargetCapture* aCapture,
-                         CompositorBridgeChild* aBridge)
-  : mTarget(aTarget)
-  , mRestorePermitsSubpixelAA(aTarget->GetPermitSubpixelAA())
-  , mOldTransform(aTarget->GetTransform())
+struct MOZ_STACK_CLASS AutoCapturedPaintSetup
+{
+  AutoCapturedPaintSetup(CapturedPaintState* aState, CompositorBridgeChild* aBridge)
+  : mState(aState)
+  , mTarget(aState->mTarget)
+  , mRestorePermitsSubpixelAA(mTarget->GetPermitSubpixelAA())
+  , mOldTransform(mTarget->GetTransform())
   , mBridge(aBridge)
   {
-    MOZ_ASSERT(mTarget);
-    MOZ_ASSERT(aCapture);
-
-    mTarget->SetTransform(aCapture->GetTransform());
-    mTarget->SetPermitSubpixelAA(aCapture->GetPermitSubpixelAA());
+    mTarget->SetTransform(aState->mCapture->GetTransform());
+    mTarget->SetPermitSubpixelAA(aState->mCapture->GetPermitSubpixelAA());
   }
 
   ~AutoCapturedPaintSetup()
@@ -54,10 +51,11 @@ struct AutoCapturedPaintSetup {
     mTarget->Flush();
 
     if (mBridge) {
-      mBridge->NotifyFinishedAsyncPaint();
+      mBridge->NotifyFinishedAsyncPaint(mState);
     }
   }
 
+  RefPtr<CapturedPaintState> mState;
   DrawTarget* mTarget;
   bool mRestorePermitsSubpixelAA;
   Matrix mOldTransform;
@@ -163,7 +161,7 @@ PaintThread::PaintContentsAsync(CompositorBridgeChild* aBridge,
   DrawTarget* target = aState->mTarget;
   DrawTargetCapture* capture = aState->mCapture;
 
-  AutoCapturedPaintSetup setup(target, capture, aBridge);
+  AutoCapturedPaintSetup setup(aState, aBridge);
 
   if (!aCallback(aState)) {
     return;
@@ -186,7 +184,7 @@ PaintThread::PaintContents(CapturedPaintState* aState,
   RefPtr<CompositorBridgeChild> cbc;
   if (!gfxPrefs::LayersOMTPForceSync()) {
     cbc = CompositorBridgeChild::Get();
-    cbc->NotifyBeginAsyncPaint();
+    cbc->NotifyBeginAsyncPaint(aState);
   }
   RefPtr<CapturedPaintState> state(aState);
   RefPtr<DrawTargetCapture> capture(aState->mCapture);

gfx/layers/PaintThread.h

@@ -11,6 +11,7 @@
 #include "mozilla/RefPtr.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/UniquePtr.h"
+#include "mozilla/layers/TextureClient.h"
 #include "nsThreadUtils.h"
 
 namespace mozilla {
@@ -41,6 +42,8 @@ class CapturedPaintState {
   {}
 
   nsIntRegion mRegionToDraw;
+  RefPtr<TextureClient> mTextureClient;
+  RefPtr<TextureClient> mTextureClientOnWhite;
   RefPtr<gfx::DrawTargetCapture> mCapture;
   RefPtr<gfx::DrawTarget> mTarget;
   RefPtr<gfx::DrawTarget> mTargetOnWhite;

gfx/layers/client/ContentClient.cpp

@@ -186,7 +186,14 @@ RefPtr<CapturedPaintState>
 ContentClientRemoteBuffer::BorrowDrawTargetForRecording(PaintState& aPaintState,
                                                         RotatedContentBuffer::DrawIterator* aIter)
 {
-  return RotatedContentBuffer::BorrowDrawTargetForRecording(aPaintState, aIter);
+  RefPtr<CapturedPaintState> cps = RotatedContentBuffer::BorrowDrawTargetForRecording(aPaintState, aIter);
+  if (!cps) {
+    return nullptr;
+  }
+
+  cps->mTextureClient = mTextureClient;
+  cps->mTextureClientOnWhite = mTextureClientOnWhite;
+  return cps.forget();
 }
 
 class RemoteBufferReadbackProcessor : public TextureReadbackSink

gfx/layers/client/TextureClient.cpp

@@ -15,6 +15,7 @@
 #include "mozilla/layers/ISurfaceAllocator.h"
 #include "mozilla/layers/ImageBridgeChild.h"
 #include "mozilla/layers/ImageDataSerializer.h"
+#include "mozilla/layers/PaintThread.h"
 #include "mozilla/layers/TextureClientRecycleAllocator.h"
 #include "mozilla/Mutex.h"
 #include "nsDebug.h"                    // for NS_ASSERTION, NS_WARNING, etc
@@ -380,6 +381,9 @@ DeallocateTextureClient(TextureDeallocParams params)
 
 void TextureClient::Destroy()
 {
+  // Async paints should have been flushed by now.
+  MOZ_RELEASE_ASSERT(mPaintThreadRefs == 0);
+
   if (mActor && !mIsLocked) {
     mActor->Lock();
   }
@@ -605,6 +609,9 @@ TextureClient::SerializeReadLock(ReadLockDescriptor& aDescriptor)
 
 TextureClient::~TextureClient()
 {
+  // TextureClients should be kept alive while there are references on the
+  // paint thread.
+  MOZ_ASSERT(mPaintThreadRefs == 0);
   mReadLock = nullptr;
   Destroy();
 }
@@ -1701,6 +1708,21 @@ TextureClient::EnableBlockingReadLock()
   }
 }
 
+void
+TextureClient::AddPaintThreadRef()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  mPaintThreadRefs += 1;
+}
+
+void
+TextureClient::DropPaintThreadRef()
+{
+  MOZ_RELEASE_ASSERT(PaintThread::IsOnPaintThread());
+  MOZ_RELEASE_ASSERT(mPaintThreadRefs >= 1);
+  mPaintThreadRefs -= 1;
+}
+
 bool
 UpdateYCbCrTextureClient(TextureClient* aTexture, const PlanarYCbCrData& aData)
 {

gfx/layers/client/TextureClient.h

@@ -11,6 +11,7 @@
 #include "GLTextureImage.h"             // for TextureImage
 #include "ImageTypes.h"                 // for StereoMode
 #include "mozilla/Assertions.h"         // for MOZ_ASSERT, etc
+#include "mozilla/Atomics.h"
 #include "mozilla/Attributes.h"         // for override
 #include "mozilla/DebugOnly.h"
 #include "mozilla/RefPtr.h"             // for RefPtr, RefCounted
@@ -630,6 +631,15 @@ class TextureClient
 
   bool SerializeReadLock(ReadLockDescriptor& aDescriptor);
 
+  // Mark that the TextureClient will be used by the paint thread, and should not
+  // free its underlying texture data. This must only be called from the main
+  // thread.
+  void AddPaintThreadRef();
+
+  // Mark that the TextureClient is no longer in use by the PaintThread. This
+  // must only be called from the PaintThread.
+  void DropPaintThreadRef();
+
 private:
   static void TextureClientRecycleCallback(TextureClient* aClient, void* aClosure);
 
@@ -719,6 +729,9 @@ class TextureClient
   // Serial id of TextureClient. It is unique in current process.
   const uint64_t mSerial;
 
+  // When non-zero, texture data must not be freed.
+  mozilla::Atomic<uintptr_t> mPaintThreadRefs;
+
   // External image id. It is unique if it is allocated.
   // The id is allocated in TextureClient::InitIPDLActor().
   // Its allocation is supported by

gfx/layers/ipc/CompositorBridgeChild.cpp

@@ -171,6 +171,9 @@ CompositorBridgeChild::Destroy()
     wrBridge->Destroy(/* aIsSync */ false);
   }
 
+  // Flush async paints before we destroy texture data.
+  FlushAsyncPaints();
+
   const ManagedContainer<PTextureChild>& textures = ManagedPTextureChild();
   for (auto iter = textures.ConstIter(); !iter.Done(); iter.Next()) {
     RefPtr<TextureClient> texture = TextureClient::AsTextureClient(iter.Get()->GetKey());
@@ -1131,7 +1134,7 @@ CompositorBridgeChild::GetNextPipelineId()
 }
 
 void
-CompositorBridgeChild::NotifyBeginAsyncPaint()
+CompositorBridgeChild::NotifyBeginAsyncPaint(CapturedPaintState* aState)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
@@ -1143,17 +1146,47 @@ CompositorBridgeChild::NotifyBeginAsyncPaint()
   MOZ_ASSERT(!mIsWaitingForPaint);
 
   mOutstandingAsyncPaints++;
+
+  // Mark texture clients that they are being used for async painting, and
+  // make sure we hold them alive on the main thread.
+  aState->mTextureClient->AddPaintThreadRef();
+  mTextureClientsForAsyncPaint.AppendElement(aState->mTextureClient);
+  if (aState->mTextureClientOnWhite) {
+    aState->mTextureClientOnWhite->AddPaintThreadRef();
+    mTextureClientsForAsyncPaint.AppendElement(aState->mTextureClientOnWhite);
+  }
 }
 
 void
-CompositorBridgeChild::NotifyFinishedAsyncPaint()
+CompositorBridgeChild::NotifyFinishedAsyncPaint(CapturedPaintState* aState)
 {
   MOZ_ASSERT(PaintThread::IsOnPaintThread());
 
   MonitorAutoLock lock(mPaintLock);
 
   mOutstandingAsyncPaints--;
 
+  // These textures should be held alive on the main thread. The ref we
+  // captured should not be the final ref.
+  MOZ_RELEASE_ASSERT(!aState->mTextureClient->HasOneRef());
+
+  // It's now safe to drop the paint thread ref we're holding, since we've
+  // flushed writes to the underlying TextureData. Note that we keep the
+  // main thread ref around until FlushAsyncPaints is called, lazily ensuring
+  // the Release occurs on the main thread (versus a message in the event
+  // loop).
+  //
+  // Note that we zap our ref immediately after. Otherwise, the main thread
+  // could wake up when we drop the lock, and we could still be holding a ref
+  // on the paint thread. If this causes TextureClient to destroy then it will
+  // be destroyed on the wrong thread.
+  aState->mTextureClient->DropPaintThreadRef();
+  aState->mTextureClient = nullptr;
+  if (aState->mTextureClientOnWhite) {
+    aState->mTextureClientOnWhite->DropPaintThreadRef();
+    aState->mTextureClientOnWhite = nullptr;
+  }
+
   // It's possible that we painted so fast that the main thread never reached
   // the code that starts delaying messages. If so, mIsWaitingForPaint will be
   // false, and we can safely return.
@@ -1209,6 +1242,9 @@ CompositorBridgeChild::FlushAsyncPaints()
   while (mIsWaitingForPaint) {
     lock.Wait();
   }
+
+  // It's now safe to free any TextureClients that were used during painting.
+  mTextureClientsForAsyncPaint.Clear();
 }
 
 } // namespace layers

gfx/layers/ipc/CompositorBridgeChild.h

@@ -45,6 +45,7 @@ class CompositorManagerChild;
 class CompositorOptions;
 class TextureClient;
 class TextureClientPool;
+class CapturedPaintState;
 struct FrameMetrics;
 
 class CompositorBridgeChild final : public PCompositorBridgeChild,
@@ -223,11 +224,11 @@ class CompositorBridgeChild final : public PCompositorBridgeChild,
 
   // Must only be called from the main thread. Notifies the CompositorBridge
   // that the paint thread is going to begin painting asynchronously.
-  void NotifyBeginAsyncPaint();
+  void NotifyBeginAsyncPaint(CapturedPaintState* aState);
 
   // Must only be called from the paint thread. Notifies the CompositorBridge
   // that the paint thread has finished an asynchronous paint request.
-  void NotifyFinishedAsyncPaint();
+  void NotifyFinishedAsyncPaint(CapturedPaintState* aState);
 
   // Must only be called from the main thread. Notifies the CompoistorBridge
   // that a transaction is about to be sent, and if the paint thread is
@@ -353,6 +354,10 @@ class CompositorBridgeChild final : public PCompositorBridgeChild,
 
   FixedSizeSmallShmemSectionAllocator* mSectionAllocator;
 
+  // TextureClients that must be kept alive during async painting. This
+  // is only accessed on the main thread.
+  nsTArray<RefPtr<TextureClient>> mTextureClientsForAsyncPaint;
+
   // Off-Main-Thread Painting state. This covers access to the OMTP-related
   // state below.
   Monitor mPaintLock;