Bug 877534 - Use a shutdown listener to destroy the compositor from nsBaseWidget. r=roc
authorMatt Woodrow <mwoodrow@mozilla.com>
Mon, 17 Jun 2013 14:50:32 +1200
changeset 146780 18a68adb330fe42be6a236512093a6fdafd5e28d
parent 146779 0f034aaaecfff5a5878dd7f1d8e189be3a2257c6
child 146781 6e84744ffa9a7c0b22d27830c4d2df6bef29f53a
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs877534
milestone24.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 877534 - Use a shutdown listener to destroy the compositor from nsBaseWidget. r=roc This prevents a case where the nsBaseWidget isn't destroyed until the cycle collector is shutdown, and we are too late in the shutdown sequence to process the events that get queued during Compositor teardown.
gfx/layers/opengl/TextureHostOGL.h
widget/xpwidgets/nsBaseWidget.cpp
widget/xpwidgets/nsBaseWidget.h
--- a/gfx/layers/opengl/TextureHostOGL.h
+++ b/gfx/layers/opengl/TextureHostOGL.h
@@ -550,17 +550,17 @@ private:
   {
     // Deduce the type that was assigned in GetFormatAndTileForImageFormat
     return mGLFormat == LOCAL_GL_RGB ? LOCAL_GL_UNSIGNED_SHORT_5_6_5 : LOCAL_GL_UNSIGNED_BYTE;
   }
 
   gfx::IntSize mSize;
   GLuint mTextureHandle;
   GLenum mGLFormat;
-  gl::GLContext* mGL;
+  nsRefPtr<gl::GLContext> mGL;
 };
 
 #ifdef MOZ_WIDGET_GONK
 
 // For direct texturing with gralloc buffers. The corresponding TextureClient is TextureClientShmem,
 // which automatically gets gralloc when it can, in which case the compositor sees that the
 // SurfaceDescriptor is gralloc, and decides to use a GrallocTextureHostOGL to do direct texturing,
 // saving the cost of a texture upload.
--- a/widget/xpwidgets/nsBaseWidget.cpp
+++ b/widget/xpwidgets/nsBaseWidget.cpp
@@ -113,18 +113,41 @@ nsBaseWidget::nsBaseWidget()
 #ifdef NOISY_WIDGET_LEAKS
   gNumWidgets++;
   printf("WIDGETS+ = %d\n", gNumWidgets);
 #endif
 
 #ifdef DEBUG
   debug_RegisterPrefCallbacks();
 #endif
+
+  mShutdownObserver = new WidgetShutdownObserver(this);
+  nsContentUtils::RegisterShutdownObserver(mShutdownObserver);
 }
 
+NS_IMPL_ISUPPORTS1(WidgetShutdownObserver, nsIObserver)
+
+NS_IMETHODIMP
+WidgetShutdownObserver::Observe(nsISupports *aSubject,
+                                const char *aTopic,
+                                const PRUnichar *aData)
+{
+  if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
+    mWidget->Shutdown();
+    nsContentUtils::UnregisterShutdownObserver(this);
+  }
+ return NS_OK;
+}
+
+void
+nsBaseWidget::Shutdown()
+{
+  DestroyCompositor();
+  mShutdownObserver = nullptr;
+}
 
 static void DeferredDestroyCompositor(CompositorParent* aCompositorParent,
                               CompositorChild* aCompositorChild)
 {
     // Bug 848949 needs to be fixed before
     // we can close the channel properly
     //aCompositorChild->Close();
     aCompositorParent->Release();
@@ -166,16 +189,20 @@ nsBaseWidget::~nsBaseWidget()
     static_cast<BasicLayerManager*>(mLayerManager.get())->ClearRetainerWidget();
   }
 
   if (mLayerManager) {
     mLayerManager->Destroy();
     mLayerManager = nullptr;
   }
 
+  if (mShutdownObserver) {
+    nsContentUtils::UnregisterShutdownObserver(mShutdownObserver);
+  }
+
   DestroyCompositor();
 
 #ifdef NOISY_WIDGET_LEAKS
   gNumWidgets--;
   printf("WIDGETS- = %d\n", gNumWidgets);
 #endif
 
   NS_IF_RELEASE(mContext);
