Bug 1255823 - Add a two-step destruction process to PAPZ. r?
MozReview-Commit-ID: 5eokgm6ufnF
--- a/gfx/layers/ipc/APZChild.cpp
+++ b/gfx/layers/ipc/APZChild.cpp
@@ -73,22 +73,27 @@ APZChild::Create(const dom::TabId& aTabI
return nullptr;
}
apz->SetObserver(observer);
}
return apz.forget();
}
+APZChild::APZChild()
+ : mDestroyed(false)
+{
+}
+
APZChild::~APZChild()
{
if (mObserver) {
nsCOMPtr<nsIObserverService> os = services::GetObserverService();
os->RemoveObserver(mObserver, "tab-child-created");
- } else {
+ } else if (mBrowser) {
mBrowser->SetAPZChild(nullptr);
}
}
bool
APZChild::RecvUpdateFrame(const FrameMetrics& aFrameMetrics)
{
return mBrowser->UpdateFrame(aFrameMetrics);
@@ -146,30 +151,47 @@ APZChild::RecvNotifyFlushComplete()
nsCOMPtr<nsIPresShell> shell;
if (nsCOMPtr<nsIDocument> doc = mBrowser->GetDocument()) {
shell = doc->GetShell();
}
APZCCallbackHelper::NotifyFlushComplete(shell.get());
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)
{
MOZ_ASSERT(!mBrowser);
mObserver = aObserver;
}
void
APZChild::SetBrowser(dom::TabChild* aBrowser)
{
MOZ_ASSERT(!mBrowser);
if (mObserver) {
nsCOMPtr<nsIObserverService> os = services::GetObserverService();
os->RemoveObserver(mObserver, "tab-child-created");
mObserver = nullptr;
}
- mBrowser = aBrowser;
- mBrowser->SetAPZChild(this);
+ // 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);
+ }
}
} // namespace layers
} // namespace mozilla
--- a/gfx/layers/ipc/APZChild.h
+++ b/gfx/layers/ipc/APZChild.h
@@ -46,24 +46,27 @@ public:
const uint64_t& aInputBlockId) override;
virtual bool RecvNotifyAPZStateChange(const ViewID& aViewId,
const APZStateChange& aChange,
const int& aArg) override;
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
} // namespace mozilla
#endif // mozilla_layers_APZChild_h
--- a/gfx/layers/ipc/PAPZ.ipdl
+++ b/gfx/layers/ipc/PAPZ.ipdl
@@ -78,26 +78,28 @@ parent:
/**
* Updates the zoom constraints for a scrollable frame in this tab.
* The zoom controller code lives on the parent side and so this allows it to
* have up-to-date zoom constraints.
*/
async UpdateZoomConstraints(uint32_t aPresShellId, ViewID aViewId,
MaybeZoomConstraints aConstraints);
+ async __delete__();
+
child:
async UpdateFrame(FrameMetrics frame);
// The following methods correspond to functions on the GeckoContentController
// interface in gfx/layers/apz/public/GeckoContentController.h. Refer to documentation
// in that file for these functions.
async AcknowledgeScrollUpdate(ViewID aScrollId, uint32_t aScrollGeneration);
async HandleDoubleTap(CSSPoint aPoint, Modifiers aModifiers, ScrollableLayerGuid aGuid);
async HandleSingleTap(CSSPoint aPoint, Modifiers aModifiers, ScrollableLayerGuid aGuid, bool aCallTakeFocusForClickFromTap);
async HandleLongTap(CSSPoint point, Modifiers aModifiers, ScrollableLayerGuid aGuid, uint64_t aInputBlockId);
async NotifyAPZStateChange(ViewID aViewId, APZStateChange aChange, int aArg);
async NotifyFlushComplete();
- async __delete__();
+ async Destroy();
};
} // layers
} // mozilla
--- a/gfx/layers/ipc/RemoteContentController.cpp
+++ b/gfx/layers/ipc/RemoteContentController.cpp
@@ -20,31 +20,30 @@
#include "Units.h"
#ifdef MOZ_WIDGET_ANDROID
#include "AndroidBridge.h"
#endif
namespace mozilla {
namespace layers {
+static std::map<uint64_t, RefPtr<RemoteContentController>> sDestroyedControllers;
+
RemoteContentController::RemoteContentController(uint64_t aLayersId,
dom::TabParent* aBrowserParent)
: mUILoop(MessageLoop::current())
, mLayersId(aLayersId)
, mBrowserParent(aBrowserParent)
, mMutex("RemoteContentController")
{
MOZ_ASSERT(NS_IsMainThread());
}
RemoteContentController::~RemoteContentController()
{
- if (mBrowserParent) {
- Unused << PAPZParent::Send__delete__(this);
- }
}
void
RemoteContentController::RequestContentRepaint(const FrameMetrics& aFrameMetrics)
{
MOZ_ASSERT(NS_IsMainThread());
if (CanSend()) {
Unused << SendUpdateFrame(aFrameMetrics);
@@ -304,44 +303,73 @@ RemoteContentController::RecvUpdateZoomC
{
if (RefPtr<APZCTreeManager> apzcTreeManager = GetApzcTreeManager()) {
apzcTreeManager->UpdateZoomConstraints(ScrollableLayerGuid(mLayersId, aPresShellId, aViewId),
aConstraints);
}
return true;
}
+static void
+ReleaseDestroyedController(uint64_t aKey)
+{
+ // Clear the RefPtr in the sDestroyedControllers map in a runnable so that
+ // this object is destroyed after we unwind from the IPC code.
+ NS_DispatchToMainThread(NS_NewRunnableFunction([aKey]() {
+ sDestroyedControllers.erase(aKey);
+ }));
+}
+
+bool
+RemoteContentController::Recv__delete__()
+{
+ // This function is only ever called after we successfully call SendDestroy(),
+ // so we are guaranteed that sDestroyedControllers will contain this
+ // RemoteContentController.
+ ReleaseDestroyedController(mLayersId);
+ return true;
+}
+
void
RemoteContentController::ActorDestroy(ActorDestroyReason aWhy)
{
{
MutexAutoLock lock(mMutex);
mApzcTreeManager = nullptr;
}
mBrowserParent = nullptr;
-}
-// 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);
+ if (aWhy == ActorDestroyReason::NormalShutdown) {
+ // Normal shutdown can happen after we call SendDestroy(), but before
+ // we Recv__delete__. In that case, the __delete__ message will never
+ // arrive, so we should release the controller from here. If this gets
+ // called before Destroy(), then this will be a no-op, and CanSend() will
+ // return false so the SendDestroy() will never happen.
+ ReleaseDestroyedController(mLayersId);
+ }
}
void
RemoteContentController::Destroy()
{
RefPtr<RemoteContentController> controller = this;
NS_DispatchToMainThread(NS_NewRunnableFunction([controller] {
- if (controller->CanSend()) {
- DeletePAPZParent(controller);
+ if (controller->CanSend() && 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;
+ } else {
+ // If the send failed, then this object must already have had ActorDestroy
+ // called on it, and IPC code is no longer holding on to it, so we can
+ // let it get destroyed.
+ MOZ_ASSERT(controller->mBrowserParent == nullptr);
}
}));
}
void
RemoteContentController::ChildAdopted()
{
// Clear the cached APZCTreeManager.
--- a/gfx/layers/ipc/RemoteContentController.h
+++ b/gfx/layers/ipc/RemoteContentController.h
@@ -90,16 +90,17 @@ public:
virtual bool RecvSetAllowedTouchBehavior(const uint64_t& aInputBlockId,
nsTArray<TouchBehaviorFlags>&& aFlags) override;
virtual bool RecvUpdateZoomConstraints(const uint32_t& aPresShellId,
const ViewID& aViewId,
const MaybeZoomConstraints& aConstraints) override;
+ virtual bool Recv__delete__() override;
virtual void ActorDestroy(ActorDestroyReason aWhy) override;
virtual void Destroy() override;
virtual void ChildAdopted() override;
private:
bool CanSend()