Bug 1504659 Part 2: Update innerWidth/Height getters and setters to better handle overridden visual viewports. r=botond
☠☠ backed out by 73cd4b7b53ba ☠ ☠
authorBrad Werth <bwerth@mozilla.com>
Mon, 14 Jan 2019 21:00:33 +0000
changeset 513862 a88ccc9308e0003edc0866f820fed9e671433ed8
parent 513861 738e1ee854eb24b72679b35252a4889b9603c003
child 513863 231b16b0091e96e1aa15cd34c2a9367df8f4d1b8
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1504659
milestone66.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 1504659 Part 2: Update innerWidth/Height getters and setters to better handle overridden visual viewports. r=botond This change also stylistically restructures the getters to make the logic match up cleanly with the new logic in the setters. Differential Revision: https://phabricator.services.mozilla.com/D15995
dom/base/nsGlobalWindowOuter.cpp
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -3029,33 +3029,30 @@ nsresult nsGlobalWindowOuter::GetInnerSi
   RefPtr<nsPresContext> presContext = mDocShell->GetPresContext();
   RefPtr<nsIPresShell> presShell = mDocShell->GetPresShell();
 
   if (!presContext || !presShell) {
     aSize = CSSIntSize(0, 0);
     return NS_OK;
   }
 
-  /*
-   * On platforms with resolution-based zooming, the CSS viewport
-   * and visual viewport may not be the same. The inner size should
-   * be the visual viewport, but we fall back to the CSS viewport
-   * if it is not set.
-   */
+  // If the visual viewport has been overriden, return that.
   if (presShell->IsVisualViewportSizeSet()) {
     aSize = CSSIntRect::FromAppUnitsRounded(presShell->GetVisualViewportSize());
-  } else {
-    RefPtr<nsViewManager> viewManager = presShell->GetViewManager();
-    if (viewManager) {
-      viewManager->FlushDelayedResize(false);
-    }
-
-    aSize =
-        CSSIntRect::FromAppUnitsRounded(presContext->GetVisibleArea().Size());
-  }
+    return NS_OK;
+  }
+
+  // Whether or not the css viewport has been overridden, we can get the
+  // correct value by looking at the visible area of the presContext.
+  RefPtr<nsViewManager> viewManager = presShell->GetViewManager();
+  if (viewManager) {
+    viewManager->FlushDelayedResize(false);
+  }
+
+  aSize = CSSIntRect::FromAppUnitsRounded(presContext->GetVisibleArea().Size());
   return NS_OK;
 }
 
 int32_t nsGlobalWindowOuter::GetInnerWidthOuter(ErrorResult& aError) {
   CSSIntSize size;
   aError = GetInnerSize(size);
   return size.width;
 }
