Bug 1460289 - Reduce scope of indirect layer tree lock in RecvAdoptChild. r?sotaro draft
authorKartikaya Gupta <kgupta@mozilla.com>
Wed, 09 May 2018 15:23:30 -0400
changeset 793217 80d7b74c1f5ce06f56344694bd2e1ffe591c80a2
parent 793056 9294f67b3f3bd4a3dd898961148cecd8bfc1ce9c
push id109321
push userkgupta@mozilla.com
push dateWed, 09 May 2018 19:23:53 +0000
reviewerssotaro
bugs1460289
milestone62.0a1
Bug 1460289 - Reduce scope of indirect layer tree lock in RecvAdoptChild. r?sotaro In particular, this change makes it so that we are not holding the lock when we clone the WebRenderAPI, because that can result in a deadlock. MozReview-Commit-ID: 33OGZdFMjEA
gfx/layers/ipc/CompositorBridgeParent.cpp
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -1727,48 +1727,61 @@ CompositorBridgeParent::RecvMapAndNotify
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 CompositorBridgeParent::RecvAdoptChild(const LayersId& child)
 {
   RefPtr<APZUpdater> oldApzUpdater;
   APZCTreeManagerParent* parent;
-  {
+  bool scheduleComposition = false;
+  RefPtr<CrossProcessCompositorBridgeParent> cpcp;
+  RefPtr<WebRenderBridgeParent> childWrBridge;
+
+  { // scope lock
     MonitorAutoLock lock(*sIndirectLayerTreesLock);
     if (sIndirectLayerTrees[child].mParent) {
       // We currently don't support adopting children from one compositor to
       // another if the two compositors don't have the same options.
       MOZ_ASSERT(sIndirectLayerTrees[child].mParent->mOptions == mOptions);
       oldApzUpdater = sIndirectLayerTrees[child].mParent->mApzUpdater;
     }
     NotifyChildCreated(child);
     if (sIndirectLayerTrees[child].mLayerTree) {
       sIndirectLayerTrees[child].mLayerTree->SetLayerManager(mLayerManager, GetAnimationStorage());
       // Trigger composition to handle a case that mLayerTree was not composited yet
       // by previous CompositorBridgeParent, since nsRefreshDriver might wait composition complete.
-      ScheduleComposition();
+      scheduleComposition = true;
     }
-    if (mWrBridge && sIndirectLayerTrees[child].mWrBridge) {
-      RefPtr<wr::WebRenderAPI> api = mWrBridge->GetWebRenderAPI();
-      api = api->Clone();
-      sIndirectLayerTrees[child].mWrBridge->UpdateWebRender(mWrBridge->CompositorScheduler(),
-                                                            api,
-                                                            mWrBridge->AsyncImageManager(),
-                                                            GetAnimationStorage());
-      // Pretend we composited, since parent CompositorBridgeParent was replaced.
-      CrossProcessCompositorBridgeParent* cpcp = sIndirectLayerTrees[child].mCrossProcessParent;
-      if (cpcp) {
-        TimeStamp now = TimeStamp::Now();
-        cpcp->DidCompositeLocked(child, now, now);
-      }
+    if (mWrBridge) {
+      childWrBridge = sIndirectLayerTrees[child].mWrBridge;
+      cpcp = sIndirectLayerTrees[child].mCrossProcessParent;
     }
     parent = sIndirectLayerTrees[child].mApzcTreeManagerParent;
   }
 
+  if (scheduleComposition) {
+    ScheduleComposition();
+  }
+
+  if (childWrBridge) {
+    MOZ_ASSERT(mWrBridge);
+    RefPtr<wr::WebRenderAPI> api = mWrBridge->GetWebRenderAPI();
+    api = api->Clone();
+    childWrBridge->UpdateWebRender(mWrBridge->CompositorScheduler(),
+                                   api,
+                                   mWrBridge->AsyncImageManager(),
+                                   GetAnimationStorage());
+    // Pretend we composited, since parent CompositorBridgeParent was replaced.
+    if (cpcp) {
+      TimeStamp now = TimeStamp::Now();
+      cpcp->DidComposite(child, now, now);
+    }
+  }
+
   if (oldApzUpdater) {
     // We don't support moving a child from an APZ-enabled compositor to a
     // APZ-disabled compositor. The mOptions assertion above should already
     // ensure this, since APZ-ness is one of the things in mOptions. Note
     // however it is possible for mApzUpdater to be non-null here with
     // oldApzUpdater null, because the child may not have been previously
     // composited.
     MOZ_ASSERT(mApzUpdater);