Bug 1463819 - Account for the possibility of SyncAttributesToWidget destroying the window. r=enn, a=RyanVM
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 25 May 2018 20:58:56 +0200
changeset 473528 7aa58e74173962f26e61191567091afc35202abf
parent 473527 96eeb9f7e318718bbd01ce04ee019931c354fe57
child 473529 8735fe57245edec03c7a99b82626e57f9a4997bd
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersenn, RyanVM
bugs1463819
milestone61.0
Bug 1463819 - Account for the possibility of SyncAttributesToWidget destroying the window. r=enn, a=RyanVM MozReview-Commit-ID: 8O3We8wnSGk
xpfe/appshell/nsXULWindow.cpp
xpfe/appshell/nsXULWindow.h
--- a/xpfe/appshell/nsXULWindow.cpp
+++ b/xpfe/appshell/nsXULWindow.cpp
@@ -446,19 +446,16 @@ NS_IMETHODIMP nsXULWindow::Create()
 {
   //XXX First Check In
   NS_ASSERTION(false, "Not Yet Implemented");
   return NS_OK;
 }
 
 NS_IMETHODIMP nsXULWindow::Destroy()
 {
-  MOZ_DIAGNOSTIC_ASSERT(!mSyncingAttributesToWidget,
-                        "Destroying the window from SyncAttributesToWidget?");
-
   if (!mWindow)
      return NS_OK;
 
   // Ensure we don't reenter this code
   if (mDestroying)
     return NS_OK;
 
   mozilla::AutoRestore<bool> guard(mDestroying);
@@ -1122,23 +1119,25 @@ NS_IMETHODIMP nsXULWindow::ForceRoundedD
 void nsXULWindow::OnChromeLoaded()
 {
   nsresult rv = EnsureContentTreeOwner();
 
   if (NS_SUCCEEDED(rv)) {
     mChromeLoaded = true;
     ApplyChromeFlags();
     SyncAttributesToWidget();
-    SizeShell();
-
-    if (mShowAfterLoad) {
-      SetVisibility(true);
-      // At this point the window may have been closed during Show(), so
-      // nsXULWindow::Destroy may already have been called. Take care!
+    if (mWindow) {
+      SizeShell();
+      if (mShowAfterLoad) {
+        SetVisibility(true);
+      }
     }
+    // At this point the window may have been closed already during Show() or
+    // SyncAttributesToWidget(), so nsXULWindow::Destroy may already have been
+    // called. Take care!
   }
   mPersistentAttributesMask |= PAD_POSITION | PAD_SIZE | PAD_MISC;
 }
 
 // If aSpecWidth and/or aSpecHeight are > 0, we will use these CSS px sizes
 // to fit to the screen when staggering windows; if they're negative,
 // we use the window's current size instead.
 bool nsXULWindow::LoadPositionFromXUL(int32_t aSpecWidth, int32_t aSpecHeight)
@@ -1548,19 +1547,16 @@ void nsXULWindow::StaggerPosition(int32_
 }
 
 void nsXULWindow::SyncAttributesToWidget()
 {
   nsCOMPtr<dom::Element> windowElement = GetWindowDOMElement();
   if (!windowElement)
     return;
 
-  AutoRestore<bool> scope { mSyncingAttributesToWidget };
-  mSyncingAttributesToWidget = true;
-
   MOZ_DIAGNOSTIC_ASSERT(mWindow, "No widget on SyncAttributesToWidget?");
 
   nsAutoString attr;
 
   // "hidechrome" attribute
   if (windowElement->AttrValueIs(kNameSpaceID_None, nsGkAtoms::hidechrome,
                                  nsGkAtoms::_true, eCaseMatters)) {
     mWindow->HideWindowChrome(true);
@@ -2298,17 +2294,19 @@ nsXULWindow::ApplyChromeFlags()
   window->SetAttribute(NS_LITERAL_STRING("chromehidden"), newvalue, rv);
 }
 
 NS_IMETHODIMP
 nsXULWindow::BeforeStartLayout()
 {
   ApplyChromeFlags();
   SyncAttributesToWidget();
-  SizeShell();
+  if (mWindow) {
+    SizeShell();
+  }
   return NS_OK;
 }
 
 void
 nsXULWindow::SizeShell()
 {
   AutoRestore<bool> sizingShellFromXUL(mSizingShellFromXUL);
   mSizingShellFromXUL = true;
--- a/xpfe/appshell/nsXULWindow.h
+++ b/xpfe/appshell/nsXULWindow.h
@@ -165,17 +165,16 @@ protected:
    bool                    mLockedUntilChromeLoad;
    bool                    mIgnoreXULSize;
    bool                    mIgnoreXULPosition;
    bool                    mChromeFlagsFrozen;
    bool                    mIgnoreXULSizeMode;
    // mDestroying is used to prevent reentry into into Destroy(), which can
    // otherwise happen due to script running as we tear down various things.
    bool                    mDestroying;
-   bool                    mSyncingAttributesToWidget = false;
    bool                    mRegistered;
    uint32_t                mPersistentAttributesDirty; // persistentAttributes
    uint32_t                mPersistentAttributesMask;
    uint32_t                mChromeFlags;
    uint64_t                mNextTabParentId;
    nsString                mTitle;
    nsIntRect               mOpenerScreenRect; // the screen rect of the opener