Bug 1444525: Persist XUL window size attributes as the outer size, not inner size. r=bz
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 10 Mar 2018 02:42:57 +0100
changeset 462225 0038e9f8858b1309ce2d2d2b8a1fcf18e7a5efe7
parent 462224 e51c36c4cecda981facddbbbbf5dbe389031e1a1
child 462226 0d1ab0ba4aea986499513e35f64d014aa8216448
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1444525
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1444525: Persist XUL window size attributes as the outer size, not inner size. r=bz MozReview-Commit-ID: 7U5tWxTaxGi
dom/xul/XULDocument.cpp
xpfe/appshell/nsIXULWindow.idl
xpfe/appshell/nsXULWindow.cpp
--- a/dom/xul/XULDocument.cpp
+++ b/dom/xul/XULDocument.cpp
@@ -1155,16 +1155,51 @@ XULDocument::Persist(const nsAString& aI
         }
 
         nameSpaceID = kNameSpaceID_None;
     }
 
     aRv = Persist(element, nameSpaceID, tag);
 }
 
+enum class ConversionDirection {
+    InnerToOuter,
+    OuterToInner,
+};
+
+static void
+ConvertWindowSize(nsIXULWindow* aWin,
+                  nsAtom* aAttr,
+                  ConversionDirection aDirection,
+                  nsAString& aInOutString)
+{
+    MOZ_ASSERT(aWin);
+    MOZ_ASSERT(aAttr == nsGkAtoms::width || aAttr == nsGkAtoms::height);
+
+    nsresult rv;
+    int32_t size = aInOutString.ToInteger(&rv);
+    if (NS_FAILED(rv)) {
+        return;
+    }
+
+    int32_t sizeDiff = aAttr == nsGkAtoms::width
+        ? aWin->GetOuterToInnerWidthDifferenceInCSSPixels()
+        : aWin->GetOuterToInnerHeightDifferenceInCSSPixels();
+
+    if (!sizeDiff) {
+        return;
+    }
+
+    int32_t multiplier =
+        aDirection == ConversionDirection::InnerToOuter ? 1 : - 1;
+
+    CopyASCIItoUTF16(nsPrintfCString("%d", size + multiplier * sizeDiff),
+                     aInOutString);
+}
+
 nsresult
 XULDocument::Persist(Element* aElement, int32_t aNameSpaceID,
                      nsAtom* aAttribute)
 {
     // For non-chrome documents, persistance is simply broken
     if (!nsContentUtils::IsSystemPrincipal(NodePrincipal()))
         return NS_ERROR_NOT_AVAILABLE;
 
@@ -1193,19 +1228,31 @@ XULDocument::Persist(Element* aElement, 
     bool hasAttr;
     rv = mLocalStore->HasValue(uri, id, attrstr, &hasAttr);
     if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
     }
 
     if (hasAttr && valuestr.IsEmpty()) {
         return mLocalStore->RemoveValue(uri, id, attrstr);
-    } else {
-        return mLocalStore->SetValue(uri, id, attrstr, valuestr);
     }
+
+    // Make sure we store the <window> attributes as outer window size, see
+    // bug 1444525 & co.
+    if (aElement->IsXULElement(nsGkAtoms::window) &&
+        (aAttribute == nsGkAtoms::width || aAttribute == nsGkAtoms::height)) {
+        if (nsCOMPtr<nsIXULWindow> win = GetXULWindowIfToplevelChrome()) {
+            ConvertWindowSize(win,
+                              aAttribute,
+                              ConversionDirection::InnerToOuter,
+                              valuestr);
+        }
+    }
+
+    return mLocalStore->SetValue(uri, id, attrstr, valuestr);
 }
 
 
 nsresult
 XULDocument::GetViewportSize(int32_t* aWidth,
                              int32_t* aHeight)
 {
     *aWidth = *aHeight = 0;
@@ -1743,17 +1790,16 @@ XULDocument::ApplyPersistentAttributesIn
         if (NS_WARN_IF(NS_FAILED(rv))) {
             return rv;
         }
     }
 
     return NS_OK;
 }
 
