Bug 1125848 - Consolidate PCompositor's creation-destruction logic. r=sotaro a=lmandel
💩💩 backed out by 17af3ddb4a24 💩 💩
authorNicolas Silva <nsilva@mozilla.com>
Thu, 12 Mar 2015 20:47:26 +0100
changeset 250366 81009105d11d
parent 250365 e15e6597d699
child 250367 f8c988045bb5
push id4562
push usernsilva@mozilla.com
push date2015-03-12 19:47 +0000
treeherdermozilla-beta@81009105d11d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro, lmandel
bugs1125848
milestone37.0
Bug 1125848 - Consolidate PCompositor's creation-destruction logic. r=sotaro a=lmandel
gfx/layers/ipc/CompositorChild.cpp
gfx/layers/ipc/CompositorChild.h
widget/nsBaseWidget.cpp
widget/nsBaseWidget.h
widget/windows/nsWindow.cpp
widget/windows/nsWindowGfx.cpp
--- a/gfx/layers/ipc/CompositorChild.cpp
+++ b/gfx/layers/ipc/CompositorChild.cpp
@@ -1,15 +1,16 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set sw=2 ts=2 et tw=80 : */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/layers/CompositorChild.h"
+#include "mozilla/layers/CompositorParent.h"
 #include <stddef.h>                     // for size_t
 #include "ClientLayerManager.h"         // for ClientLayerManager
 #include "base/message_loop.h"          // for MessageLoop
 #include "base/process_util.h"          // for OpenProcessHandle
 #include "base/task.h"                  // for NewRunnableMethod, etc
 #include "base/tracked.h"               // for FROM_HERE
 #include "mozilla/layers/LayerTransactionChild.h"
 #include "mozilla/layers/PLayerTransactionChild.h"