@@ -3068,32 +3065,51 @@ void nsGlobalWindowOuter::SetInnerWidthO
                                              CallerType aCallerType,
                                              ErrorResult& aError) {
   if (!mDocShell) {
     aError.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
 
   CheckSecurityWidthAndHeight(&aInnerWidth, nullptr, aCallerType);
-
   RefPtr<nsIPresShell> presShell = mDocShell->GetPresShell();
 
+  // Setting inner width should set the visual viewport. Most of the
+  // time, this is the same as the CSS viewport, and when we set one,
+  // we implicitly set both of them. But if
+  // presShell->IsVisualViewportSizeSet() returns true, that means
+  // that the two diverge. In that case we only set the visual viewport.
+  // This mirrors the logic in ::GetInnerSize() and ensures that JS
+  // behaves sanely when setting and then getting the innerWidth.
+  if (presShell && presShell->IsVisualViewportSizeSet()) {
+    CSSSize viewportSize =
+        CSSRect::FromAppUnits(presShell->GetVisualViewportSize());
+    viewportSize.width = aInnerWidth;
+    nsLayoutUtils::SetVisualViewportSize(presShell, viewportSize);
+    return;
+  }
+
+  // We're going to set both viewports. If the css viewport has been
+  // overridden, change the css viewport override. The visual viewport
+  // will adopt this value via the logic in ::GetInnerSize().
   if (presShell && presShell->GetIsViewportOverridden()) {
     nscoord height = 0;
 
     RefPtr<nsPresContext> presContext;
     presContext = presShell->GetPresContext();
 
     nsRect shellArea = presContext->GetVisibleArea();
     height = shellArea.Height();
     SetCSSViewportWidthAndHeight(
         nsPresContext::CSSPixelsToAppUnits(aInnerWidth), height);
     return;
   }
 
+  // Nothing has been overriden, so change the docshell itself, which will
+  // affect both viewports.
   int32_t height = 0;
   int32_t unused = 0;
 
   nsCOMPtr<nsIBaseWindow> docShellAsWin(do_QueryInterface(mDocShell));
   docShellAsWin->GetSize(&unused, &height);
   aError = SetDocShellWidthAndHeight(CSSToDevIntPixels(aInnerWidth), height);
 }
 
@@ -3110,37 +3126,57 @@ nsresult nsGlobalWindowOuter::GetInnerHe
 void nsGlobalWindowOuter::SetInnerHeightOuter(int32_t aInnerHeight,
                                               CallerType aCallerType,
                                               ErrorResult& aError) {
   if (!mDocShell) {
     aError.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
 
+  CheckSecurityWidthAndHeight(nullptr, &aInnerHeight, aCallerType);
   RefPtr<nsIPresShell> presShell = mDocShell->GetPresShell();
 
+  // Setting inner height should set the visual viewport. Most of the
+  // time, this is the same as the CSS viewport, and when we set one,
+  // we implicitly set both of them. But if
+  // presShell->IsVisualViewportSizeSet() returns true, that means
+  // that the two diverge. In that case we only set the visual viewport.
+  // This mirrors the logic in ::GetInnerSize() and ensures that JS
+  // behaves sanely when setting and then getting the innerHeight.
+  if (presShell && presShell->IsVisualViewportSizeSet()) {
+    CSSSize viewportSize =
+        CSSRect::FromAppUnits(presShell->GetVisualViewportSize());
+    viewportSize.height = aInnerHeight;
+    nsLayoutUtils::SetVisualViewportSize(presShell, viewportSize);
+    return;
+  }
+
+  // We're going to set both viewports. If the css viewport has been
+  // overridden, change the css viewport override. The visual viewport
+  // will adopt this value via the logic in ::GetInnerSize().
   if (presShell && presShell->GetIsViewportOverridden()) {
+    nscoord width = 0;
+
     RefPtr<nsPresContext> presContext;
     presContext = presShell->GetPresContext();
 
     nsRect shellArea = presContext->GetVisibleArea();
-    nscoord height = aInnerHeight;
-    nscoord width = shellArea.Width();
-    CheckSecurityWidthAndHeight(nullptr, &height, aCallerType);
-    SetCSSViewportWidthAndHeight(width,
-                                 nsPresContext::CSSPixelsToAppUnits(height));
+    width = shellArea.Width();
+    SetCSSViewportWidthAndHeight(
+        width, nsPresContext::CSSPixelsToAppUnits(aInnerHeight));
     return;
   }
 
+  // Nothing has been overriden, so change the docshell itself, which will
+  // affect both viewports.
   int32_t height = 0;
   int32_t width = 0;
 
   nsCOMPtr<nsIBaseWindow> docShellAsWin(do_QueryInterface(mDocShell));
   docShellAsWin->GetSize(&width, &height);
-  CheckSecurityWidthAndHeight(nullptr, &aInnerHeight, aCallerType);
   aError = SetDocShellWidthAndHeight(width, CSSToDevIntPixels(aInnerHeight));
 }
 
 nsIntSize nsGlobalWindowOuter::GetOuterSize(CallerType aCallerType,
                                             ErrorResult& aError) {
   if (nsContentUtils::ResistFingerprinting(aCallerType)) {
     CSSIntSize size;
     aError = GetInnerSize(size);