Bug 1363280 - Fix Updating sImageBridges handling r=dvander,nical

gfx/layers/ipc/CompositorThread.cpp

@@ -7,6 +7,7 @@
 #include "MainThreadUtils.h"
 #include "nsThreadUtils.h"
 #include "CompositorBridgeParent.h"
+#include "mozilla/layers/ImageBridgeParent.h"
 #include "mozilla/media/MediaSystemResourceService.h"
 
 namespace mozilla {
@@ -105,6 +106,7 @@ CompositorThreadHolder::CreateCompositorThread()
   }
 
   CompositorBridgeParent::Setup();
+  ImageBridgeParent::Setup();
 
   return compositorThread;
 }

gfx/layers/ipc/ImageBridgeParent.cpp

@@ -10,6 +10,7 @@
 #include "base/message_loop.h"          // for MessageLoop
 #include "base/process.h"               // for ProcessId
 #include "base/task.h"                  // for CancelableTask, DeleteTask, etc
+#include "mozilla/ClearOnShutdown.h"
 #include "mozilla/gfx/Point.h"                   // for IntSize
 #include "mozilla/Hal.h"                // for hal::SetCurrentThreadPriority()
 #include "mozilla/HalTypes.h"           // for hal::THREAD_PRIORITY_COMPOSITOR
@@ -23,6 +24,7 @@
 #include "mozilla/layers/PImageBridgeParent.h"
 #include "mozilla/layers/TextureHostOGL.h"  // for TextureHostOGL
 #include "mozilla/layers/Compositor.h"
+#include "mozilla/Monitor.h"
 #include "mozilla/mozalloc.h"           // for operator new, etc
 #include "mozilla/Unused.h"
 #include "nsDebug.h"                    // for NS_RUNTIMEABORT, etc
@@ -42,11 +44,21 @@ using namespace mozilla::media;
 
 std::map<base::ProcessId, ImageBridgeParent*> ImageBridgeParent::sImageBridges;
 
-MessageLoop* ImageBridgeParent::sMainLoop = nullptr;
+StaticAutoPtr<mozilla::Monitor> sImageBridgesLock;
 
 // defined in CompositorBridgeParent.cpp
 CompositorThreadHolder* GetCompositorThreadHolder();
 
+/* static */ void
+ImageBridgeParent::Setup()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  if (!sImageBridgesLock) {
+    sImageBridgesLock = new Monitor("ImageBridges");
+    mozilla::ClearOnShutdown(&sImageBridgesLock);
+  }
+}
+
 ImageBridgeParent::ImageBridgeParent(MessageLoop* aLoop,
                                      ProcessId aChildProcessId)
   : mMessageLoop(aLoop)
@@ -54,17 +66,18 @@ ImageBridgeParent::ImageBridgeParent(MessageLoop* aLoop,
   , mClosed(false)
 {
   MOZ_ASSERT(NS_IsMainThread());
-  sMainLoop = MessageLoop::current();
 
   // creates the map only if it has not been created already, so it is safe
   // with several bridges
+  {
+    MonitorAutoLock lock(*sImageBridgesLock);
     sImageBridges[aChildProcessId] = this;
+  }
   SetOtherProcessId(aChildProcessId);
 }
 
 ImageBridgeParent::~ImageBridgeParent()
 {
-  sImageBridges.erase(OtherPid());
 }
 
 static StaticRefPtr<ImageBridgeParent> sImageBridgeParentSingleton;
@@ -105,7 +118,10 @@ ImageBridgeParent::ActorDestroy(ActorDestroyReason aWhy)
   // Can't alloc/dealloc shmems from now on.
   mClosed = true;
   mCompositables.clear();
-
+  {
+    MonitorAutoLock lock(*sImageBridgesLock);
+    sImageBridges.erase(OtherPid());
+  }
   MessageLoop::current()->PostTask(NewRunnableMethod(this, &ImageBridgeParent::DeferredDestroy));
 
   // It is very important that this method gets called at shutdown (be it a clean
@@ -327,9 +343,11 @@ ImageBridgeParent::DeferredDestroy()
   mSelfRef = nullptr; // "this" ImageBridge may get deleted here.
 }
 
-ImageBridgeParent*
+RefPtr<ImageBridgeParent>
 ImageBridgeParent::GetInstance(ProcessId aId)
 {
+  MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
+  MonitorAutoLock lock(*sImageBridgesLock);
   NS_ASSERTION(sImageBridges.count(aId) == 1, "ImageBridgeParent for the process");
   return sImageBridges[aId];
 }