@@ -31,38 +32,75 @@ namespace mozilla {
 namespace layers {
 
 /*static*/ CompositorChild* CompositorChild::sCompositor;
 
 Atomic<int32_t> 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<CompositorParent> aCompositorParent,
+                                      nsRefPtr<CompositorChild> aCompositorChild)
+{
+    // Bug 848949 needs to be fixed before
+    // we can close the channel properly
+    //aCompositorChild->Close();
 }
 
 void
 CompositorChild::Destroy()
 {
-  mLayerManager->Destroy();
-  mLayerManager = nullptr;
+  if (!mCanSend) {
+    NS_WARNING("Trying to deinitialize a CompositorChild twice");
+    return;
+  }
+
+  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.
+  mCanSend = false;
+
+  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) {
     RefPtr<LayerTransactionChild> layers =
       static_cast<LayerTransactionChild*>(ManagedPLayerTransactionChild()[i]);
     layers->Destroy();
   }
-  MOZ_ASSERT(!mCanSend);
+
   SendStop();
+
+  // The DeferredDestroyCompositor task takes ownership of compositorParent and
+  // will release them when it runs.
+  nsRefPtr<CompositorChild> selfRef = this;
+  MessageLoop::current()->PostTask(FROM_HERE,
+             NewRunnableFunction(DeferredDestroyCompositor, mCompositorParent, selfRef));
 }
 
 bool
 CompositorChild::LookupCompositorFrameMetrics(const FrameMetrics::ViewID aId,
                                               FrameMetrics& aFrame)
 {
   SharedFrameMetricsData* data = mFrameMetricsTable.Get(aId);
   if (data) {
@@ -85,27 +123,41 @@ CompositorChild::Create(Transport* aTran
     NS_RUNTIMEABORT("Couldn't OpenProcessHandle() to parent process.");
     return nullptr;
   }
   if (!child->Open(aTransport, handle, XRE_GetIOMessageLoop(), ipc::ChildSide)) {
     NS_RUNTIMEABORT("Couldn't Open() Compositor channel.");
     return nullptr;
   }
 
+  child->mCanSend = true;
+
   // We release this ref in ActorDestroy().
   sCompositor = child.forget().take();
 
   int32_t width;
   int32_t height;
   sCompositor->SendGetTileSize(&width, &height);
   gfxPlatform::GetPlatform()->SetTileSize(width, height);
 
   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()
 {
   // This is only expected to be used in child processes.
   MOZ_ASSERT(XRE_GetProcessType() != GeckoProcessType_Default);
   return sCompositor;
 }
 
@@ -183,21 +235,24 @@ CompositorChild::ActorDestroy(ActorDestr
     NS_RUNTIMEABORT("ActorDestroy by IPC channel failure at CompositorChild");
   }
 #endif
 
   // 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.
-  sCompositor = nullptr;
+  if (mCanSend) {
+    Destroy();
+  }
 
-  MessageLoop::current()->PostTask(
-    FROM_HERE,
-    NewRunnableMethod(this, &CompositorChild::Release));
+  if (sCompositor) {
+    sCompositor->Release();
+    sCompositor = nullptr;
+  }
 }
 
 void
 CompositorChild::ShutDown()
 {
   if (sCompositor) {
     sCompositor->ActorDestroy(NormalShutdown);
   }
@@ -322,18 +377,16 @@ CompositorChild::CancelNotifyAfterRemote
     mWeakTabChild = nullptr;
   }
 }
 
 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();
 }
 
 bool
 CompositorChild::SendPause()
 {
   MOZ_ASSERT(mCanSend);
   if (!mCanSend) {
--- a/gfx/layers/ipc/CompositorChild.h
+++ b/gfx/layers/ipc/CompositorChild.h
@@ -55,16 +55,22 @@ public:
   /**
    * We're asked to create a new Compositor in response to an Opens()
    * or Bridge() request from our parent process.  The Transport is to
    * the compositor's context.
    */
   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; }
 
   virtual bool RecvInvalidateAll() MOZ_OVERRIDE;
   virtual bool RecvOverfill(const uint32_t &aOverfill) MOZ_OVERRIDE;
   void AddOverfillObserver(ClientLayerManager* aLayerManager);
 
@@ -144,16 +150,19 @@ private:
     // the shared FrameMetrics
     nsRefPtr<mozilla::ipc::SharedMemoryBasic> mBuffer;
     CrossProcessMutex* mMutex;
     // Unique ID of the APZC that is sharing the FrameMetrics
     uint32_t mAPZCId;
   };
 
   nsRefPtr<ClientLayerManager> mLayerManager;
