Bug 1575874: Create the CanvasTranslator on the Compositor thread to prevent race with WaitForSurfaceDescriptor. r=jrmuizel

Differential Revision: https://phabricator.services.mozilla.com/D43774

--HG--
extra : moz-landing-system : lando

gfx/2d/InlineTranslator.cpp

@@ -18,6 +18,9 @@ using namespace mozilla::gfx;
 namespace mozilla {
 namespace gfx {
 
+InlineTranslator::InlineTranslator()
+    : mFontContext(nullptr) {}
+
 InlineTranslator::InlineTranslator(DrawTarget* aDT, void* aFontContext)
     : mBaseDT(aDT), mFontContext(aFontContext) {}
 
@@ -101,6 +104,8 @@ bool InlineTranslator::TranslateRecording(char* aData, size_t aLen) {
 already_AddRefed<DrawTarget> InlineTranslator::CreateDrawTarget(
     ReferencePtr aRefPtr, const gfx::IntSize& aSize,
     gfx::SurfaceFormat aFormat) {
+  MOZ_ASSERT(mBaseDT, "mBaseDT has not been initialized.");
+
   RefPtr<DrawTarget> drawTarget = mBaseDT;
   AddDrawTarget(aRefPtr, drawTarget);
   return drawTarget.forget();

gfx/2d/InlineTranslator.h

@@ -30,6 +30,8 @@ using gfx::Translator;
 
 class InlineTranslator : public Translator {
  public:
+  InlineTranslator();
+
   explicit InlineTranslator(DrawTarget* aDT, void* aFontContext = nullptr);
 
   bool TranslateRecording(char*, size_t len);
@@ -163,13 +165,18 @@ class InlineTranslator : public Translator {
       ReferencePtr aRefPtr, const gfx::IntSize& aSize,
       gfx::SurfaceFormat aFormat) override;
 
-  mozilla::gfx::DrawTarget* GetReferenceDrawTarget() final { return mBaseDT; }
+  mozilla::gfx::DrawTarget* GetReferenceDrawTarget() final {
+    MOZ_ASSERT(mBaseDT, "mBaseDT has not been initialized.");
+    return mBaseDT;
+  }
 
   void* GetFontContext() final { return mFontContext; }
   std::string GetError() { return mError; }
 
- private:
+ protected:
   RefPtr<DrawTarget> mBaseDT;
+
+ private:
   void* mFontContext;
   std::string mError;
 

gfx/layers/CanvasTranslator.cpp

@@ -50,36 +50,33 @@ static TextureData* CreateTextureData(TextureType aTextureType,
 }
 
 /* static */
-UniquePtr<CanvasTranslator> CanvasTranslator::Create(
+UniquePtr<CanvasTranslator> CanvasTranslator::Create() {
+  return UniquePtr<CanvasTranslator>(new CanvasTranslator());
+}
+
+CanvasTranslator::CanvasTranslator() : gfx::InlineTranslator() {}
+
+CanvasTranslator::~CanvasTranslator() {
+  if (mReferenceTextureData) {
+    mReferenceTextureData->Unlock();
+  }
+}
+
+bool CanvasTranslator::Init(
     const TextureType& aTextureType,
     const ipc::SharedMemoryBasic::Handle& aReadHandle,
     const CrossProcessSemaphoreHandle& aReaderSem,
     const CrossProcessSemaphoreHandle& aWriterSem,
     UniquePtr<CanvasEventRingBuffer::ReaderServices> aReaderServices) {
-  TextureData* textureData = CreateTextureData(aTextureType, gfx::IntSize(1, 1),
-                                               gfx::SurfaceFormat::B8G8R8A8);
-  textureData->Lock(OpenMode::OPEN_READ_WRITE);
-  RefPtr<gfx::DrawTarget> dt = textureData->BorrowDrawTarget();
-  return UniquePtr<CanvasTranslator>(
-      new CanvasTranslator(aTextureType, textureData, dt, aReadHandle,
-                           aReaderSem, aWriterSem, std::move(aReaderServices)));
+  mTextureType = aTextureType;
+  mReferenceTextureData.reset(CreateTextureData(
+      aTextureType, gfx::IntSize(1, 1), gfx::SurfaceFormat::B8G8R8A8));
+  mReferenceTextureData->Lock(OpenMode::OPEN_READ_WRITE);
+  mBaseDT = mReferenceTextureData->BorrowDrawTarget();
+  return mStream.InitReader(aReadHandle, aReaderSem, aWriterSem,
+                            std::move(aReaderServices));
 }
 
-CanvasTranslator::CanvasTranslator(
-    const TextureType& aTextureType, TextureData* aTextureData,
-    gfx::DrawTarget* aDT, const ipc::SharedMemoryBasic::Handle& aReadHandle,
-    const CrossProcessSemaphoreHandle& aReaderSem,
-    const CrossProcessSemaphoreHandle& aWriterSem,
-    UniquePtr<CanvasEventRingBuffer::ReaderServices> aReaderServices)
-    : gfx::InlineTranslator(aDT),
-      mTextureType(aTextureType),
-      mReferenceTextureData(aTextureData) {
-  mStream.InitReader(aReadHandle, aReaderSem, aWriterSem,
-                     std::move(aReaderServices));
-}
-
-CanvasTranslator::~CanvasTranslator() { mReferenceTextureData->Unlock(); }
-
 bool CanvasTranslator::TranslateRecording() {
   int32_t eventType = mStream.ReadNextEvent();
   while (mStream.good()) {

gfx/layers/CanvasTranslator.h

@@ -25,8 +25,22 @@ class TextureData;
 class CanvasTranslator final : public gfx::InlineTranslator {
  public:
   /**
-   * Create a canvas translator for a particular TextureType, which translates
-   * events from a CanvasEventRingBuffer.
+   * Create an uninitialized CanvasTranslator. This must be initialized via the
+   * Init function before any translation can occur. We need this to be able
+   * to create the CanvasTranslator on a different thread where we will need
+   * to call WaitForSurfaceDescriptor. Otherwise we have a race between the
+   * CanvasTranslator being created on the canvas thread and the first texture
+   * being required on the Compositor thread.
+   *
+   * @returns the new CanvasTranslator
+   */
+  static UniquePtr<CanvasTranslator> Create();
+
+  ~CanvasTranslator();
+
+  /**
+   * Initializes a canvas translator for a particular TextureType, which
+   * translates events from a CanvasEventRingBuffer.
    *
    * @param aTextureType the TextureType the translator will create
    * @param aReadHandle handle to the shared memory for the
@@ -34,16 +48,13 @@ class CanvasTranslator final : public gfx::InlineTranslator {
    * @param aReaderSem reading blocked semaphore for the CanvasEventRingBuffer
    * @param aWriterSem writing blocked semaphore for the CanvasEventRingBuffer
    * @param aReaderServices provides functions required by the reader
-   * @returns the new CanvasTranslator
+   * @returns true if the initialization works, false otherwise
    */
-  static UniquePtr<CanvasTranslator> Create(
-      const TextureType& aTextureType,
-      const ipc::SharedMemoryBasic::Handle& aReadHandle,
-      const CrossProcessSemaphoreHandle& aReaderSem,
-      const CrossProcessSemaphoreHandle& aWriterSem,
-      UniquePtr<CanvasEventRingBuffer::ReaderServices> aReaderServices);
-
-  ~CanvasTranslator();
+  bool Init(const TextureType& aTextureType,
+            const ipc::SharedMemoryBasic::Handle& aReadHandle,
+            const CrossProcessSemaphoreHandle& aReaderSem,
+            const CrossProcessSemaphoreHandle& aWriterSem,
+            UniquePtr<CanvasEventRingBuffer::ReaderServices> aReaderServices);
 
   bool IsValid() { return mIsValid; }
 
@@ -185,13 +196,7 @@ class CanvasTranslator final : public gfx::InlineTranslator {
       gfx::ReferencePtr aSurface);
 
  private:
-  CanvasTranslator(
-      const TextureType& aTextureType, TextureData* aTextureData,
-      gfx::DrawTarget* aDrawTarget,
-      const ipc::SharedMemoryBasic::Handle& aReadHandle,
-      const CrossProcessSemaphoreHandle& aReaderSem,
-      const CrossProcessSemaphoreHandle& aWriterSem,
-      UniquePtr<CanvasEventRingBuffer::ReaderServices> aReaderServices);
+  CanvasTranslator(); 
 
   void AddSurfaceDescriptor(gfx::ReferencePtr aRefPtr,
                             TextureData* atextureData);

gfx/layers/composite/TextureHost.cpp

@@ -222,6 +222,11 @@ already_AddRefed<TextureHost> TextureHost::Create(
       UniquePtr<SurfaceDescriptor> realDesc =
           aDeallocator->AsCompositorBridgeParentBase()
               ->LookupSurfaceDescriptorForClientDrawTarget(desc.drawTarget());
+      if (!realDesc) {
+        NS_WARNING("Failed to get descriptor for recorded texture.");
+        return nullptr;
+      }
+
       result = TextureHost::Create(*realDesc, aReadLock, aDeallocator, aBackend,
                                    aFlags, aExternalImageId);
       return result.forget();

gfx/layers/ipc/CanvasChild.cpp

@@ -124,8 +124,7 @@ void CanvasChild::EnsureRecorder(TextureType aTextureType) {
                     MakeUnique<RingBufferWriterServices>(this));
 
     if (CanSend()) {
-      Unused << SendCreateTranslator(mTextureType, handle, readerSem,
-                                     writerSem);
+      Unused << SendInitTranslator(mTextureType, handle, readerSem, writerSem);
     }
   }
 

gfx/layers/ipc/CanvasParent.cpp

@@ -138,7 +138,7 @@ static void EnsureAllClosed() {
   }
 }
 
-CanvasParent::CanvasParent() {}
+CanvasParent::CanvasParent() : mTranslator(CanvasTranslator::Create()) {}
 
 CanvasParent::~CanvasParent() {}
 
@@ -150,14 +150,16 @@ void CanvasParent::Bind(Endpoint<PCanvasParent>&& aEndpoint) {
   CanvasParents().PutEntry(this);
 }
 
-mozilla::ipc::IPCResult CanvasParent::RecvCreateTranslator(
+mozilla::ipc::IPCResult CanvasParent::RecvInitTranslator(
     const TextureType& aTextureType,
     const ipc::SharedMemoryBasic::Handle& aReadHandle,
     const CrossProcessSemaphoreHandle& aReaderSem,
     const CrossProcessSemaphoreHandle& aWriterSem) {
-  mTranslator = CanvasTranslator::Create(
-      aTextureType, aReadHandle, aReaderSem, aWriterSem,
-      MakeUnique<RingBufferReaderServices>(this));
+  if (!mTranslator->Init(aTextureType, aReadHandle, aReaderSem, aWriterSem,
+                         MakeUnique<RingBufferReaderServices>(this))) {
+    return IPC_FAIL(this, "Failed to initialize CanvasTranslator.");
+  }
+
   return RecvResumeTranslation();
 }
 
@@ -198,6 +200,11 @@ void CanvasParent::StartTranslation() {
 UniquePtr<SurfaceDescriptor>
 CanvasParent::LookupSurfaceDescriptorForClientDrawTarget(
     const uintptr_t aDrawTarget) {
+  if (!mTranslator) {
+    // We are shutting down.
+    return nullptr;
+  }
+
   return mTranslator->WaitForSurfaceDescriptor(
       reinterpret_cast<void*>(aDrawTarget));
 }

gfx/layers/ipc/CanvasParent.h

@@ -40,8 +40,8 @@ class CanvasParent final : public PCanvasParent {
   static void Shutdown();
 
   /**
-   * Create a canvas translator for a particular TextureType, which translates
-   * events from a CanvasEventRingBuffer.
+   * Initialize a canvas translator for a particular TextureType, which
+   * translates events from a CanvasEventRingBuffer.
    *
    * @param aTextureType the TextureType the translator will create
    * @param aReadHandle handle to the shared memory for the
@@ -49,7 +49,7 @@ class CanvasParent final : public PCanvasParent {
    * @param aReaderSem reading blocked semaphore for the CanvasEventRingBuffer
    * @param aWriterSem writing blocked semaphore for the CanvasEventRingBuffer
    */
-  ipc::IPCResult RecvCreateTranslator(
+  ipc::IPCResult RecvInitTranslator(
       const TextureType& aTextureType,
       const ipc::SharedMemoryBasic::Handle& aReadHandle,
       const CrossProcessSemaphoreHandle& aReaderSem,

gfx/layers/ipc/PCanvas.ipdl

@@ -17,14 +17,14 @@ namespace layers {
 sync protocol PCanvas {
 parent:
   /**
-   * Create a CanvasTranslator for a particular TextureType, which translates
-   * events from a CanvasEventRingBuffer. aReadHandle is the shared memory
-   * handle for the ring buffer. aReaderSem and aWriterSem are handles for the
-   * semaphores to handle waiting on either side.
+   * Initialize a CanvasTranslator for a particular TextureType, which
+   * translates events from a CanvasEventRingBuffer. aReadHandle is the shared
+   * memory handle for the ring buffer. aReaderSem and aWriterSem are handles
+   * for the semaphores to handle waiting on either side.
    */
-  async CreateTranslator(TextureType aTextureType, Handle aReadHandle,
-                         CrossProcessSemaphoreHandle aReaderSem,
-                         CrossProcessSemaphoreHandle aWriterSem);
+  async InitTranslator(TextureType aTextureType, Handle aReadHandle,
+                       CrossProcessSemaphoreHandle aReaderSem,
+                       CrossProcessSemaphoreHandle aWriterSem);
 
   /**
    * Used to tell the CanvasTranslator to start translating again after it has