Backed out 2 changesets (bug 1516722) for wpt failures on scroll-restoration-fragment-scrolling-samedoc.html. a=backout
authorCsoregi Natalia <ncsoregi@mozilla.com>
Tue, 30 Apr 2019 13:15:29 +0300
changeset 530672 e8766f96041a7f5a56aecf2a9dc94787a9e297ce
parent 530671 20d44dfc1b05a7d4715fedbd786d5946691c0630
child 530673 2258dc01bcd175ce0d2f31ae69a88fadfbecdb12
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1516722
milestone68.0a1
backs out8f2db95f0610ed8c51419bb94ed97ab25a2e3c8c
55c8e6f3e522adf4db6a3a6f9cec450ab8ff8926
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
Backed out 2 changesets (bug 1516722) for wpt failures on scroll-restoration-fragment-scrolling-samedoc.html. a=backout Backed out changeset 8f2db95f0610 (bug 1516722) Backed out changeset 55c8e6f3e522 (bug 1516722)
gfx/layers/apz/test/mochitest/apz_test_utils.js
gfx/layers/apz/test/mochitest/helper_fixed_pos_displayport.html
gfx/layers/apz/test/mochitest/helper_scroll_into_view_bug1516056.html
layout/base/PresShell.cpp
--- a/gfx/layers/apz/test/mochitest/apz_test_utils.js
+++ b/gfx/layers/apz/test/mochitest/apz_test_utils.js
@@ -14,18 +14,16 @@
 function convertEntries(entries) {
   var result = {};
   for (var i = 0; i < entries.length; ++i) {
     result[entries[i].key] = entries[i].value;
   }
   return result;
 }
 
-// TODO: Clean up these rect-handling functions so that e.g. a rect returned
-//       by Element.getBoundingClientRect() Just Works with them.
 function parseRect(str) {
   var pieces = str.replace(/[()\s]+/g, "").split(",");
   SimpleTest.is(pieces.length, 4, "expected string of form (x,y,w,h)");
   return { x: parseInt(pieces[0]),
            y: parseInt(pieces[1]),
            w: parseInt(pieces[2]),
            h: parseInt(pieces[3]) };
 }
@@ -36,21 +34,16 @@ function rectContains(haystack, needle) 
   return haystack.x <= needle.x
       && haystack.y <= needle.y
       && (haystack.x + haystack.w) >= (needle.x + needle.w)
       && (haystack.y + haystack.h) >= (needle.y + needle.h);
 }
 function rectToString(rect) {
   return "(" + rect.x + "," + rect.y + "," + rect.w + "," + rect.h + ")";
 }
-function assertRectContainment(haystackRect, haystackDesc, needleRect, needleDesc) {
-  SimpleTest.ok(rectContains(haystackRect, needleRect),
-                haystackDesc + " " + rectToString(haystackRect) + " should contain " +
-                needleDesc + " " + rectToString(needleRect));
-}
 
 function getPropertyAsRect(scrollFrames, scrollId, prop) {
   SimpleTest.ok(scrollId in scrollFrames,
                 "expected scroll frame data for scroll id " + scrollId);
   var scrollFrameData = scrollFrames[scrollId];
   SimpleTest.ok("displayport" in scrollFrameData,
                 "expected a " + prop + " for scroll id " + scrollId);
   var value = scrollFrameData[prop];
--- a/gfx/layers/apz/test/mochitest/helper_fixed_pos_displayport.html
+++ b/gfx/layers/apz/test/mochitest/helper_fixed_pos_displayport.html
@@ -68,18 +68,19 @@
       // relative to the layout viewport (but not relative to the page), since
       // fixed-position elements are attached to the layout viewport.
       // This is accomplished by checking the fixed-pos display port contains
       // the visual viewport rect as expressed relative to the layout viewport.
       let vvRect = { x: vv.offsetLeft,  // offsets relative to layout viewport
                      y: vv.offsetTop,
                      w: vv.width,
                      h: vv.height };
-      assertRectContainment(fixedPosDisplayport, "fixed-pos displayport",
-                            vvRect, "visual viewport");
+      ok(rectContains(fixedPosDisplayport, vvRect),
+         "fixed-pos displayport " + rectToString(fixedPosDisplayport) +
+         " should contain visual viewport " + rectToString(vvRect));
     }
 
     function* test(testDriver) {
       // First, check size and position on page load.
       checkFixedPosDisplayport();
 
       // Scroll the visual viewport within the layout viewport, without
       // scrolling the layout viewport itself, and check the size and
--- a/gfx/layers/apz/test/mochitest/helper_scroll_into_view_bug1516056.html
+++ b/gfx/layers/apz/test/mochitest/helper_scroll_into_view_bug1516056.html
@@ -14,54 +14,31 @@
       margin-right: 50%;
       background: cyan;
     }
   </style>
 </head>
 <body>
   <div id="target"></div>
   <script>
-    let vv = window.visualViewport;
     function getVisualScrollRange() {
       let rootScroller = document.scrollingElement;
+      let vv = window.visualViewport;
       return {
         width: rootScroller.scrollWidth - vv.width,
         height: rootScroller.scrollHeight - vv.height,
       };
     }