-
 nsresult
 XULDocument::ApplyPersistentAttributesToElements(const nsAString &aID,
                                                  nsCOMArray<Element>& aElements)
 {
     nsAutoCString utf8uri;
     nsresult rv = mDocumentURI->GetSpec(utf8uri);
     if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
@@ -1784,23 +1830,39 @@ XULDocument::ApplyPersistentAttributesTo
         }
 
         RefPtr<nsAtom> attr = NS_Atomize(attrstr);
         if (NS_WARN_IF(!attr)) {
             return NS_ERROR_OUT_OF_MEMORY;
         }
 
         uint32_t cnt = aElements.Count();
-
         for (int32_t i = int32_t(cnt) - 1; i >= 0; --i) {
             RefPtr<Element> element = aElements.SafeObjectAt(i);
             if (!element) {
                  continue;
             }
 
+            // Convert attributes from outer size to inner size for top-level
+            // windows, see bug 1444525 & co.
+            if (element->IsXULElement(nsGkAtoms::window) &&
+                (attr == nsGkAtoms::width || attr == nsGkAtoms::height)) {
+                if (nsCOMPtr<nsIXULWindow> win = GetXULWindowIfToplevelChrome()) {
+                    nsAutoString maybeConvertedValue = value;
+                    ConvertWindowSize(win,
+                                      attr,
+                                      ConversionDirection::OuterToInner,
+                                      maybeConvertedValue);
+                    Unused <<
+                        element->SetAttr(kNameSpaceID_None, attr, maybeConvertedValue, true);
+
+                    continue;
+                }
+            }
+
             Unused << element->SetAttr(kNameSpaceID_None, attr, value, true);
         }
     }
 
     return NS_OK;
 }
 
 void
