Bug 1586144 - Expand the FrameMetrics.mLayoutViewport to the size for viewport units. r=botond
authorHiroyuki Ikezoe <hikezoe.birchill@mozilla.com>
Thu, 14 Nov 2019 06:00:42 +0000
changeset 501891 828246fa2bdf81639ca22092cb330e2473c1d4e8
parent 501890 1f6692cbcebad058f1c4d322e3306314e5d534eb
child 501892 5dcaf36d218a33e8d20563f6210a296e0d47f2fa
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1586144
milestone72.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 1586144 - Expand the FrameMetrics.mLayoutViewport to the size for viewport units. r=botond Note that this FrameMetrics.mLayoutViewport doesn't represent exact size of the layout viewport on the main thread, it means the maximum layout viewport in future on the compositor thread once after the dynamic toolbar is completely hidden. During the dynamic toolbar transition we don't update any information on the main thread, which means it's possible that the visual viewport on the compositor gets bigger than the layout viewport at the time when we send it to the compositor thread, we have to avoid the situation. Depends on D50419 Differential Revision: https://phabricator.services.mozilla.com/D50420
layout/base/nsLayoutUtils.cpp
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/DynamicToolbarTest.kt
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -8997,16 +8997,28 @@ ScrollMetadata nsLayoutUtils::ComputeScr
 
       if (const Maybe<PresShell::VisualScrollUpdate>& visualUpdate =
               presShell->GetPendingVisualScrollUpdate()) {
         metrics.SetVisualViewportOffset(
             CSSPoint::FromAppUnits(visualUpdate->mVisualScrollOffset));
         metrics.SetVisualScrollUpdateType(visualUpdate->mUpdateType);
         presShell->AcknowledgePendingVisualScrollUpdate();
       }
+      // Expand the layout viewport to the size including the area covered by
+      // the dynamic toolbar in the case where the dynamic toolbar is being
+      // used, otherwise when the dynamic toolbar transitions on the compositor,
+      // the layout viewport will be smaller than the visual viewport on the
+      // compositor, thus the layout viewport offset will be forced to be moved
+      // in FrameMetrics::KeepLayoutViewportEnclosingVisualViewport.
+      if (presContext->HasDynamicToolbar()) {
+        CSSRect viewport = metrics.GetLayoutViewport();
+        viewport.SizeTo(nsLayoutUtils::ExpandHeightForViewportUnits(
+            presContext, viewport.Size()));
+        metrics.SetLayoutViewport(viewport);
+      }
     }
 
     CSSRect viewport = metrics.GetLayoutViewport();
     viewport.MoveTo(scrollPosition);
     metrics.SetLayoutViewport(viewport);
 
     nsPoint smoothScrollPosition = scrollableFrame->LastScrollDestination();
     metrics.SetSmoothScrollOffset(CSSPoint::FromAppUnits(smoothScrollPosition));
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/DynamicToolbarTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/DynamicToolbarTest.kt
@@ -100,9 +100,37 @@ class DynamicToolbarTest : BaseSessionTe
         // waiting for a corresponding MozAfterPaint on the main-thread so it's possible
         // to take a stale snapshot even if it's a result of syncronous scrolling.
         mainSession.evaluateJS("new Promise(resolve => window.setTimeout(resolve, 1000))")
 
         sessionRule.display?.let {
             assertScreenshotResult(it.capturePixels(), reference)
         }
     }
+
+    // Asynchronous scrolling with the dynamic toolbar max height causes
+    // situations where the visual viewport size gets bigger than the layout
+    // viewport on the compositor thread because of 200vh position:fixed
+    // elements.  This is a test case that a 200vh position element is
+    // properly rendered its positions.
+    @WithDisplay(height = SCREEN_HEIGHT, width = SCREEN_WIDTH)
+    @Test
+    fun layoutViewportExpansion() {
+        sessionRule.display?.run { setDynamicToolbarMaxHeight(SCREEN_HEIGHT / 2) }
+
+        val reference = getComparisonScreenshot(SCREEN_WIDTH, SCREEN_HEIGHT)
+
+        mainSession.loadTestPath(BaseSessionTest.FIXED_VH)
+        mainSession.waitForPageStop()
+
+        mainSession.evaluateJS("window.scrollTo(0, 100)")
+
+        // Scroll back to the original position by asynchronous scrolling.
+        mainSession.evaluateJS("window.scrollTo({ top: 0, behavior: 'smooth' })")
+
+        mainSession.evaluateJS("new Promise(resolve => window.setTimeout(resolve, 1000))")
+
+        sessionRule.display?.let {
+            assertScreenshotResult(it.capturePixels(), reference)
+        }
+    }
+
 }