Bug 1439875: Size the XUL window before doing layout. r=smaug

The only subtle thing is the mCenterAfterLoad stuff, which is gated after a
mChromeLoaded.

Other than that it follows the same pattern as bug 345560.

MozReview-Commit-ID: 8qDiA2yn9DB

dom/xul/XULDocument.cpp

@@ -2663,19 +2663,17 @@ XULDocument::DoneWalking()
         NotifyPossibleTitleChange(false);
 
         // Before starting layout, check whether we're a toplevel chrome
-        // window.  If we are, set our chrome flags now, so that we don't have
+        // window.  If we are, setup some state so that we don't have to restyle
-        // to restyle the whole frame tree after StartLayout.
+        // the whole tree after StartLayout.
-        nsCOMPtr<nsIDocShellTreeItem> item = GetDocShell();
+        if (nsCOMPtr<nsIDocShellTreeItem> item = GetDocShell()) {
-        if (item) {
             nsCOMPtr<nsIDocShellTreeOwner> owner;
             item->GetTreeOwner(getter_AddRefs(owner));
-            nsCOMPtr<nsIXULWindow> xulWin = do_GetInterface(owner);
+            if (nsCOMPtr<nsIXULWindow> xulWin = do_GetInterface(owner)) {
-            if (xulWin) {
                 nsCOMPtr<nsIDocShell> xulWinShell;
                 xulWin->GetDocShell(getter_AddRefs(xulWinShell));
                 if (SameCOMIdentity(xulWinShell, item)) {
-                    // We're the chrome document!  Apply our chrome flags now.
+                    // We're the chrome document!
-                    xulWin->ApplyChromeFlags();
+                    xulWin->BeforeStartLayout();
                 }
             }
         }

xpfe/appshell/nsIXULWindow.idl

@@ -128,12 +128,14 @@ interface nsIXULWindow : nsISupports
   attribute nsIXULBrowserWindow XULBrowserWindow;
 
   /**
-   * Back-door method to force application of chrome flags at a particular
+   * Back-door method to make sure some stuff is done when the document is
-   * time.  Do NOT call this unless you know what you're doing!  In particular,
+   * ready for layout, that would cause expensive computation otherwise later.
+   *
+   * Do NOT call this unless you know what you're doing!  In particular,
    * calling this when this XUL window doesn't yet have a document in its
    * docshell could cause problems.
    */
-  [noscript] void applyChromeFlags();
+  [noscript] void beforeStartLayout();
 
   /**
    * Given the dimensions of some content area held within this

xpfe/appshell/nsXULWindow.cpp

@@ -274,8 +274,9 @@ NS_IMETHODIMP nsXULWindow::SetChromeFlags(uint32_t aChromeFlags)
                "SetChromeFlags() after AssumeChromeFlagsAreFrozen()!");
 
   mChromeFlags = aChromeFlags;
-  if (mChromeLoaded)
+  if (mChromeLoaded) {
-    NS_ENSURE_SUCCESS(ApplyChromeFlags(), NS_ERROR_FAILURE);
+    ApplyChromeFlags();
+  }
   return NS_OK;
 }
 
@@ -1083,82 +1084,7 @@ void nsXULWindow::OnChromeLoaded()
     mChromeLoaded = true;
     ApplyChromeFlags();
     SyncAttributesToWidget();
-
+    SizeShell();
-    int32_t specWidth = -1, specHeight = -1;
-    bool gotSize = false;
-    bool isContent = false;
-
-    GetHasPrimaryContent(&isContent);
-
-    CSSIntSize windowDiff = mWindow
-      ? RoundedToInt(GetWindowOuterInnerDiff(mWindow) /
-                     mWindow->GetDefaultScale())
-      : CSSIntSize();
-
-    // If this window has a primary content and fingerprinting resistance is
-    // enabled, we enforce this window to rounded dimensions.
-    if (isContent && nsContentUtils::ShouldResistFingerprinting()) {
-      ForceRoundedDimensions();
-    } else if (!mIgnoreXULSize) {
-      gotSize = LoadSizeFromXUL(specWidth, specHeight);
-      specWidth += windowDiff.width;
-      specHeight += windowDiff.height;
-    }
-
-    bool positionSet = !mIgnoreXULPosition;
-    nsCOMPtr<nsIXULWindow> parentWindow(do_QueryReferent(mParentWindow));
-#if defined(XP_UNIX) && !defined(XP_MACOSX)
-    // don't override WM placement on unix for independent, top-level windows
-    // (however, we think the benefits of intelligent dependent window placement
-    // trump that override.)
-    if (!parentWindow)
-      positionSet = false;
-#endif
-    if (positionSet) {
-      // We have to do this before sizing the window, because sizing depends
-      // on the resolution of the screen we're on. But positioning needs to
-      // know the size so that it can constrain to screen bounds.... as an
-      // initial guess here, we'll use the specified size (if any).
-      positionSet = LoadPositionFromXUL(specWidth, specHeight);
-    }
-
-    if (gotSize) {
-      SetSpecifiedSize(specWidth, specHeight);
-    }
-
-    if (mIntrinsicallySized) {
-      // (if LoadSizeFromXUL set the size, mIntrinsicallySized will be false)
-      nsCOMPtr<nsIContentViewer> cv;
-      mDocShell->GetContentViewer(getter_AddRefs(cv));
-      if (cv) {
-        nsCOMPtr<nsIDocShellTreeItem> docShellAsItem = do_QueryInterface(mDocShell);
-        nsCOMPtr<nsIDocShellTreeOwner> treeOwner;
-        docShellAsItem->GetTreeOwner(getter_AddRefs(treeOwner));
-        if (treeOwner) {
-          // GetContentSize can fail, so initialise |width| and |height| to be
-          // on the safe side.
-          int32_t width = 0, height = 0;
-          if (NS_SUCCEEDED(cv->GetContentSize(&width, &height))) {
-            treeOwner->SizeShellTo(docShellAsItem, width, height);
-            // Update specified size for the final LoadPositionFromXUL call.
-            specWidth = width + windowDiff.width;
-            specHeight = height + windowDiff.height;
-          }
-        }
-      }
-    }
-
-    // Now that we have set the window's final size, we can re-do its
-    // positioning so that it is properly constrained to the screen.
-    if (positionSet) {
-      LoadPositionFromXUL(specWidth, specHeight);
-    }
-
-    LoadMiscPersistentAttributesFromXUL();
-
-    if (mCenterAfterLoad && !positionSet) {
-      Center(parentWindow, parentWindow ? false : true, false);
-    }
 
     if (mShowAfterLoad) {
       SetVisibility(true);
@@ -2262,10 +2188,13 @@ void nsXULWindow::PersistentAttributesDirty(uint32_t aDirtyFlags)
   mPersistentAttributesDirty |= aDirtyFlags & mPersistentAttributesMask;
 }
 
-NS_IMETHODIMP nsXULWindow::ApplyChromeFlags()
+void
+nsXULWindow::ApplyChromeFlags()
 {
   nsCOMPtr<dom::Element> window = GetWindowDOMElement();
-  NS_ENSURE_TRUE(window, NS_ERROR_FAILURE);
+  if (!window) {
+    return;
+  }
 
   if (mChromeLoaded) {
     // The two calls in this block don't need to happen early because they
@@ -2304,12 +2233,99 @@ NS_IMETHODIMP nsXULWindow::ApplyChromeFlags()
 
   // Note that if we're not actually changing the value this will be a no-op,
   // so no need to compare to the old value.
-  ErrorResult rv;
+  IgnoredErrorResult rv;
   window->SetAttribute(NS_LITERAL_STRING("chromehidden"), newvalue, rv);
+}
 
+NS_IMETHODIMP
+nsXULWindow::BeforeStartLayout()
+{
+  ApplyChromeFlags();
+  SyncAttributesToWidget();
+  SizeShell();
   return NS_OK;
 }
 
+void
+nsXULWindow::SizeShell()
+{
+  int32_t specWidth = -1, specHeight = -1;
+  bool gotSize = false;
+  bool isContent = false;
+
+  GetHasPrimaryContent(&isContent);
+
+  CSSIntSize windowDiff = mWindow
+    ? RoundedToInt(GetWindowOuterInnerDiff(mWindow) /
+                   mWindow->GetDefaultScale())
+    : CSSIntSize();
+
+  // If this window has a primary content and fingerprinting resistance is
+  // enabled, we enforce this window to rounded dimensions.
+  if (isContent && nsContentUtils::ShouldResistFingerprinting()) {
+    ForceRoundedDimensions();
+  } else if (!mIgnoreXULSize) {
+    gotSize = LoadSizeFromXUL(specWidth, specHeight);
+    specWidth += windowDiff.width;
+    specHeight += windowDiff.height;
+  }
+
+  bool positionSet = !mIgnoreXULPosition;
+  nsCOMPtr<nsIXULWindow> parentWindow(do_QueryReferent(mParentWindow));
+#if defined(XP_UNIX) && !defined(XP_MACOSX)
+  // don't override WM placement on unix for independent, top-level windows
+  // (however, we think the benefits of intelligent dependent window placement
+  // trump that override.)
+  if (!parentWindow)
+    positionSet = false;
+#endif
+  if (positionSet) {
+    // We have to do this before sizing the window, because sizing depends
+    // on the resolution of the screen we're on. But positioning needs to
+    // know the size so that it can constrain to screen bounds.... as an
+    // initial guess here, we'll use the specified size (if any).
+    positionSet = LoadPositionFromXUL(specWidth, specHeight);
+  }
+
+  if (gotSize) {
+    SetSpecifiedSize(specWidth, specHeight);
+  }
+
+  if (mIntrinsicallySized) {
+    // (if LoadSizeFromXUL set the size, mIntrinsicallySized will be false)
+    nsCOMPtr<nsIContentViewer> cv;
+    mDocShell->GetContentViewer(getter_AddRefs(cv));
+    if (cv) {
+      nsCOMPtr<nsIDocShellTreeItem> docShellAsItem = do_QueryInterface(mDocShell);
+      nsCOMPtr<nsIDocShellTreeOwner> treeOwner;
+      docShellAsItem->GetTreeOwner(getter_AddRefs(treeOwner));
+      if (treeOwner) {
+        // GetContentSize can fail, so initialise |width| and |height| to be
+        // on the safe side.
+        int32_t width = 0, height = 0;
+        if (NS_SUCCEEDED(cv->GetContentSize(&width, &height))) {
+          treeOwner->SizeShellTo(docShellAsItem, width, height);
+          // Update specified size for the final LoadPositionFromXUL call.
+          specWidth = width + windowDiff.width;
+          specHeight = height + windowDiff.height;
+        }
+      }
+    }
+  }
+
+  // Now that we have set the window's final size, we can re-do its
+  // positioning so that it is properly constrained to the screen.
+  if (positionSet) {
+    LoadPositionFromXUL(specWidth, specHeight);
+  }
+
+  LoadMiscPersistentAttributesFromXUL();
+
+  if (mChromeLoaded && mCenterAfterLoad && !positionSet) {
+    Center(parentWindow, parentWindow ? false : true, false);
+  }
+}
+
 NS_IMETHODIMP nsXULWindow::GetXULBrowserWindow(nsIXULBrowserWindow * *aXULBrowserWindow)
 {
   NS_IF_ADDREF(*aXULBrowserWindow = mXULBrowserWindow);

xpfe/appshell/nsXULWindow.h

@@ -93,6 +93,8 @@ protected:
    NS_IMETHOD ForceRoundedDimensions();
    NS_IMETHOD GetAvailScreenSize(int32_t* aAvailWidth, int32_t* aAvailHeight);
 
+   void ApplyChromeFlags();
+   void SizeShell();
    void OnChromeLoaded();
    void StaggerPosition(int32_t &aRequestedX, int32_t &aRequestedY,
                         int32_t aSpecWidth, int32_t aSpecHeight);