@@ -888,16 +915,22 @@ nsBaseWidget::GetPreferredCompositorBack
   return mozilla::layers::LAYERS_OPENGL;
 }
 
 void nsBaseWidget::CreateCompositor(int aWidth, int aHeight)
 {
   // 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;
+  }
+
   mCompositorParent = NewCompositorParent(aWidth, aHeight);
   AsyncChannel *parentChannel = mCompositorParent->GetIPCChannel();
   LayerManager* lm = new ClientLayerManager(this);
   MessageLoop *childMessageLoop = CompositorParent::CompositorLoop();
   mCompositorChild = new CompositorChild(lm);
   AsyncChannel::Side childSide = mozilla::ipc::AsyncChannel::Child;
   mCompositorChild->Open(parentChannel, childMessageLoop, childSide);
 
--- a/widget/xpwidgets/nsBaseWidget.h
+++ b/widget/xpwidgets/nsBaseWidget.h
@@ -10,16 +10,17 @@
 #include "nsIWidget.h"
 #include "nsWidgetsCID.h"
 #include "nsIFile.h"
 #include "nsString.h"
 #include "nsCOMPtr.h"
 #include "nsGUIEvent.h"
 #include "nsAutoPtr.h"
 #include "nsIRollupListener.h"
+#include "nsIObserver.h"
 #include <algorithm>
 class nsIContent;
 class nsAutoRollup;
 class gfxContext;
 
 namespace mozilla {
 #ifdef ACCESSIBILITY
 namespace a11y {
@@ -33,16 +34,32 @@ class CompositorChild;
 class CompositorParent;
 }
 }
 
 namespace base {
 class Thread;
 }
 
+class nsBaseWidget;
+
+class WidgetShutdownObserver MOZ_FINAL : public nsIObserver
+{
+public:
+  WidgetShutdownObserver(nsBaseWidget* aWidget)
+    : mWidget(aWidget)
+  { }
+
+  NS_DECL_ISUPPORTS
+  NS_DECL_NSIOBSERVER
+
+private:
+  nsBaseWidget *mWidget;
+};
+
 /**
  * Common widget implementation used as base class for native
  * or crossplatform implementations of Widgets. 
  * All cross-platform behavior that all widgets need to implement 
  * should be placed in this class. 
  * (Note: widget implementations are not required to use this
  * class, but it gives them a head start.)
  */
@@ -249,16 +266,18 @@ public:
   friend class AutoUseBasicLayerManager;
 
   nsWindowType            GetWindowType() { return mWindowType; }
 
   virtual bool            ShouldUseOffMainThreadCompositing();
 
   static nsIRollupListener* GetActiveRollupListener();
 
+  void Shutdown();
+
 protected:
 
   virtual void            ResolveIconName(const nsAString &aIconName,
                                           const nsAString &aIconSuffix,
                                           nsIFile **aResult);
   virtual void            OnDestroy();
   virtual void            BaseCreate(nsIWidget *aParent,
                                      const nsIntRect &aRect,
@@ -351,16 +370,17 @@ protected:
 
   nsIWidgetListener* mWidgetListener;
   nsIWidgetListener* mAttachedWidgetListener;
   nsDeviceContext* mContext;
   nsRefPtr<LayerManager> mLayerManager;
   nsRefPtr<LayerManager> mBasicLayerManager;
   nsRefPtr<CompositorChild> mCompositorChild;
   nsRefPtr<CompositorParent> mCompositorParent;
+  nsCOMPtr<WidgetShutdownObserver> mShutdownObserver;
   nscolor           mBackground;
   nscolor           mForeground;
   nsCursor          mCursor;
   nsWindowType      mWindowType;
   nsBorderStyle     mBorderStyle;
   bool              mUseLayersAcceleration;
   bool              mForceLayersAcceleration;
   bool              mTemporarilyUseBasicLayerManager;