Bug 1125848 - Backout because of crashes. r=me

gfx/layers/ipc/CompositorChild.cpp

@@ -5,7 +5,6 @@
  * 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
@@ -41,53 +40,19 @@ Atomic<int32_t> CompositableForwarder::sSerialCounter(0);
 
 CompositorChild::CompositorChild(ClientLayerManager *aLayerManager)
   : mLayerManager(aLayerManager)
-  , mCanSend(false)
+  , mCanSend(true)
 {
 }
 
 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()
 {
-  // This must not be called from the destructor!
-  MOZ_ASSERT(mRefCnt != 0);
-
-  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;
-  }
-
+  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) {
@@ -95,14 +60,8 @@ CompositorChild::Destroy()
       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
@@ -135,8 +94,6 @@ CompositorChild::Create(Transport* aTransport, ProcessId aOtherProcess)
     return nullptr;
   }
 
-  child->mCanSend = true;
-
   // We release this ref in ActorDestroy().
   sCompositor = child.forget().take();
 
@@ -149,18 +106,6 @@ CompositorChild::Create(Transport* aTransport, ProcessId aOtherProcess)
   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()
 {
@@ -537,6 +482,8 @@ 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();
 }
 

gfx/layers/ipc/CompositorChild.h

@@ -60,12 +60,6 @@ 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; }
@@ -174,9 +168,6 @@ private:
                                                         void* aLayerTransactionChild);
 
   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

widget/nsBaseWidget.cpp

@@ -181,22 +181,39 @@ nsBaseWidget::Shutdown()
   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) {
-    mCompositorChild->Destroy();
-    mCompositorChild = nullptr;
-    mCompositorParent = nullptr;
-  }
-}
+    nsRefPtr<CompositorChild> compositorChild = mCompositorChild.forget();
+    nsRefPtr<CompositorParent> compositorParent = mCompositorParent.forget();
 
-void nsBaseWidget::DestroyLayerManager()
-{
-  if (mLayerManager) {
-    mLayerManager->Destroy();
-    mLayerManager = nullptr;
+    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));
   }
-  DestroyCompositor();
 }
 
 //-------------------------------------------------------------------------
@@ -211,6 +228,11 @@ nsBaseWidget::~nsBaseWidget()
     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
@@ -220,7 +242,7 @@ nsBaseWidget::~nsBaseWidget()
     nsContentUtils::UnregisterShutdownObserver(mShutdownObserver);
   }
 
-  DestroyLayerManager();
+  DestroyCompositor();
 
 #ifdef NOISY_WIDGET_LEAKS
   gNumWidgets--;
@@ -1059,12 +1081,8 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight)
   MOZ_ASSERT(gfxPlatform::UsesOffMainThreadCompositing(),
              "This function assumes OMTC");
 
-  MOZ_ASSERT(!mCompositorParent && !mCompositorChild,
-    "Should have properly cleaned up the previous PCompositor pair beforehand");
-
-  if (mCompositorChild) {
-    mCompositorChild->Destroy();
-  }
+  MOZ_ASSERT(!mCompositorParent,
+    "Should have properly cleaned up the previous CompositorParent beforehand");
 
   // Recreating this is tricky, as we may still have an old and we need
   // to make sure it's properly destroyed by calling DestroyCompositor!
@@ -1077,9 +1095,11 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight)
 
   CreateCompositorVsyncDispatcher();
   mCompositorParent = NewCompositorParent(aWidth, aHeight);
+  MessageChannel *parentChannel = mCompositorParent->GetIPCChannel();
   nsRefPtr<ClientLayerManager> lm = new ClientLayerManager(this);
+  MessageLoop *childMessageLoop = CompositorParent::CompositorLoop();
   mCompositorChild = new CompositorChild(lm);
-  mCompositorChild->OpenSameProcess(mCompositorParent);
+  mCompositorChild->Open(parentChannel, childMessageLoop, ipc::ChildSide);
 
   if (gfxPrefs::AsyncPanZoomEnabled() &&
       (WindowType() == eWindowType_toplevel || WindowType() == eWindowType_child)) {
@@ -1108,20 +1128,26 @@ void nsBaseWidget::CreateCompositor(int aWidth, int aHeight)
       backendHints, 0, &textureFactoryIdentifier, &success);
   }
 
-  ShadowLayerForwarder* lf = lm->AsShadowForwarder();
+  if (success) {
+    ShadowLayerForwarder* lf = lm->AsShadowForwarder();
+    if (!lf) {
+      lm = nullptr;
+      mCompositorChild = nullptr;
+      return;
+    }
+    lf->SetShadowManager(shadowManager);
+    lf->IdentifyTextureHost(textureFactoryIdentifier);
+    ImageBridgeChild::IdentifyCompositorTextureHost(textureFactoryIdentifier);
+    WindowUsesOMTC();
 
-  if (!success || !lf) {
-    NS_WARNING("Failed to create an OMT compositor.");
-    DestroyCompositor();
+    mLayerManager = lm.forget();
     return;
   }
 
-  lf->SetShadowManager(shadowManager);
-  lf->IdentifyTextureHost(textureFactoryIdentifier);
-  ImageBridgeChild::IdentifyCompositorTextureHost(textureFactoryIdentifier);
-  WindowUsesOMTC();
-
-  mLayerManager = lm.forget();
+  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.
 }
 
 bool nsBaseWidget::ShouldUseOffMainThreadCompositing()

widget/nsBaseWidget.h

@@ -441,7 +441,6 @@ protected:
    * reached (This is the case with gtk2 for instance).
    */
   void DestroyCompositor();
-  void DestroyLayerManager();
 
   nsIWidgetListener* mWidgetListener;
   nsIWidgetListener* mAttachedWidgetListener;

widget/windows/nsWindow.cpp

@@ -671,7 +671,11 @@ 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.
    */
-  DestroyLayerManager();
+  if (mLayerManager) {
+    mLayerManager->Destroy();
+  }
+  mLayerManager = nullptr;
+  DestroyCompositor();
 
   /* We should clear our cached resources now and not wait for the GC to
    * delete the nsWindow. */
@@ -6532,7 +6536,10 @@ bool nsWindow::AutoErase(HDC dc)
 void
 nsWindow::ClearCompositor(nsWindow* aWindow)
 {
-  aWindow->DestroyLayerManager();
+  if (aWindow->mLayerManager) {
+    aWindow->mLayerManager = nullptr;
+    aWindow->DestroyCompositor();
+  }
 }
 
 bool