+  // When not multi-process, hold a reference to the CompositorParent to keep it
+  // alive. This reference should be null in multi-process.
+  nsRefPtr<CompositorParent> 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
   nsClassHashtable<nsUint64HashKey, SharedFrameMetricsData> mFrameMetricsTable;
 
   // When we're in a child process, this is the process-global
   // compositor that we use to forward transactions directly to the
   // compositor context in another process.
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@@ -160,78 +160,56 @@ WidgetShutdownObserver::Observe(nsISuppo
 
 void
 nsBaseWidget::Shutdown()
 {
   DestroyCompositor();
   mShutdownObserver = nullptr;
 }
 
-static void DeferredDestroyCompositor(nsRefPtr<CompositorParent> aCompositorParent,
-                                      nsRefPtr<CompositorChild> aCompositorChild)
-{
-    // Bug 848949 needs to be fixed before
-    // we can close the channel properly
-    //aCompositorChild->Close();
-}
-
 void nsBaseWidget::DestroyCompositor()
 {
   if (mCompositorChild) {
-    nsRefPtr<CompositorChild> compositorChild = mCompositorChild.forget();
-    nsRefPtr<CompositorParent> compositorParent = mCompositorParent.forget();
-
-    compositorChild->SendWillStop();
-    // New LayerManager, CompositorParent and CompositorChild might be created
-    // as a result of internal GetLayerManager() call.
-    compositorChild->Destroy();
+    mCompositorChild->Destroy();
+    mCompositorChild = nullptr;
+    mCompositorParent = nullptr;
+  }
+}
 
-    // 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));
+void nsBaseWidget::DestroyLayerManager()
+{
+  if (mLayerManager) {
+    mLayerManager->Destroy();
+    mLayerManager = nullptr;
   }
+  DestroyCompositor();
 }
 
 //-------------------------------------------------------------------------
 //
 // nsBaseWidget destructor
 //
 //-------------------------------------------------------------------------
 nsBaseWidget::~nsBaseWidget()
 {
   if (mLayerManager &&
       mLayerManager->GetBackendType() == LayersBackend::LAYERS_BASIC) {
     static_cast<BasicLayerManager*>(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
     // function from being called. Make sure we don't try
     // to reference the dead widget.
     mShutdownObserver->mWidget = nullptr;
     nsContentUtils::UnregisterShutdownObserver(mShutdownObserver);
   }
 
-  DestroyCompositor();
+  DestroyLayerManager();
 
 #ifdef NOISY_WIDGET_LEAKS
   gNumWidgets--;
   printf("WIDGETS- = %d\n", gNumWidgets);
 #endif
 
   NS_IF_RELEASE(mContext);
   delete mOriginalBounds;
@@ -991,35 +969,37 @@ nsBaseWidget::GetCompositorVsyncDispatch
 void nsBaseWidget::CreateCompositor(int aWidth, int aHeight)
 {
   // This makes sure that gfxPlatforms gets initialized if it hasn't by now.
   gfxPlatform::GetPlatform();
 
   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!
 
   // If we've already received a shutdown notification, don't try
   // create a new compositor.
   if (!mShutdownObserver) {
     return;
   }
 
   CreateCompositorVsyncDispatcher();
   mCompositorParent = NewCompositorParent(aWidth, aHeight);
-  MessageChannel *parentChannel = mCompositorParent->GetIPCChannel();
   nsRefPtr<ClientLayerManager> lm = new ClientLayerManager(this);
-  MessageLoop *childMessageLoop = CompositorParent::CompositorLoop();
   mCompositorChild = new CompositorChild(lm);
-  mCompositorChild->Open(parentChannel, childMessageLoop, ipc::ChildSide);
+  mCompositorChild->OpenSameProcess(mCompositorParent);
 
   if (gfxPrefs::AsyncPanZoomEnabled() &&
       (WindowType() == eWindowType_toplevel || WindowType() == eWindowType_child)) {
     ConfigureAPZCTreeManager();
   }
 
   TextureFactoryIdentifier textureFactoryIdentifier;
   PLayerTransactionChild* shadowManager = nullptr;
@@ -1038,36 +1018,30 @@ void nsBaseWidget::CreateCompositor(int 
 #endif
 
   bool success = false;
   if (!backendHints.IsEmpty()) {
     shadowManager = mCompositorChild->SendPLayerTransactionConstructor(
       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()
 {
   return gfxPlatform::UsesOffMainThreadCompositing();
 }
 
 LayerManager* nsBaseWidget::GetLayerManager(PLayerTransactionChild* aShadowManager,
--- a/widget/nsBaseWidget.h
+++ b/widget/nsBaseWidget.h
@@ -420,16 +420,17 @@ protected:
    *
    * When this function returns, the compositor should not be 
    * able to access the opengl context anymore.
    * It is safe to call it several times if platform implementations
    * require the compositor to be destroyed before ~nsBaseWidget is
    * reached (This is the case with gtk2 for instance).
    */
   void DestroyCompositor();
+  void DestroyLayerManager();
 
   nsIWidgetListener* mWidgetListener;
   nsIWidgetListener* mAttachedWidgetListener;
   nsDeviceContext* mContext;
   nsRefPtr<LayerManager> mLayerManager;
   nsRefPtr<LayerManager> mBasicLayerManager;
   nsRefPtr<CompositorChild> mCompositorChild;
   nsRefPtr<CompositorParent> mCompositorParent;
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -683,21 +683,17 @@ NS_METHOD nsWindow::Destroy()
 
   // During the destruction of all of our children, make sure we don't get deleted.
   nsCOMPtr<nsIWidget> kungFuDeathGrip(this);
 
   /**
    * 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. */
   ClearCachedResources();
 
   // The DestroyWindow function destroys the specified window. The function sends WM_DESTROY
   // and WM_NCDESTROY messages to the window to deactivate it and remove the keyboard focus
   // from it. The function also destroys the window's menu, flushes the thread message queue,
@@ -3334,18 +3330,17 @@ nsWindow::GetLayerManager(PLayerTransact
     {
       LayerManagerD3D10 *layerManagerD3D10 =
         static_cast<LayerManagerD3D10*>(mLayerManager.get());
       if (layerManagerD3D10->device() !=
           gfxWindowsPlatform::GetPlatform()->GetD3D10Device())
       {
         MOZ_ASSERT(!mLayerManager->IsInTransaction());
 
-        mLayerManager->Destroy();
-        mLayerManager = nullptr;
+        DestroyLayerManager();
       }
     }
   }
 #endif
 
   RECT windowRect;
   ::GetClientRect(mWnd, &windowRect);
 
@@ -6689,27 +6684,25 @@ bool nsWindow::AutoErase(HDC dc)
 {
   return false;
 }
 
 void
 nsWindow::AllowD3D9Callback(nsWindow *aWindow)
 {
   if (aWindow->mLayerManager && !aWindow->ShouldUseOffMainThreadCompositing()) {
-    aWindow->mLayerManager->Destroy();
-    aWindow->mLayerManager = nullptr;
+    aWindow->DestroyLayerManager();
   }
 }
 
 void
 nsWindow::AllowD3D9WithReinitializeCallback(nsWindow *aWindow)
 {
   if (aWindow->mLayerManager && !aWindow->ShouldUseOffMainThreadCompositing()) {
-    aWindow->mLayerManager->Destroy();
-    aWindow->mLayerManager = nullptr;
+    aWindow->DestroyLayerManager();
     (void) aWindow->GetLayerManager();
   }
 }
 
 void
 nsWindow::StartAllowingD3D9(bool aReinitialize)
 {
   sAllowD3D9 = true;
--- a/widget/windows/nsWindowGfx.cpp
+++ b/widget/windows/nsWindowGfx.cpp
@@ -184,18 +184,17 @@ bool nsWindow::OnPaint(HDC aDC, uint32_t
   // windows event spin loop. If we don't trap for this, we'll try to paint,
   // but view manager will refuse to paint the surface, resulting is black
   // flashes on the plugin rendering surface.
   if (mozilla::ipc::MessageChannel::IsSpinLoopActive() && mPainting)
     return false;
 
   if (gfxWindowsPlatform::GetPlatform()->DidRenderingDeviceReset()) {
     gfxWindowsPlatform::GetPlatform()->UpdateRenderMode();
-    mLayerManager = nullptr;
-    DestroyCompositor();
+    DestroyLayerManager();
     return false;
   }
 
   // After we CallUpdateWindow to the child, occasionally a WM_PAINT message
   // is posted to the parent event loop with an empty update rect. Do a
   // dummy paint so that Windows stops dispatching WM_PAINT in an inifinite
   // loop. See bug 543788.
   if (IsPlugin()) {
@@ -524,18 +523,17 @@ bool nsWindow::OnPaint(HDC aDC, uint32_t
 #ifdef MOZ_ENABLE_D3D9_LAYER
       case LayersBackend::LAYERS_D3D9:
         {
           nsRefPtr<LayerManagerD3D9> layerManagerD3D9 =
             static_cast<mozilla::layers::LayerManagerD3D9*>(GetLayerManager());
           layerManagerD3D9->SetClippingRegion(region);
           result = listener->PaintWindow(this, region);
           if (layerManagerD3D9->DeviceWasRemoved()) {
-            mLayerManager->Destroy();
-            mLayerManager = nullptr;
+            DestroyLayerManager();
             // When our device was removed, we should have gfxWindowsPlatform
             // check if its render mode is up to date!
             gfxWindowsPlatform::GetPlatform()->UpdateRenderMode();
             Invalidate();
           }
         }
         break;
 #endif