-    function getVisualViewportRect() {
-      return {
-        x: vv.pageLeft,
-        y: vv.pageTop,
-        w: vv.width,
-        h: vv.height,
-      };
-    }
     function* test(testDriver) {
       SimpleTest.is(window.scrollMaxX, 0, "page should have a zero horizontal layout scroll range");
       SimpleTest.is(window.scrollMaxY, 0, "page should have a zero vertical layout scroll range");
       let visualScrollRange = getVisualScrollRange();
       SimpleTest.ok(visualScrollRange.width > 0, "page should have a nonzero horizontal visual scroll range");
       SimpleTest.ok(visualScrollRange.height > 0, "page should have a nonzero vertical visual scroll range");
       let target = document.getElementById("target");
-
-      // Scroll target element into view. Wait until any visual scrolling is done before doing checks.
-      vv.addEventListener("scroll", testDriver, { once: true });
       target.scrollIntoView();
-      yield; // wait for visual viewport "scroll" event
-      yield waitForApzFlushedRepaints(testDriver);
-
-      // Test that scrollIntoView() respected the layout scroll range.
       SimpleTest.is(window.scrollX, 0, "page should not layout-scroll with a zero layout scroll range");
       SimpleTest.is(window.scrollY, 0, "page should not layout-scroll with a zero layout scroll range");
-
-      // Test that scrollIntoView() did perform visual scrolling.
-      let vvRect = getVisualViewportRect();
-      let targetBounds = target.getBoundingClientRect();
-      // set property names expected by rectContains()
-      targetBounds.w = targetBounds.width;
-      targetBounds.h = targetBounds.height;
-      assertRectContainment(vvRect, "visual viewport", targetBounds, "target element bounding rect");
     }
     waitUntilApzStable().then(runContinuation(test)).then(subtestDone);
   </script>
 </body>
 </html>
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -3308,18 +3308,17 @@ static nscoord ComputeWhereToScroll(Wher
  * This needs to work even if aRect has a width or height of zero.
  *
  * Note that, since we are performing a layout scroll, it's possible that
  * this fnction will sometimes be unsuccessful; the content will move as
  * fast as it can on the screen using layout viewport scrolling, and then
  * stop there, even if it could get closer to the desired position by
  * moving the visual viewport within the layout viewport.
  */
-static void ScrollToShowRect(nsIPresShell* aPresShell,
-                             nsIScrollableFrame* aFrameAsScrollable,
+static void ScrollToShowRect(nsIScrollableFrame* aFrameAsScrollable,
                              const nsRect& aRect,
                              nsIPresShell::ScrollAxis aVertical,
                              nsIPresShell::ScrollAxis aHorizontal,
                              ScrollFlags aScrollFlags) {
   nsPoint scrollPt = aFrameAsScrollable->GetVisualViewportOffset();
   nsRect visibleRect(scrollPt, aFrameAsScrollable->GetVisualViewportSize());
 
   nsSize lineSize;
@@ -3381,25 +3380,16 @@ static void ScrollToShowRect(nsIPresShel
                          autoBehaviorIsSmooth);
     if (gfxPrefs::ScrollBehaviorEnabled() && smoothScroll) {
       scrollMode = ScrollMode::SmoothMsd;
     }
     aFrameAsScrollable->ScrollTo(scrollPt, scrollMode, &allowedRange,
                                  aScrollFlags & ScrollFlags::ScrollSnap
                                      ? nsIScrollbarMediator::ENABLE_SNAP
                                      : nsIScrollbarMediator::DISABLE_SNAP);
-    // If this is the RCD-RSF, also call ScrollToVisual() since we want to
-    // scroll the rect into view visually, and that may require scrolling
-    // the visual viewport in scenarios where there is not enough layout
-    // scroll range.
-    if (aFrameAsScrollable->IsRootScrollFrameOfDocument() &&
-        aPresShell->GetPresContext()->IsRootContentDocument()) {
-      aPresShell->ScrollToVisual(scrollPt, FrameMetrics::eMainThread,
-                                 scrollMode);
-    }
   }
 }
 
 nsresult PresShell::ScrollContentIntoView(nsIContent* aContent,
                                           nsIPresShell::ScrollAxis aVertical,
                                           nsIPresShell::ScrollAxis aHorizontal,
                                           ScrollFlags aScrollFlags) {
   NS_ENSURE_TRUE(aContent, NS_ERROR_NULL_POINTER);
@@ -3555,18 +3545,17 @@ bool nsIPresShell::ScrollFrameRectIntoVi
 
       targetRect -= sf->GetScrolledFrame()->GetPosition();
       if (!(aScrollFlags & ScrollFlags::IgnoreMarginAndPadding)) {
         nsMargin scrollPadding = sf->GetScrollPadding();
         targetRect.Inflate(scrollPadding);
         targetRect = targetRect.Intersect(sf->GetScrolledRect());
       }
 
-      ScrollToShowRect(this, sf, targetRect, aVertical, aHorizontal,
-                       aScrollFlags);
+      ScrollToShowRect(sf, targetRect, aVertical, aHorizontal, aScrollFlags);
 
       nsPoint newPosition = sf->LastScrollDestination();
       // If the scroll position increased, that means our content moved up,
       // so our rect's offset should decrease
       rect += oldPosition - newPosition;
 
       if (oldPosition != newPosition) {
         didScroll = true;