Bug 1586144 - Expand the clipt rect for visual viewport scrollable display item. r=botond
authorHiroyuki Ikezoe <hikezoe.birchill@mozilla.com>
Thu, 14 Nov 2019 06:00:25 +0000
changeset 501889 1f6692cbcebad058f1c4d322e3306314e5d534eb
parent 501888 9f96406e2da19b6d4e9f89163ff51c4e529779ae
child 501890 828246fa2bdf81639ca22092cb330e2473c1d4e8
push id36801
push userdvarga@mozilla.com
push dateThu, 14 Nov 2019 17:12:31 +0000
treeherdermozilla-central@a19a226a8c6a [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 clipt rect for visual viewport scrollable display item. r=botond Without this change, the junit test in this commit fail, the failure rendered result is the area of the position:fixed element covered by the dynamic toolbar before scrolling is rendered as blank. Depends on D50418 Differential Revision: https://phabricator.services.mozilla.com/D50419
layout/base/nsLayoutUtils.h
layout/base/nsPresContext.h
layout/generic/nsGfxScrollFrame.cpp
mobile/android/geckoview/src/androidTest/assets/www/fixedvh.html
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/DynamicToolbarTest.kt
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -2991,16 +2991,26 @@ class nsLayoutUtils {
   /**
    * Similar to above FrameIsScrolledOutViewInCrossProcess but returns true even
    * if |aFrame| is not fully scrolled out of view and its visible area width or
    * height is smaller than |aMargin|.
    **/
   static bool FrameIsMostlyScrolledOutOfViewInCrossProcess(
       const nsIFrame* aFrame, nscoord aMargin);
 
+  /**
+   * Expand the height of |aSize| to the size of `vh` units.
+   *
+   * With dynamic toolbar(s) the height for `vh` units is greater than the
+   * ICB height, we need to expand it in some places.
+   **/
+  template <typename SizeType>
+  static SizeType ExpandHeightForViewportUnits(nsPresContext* aPresContext,
+                                               const SizeType& aSize);
+
  private:
   /**
    * Helper function for LogTestDataForPaint().
    */
   static void DoLogTestDataForPaint(mozilla::layers::LayerManager* aManager,
                                     ViewID aScrollId, const std::string& aKey,
                                     const std::string& aValue);
 
@@ -3043,16 +3053,33 @@ template <typename PointType, typename R
       aClosestYDistance = yDistance;
       return true;
     }
   }
 
   return false;
 }
 
