Bug 1533673 - Prevent the GPU RemoteContentController from sending messages to a dead actor. r=rhunt
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 06 May 2019 16:20:05 +0000
changeset 531529 c737fc642ddcaa5846383f906ec3f24145d4a7a5
parent 531528 036a4b9e84262578b682d6e36ac60416db334d07
child 531530 4abfc5c2a4ee418302a99a2662a78f88b84d44b9
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrhunt
bugs1533673
milestone68.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 1533673 - Prevent the GPU RemoteContentController from sending messages to a dead actor. r=rhunt Currently it's possible for the RemoteContentController in the GPU process to be sending a message to the UI process while the UI process is running RemoteCompositorSession::Shutdown. This means that by the time that message is processed, the UI process actor is destroyed and the message produces a routing error. This patch ensures that before RemoteCompositorSession::Shutdown completes, it notifies the RemoteContentController in the GPU process that it's about to destroy the APZChild actor. This eliminates the race because it ensures the RemoteContentController is synchronously notified of the impending actor destruction before it tries to send the message. Differential Revision: https://phabricator.services.mozilla.com/D29941
gfx/layers/ipc/CompositorBridgeChild.cpp
gfx/layers/ipc/PAPZ.ipdl
gfx/layers/ipc/RemoteContentController.cpp
gfx/layers/ipc/RemoteContentController.h
--- a/gfx/layers/ipc/CompositorBridgeChild.cpp
+++ b/gfx/layers/ipc/CompositorBridgeChild.cpp
@@ -170,26 +170,35 @@ void CompositorBridgeChild::Destroy() {
   AutoTArray<PWebRenderBridgeChild*, 16> wrBridges;
   ManagedPWebRenderBridgeChild(wrBridges);
   for (int i = wrBridges.Length() - 1; i >= 0; --i) {
     RefPtr<WebRenderBridgeChild> wrBridge =
         static_cast<WebRenderBridgeChild*>(wrBridges[i]);
     wrBridge->Destroy(/* aIsSync */ false);
   }
 
+  AutoTArray<PAPZChild*, 16> apzChildren;
+  ManagedPAPZChild(apzChildren);
+  for (PAPZChild* child : apzChildren) {
+    Unused << child->SendDestroy();
+  }
+
   const ManagedContainer<PTextureChild>& textures = ManagedPTextureChild();
   for (auto iter = textures.ConstIter(); !iter.Done(); iter.Next()) {
     RefPtr<TextureClient> texture =
         TextureClient::AsTextureClient(iter.Get()->GetKey());
 
     if (texture) {
       texture->Destroy();
     }
   }
 
+  // The WillClose message is synchronous, so we know that after it returns
+  // any messages sent by the above code will have been processed on the
+  // other side.
   SendWillClose();
   mCanSend = false;
 
   // We no longer care about unexpected shutdowns, in the remote process case.
   mProcessToken = 0;
 
   // The call just made to SendWillClose can result in IPC from the
   // CompositorBridgeParent to the CompositorBridgeChild (e.g. caused by the
--- a/gfx/layers/ipc/PAPZ.ipdl
+++ b/gfx/layers/ipc/PAPZ.ipdl
@@ -42,17 +42,16 @@ namespace layers {
  * are implemented. If a new method is needed then PAPZ, APZChild, and RemoteContentController
  * must be updated to handle it.
  */
 sync protocol PAPZ
 {
   manager PCompositorBridge;
 
 parent:
-
   async __delete__();
 
 child:
   async LayerTransforms(MatrixMessage[] aTransforms);
 
   async RequestContentRepaint(RepaintRequest request);
 
   async UpdateOverscrollVelocity(float aX, float aY, bool aIsRootContent);
@@ -66,13 +65,14 @@ child:
   async NotifyFlushComplete();
 
   async NotifyAsyncScrollbarDragInitiated(uint64_t aDragBlockId, ViewID aScrollId, ScrollDirection aDirection);
 
   async NotifyAsyncScrollbarDragRejected(ViewID aScrollId);
 
   async NotifyAsyncAutoscrollRejected(ViewID aScrollId);
 
+both:
   async Destroy();
 };
 
 } // layers
 } // mozilla
--- a/gfx/layers/ipc/RemoteContentController.cpp
+++ b/gfx/layers/ipc/RemoteContentController.cpp
@@ -365,12 +365,19 @@ void RemoteContentController::ActorDestr
 
 void RemoteContentController::Destroy() {
   if (mCanSend) {
     mCanSend = false;
     Unused << SendDestroy();
   }
 }
 
+mozilla::ipc::IPCResult RemoteContentController::RecvDestroy() {
+  // The actor on the other side is about to get destroyed, so let's not send
+  // it any more messages.
+  mCanSend = false;
+  return IPC_OK();
+}
+
 bool RemoteContentController::IsRemote() { return true; }
 
 }  // namespace layers
 }  // namespace mozilla
--- a/gfx/layers/ipc/RemoteContentController.h
+++ b/gfx/layers/ipc/RemoteContentController.h
@@ -80,16 +80,17 @@ class RemoteContentController : public G
   void NotifyAsyncAutoscrollRejected(
       const ScrollableLayerGuid::ViewID& aScrollId) override;
 
   void CancelAutoscroll(const ScrollableLayerGuid& aScrollId) override;
 
   void ActorDestroy(ActorDestroyReason aWhy) override;
 
   void Destroy() override;
+  mozilla::ipc::IPCResult RecvDestroy();
 
   bool IsRemote() override;
 
  private:
   MessageLoop* mCompositorThread;
   bool mCanSend;
 
   void HandleTapOnMainThread(TapType aType, LayoutDevicePoint aPoint,