Bug 1439875: Size the XUL window before doing layout. r?smaug draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 21 Feb 2018 12:47:05 +0100
changeset 759140 5f82dfe66f2514eff52264ee55afb78f0cc6cee9
parent 759139 a099391c41645c54439edcab66500df7fa6f8e6c
child 759141 20c7e77698fe51a241552b9d98eaed85d034c675
push id100275
push userbmo:emilio@crisal.io
push dateFri, 23 Feb 2018 18:33:59 +0000
reviewerssmaug
bugs1439875, 345560
milestone60.0a1
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
xpfe/appshell/nsIXULWindow.idl
xpfe/appshell/nsXULWindow.cpp
xpfe/appshell/nsXULWindow.h
--- a/dom/xul/XULDocument.cpp
+++ b/dom/xul/XULDocument.cpp
@@ -2665,29 +2665,27 @@ XULDocument::DoneWalking()
         // mInitialLayoutComplete will be false will follow the else branch
         // there too.  See the big comment there for how such reentry can
         // happen.
         mDocumentLoaded = true;
 
         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
-        // to restyle the whole frame tree after StartLayout.
-        nsCOMPtr<nsIDocShellTreeItem> item = GetDocShell();
-        if (item) {
+        // window.  If we are, setup some state so that we don't have to restyle
+        // the whole tree after StartLayout.
+        if (nsCOMPtr<nsIDocShellTreeItem> item = GetDocShell()) {
             nsCOMPtr<nsIDocShellTreeOwner> owner;
             item->GetTreeOwner(getter_AddRefs(owner));
-            nsCOMPtr<nsIXULWindow> xulWin = do_GetInterface(owner);
-            if (xulWin) {
+            if (nsCOMPtr<nsIXULWindow> xulWin = do_GetInterface(owner)) {
                 nsCOMPtr<nsIDocShell> xulWinShell;
                 xulWin->GetDocShell(getter_AddRefs(xulWinShell));
                 if (SameCOMIdentity(xulWinShell, item)) {
-                    // We're the chrome document!  Apply our chrome flags now.
-                    xulWin->ApplyChromeFlags();
+                    // We're the chrome document!
+                    xulWin->BeforeStartLayout();
                 }
             }
         }
 
         nsContentUtils::DispatchTrustedEvent(this,
                 static_cast<nsIDocument*>(this),
                 NS_LITERAL_STRING("MozBeforeInitialXULLayout"),
                 true,
--- a/xpfe/appshell/nsIXULWindow.idl
+++ b/xpfe/appshell/nsIXULWindow.idl
@@ -123,22 +123,24 @@ interface nsIXULWindow : nsISupports
   nsIXULWindow createNewWindow(in int32_t aChromeFlags,
                                in nsITabParent aOpeningTab,
                                in mozIDOMWindowProxy aOpener,
                                in unsigned long long aNextTabParentId);
 
   attribute nsIXULBrowserWindow XULBrowserWindow;
 
   /**
-   * Back-door method to force application of chrome flags at a particular
-   * time.  Do NOT call this unless you know what you're doing!  In particular,
+   * Back-door method to make sure some stuff is done when the document is
+   * 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
    * XUL window, and assuming that that content area will change
    * its dimensions in linear proportion to the dimensions of this
    * XUL window, changes the size of the XUL window so that the
    * content area reaches a particular size.
    *
--- a/xpfe/appshell/nsXULWindow.cpp
+++ b/xpfe/appshell/nsXULWindow.cpp
@@ -269,18 +269,19 @@ NS_IMETHODIMP nsXULWindow::GetChromeFlag
 }
 
 NS_IMETHODIMP nsXULWindow::SetChromeFlags(uint32_t aChromeFlags)
 {
   NS_ASSERTION(!mChromeFlagsFrozen,
                "SetChromeFlags() after AssumeChromeFlagsAreFrozen()!");
 
   mChromeFlags = aChromeFlags;
-  if (mChromeLoaded)
-    NS_ENSURE_SUCCESS(ApplyChromeFlags(), NS_ERROR_FAILURE);
+  if (mChromeLoaded) {
+    ApplyChromeFlags();
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP nsXULWindow::AssumeChromeFlagsAreFrozen()
 {
   mChromeFlagsFrozen = true;
   return NS_OK;
 }
@@ -1067,85 +1068,17 @@ NS_IMETHODIMP nsXULWindow::ForceRoundedD
 void nsXULWindow::OnChromeLoaded()
 {
   nsresult rv = EnsureContentTreeOwner();
 
   if (NS_SUCCEEDED(rv)) {
     mChromeLoaded = true;
     ApplyChromeFlags();
     SyncAttributesToWidget();
-
-    int32_t specWidth = -1, specHeight = -1;
-    bool gotSize = false;
-    bool isContent = false;
-
-    GetHasPrimaryContent(&isContent);
-
-    // 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);
-    }
-
-    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;
-            specHeight = 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);
-    }
+    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!
     }
   }
   mPersistentAttributesMask |= PAD_POSITION | PAD_SIZE | PAD_MISC;
@@ -2205,20 +2138,23 @@ bool nsXULWindow::GetContentScrollbarVis
 }
 
 // during spinup, attributes that haven't been loaded yet can't be dirty
 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
     // don't cause a global restyle on the document.  Not only that, but the
     // scrollbar stuff needs a content area to toggle the scrollbars on anyway.
     // So just don't do these until mChromeLoaded is true.
 
     // Scrollbars have their own special treatment.
@@ -2247,22 +2183,101 @@ NS_IMETHODIMP nsXULWindow::ApplyChromeFl
   if (! (mChromeFlags & nsIWebBrowserChrome::CHROME_STATUSBAR))
     newvalue.AppendLiteral("status ");
 
   if (! (mChromeFlags & nsIWebBrowserChrome::CHROME_EXTRA))
     newvalue.AppendLiteral("extrachrome ");
 
   // 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();
+  SizeShell();
   return NS_OK;
 }
 
+void
+nsXULWindow::SizeShell()
+{
+    int32_t specWidth = -1, specHeight = -1;
+    bool gotSize = false;
+    bool isContent = false;
+
+    GetHasPrimaryContent(&isContent);
+
+    // 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);
+    }
+
+    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;
+            specHeight = 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);
   return NS_OK;
 }
 
 NS_IMETHODIMP nsXULWindow::SetXULBrowserWindow(nsIXULBrowserWindow * aXULBrowserWindow)
 {
--- a/xpfe/appshell/nsXULWindow.h
+++ b/xpfe/appshell/nsXULWindow.h
@@ -88,16 +88,18 @@ protected:
    NS_IMETHOD EnsureChromeTreeOwner();
    NS_IMETHOD EnsureContentTreeOwner();
    NS_IMETHOD EnsurePrimaryContentTreeOwner();
    NS_IMETHOD EnsurePrompter();
    NS_IMETHOD EnsureAuthPrompter();
    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);
    bool       LoadPositionFromXUL(int32_t aSpecWidth, int32_t aSpecHeight);
    bool       LoadSizeFromXUL(int32_t& aSpecWidth, int32_t& aSpecHeight);
    void       SetSpecifiedSize(int32_t aSpecWidth, int32_t aSpecHeight);
    bool       LoadMiscPersistentAttributesFromXUL();
    void       SyncAttributesToWidget();