Bug 1575832: Ensure canvas recorded event translation has finished before shutting down the canvas thread. r=jrmuizel

Null checks have been added for the recorder in CanvasChild to prevent crashes
during shutdown.
This also removes mCanSend in CanvasParent and uses CanSend() instead.

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

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

gfx/layers/ipc/CanvasChild.cpp

@@ -79,7 +79,6 @@ class SourceSurfaceCanvasRecording final : public gfx::SourceSurface {
 
 CanvasChild::CanvasChild(Endpoint<PCanvasChild>&& aEndpoint) {
   aEndpoint.Bind(this);
-  mCanSend = true;
 }
 
 CanvasChild::~CanvasChild() {}
@@ -95,7 +94,7 @@ void CanvasChild::EnsureRecorder(TextureType aTextureType) {
     mRecorder->Init(OtherPid(), &handle, &readerSem, &writerSem,
                     MakeUnique<RingBufferWriterServices>(this));
 
-    if (mCanSend) {
+    if (CanSend()) {
       Unused << SendCreateTranslator(mTextureType, handle, readerSem,
                                      writerSem);
     }
@@ -106,27 +105,37 @@ void CanvasChild::EnsureRecorder(TextureType aTextureType) {
 }
 
 void CanvasChild::ActorDestroy(ActorDestroyReason aWhy) {
-  mCanSend = false;
-
   // Explicitly drop our reference to the recorder, because it holds a reference
   // to us via the ResumeTranslation callback.
   mRecorder = nullptr;
 }
 
 void CanvasChild::ResumeTranslation() {
-  if (mCanSend) {
+  if (CanSend()) {
     SendResumeTranslation();
   }
 }
 
-void CanvasChild::Destroy() { Close(); }
+void CanvasChild::Destroy() {
+  if (CanSend()) {
+    Close();
+  }
+}
 
 void CanvasChild::OnTextureWriteLock() {
+  if (!mRecorder) {
+    return;
+  }
+
   mHasOutstandingWriteLock = true;
   mLastWriteLockCheckpoint = mRecorder->CreateCheckpoint();
 }
 
 void CanvasChild::OnTextureForwarded() {
+  if (!mRecorder) {
+    return;
+  }
+
   if (mHasOutstandingWriteLock) {
     mRecorder->RecordEvent(RecordedCanvasFlush());
     if (!mRecorder->WaitForCheckpoint(mLastWriteLockCheckpoint)) {
@@ -138,6 +147,10 @@ void CanvasChild::OnTextureForwarded() {
 }
 
 void CanvasChild::EnsureBeginTransaction() {
+  if (!mRecorder) {
+    return;
+  }
+
   if (!mIsInTransaction) {
     mRecorder->RecordEvent(RecordedCanvasBeginTransaction());
     mIsInTransaction = true;
@@ -145,6 +158,10 @@ void CanvasChild::EnsureBeginTransaction() {
 }
 
 void CanvasChild::EndTransaction() {
+  if (!mRecorder) {
+    return;
+  }
+
   if (mIsInTransaction) {
     mRecorder->RecordEvent(RecordedCanvasEndTransaction());
     mIsInTransaction = false;
@@ -165,6 +182,10 @@ already_AddRefed<gfx::DrawTarget> CanvasChild::CreateDrawTarget(
 }
 
 void CanvasChild::RecordEvent(const gfx::RecordedEvent& aEvent) {
+  if (!mRecorder) {
+    return;
+  }
+
   mRecorder->RecordEvent(aEvent);
 }
 
@@ -173,6 +194,10 @@ already_AddRefed<gfx::DataSourceSurface> CanvasChild::GetDataSurface(
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aSurface);
 
+  if (!mRecorder) {
+    return nullptr;
+  }
+
   mTransactionsSinceGetDataSurface = 0;
   EnsureBeginTransaction();
   mRecorder->RecordEvent(RecordedPrepareDataForSurface(aSurface));

gfx/layers/ipc/CanvasChild.h

@@ -123,7 +123,6 @@ class CanvasChild final : public PCanvasChild {
   TextureType mTextureType = TextureType::Unknown;
   uint32_t mLastWriteLockCheckpoint = 0;
   uint32_t mTransactionsSinceGetDataSurface = kCacheDataSurfaceThreshold;
-  bool mCanSend = false;
   bool mIsInTransaction = false;
   bool mHasOutstandingWriteLock = false;
 };

gfx/layers/ipc/CanvasParent.cpp

@@ -10,6 +10,7 @@
 #include "mozilla/layers/SourceSurfaceSharedData.h"
 #include "mozilla/layers/TextureClient.h"
 #include "mozilla/SharedThreadPool.h"
+#include "nsTHashtable.h"
 #include "prsystem.h"
 
 #if defined(XP_WIN)
@@ -56,6 +57,13 @@ static MessageLoop* CanvasPlaybackLoop() {
   return sCanvasThread ? sCanvasThread->message_loop() : nullptr;
 }
 
+typedef nsTHashtable<nsRefPtrHashKey<CanvasParent>> CanvasParentSet;
+
+static CanvasParentSet& CanvasParents() {
+  static CanvasParentSet* sCanvasParents = new CanvasParentSet();
+  return *sCanvasParents;
+}
+
 /* static */
 already_AddRefed<CanvasParent> CanvasParent::Create(
     ipc::Endpoint<PCanvasParent>&& aEndpoint) {
@@ -96,10 +104,29 @@ static already_AddRefed<nsIThreadPool> GetCanvasWorkers() {
   return do_AddRef(sCanvasWorkers);
 }
 
+static void EnsureAllClosed() {
+  // Close removes the CanvasParent from the set so take a copy first.
+  CanvasParentSet canvasParents;
+  for (auto iter = CanvasParents().Iter(); !iter.Done(); iter.Next()) {
+    canvasParents.PutEntry(iter.Get()->GetKey());
+  }
+
+  for (auto iter = canvasParents.Iter(); !iter.Done(); iter.Next()) {
+    iter.Get()->GetKey()->Close();
+    iter.Remove();
+  }
+
+  MOZ_DIAGNOSTIC_ASSERT(CanvasParents().IsEmpty(),
+                        "Closing should have cleared all entries.");
+}
+
 /* static */ void CanvasParent::Shutdown() {
   sShuttingDown = true;
 
   if (sCanvasThread) {
+    RefPtr<Runnable> runnable =
+        NewRunnableFunction("CanvasParent::EnsureAllClosed", &EnsureAllClosed);
+    sCanvasThread->message_loop()->PostTask(runnable.forget());
     sCanvasThread->Stop();
     delete sCanvasThread;
     sCanvasThread = nullptr;
@@ -120,7 +147,7 @@ void CanvasParent::Bind(Endpoint<PCanvasParent>&& aEndpoint) {
     return;
   }
 
-  mSelfRef = this;
+  CanvasParents().PutEntry(this);
 }
 
 mozilla::ipc::IPCResult CanvasParent::RecvCreateTranslator(
@@ -188,7 +215,7 @@ void CanvasParent::ActorDealloc() {
   MOZ_DIAGNOSTIC_ASSERT(!mTranslating);
   MOZ_DIAGNOSTIC_ASSERT(!mTranslator);
 
-  mSelfRef = nullptr;
+  CanvasParents().RemoveEntry(this);
 }
 
 }  // namespace layers

gfx/layers/ipc/CanvasParent.h

@@ -87,7 +87,6 @@ class CanvasParent final : public PCanvasParent {
 
   void StartTranslation();
 
-  RefPtr<CanvasParent> mSelfRef;
   UniquePtr<CanvasTranslator> mTranslator;
   volatile bool mTranslating = false;
 };