Backed out changeset 65e9d46daf3e (bug 1255823) for various failures that end up crashing in [@ mozilla::layers::PAPZParent::DestroySubtree]

MozReview-Commit-ID: 31J6pCNfW2D

gfx/layers/ipc/APZChild.cpp

@@ -78,19 +78,13 @@ APZChild::Create(const dom::TabId& aTabId)
   return apz.forget();
 }
 
-APZChild::APZChild()
-  : mDestroyed(false)
-{
-}
-
 APZChild::~APZChild()
 {
   if (mObserver) {
     nsCOMPtr<nsIObserverService> os = services::GetObserverService();
     os->RemoveObserver(mObserver, "tab-child-created");
-  } else if (mBrowser) {
+  } else {
     mBrowser->SetAPZChild(nullptr);
-    mBrowser = nullptr;
   }
 }
 
@@ -157,18 +151,6 @@ APZChild::RecvNotifyFlushComplete()
   return true;
 }
 
-bool
-APZChild::RecvDestroy()
-{
-  mDestroyed = true;
-  if (mBrowser) {
-    mBrowser->SetAPZChild(nullptr);
-    mBrowser = nullptr;
-  }
-  PAPZChild::Send__delete__(this);
-  return true;
-}
-
 void
 APZChild::SetObserver(nsIObserver* aObserver)
 {
@@ -185,13 +167,8 @@ APZChild::SetBrowser(dom::TabChild* aBrowser)
     os->RemoveObserver(mObserver, "tab-child-created");
     mObserver = nullptr;
   }
-  // We might get the tab-child-created notification after we receive a
-  // Destroy message from the parent. In that case we don't want to install
-  // ourselves with the browser.
-  if (!mDestroyed) {
-    mBrowser = aBrowser;
-    mBrowser->SetAPZChild(this);
-  }
+  mBrowser = aBrowser;
+  mBrowser->SetAPZChild(this);
 }
 
 } // namespace layers

gfx/layers/ipc/APZChild.h

@@ -51,18 +51,15 @@ public:
 
   virtual bool RecvNotifyFlushComplete() override;
 
-  virtual bool RecvDestroy() override;
-
   void SetBrowser(dom::TabChild* aBrowser);
 
 private:
-  APZChild();
+  APZChild() {};
 
   void SetObserver(nsIObserver* aObserver);
 
   RefPtr<dom::TabChild> mBrowser;
   RefPtr<nsIObserver> mObserver;
-  bool mDestroyed;
 };
 
 } // namespace layers

gfx/layers/ipc/PAPZ.ipdl

@@ -83,8 +83,6 @@ parent:
   async UpdateZoomConstraints(uint32_t aPresShellId, ViewID aViewId,
                               MaybeZoomConstraints aConstraints);
 
-  async __delete__();
-
 child:
   async UpdateFrame(FrameMetrics frame);
 
@@ -98,7 +96,7 @@ child:
   async NotifyAPZStateChange(ViewID aViewId, APZStateChange aChange, int aArg);
   async NotifyFlushComplete();
 
-  async Destroy();
+  async __delete__();
 };
 
 } // layers

gfx/layers/ipc/RemoteContentController.cpp

@@ -25,8 +25,6 @@
 namespace mozilla {
 namespace layers {
 
-static std::map<uint64_t, RefPtr<RemoteContentController>> sDestroyedControllers;
-
 RemoteContentController::RemoteContentController(uint64_t aLayersId,
                                                  dom::TabParent* aBrowserParent)
   : mUILoop(MessageLoop::current())
@@ -39,6 +37,9 @@ RemoteContentController::RemoteContentController(uint64_t aLayersId,
 
 RemoteContentController::~RemoteContentController()
 {
+  if (mBrowserParent) {
+    Unused << PAPZParent::Send__delete__(this);
+  }
 }
 
 void
@@ -316,18 +317,17 @@ RemoteContentController::ActorDestroy(ActorDestroyReason aWhy)
     mApzcTreeManager = nullptr;
   }
   mBrowserParent = nullptr;
+}
 
-  // Clear the RefPtr in the sDestroyedControllers map in a runnable so that
-  // this object is destroyed after we unwind from the IPC code. Note that for
-  // some values of ActorDestroyReason sDestroyedControllers may not even
-  // contain this RemoteContentController. In those cases the gfx code will
-  // eventually call Destroy() on this object, but CanSend() will return false
-  // and so it will be a no-op. The IPC code should take care of destroying
-  // the child-side stuff in those cases.
-  uint64_t key = mLayersId;
-  NS_DispatchToMainThread(NS_NewRunnableFunction([key] {
-    sDestroyedControllers.erase(key);
-  }));
+// TODO: Remove once upgraded to GCC 4.8+ on linux. Calling a static member
+//       function (like PAPZParent::Send__delete__) in a lambda leads to a bogus
+//       error: "'this' was not captured for this lambda function".
+//
+//       (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51494)
+static void
+DeletePAPZParent(PAPZParent* aPAPZ)
+{
+  Unused << PAPZParent::Send__delete__(aPAPZ);
 }
 
 void
@@ -336,15 +336,7 @@ RemoteContentController::Destroy()
   RefPtr<RemoteContentController> controller = this;
   NS_DispatchToMainThread(NS_NewRunnableFunction([controller] {
     if (controller->CanSend()) {
-      if (controller->SendDestroy()) {
-        // Gfx code is done with this object, and it will probably get destroyed
-        // soon. We need to keep a RefPtr to this object until we get the
-        // __delete__ back, otherwise we might get destroyed in the meantime and
-        // the IPC code will crash on try to handle the __delete__.
-        uint64_t key = controller->mLayersId;
-        MOZ_ASSERT(sDestroyedControllers.find(key) == sDestroyedControllers.end());
-        sDestroyedControllers[key] = controller;
-      }
+      DeletePAPZParent(controller);
     }
   }));
 }