--- a/xpfe/appshell/nsIXULWindow.idl
+++ b/xpfe/appshell/nsIXULWindow.idl
@@ -62,16 +62,23 @@ interface nsIXULWindow : nsISupports
 
   /**
    * Tell this window that it has picked up a child XUL window
    * @param aChild the child window being added
    */
   void addChildWindow(in nsIXULWindow aChild);
 
   /**
+   * Returns the difference between the inner window size (client size) and the
+   * outer window size, in CSS pixels.
+   */
+  [noscript,notxpcom] uint32_t getOuterToInnerHeightDifferenceInCSSPixels();
+  [noscript,notxpcom] uint32_t getOuterToInnerWidthDifferenceInCSSPixels();
+
+  /**
    * Tell this window that it has lost a child XUL window
    * @param aChild the child window being removed
    */
   void removeChildWindow(in nsIXULWindow aChild);
 
   /**
    * Move the window to a centered position.
    * @param aRelative If not null, the window relative to which the window is
--- a/xpfe/appshell/nsXULWindow.cpp
+++ b/xpfe/appshell/nsXULWindow.cpp
@@ -334,16 +334,49 @@ nsXULWindow::TabParentRemoved(nsITabPare
 NS_IMETHODIMP
 nsXULWindow::GetPrimaryTabParent(nsITabParent** aTab)
 {
   nsCOMPtr<nsITabParent> tab = mPrimaryTabParent;
   tab.forget(aTab);
   return NS_OK;
 }
 
+static LayoutDeviceIntSize
+GetOuterToInnerSizeDifference(nsIWidget* aWindow)
+{
+  if (!aWindow) {
+    return LayoutDeviceIntSize();
+  }
+  LayoutDeviceIntSize baseSize(200, 200);
+  LayoutDeviceIntSize windowSize = aWindow->ClientToWindowSize(baseSize);
+  return windowSize - baseSize;
+}
+
+static CSSIntSize
+GetOuterToInnerSizeDifferenceInCSSPixels(nsIWidget* aWindow)
+{
+  if (!aWindow) {
+    return { };
+  }
+  LayoutDeviceIntSize devPixelSize = GetOuterToInnerSizeDifference(aWindow);
+  return RoundedToInt(devPixelSize / aWindow->GetDefaultScale());
+}
+
+uint32_t
+nsXULWindow::GetOuterToInnerHeightDifferenceInCSSPixels()
+{
+  return GetOuterToInnerSizeDifferenceInCSSPixels(mWindow).height;
+}
+
+uint32_t
+nsXULWindow::GetOuterToInnerWidthDifferenceInCSSPixels()
+{
+  return GetOuterToInnerSizeDifferenceInCSSPixels(mWindow).width;
+}
+
 nsTArray<RefPtr<mozilla::LiveResizeListener>>
 nsXULWindow::GetLiveResizeListeners()
 {
   nsTArray<RefPtr<mozilla::LiveResizeListener>> listeners;
   if (mPrimaryTabParent) {
     TabParent* parent = static_cast<TabParent*>(mPrimaryTabParent.get());
     listeners.AppendElement(parent);
   }
@@ -1060,27 +1093,16 @@ NS_IMETHODIMP nsXULWindow::ForceRoundedD
   SetPrimaryContentSize(targetContentWidth, targetContentHeight);
 
   mIgnoreXULSize = true;
   mIgnoreXULSizeMode = true;
 
   return NS_OK;
 }
 
-static LayoutDeviceIntSize
-GetWindowOuterInnerDiff(nsIWidget* aWindow)
-{
-  if (!aWindow) {
-    return LayoutDeviceIntSize();
-  }
-  LayoutDeviceIntSize baseSize(200, 200);
-  LayoutDeviceIntSize windowSize = aWindow->ClientToWindowSize(baseSize);
-  return windowSize - baseSize;
-}
-
 void nsXULWindow::OnChromeLoaded()
 {
   nsresult rv = EnsureContentTreeOwner();
 
   if (NS_SUCCEEDED(rv)) {
     mChromeLoaded = true;
     ApplyChromeFlags();
     SyncAttributesToWidget();
@@ -1595,20 +1617,20 @@ NS_IMETHODIMP nsXULWindow::SavePersisten
     if (NS_SUCCEEDED(parent->GetPosition(&parentX, &parentY))) {
       rect.MoveBy(-parentX, -parentY);
     }
   }
 
   char                        sizeBuf[10];
   nsAutoString                sizeString;
   nsAutoString                windowElementId;
-  RefPtr<dom::XULDocument>    ownerXULDoc;
 
   // fetch docShellElement's ID and XUL owner document
-  ownerXULDoc = docShellElement->OwnerDoc()->AsXULDocument();
+  RefPtr<dom::XULDocument> ownerXULDoc =
+    docShellElement->OwnerDoc()->AsXULDocument();
   if (docShellElement->IsXULElement()) {
     docShellElement->GetId(windowElementId);
   }
 
   bool shouldPersist = !isFullscreen && ownerXULDoc;
   ErrorResult rv;
   // (only for size elements which are persisted)
   if ((mPersistentAttributesDirty & PAD_POSITION) && gotRestoredBounds) {
@@ -1628,30 +1650,28 @@ NS_IMETHODIMP nsXULWindow::SavePersisten
       if (shouldPersist) {
         IgnoredErrorResult err;
         ownerXULDoc->Persist(windowElementId, SCREENY_ATTRIBUTE, err);
       }
     }
   }
 
   if ((mPersistentAttributesDirty & PAD_SIZE) && gotRestoredBounds) {
-    LayoutDeviceIntSize winDiff = GetWindowOuterInnerDiff(mWindow);
+    LayoutDeviceIntRect innerRect = rect - GetOuterToInnerSizeDifference(mWindow);
     if (persistString.Find("width") >= 0) {
-      auto width = rect.Width() - winDiff.width;
-      SprintfLiteral(sizeBuf, "%d", NSToIntRound(width / sizeScale.scale));
+      SprintfLiteral(sizeBuf, "%d", NSToIntRound(innerRect.Width() / sizeScale.scale));
       CopyASCIItoUTF16(sizeBuf, sizeString);
       docShellElement->SetAttribute(WIDTH_ATTRIBUTE, sizeString, rv);
       if (shouldPersist) {
         IgnoredErrorResult err;
         ownerXULDoc->Persist(windowElementId, WIDTH_ATTRIBUTE, err);
       }
     }
     if (persistString.Find("height") >= 0) {
-      auto height = rect.Height() - winDiff.height;
-      SprintfLiteral(sizeBuf, "%d", NSToIntRound(height / sizeScale.scale));
+      SprintfLiteral(sizeBuf, "%d", NSToIntRound(innerRect.Height() / sizeScale.scale));
       CopyASCIItoUTF16(sizeBuf, sizeString);
       docShellElement->SetAttribute(HEIGHT_ATTRIBUTE, sizeString, rv);
       if (shouldPersist) {
         IgnoredErrorResult err;
         ownerXULDoc->Persist(windowElementId, HEIGHT_ATTRIBUTE, err);
       }
     }
   }
@@ -2250,20 +2270,17 @@ 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();
+  CSSIntSize windowDiff = GetOuterToInnerSizeDifferenceInCSSPixels(mWindow);
 
   // 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;