+template <typename SizeType>
+/* static */ SizeType nsLayoutUtils::ExpandHeightForViewportUnits(
+    nsPresContext* aPresContext, const SizeType& aSize) {
+  nsSize sizeForViewportUnits = aPresContext->GetSizeForViewportUnits();
+
+  // |aSize| might be the size expanded to the minimum-scale size whereas the
+  // size for viewport units is not scaled so that we need to expand the |aSize|
+  // height with the aspect ratio of the size for viewport units instead of just
+  // expanding to the size for viewport units.
+  float ratio = (float)sizeForViewportUnits.height / sizeForViewportUnits.width;
+
+  MOZ_ASSERT(aSize.height <=
+             NSCoordSaturatingNonnegativeMultiply(aSize.width, ratio));
+  return SizeType(aSize.width,
+                  NSCoordSaturatingNonnegativeMultiply(aSize.width, ratio));
+}
+
 namespace mozilla {
 
 /**
  * Converts an nsPoint in app units to a Moz2D Point in pixels (whether those
  * are device pixels or CSS px depends on what the caller chooses to pass as
  * aAppUnitsPerPixel).
  */
 inline gfx::Point NSPointToPoint(const nsPoint& aPoint,
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -366,16 +366,18 @@ class nsPresContext : public nsISupports
   nsRect GetVisibleArea() const { return mVisibleArea; }
 
   /**
    * Set the currently visible area. The units for r are standard
    * nscoord units (as scaled by the device context).
    */
   void SetVisibleArea(const nsRect& r);
 
+  nsSize GetSizeForViewportUnits() const { return mSizeForViewportUnits; }
+
   /**
    * Set the maximum height of the dynamic toolbar in nscoord units.
    */
   MOZ_CAN_RUN_SCRIPT
   void SetDynamicToolbarMaxHeight(mozilla::ScreenIntCoord aHeight);
 
   mozilla::ScreenIntCoord GetDynamicToolbarMaxHeight() const {
     MOZ_ASSERT(IsRootContentDocumentCrossProcess());
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -3539,16 +3539,27 @@ void ScrollFrameHelper::BuildDisplayList
   nsDisplayListCollection set(aBuilder);
 
   bool isRootContent =
       mIsRoot && mOuter->PresContext()->IsRootContentDocumentCrossProcess();
   bool willBuildAsyncZoomContainer =
       aBuilder->ShouldBuildAsyncZoomContainer() && isRootContent;
 
   nsRect scrollPortClip = mScrollPort + aBuilder->ToReferenceFrame(mOuter);
+  // Expand the clip rect to the size including the area covered by dynamic
+  // toolbar in the case where the dynamic toolbar is being used since
+  // position:fixed elements attached to this root scroller might be taller than
+  // its scroll port (e.g 100vh). Even if the dynamic toolbar covers the taller
+  // area, it doesn't mean the area is clipped by the toolbar because the
+  // dynamic toolbar is laid out outside of our topmost window and it
+  // transitions without changing our topmost window size.
+  if (isRootContent && mOuter->PresContext()->HasDynamicToolbar()) {
+    scrollPortClip.SizeTo(nsLayoutUtils::ExpandHeightForViewportUnits(
+        mOuter->PresContext(), scrollPortClip.Size()));
+  }
   nsRect clipRect = scrollPortClip;
   // Our override of GetBorderRadii ensures we never have a radius at
   // the corners where we have a scrollbar.
   nscoord radii[8];
   bool haveRadii = mOuter->GetPaddingBoxBorderRadii(radii);
   if (mIsRoot) {
     clipRect.SizeTo(nsLayoutUtils::CalculateCompositionSizeForFrame(mOuter));
     if (!aBuilder->IsPaintingToWindow() &&
new file mode 100644
--- /dev/null
+++ b/mobile/android/geckoview/src/androidTest/assets/www/fixedvh.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<meta name="viewport" content="width=device-width, minimum-scale=0.5">
+<style>
+html {
+  width: 100%;
+  height: 100%;
+  scrollbar-width: none;
+}
+body {
+  width: 200%;
+  height: 2000px;
+  margin: 0;
+  padding: 0;
+}
+
+#fixed-element {
+  width: 100%;
+  height: 200vh;
+  position: fixed;
+  top: 0px;
+  background-color: green;
+}
+</style>
+<div id="fixed-element"></div>
--- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
+++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/BaseSessionTest.kt
@@ -54,16 +54,17 @@ open class BaseSessionTest(noErrorCollec
         const val FULLSCREEN_PATH = "/assets/www/fullscreen.html"
         const val VIEWPORT_PATH = "/assets/www/viewport.html"
         const val IFRAME_REDIRECT_LOCAL = "/assets/www/iframe_redirect_local.html"
         const val IFRAME_REDIRECT_AUTOMATION = "/assets/www/iframe_redirect_automation.html"
         const val AUTOPLAY_PATH = "/assets/www/autoplay.html"
         const val SCROLL_TEST_PATH = "/assets/www/scroll.html"
         const val COLORS_HTML_PATH = "/assets/www/colors.html"
         const val FIXED_BOTTOM = "/assets/www/fixedbottom.html"
+        const val FIXED_VH = "/assets/www/fixedvh.html"
         const val STORAGE_TITLE_HTML_PATH = "/assets/www/reflect_local_storage_into_title.html"
         const val HUNG_SCRIPT = "/assets/www/hungScript.html"
         const val PUSH_HTML_PATH = "/assets/www/push/push.html"
         const val OPEN_WINDOW_PATH = "/assets/www/worker/open_window.html"
         const val OPEN_WINDOW_TARGET_PATH = "/assets/www/worker/open_window_target.html"
     }
 
     @get:Rule val sessionRule = GeckoSessionTestRule()
--- 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
@@ -1,32 +1,108 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*-
  * Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 package org.mozilla.geckoview.test
 
+import android.graphics.*
+import android.graphics.Bitmap
 import android.support.test.filters.MediumTest
 import android.support.test.runner.AndroidJUnit4
+import android.util.Base64
+import java.io.ByteArrayOutputStream
 import org.hamcrest.Matchers.*
 import org.junit.Assert.fail
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mozilla.geckoview.GeckoResult
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.WithDisplay
+import org.hamcrest.Matchers.equalTo
 
-private const val SCREEN_HEIGHT = 100
-private const val SCREEN_WIDTH = 200
+private const val SCREEN_WIDTH = 100
+private const val SCREEN_HEIGHT = 200
 
 @RunWith(AndroidJUnit4::class)
 @MediumTest
 class DynamicToolbarTest : BaseSessionTest() {
     @WithDisplay(height = SCREEN_HEIGHT, width = SCREEN_WIDTH)
     @Test
     fun outOfRangeValue() {
         try {
             sessionRule.display?.run { setDynamicToolbarMaxHeight(SCREEN_HEIGHT + 1) }
             fail("Request should have failed")
         } catch (e: AssertionError) {
             assertThat("Throws an exception when setting values greater than the client height",
                        e.toString(), containsString("maximum height of the dynamic toolbar"))
         }
     }
+
+    private fun assertScreenshotResult(result: GeckoResult<Bitmap>, comparisonImage: Bitmap) {
+        sessionRule.waitForResult(result).let {
+            assertThat("Screenshot is not null",
+                    it, notNullValue())
+            assertThat("Widths are the same", comparisonImage.width, equalTo(it.width))
+            assertThat("Heights are the same", comparisonImage.height, equalTo(it.height))
+            assertThat("Byte counts are the same", comparisonImage.byteCount, equalTo(it.byteCount))
+            assertThat("Configs are the same", comparisonImage.config, equalTo(it.config))
+
+            if (!comparisonImage.sameAs(it)) {
+              val outputForComparison = ByteArrayOutputStream()
+              comparisonImage.compress(Bitmap.CompressFormat.PNG, 100, outputForComparison)
+
+              val outputForActual = ByteArrayOutputStream()
+              it.compress(Bitmap.CompressFormat.PNG, 100, outputForActual)
+              val actualString: String = Base64.encodeToString(outputForActual.toByteArray(), Base64.DEFAULT)
+              val comparisonString: String = Base64.encodeToString(outputForComparison.toByteArray(), Base64.DEFAULT)
+
+              assertThat("Encoded strings are the same", comparisonString, equalTo(actualString))
+            }
+
+            assertThat("Bytes are the same", comparisonImage.sameAs(it), equalTo(true))
+        }
+    }
+
+    /**
+     * Returns a whole green Bitmap.
+     * This Bitmap would be a reference image of tests in this file.
+     */
+    private fun getComparisonScreenshot(width: Int, height: Int): Bitmap {
+        val screenshotFile = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888)
+        val canvas = Canvas(screenshotFile)
+        val paint = Paint()
+        paint.color = Color.rgb(0, 128, 0)
+        canvas.drawRect(0f, 0f, width.toFloat(), height.toFloat(), paint)
+        return screenshotFile
+    }
+
+    // With the dynamic toolbar max height vh units values exceed
+    // the top most window height. This is a test case that exceeded area
+    // is rendered properly (on the compositor).
+    @WithDisplay(height = SCREEN_HEIGHT, width = SCREEN_WIDTH)
+    @Test
+    fun positionFixedElementClipping() {
+        sessionRule.display?.run { setDynamicToolbarMaxHeight(SCREEN_HEIGHT / 2) }
+
+        val reference = getComparisonScreenshot(SCREEN_WIDTH, SCREEN_HEIGHT)
+
+        // FIXED_VH is an HTML file which has a position:fixed element whose
+        // style is "width: 100%; height: 200vh" and the document is scaled by
+        // minimum-scale 0.5, so that the height of the element exceeds the
+        // window height.
+        mainSession.loadTestPath(BaseSessionTest.FIXED_VH)
+        mainSession.waitForPageStop()
+
+        // Scroll down bit, if we correctly render the document, the position
+        // fixed element still covers whole the document area.
+        mainSession.evaluateJS("window.scrollTo({ top: 100, behevior: 'instant' })")
+
+        // Wait a while to make sure the scrolling result is composited on the compositor
+        // since capturePixels() takes a snapshot directly from the compositor without
+        // 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)
+        }
+    }
 }