Bug 1510058 - Work around mWidget becoming null during DidComposite, and add a diagnostic assert that will hopefully root out the culprit. r=jrmuizel, a=jcristau
authorMarkus Stange <mstange@themasta.com>
Thu, 29 Nov 2018 17:48:43 +0000
changeset 501490 97f7aec00819
parent 501489 e4c03449c5ec
child 501491 c58ea2229c33
push id1891
push userjcristau@mozilla.com
push dateTue, 08 Jan 2019 16:05:30 +0000
treeherdermozilla-release@c58ea2229c33 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, jcristau
bugs1510058
milestone64.0.2
Bug 1510058 - Work around mWidget becoming null during DidComposite, and add a diagnostic assert that will hopefully root out the culprit. r=jrmuizel, a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D13309
gfx/layers/client/ClientLayerManager.cpp
gfx/layers/client/ClientLayerManager.h
--- a/gfx/layers/client/ClientLayerManager.cpp
+++ b/gfx/layers/client/ClientLayerManager.cpp
@@ -53,16 +53,17 @@ ClientLayerManager::ClientLayerManager(n
   , mLastPaintTime(TimeDuration::Forever())
   , mTargetRotation(ROTATION_0)
   , mRepeatTransaction(false)
   , mIsRepeatTransaction(false)
   , mTransactionIncomplete(false)
   , mCompositorMightResample(false)
   , mNeedsComposite(false)
   , mQueuedAsyncPaints(false)
+  , mNotifyingWidgetListener(false)
   , mPaintSequenceNumber(0)
   , mForwarder(new ShadowLayerForwarder(this))
 {
   MOZ_COUNT_CTOR(ClientLayerManager);
   mMemoryPressureObserver = MemoryPressureObserver::Create(this);
 }
 
 
@@ -80,16 +81,19 @@ ClientLayerManager::~ClientLayerManager(
   mRoot = nullptr;
 
   MOZ_COUNT_DTOR(ClientLayerManager);
 }
 
 void
 ClientLayerManager::Destroy()
 {
+  MOZ_DIAGNOSTIC_ASSERT(!mNotifyingWidgetListener,
+    "Try to avoid destroying widgets and layer managers during DidCompositeWindow, if you can");
+
   // It's important to call ClearCachedResource before Destroy because the
   // former will early-return if the later has already run.
   ClearCachedResources();
   LayerManager::Destroy();
 
   if (mTransactionIdAllocator) {
     // Make sure to notify the refresh driver just in case it's waiting on a
     // pending transaction. Do this at the top of the event loop so we don't
@@ -489,21 +493,28 @@ ClientLayerManager::DidComposite(Transac
   // the end of the method invocation.
   RefPtr<ClientLayerManager> selfRef = this;
 
   // |aTransactionId| will be > 0 if the compositor is acknowledging a shadow
   // layers transaction.
   if (aTransactionId.IsValid()) {
     nsIWidgetListener *listener = mWidget->GetWidgetListener();
     if (listener) {
+      mNotifyingWidgetListener = true;
       listener->DidCompositeWindow(aTransactionId, aCompositeStart, aCompositeEnd);
+      mNotifyingWidgetListener = false;
     }
-    listener = mWidget->GetAttachedWidgetListener();
-    if (listener) {
-      listener->DidCompositeWindow(aTransactionId, aCompositeStart, aCompositeEnd);
+    // DidCompositeWindow might have called Destroy on us and nulled out mWidget,
+    // see bug 1510058.
+    // Re-check it here.
+    if (mWidget) {
+      listener = mWidget->GetAttachedWidgetListener();
+      if (listener) {
+        listener->DidCompositeWindow(aTransactionId, aCompositeStart, aCompositeEnd);
+      }
     }
     if (mTransactionIdAllocator) {
       mTransactionIdAllocator->NotifyTransactionCompleted(aTransactionId);
     }
   }
 
   // These observers fire whether or not we were in a transaction.
   for (size_t i = 0; i < mDidCompositeObservers.Length(); i++) {
--- a/gfx/layers/client/ClientLayerManager.h
+++ b/gfx/layers/client/ClientLayerManager.h
@@ -322,16 +322,17 @@ private:
   // Used to repeat the transaction right away (to avoid rebuilding
   // a display list) to support progressive drawing.
   bool mRepeatTransaction;
   bool mIsRepeatTransaction;
   bool mTransactionIncomplete;
   bool mCompositorMightResample;
   bool mNeedsComposite;
   bool mQueuedAsyncPaints;
+  bool mNotifyingWidgetListener;
 
   // An incrementing sequence number for paints.
   // Incremented in BeginTransaction(), but not for repeat transactions.
   uint32_t mPaintSequenceNumber;
 
   APZTestData mApzTestData;
 
   RefPtr<ShadowLayerForwarder> mForwarder;