Bug 1289650 - Simplify RemoteContentController destruction. r?kats draft
authorRyan Hunt <rhunt@mozilla.com>
Sun, 21 Aug 2016 17:43:08 -0700
changeset 404717 d9d3ee883bb8c2d0aaa1884f19ab96537bb3e265
parent 404716 33a1f7476d96d2b09be71d1c776b1e3103c7967f
child 404718 86f582c354fed97448f139088a40521608084a21
push id27272
push userbmo:rhunt@mozilla.com
push dateTue, 23 Aug 2016 22:55:33 +0000
reviewerskats
bugs1289650
milestone51.0a1
Bug 1289650 - Simplify RemoteContentController destruction. r?kats MozReview-Commit-ID: 3E3kVvychou
gfx/layers/ipc/APZChild.cpp
gfx/layers/ipc/CompositorBridgeParent.cpp
gfx/layers/ipc/RemoteContentController.cpp
--- a/gfx/layers/ipc/APZChild.cpp
+++ b/gfx/layers/ipc/APZChild.cpp
@@ -145,16 +145,17 @@ APZChild::RecvNotifyFlushComplete()
 bool
 APZChild::RecvDestroy()
 {
   mDestroyed = true;
   if (mBrowser) {
     mBrowser->SetAPZChild(nullptr);
     mBrowser = nullptr;
   }
+  PAPZChild::Send__delete__(this);
   return true;
 }
 
 void
 APZChild::SetObserver(nsIObserver* aObserver)
 {
   MOZ_ASSERT(!mBrowser);
   mObserver = aObserver;
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -2551,29 +2551,35 @@ PAPZParent*
 CrossProcessCompositorBridgeParent::AllocPAPZParent(const uint64_t& aLayersId)
 {
   // Check to see if this child process has access to this layer tree.
   if (!LayerTreeOwnerTracker::Get()->IsMapped(aLayersId, OtherPid())) {
     NS_ERROR("Unexpected layers id in AllocPAPZParent; dropping message...");
     return nullptr;
   }
 
-  RefPtr<RemoteContentController> controller = new RemoteContentController(aLayersId);
+  RemoteContentController* controller = new RemoteContentController(aLayersId);
+
+  // Increment the controller's refcount before we return it. This will keep the
+  // controller alive until it is released by IPDL in DeallocPAPZParent.
+  controller->AddRef();
 
   MonitorAutoLock lock(*sIndirectLayerTreesLock);
   CompositorBridgeParent::LayerTreeState& state = sIndirectLayerTrees[aLayersId];
   MOZ_ASSERT(!state.mController);
   state.mController = controller;
 
   return controller;
 }
 
 bool
 CrossProcessCompositorBridgeParent::DeallocPAPZParent(PAPZParent* aActor)
 {
+  RemoteContentController* controller = static_cast<RemoteContentController*>(aActor);
+  controller->Release();
   return true;
 }
 
 bool
 CrossProcessCompositorBridgeParent::RecvNotifyChildCreated(const uint64_t& child)
 {
   MonitorAutoLock lock(*sIndirectLayerTreesLock);
   for (LayerTreeMap::iterator it = sIndirectLayerTrees.begin();
--- a/gfx/layers/ipc/RemoteContentController.cpp
+++ b/gfx/layers/ipc/RemoteContentController.cpp
@@ -21,18 +21,16 @@
 #include "AndroidBridge.h"
 #endif
 
 namespace mozilla {
 namespace layers {
 
 using namespace mozilla::gfx;
 
-static std::map<uint64_t, RefPtr<RemoteContentController>> sDestroyedControllers;
-
 RemoteContentController::RemoteContentController(uint64_t aLayersId)
   : mCompositorThread(MessageLoop::current())
   , mLayersId(aLayersId)
   , mCanSend(true)
   , mMutex("RemoteContentController")
 {
 }
 
@@ -164,32 +162,23 @@ RemoteContentController::RecvUpdateHitRe
   MutexAutoLock lock(mMutex);
   mTouchSensitiveRegion = aRegion;
   return true;
 }
 
 void
 RemoteContentController::ActorDestroy(ActorDestroyReason aWhy)
 {
+  // This controller could possibly be kept alive longer after this
+  // by a RefPtr, but it is no longer valid to send messages.
   mCanSend = false;
-
-  // sDestroyedControllers may or may not contain the key, depending on
-  // whether or not SendDestroy() was successfully sent out or not.
-  sDestroyedControllers.erase(mLayersId);
 }
 
 void
 RemoteContentController::Destroy()
 {
   if (mCanSend) {
-    // Gfx code is done with this object, and it will probably get destroyed
-    // soon. However, if mCanSend is true, ActorDestroy has not yet been
-    // called, which means IPC code still has a handle to this object. We need
-    // to keep it alive until we get the ActorDestroy call, either via the
-    // __delete__ message or via IPC shutdown on our end.
-    MOZ_ASSERT(sDestroyedControllers.find(mLayersId) == sDestroyedControllers.end());
-    sDestroyedControllers[mLayersId] = this;
     Unused << SendDestroy();
   }
 }
 
 } // namespace layers
 } // namespace mozilla