@@ -377,26 +395,6 @@ bool ImageBridgeParent::IsSameProcess() const
   return OtherPid() == base::GetCurrentProcId();
 }
 
-/*static*/ void
-ImageBridgeParent::SetAboutToSendAsyncMessages(base::ProcessId aChildProcessId)
-{
-  ImageBridgeParent* imageBridge = ImageBridgeParent::GetInstance(aChildProcessId);
-  if (!imageBridge) {
-    return;
-  }
-  imageBridge->SetAboutToSendAsyncMessages();
-}
-
-/*static*/ void
-ImageBridgeParent::SendPendingAsyncMessages(base::ProcessId aChildProcessId)
-{
-  ImageBridgeParent* imageBridge = ImageBridgeParent::GetInstance(aChildProcessId);
-  if (!imageBridge) {
-    return;
-  }
-  imageBridge->SendPendingAsyncMessages();
-}
-
 void
 ImageBridgeParent::NotifyNotUsed(PTextureParent* aTexture, uint64_t aTransactionId)
 {

gfx/layers/ipc/ImageBridgeParent.h

@@ -50,6 +50,11 @@ protected:
 public:
   ~ImageBridgeParent();
 
+  /**
+   * Creates the globals of ImageBridgeParent.
+   */
+  static void Setup();
+
   static ImageBridgeParent* CreateSameProcess();
   static bool CreateForGPUProcess(Endpoint<PImageBridgeParent>&& aEndpoint);
   static bool CreateForContent(Endpoint<PImageBridgeParent>&& aEndpoint);
@@ -107,13 +112,7 @@ public:
 
   virtual bool IsSameProcess() const override;
 
-  using CompositableParentManager::SetAboutToSendAsyncMessages;
-  static void SetAboutToSendAsyncMessages(base::ProcessId aChildProcessId);
-
-  using CompositableParentManager::SendPendingAsyncMessages;
-  static void SendPendingAsyncMessages(base::ProcessId aChildProcessId);
-
-  static ImageBridgeParent* GetInstance(ProcessId aId);
+  static RefPtr<ImageBridgeParent> GetInstance(ProcessId aId);
 
   static bool NotifyImageComposites(nsTArray<ImageCompositeNotificationInfo>& aNotifications);
 
@@ -141,8 +140,6 @@ private:
    */
   static std::map<base::ProcessId, ImageBridgeParent*> sImageBridges;
 
-  static MessageLoop* sMainLoop;
-
   RefPtr<CompositorThreadHolder> mCompositorThreadHolder;
 };
 

gfx/layers/ipc/LayerTransactionParent.cpp

@@ -111,13 +111,11 @@ public:
     , mActorsToDestroy(aDestroyActors)
   {
     mLayerTransaction->SetAboutToSendAsyncMessages();
-    ImageBridgeParent::SetAboutToSendAsyncMessages(mLayerTransaction->GetChildProcessId());
   }
 
   ~AutoLayerTransactionParentAsyncMessageSender()
   {
     mLayerTransaction->SendPendingAsyncMessages();
-    ImageBridgeParent::SendPendingAsyncMessages(mLayerTransaction->GetChildProcessId());
     if (mActorsToDestroy) {
       // Destroy the actors after sending the async messages because the latter may contain
       // references to some actors.
@@ -409,7 +407,7 @@ LayerTransactionParent::RecvUpdate(const TransactionInfo& aInfo)
     }
     case Edit::TOpAttachAsyncCompositable: {
       const OpAttachAsyncCompositable& op = edit.get_OpAttachAsyncCompositable();
-      ImageBridgeParent* imageBridge = ImageBridgeParent::GetInstance(OtherPid());
+      RefPtr<ImageBridgeParent> imageBridge = ImageBridgeParent::GetInstance(OtherPid());
       if (!imageBridge) {
         return IPC_FAIL_NO_REASON(this);
       }

gfx/layers/wr/WebRenderBridgeParent.cpp

@@ -641,7 +641,7 @@ WebRenderBridgeParent::RecvAddExternalImageId(const ExternalImageId& aImageId,
 
   MOZ_ASSERT(!mExternalImageIds.Get(wr::AsUint64(aImageId)).get());
 
-  ImageBridgeParent* imageBridge = ImageBridgeParent::GetInstance(OtherPid());
+  RefPtr<ImageBridgeParent> imageBridge = ImageBridgeParent::GetInstance(OtherPid());
   if (!imageBridge) {
      return IPC_FAIL_NO_REASON(this);
   }