Bug 1392705 - part 3: Call nsBaseWidget::DestroyLayerManager() in nsWindow::Destroy to ensure IPC is not shutdown in the destructor for Android r=jchen draft
authorRandall Barker <rbarker@mozilla.com>
Tue, 05 Sep 2017 13:00:06 -0700
changeset 662529 b313fa65eaac14ae04ea7291d74e75ed3ce3b185
parent 662528 9c747d9d93354a8841b1098f95cb1cec275f7963
child 730897 dcefce9536809fa395e4f796333d56c7854968bd
push id79112
push userbmo:rbarker@mozilla.com
push dateMon, 11 Sep 2017 19:54:17 +0000
reviewersjchen
bugs1392705
milestone57.0a1
Bug 1392705 - part 3: Call nsBaseWidget::DestroyLayerManager() in nsWindow::Destroy to ensure IPC is not shutdown in the destructor for Android r=jchen Due to the indeterminate nature of Gecko shutdown, the ref count of the nsWindow on Android would sometimes go to zero before the XPCOM shutdown observer was called in nsBaseWindow which is where the compositor thread IPC is shutdown. If nsBaseWindow::Shutdown does not get called, then the compositor thread IPC is shutdown in the nsBaseWindow destructor. Unfortunately while the nsWindow is being deleted, it can be accessed in the compositor thread and cause a crash in LayerManagerComposite::RenderToPresentationSurface. Calling nsBaseWidget::DestroyLayerManager() in nsWindow::Destroy() ensures it gets called before the destructor gets invoked typically in the Destroy() call. MozReview-Commit-ID: KCv8SCmEjnb
widget/android/nsWindow.cpp
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -1470,16 +1470,20 @@ nsWindow::nsWindow() :
     mIsFullScreen(false)
 {
 }
 
 nsWindow::~nsWindow()
 {
     gTopLevelWindows.RemoveElement(this);
     ALOG("nsWindow %p destructor", (void*)this);
+    // The mCompositorSession should have been cleaned up in nsWindow::Destroy()
+    // DestroyLayerManager() will call DestroyCompositor() which will crash if
+    // called from nsBaseWidget destructor. See Bug 1392705
+    MOZ_ASSERT(!mCompositorSession);
 }
 
 bool
 nsWindow::IsTopLevel()
 {
     return mWindowType == eWindowType_toplevel ||
         mWindowType == eWindowType_dialog ||
         mWindowType == eWindowType_invisible;
@@ -1541,16 +1545,19 @@ nsWindow::Destroy()
     nsCOMPtr<nsIWidget> kungFuDeathGrip(this);
 
     while (mChildren.Length()) {
         // why do we still have children?
         ALOG("### Warning: Destroying window %p and reparenting child %p to null!", (void*)this, (void*)mChildren[0]);
         mChildren[0]->SetParent(nullptr);
     }
 
+    // Ensure the compositor has been shutdown before this nsWindow is potentially deleted
+    nsBaseWidget::DestroyCompositor();
+
     nsBaseWidget::Destroy();
 
     if (IsTopLevel())
         gTopLevelWindows.RemoveElement(this);
 
     SetParent(nullptr);
 
     nsBaseWidget::OnDestroy();