Bug 1289650 - Simplify RemoteContentController destruction. r=kats
authorRyan Hunt <rhunt@mozilla.com>
Sun, 21 Aug 2016 17:43:08 -0700
changeset 311097 a98e4065514ccb75eefb8052b2acad1c63d5b66e
parent 311096 1ad7640633fb14e1750f972aa70718c7c6e241e5
child 311098 b0d9956e125145216bf1c88890280f9730393b5f
push id81036
push userkgupta@mozilla.com
push dateThu, 25 Aug 2016 13:32:44 +0000
treeherdermozilla-inbound@600e4f0568c4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1289650
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -2579,29 +2579,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