diff --git a/gfx/layers/ipc/CompositorChild.cpp b/gfx/layers/ipc/CompositorChild.cpp index 4acb365cb2e4..9c941ae5728d 100644 --- a/gfx/layers/ipc/CompositorChild.cpp +++ b/gfx/layers/ipc/CompositorChild.cpp @@ -5,6 +5,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "mozilla/layers/CompositorChild.h" +#include "mozilla/layers/CompositorParent.h" #include // for size_t #include "ClientLayerManager.h" // for ClientLayerManager #include "base/message_loop.h" // for MessageLoop @@ -39,19 +40,58 @@ Atomic CompositableForwarder::sSerialCounter(0); CompositorChild::CompositorChild(ClientLayerManager *aLayerManager) : mLayerManager(aLayerManager) - , mCanSend(true) + , mCanSend(false) { } CompositorChild::~CompositorChild() { + if (mCanSend) { + gfxCriticalError() << "CompositorChild was not deinitialized"; + } +} + +static void DeferredDestroyCompositor(nsRefPtr aCompositorParent, + nsRefPtr aCompositorChild) +{ + // Bug 848949 needs to be fixed before + // we can close the channel properly + //aCompositorChild->Close(); } void CompositorChild::Destroy() { - mLayerManager->Destroy(); - mLayerManager = nullptr; + // This must not be called from the destructor! + MOZ_ASSERT(mRefCnt != 0); + + if (!mCanSend) { + return; + } + + mCanSend = false; + + // Destroying the layer manager may cause all sorts of things to happen, so + // let's make sure there is still a reference to keep this alive whatever + // happens. + nsRefPtr selfRef = this; + + SendWillStop(); + // The call just made to SendWillStop can result in IPC from the + // CompositorParent to the CompositorChild (e.g. caused by the destruction + // of shared memory). We need to ensure this gets processed by the + // CompositorChild before it gets destroyed. It suffices to ensure that + // events already in the MessageLoop get processed before the + // CompositorChild is destroyed, so we add a task to the MessageLoop to + // handle compositor desctruction. + + // From now on the only message we can send is Stop. + + if (mLayerManager) { + mLayerManager->Destroy(); + mLayerManager = nullptr; + } + // start from the end of the array because Destroy() can cause the // LayerTransactionChild to be removed from the array. for (int i = ManagedPLayerTransactionChild().Length() - 1; i >= 0; --i) { @@ -59,8 +99,13 @@ CompositorChild::Destroy() static_cast(ManagedPLayerTransactionChild()[i]); layers->Destroy(); } - MOZ_ASSERT(!mCanSend); + SendStop(); + + // The DeferredDestroyCompositor task takes ownership of compositorParent and + // will release them when it runs. + MessageLoop::current()->PostTask(FROM_HERE, + NewRunnableFunction(DeferredDestroyCompositor, mCompositorParent, selfRef)); } bool @@ -87,6 +132,8 @@ CompositorChild::Create(Transport* aTransport, ProcessId aOtherPid) return nullptr; } + child->mCanSend = true; + // We release this ref in ActorDestroy(). sCompositor = child.forget().take(); @@ -99,6 +146,18 @@ CompositorChild::Create(Transport* aTransport, ProcessId aOtherPid) return sCompositor; } +bool +CompositorChild::OpenSameProcess(CompositorParent* aParent) +{ + MOZ_ASSERT(aParent); + + mCompositorParent = aParent; + mCanSend = Open(mCompositorParent->GetIPCChannel(), + CompositorParent::CompositorLoop(), + ipc::ChildSide); + return mCanSend; +} + /*static*/ CompositorChild* CompositorChild::Get() { @@ -328,14 +387,7 @@ CompositorChild::ActorDestroy(ActorDestroyReason aWhy) NS_RUNTIMEABORT("ActorDestroy by IPC channel failure at CompositorChild"); } #endif - if (sCompositor) { - sCompositor->Release(); - sCompositor = nullptr; - } - // We don't want to release the ref to sCompositor here, during - // cleanup, because that will cause it to be deleted while it's - // still being used. So defer the deletion to after it's not in - // use. + MessageLoop::current()->PostTask( FROM_HERE, NewRunnableMethod(this, &CompositorChild::Release)); @@ -474,9 +526,6 @@ CompositorChild::CancelNotifyAfterRemotePaint(TabChild* aTabChild) bool CompositorChild::SendWillStop() { - MOZ_ASSERT(mCanSend); - // From now on the only two messages we can send are WillStop and Stop. - mCanSend = false; return PCompositorChild::SendWillStop(); } diff --git a/gfx/layers/ipc/CompositorChild.h b/gfx/layers/ipc/CompositorChild.h index 0f2afd75beec..09a9249ec51f 100644 --- a/gfx/layers/ipc/CompositorChild.h +++ b/gfx/layers/ipc/CompositorChild.h @@ -60,6 +60,12 @@ public: static PCompositorChild* Create(Transport* aTransport, ProcessId aOtherProcess); + /** + * Initialize the CompositorChild and open the connection in the non-multi-process + * case. + */ + bool OpenSameProcess(CompositorParent* aParent); + static CompositorChild* Get(); static bool ChildProcessHasCompositor() { return sCompositor != nullptr; } @@ -168,6 +174,9 @@ private: void* aLayerTransactionChild); nsRefPtr mLayerManager; + // When not multi-process, hold a reference to the CompositorParent to keep it + // alive. This reference should be null in multi-process. + nsRefPtr mCompositorParent; // The ViewID of the FrameMetrics is used as the key for this hash table. // While this should be safe to use since the ViewID is unique diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp index d7a33b9d88dd..3cc8f9bb933e 100644 --- a/widget/nsBaseWidget.cpp +++ b/widget/nsBaseWidget.cpp @@ -181,41 +181,26 @@ nsBaseWidget::Shutdown() mShutdownObserver = nullptr; } -static void DeferredDestroyCompositor(nsRefPtr aCompositorParent, - nsRefPtr aCompositorChild) -{ - // Bug 848949 needs to be fixed before - // we can close the channel properly - //aCompositorChild->Close(); -} - void nsBaseWidget::DestroyCompositor() { if (mCompositorChild) { - nsRefPtr compositorChild = mCompositorChild.forget(); - nsRefPtr compositorParent = mCompositorParent.forget(); - - compositorChild->SendWillStop(); - // New LayerManager, CompositorParent and CompositorChild might be created - // as a result of internal GetLayerManager() call. - compositorChild->Destroy(); - - // The call just made to SendWillStop can result in IPC from the - // CompositorParent to the CompositorChild (e.g. caused by the destruction - // of shared memory). We need to ensure this gets processed by the - // CompositorChild before it gets destroyed. It suffices to ensure that - // events already in the MessageLoop get processed before the - // CompositorChild is destroyed, so we add a task to the MessageLoop to - // handle compositor desctruction. - - // The DefferedDestroyCompositor task takes ownership of compositorParent and - // will release them when it runs. - MessageLoop::current()->PostTask(FROM_HERE, - NewRunnableFunction(DeferredDestroyCompositor, compositorParent, - compositorChild)); + // XXX CompositorChild and CompositorParent might be re-created in + // ClientLayerManager destructor. See bug 1133426. + nsRefPtr compositorChild = mCompositorChild; + nsRefPtr compositorParent = mCompositorParent; + mCompositorChild->Destroy(); } } +void nsBaseWidget::DestroyLayerManager() +{ + if (mLayerManager) { + mLayerManager->Destroy(); + mLayerManager = nullptr; + } + DestroyCompositor(); +} + //------------------------------------------------------------------------- // // nsBaseWidget destructor @@ -228,11 +213,6 @@ nsBaseWidget::~nsBaseWidget() static_cast(mLayerManager.get())->ClearRetainerWidget(); } - if (mLayerManager) { - mLayerManager->Destroy(); - mLayerManager = nullptr; - } - if (mShutdownObserver) { // If the shutdown observer is currently processing observers, // then UnregisterShutdownObserver won't stop our Observer @@ -242,7 +222,7 @@ nsBaseWidget::~nsBaseWidget() nsContentUtils::UnregisterShutdownObserver(mShutdownObserver); } - DestroyCompositor(); + DestroyLayerManager(); #ifdef NOISY_WIDGET_LEAKS gNumWidgets--; @@ -1127,8 +1107,12 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight) MOZ_ASSERT(gfxPlatform::UsesOffMainThreadCompositing(), "This function assumes OMTC"); - MOZ_ASSERT(!mCompositorParent, - "Should have properly cleaned up the previous CompositorParent beforehand"); + MOZ_ASSERT(!mCompositorParent && !mCompositorChild, + "Should have properly cleaned up the previous PCompositor pair beforehand"); + + if (mCompositorChild) { + mCompositorChild->Destroy(); + } // Recreating this is tricky, as we may still have an old and we need // to make sure it's properly destroyed by calling DestroyCompositor! @@ -1141,11 +1125,9 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight) CreateCompositorVsyncDispatcher(); mCompositorParent = NewCompositorParent(aWidth, aHeight); - MessageChannel *parentChannel = mCompositorParent->GetIPCChannel(); nsRefPtr lm = new ClientLayerManager(this); - MessageLoop *childMessageLoop = CompositorParent::CompositorLoop(); mCompositorChild = new CompositorChild(lm); - mCompositorChild->Open(parentChannel, childMessageLoop, ipc::ChildSide); + mCompositorChild->OpenSameProcess(mCompositorParent); // Make sure the parent knows it is same process. mCompositorParent->SetOtherProcessId(kCurrentProcessId); @@ -1177,26 +1159,20 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight) backendHints, 0, &textureFactoryIdentifier, &success); } - if (success) { - ShadowLayerForwarder* lf = lm->AsShadowForwarder(); - if (!lf) { - lm = nullptr; - mCompositorChild = nullptr; - return; - } - lf->SetShadowManager(shadowManager); - lf->IdentifyTextureHost(textureFactoryIdentifier); - ImageBridgeChild::IdentifyCompositorTextureHost(textureFactoryIdentifier); - WindowUsesOMTC(); + ShadowLayerForwarder* lf = lm->AsShadowForwarder(); - mLayerManager = lm.forget(); + if (!success || !lf) { + NS_WARNING("Failed to create an OMT compositor."); + DestroyCompositor(); return; } - NS_WARNING("Failed to create an OMT compositor."); - DestroyCompositor(); - // Compositor child had the only reference to LayerManager and will have - // deallocated it when being freed. + lf->SetShadowManager(shadowManager); + lf->IdentifyTextureHost(textureFactoryIdentifier); + ImageBridgeChild::IdentifyCompositorTextureHost(textureFactoryIdentifier); + WindowUsesOMTC(); + + mLayerManager = lm.forget(); } bool nsBaseWidget::ShouldUseOffMainThreadCompositing() diff --git a/widget/nsBaseWidget.h b/widget/nsBaseWidget.h index 9041b432e35e..7320faeb001a 100644 --- a/widget/nsBaseWidget.h +++ b/widget/nsBaseWidget.h @@ -448,6 +448,7 @@ protected: * reached (This is the case with gtk2 for instance). */ void DestroyCompositor(); + void DestroyLayerManager(); nsIWidgetListener* mWidgetListener; nsIWidgetListener* mAttachedWidgetListener; diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp index 36446a06a3ae..36df9f42fbc2 100644 --- a/widget/windows/nsWindow.cpp +++ b/widget/windows/nsWindow.cpp @@ -673,11 +673,7 @@ NS_METHOD nsWindow::Destroy() * On windows the LayerManagerOGL destructor wants the widget to be around for * cleanup. It also would like to have the HWND intact, so we nullptr it here. */ - if (mLayerManager) { - mLayerManager->Destroy(); - } - mLayerManager = nullptr; - DestroyCompositor(); + DestroyLayerManager(); /* We should clear our cached resources now and not wait for the GC to * delete the nsWindow. */ @@ -6544,10 +6540,7 @@ bool nsWindow::AutoErase(HDC dc) void nsWindow::ClearCompositor(nsWindow* aWindow) { - if (aWindow->mLayerManager) { - aWindow->mLayerManager = nullptr; - aWindow->DestroyCompositor(); - } + aWindow->DestroyLayerManager(); } bool