Fix scroll timeout by waiting for the scroll position to settle

src/Input/Mouse.php

@@ -141,6 +141,7 @@ public function click(?array $options = null)
      *
      * @throws \HeadlessChromium\Exception\CommunicationException
      * @throws \HeadlessChromium\Exception\NoResponseAvailable
+     * @throws \HeadlessChromium\Exception\OperationTimedOut
      *
      * @return $this
      */
@@ -156,6 +157,7 @@ public function scrollUp(int $distance)
      *
      * @throws \HeadlessChromium\Exception\CommunicationException
      * @throws \HeadlessChromium\Exception\NoResponseAvailable
+     * @throws \HeadlessChromium\Exception\OperationTimedOut
      *
      * @return $this
      */
@@ -180,38 +182,79 @@ private function scroll(int $distanceY, int $distanceX = 0): self
     {
         $this->page->assertNotClosed();
 
+        // make sure the mouse is on the screen
+        $this->move($this->x, $this->y);
+
+        $distances = $this->getScrollDistances($distanceY, $distanceX); // Calculated distances to scroll.
+        $startPos = $this->getCurrentScrollPosition(); // Remember positions before scrolling started.
+
+        if ($distances['x'] === 0 && $distances['y'] === 0) {
+            return $this;
+        }
+
+        $this->sendScrollMessage($distances);
+
+        // Wait until the scroll position settles (i.e. it stops changing).
+        Utils::tryWithTimeout(
+            10_000_000,
+            $this->waitForScrollToSettle($distanceY, $distanceX, $startPos['x'], $startPos['y'])
+        );
+
+        // set new position after move
+        $endPos = $this->getCurrentScrollPosition();
+        $this->x += $endPos['x'] - $startPos['x'];
+        $this->y += $endPos['y'] - $startPos['y'];
+
+        return $this;
+    }
+
+    /**
+     * Get the distances to actually scroll on each axis, clamped to the page's
+     * current scroll boundaries.
+     *
+     * @throws \HeadlessChromium\Exception\OperationTimedOut
+     * @throws \HeadlessChromium\Exception\CommunicationException
+     * @throws \HeadlessChromium\Exception\CommunicationException\ResponseHasError
+     * @throws \HeadlessChromium\Exception\NoResponseAvailable
+     *
+     * @return array{ x: int, y: int }
+     */
+    private function getScrollDistances(int $distanceY, int $distanceX = 0): array
+    {
         $scrollableArea = $this->page->getLayoutMetrics()->getCssContentSize();
         $visibleArea = $this->page->getLayoutMetrics()->getCssVisualViewport();
 
         $maximumX = $scrollableArea['width'] - $visibleArea['clientWidth'];
         $maximumY = $scrollableArea['height'] - $visibleArea['clientHeight'];
 
-        $distanceX = $this->getMaximumDistance($distanceX, $visibleArea['pageX'], $maximumX);
-        $distanceY = $this->getMaximumDistance($distanceY, $visibleArea['pageY'], $maximumY);
+        $distanceX = $this->getMaximumDistance($distanceX, (int) $visibleArea['pageX'], (int) $maximumX);
+        $distanceY = $this->getMaximumDistance($distanceY, (int) $visibleArea['pageY'], (int) $maximumY);
 
-        $targetX = $visibleArea['pageX'] + $distanceX;
-        $targetY = $visibleArea['pageY'] + $distanceY;
+        return ['x' => $distanceX, 'y' => $distanceY];
+    }
 
-        // make sure the mouse is on the screen
-        $this->move($this->x, $this->y);
+    private function getCurrentScrollPosition(): array
+    {
+        $viewport = $this->page->getLayoutMetrics()->getCssVisualViewport();
 
-        // scroll
+        return ['x' => (int) $viewport['pageX'], 'y' => (int) $viewport['pageY']];
+    }
+
+    /**
+     * @param array{ x: int, y: int } $distances
+     *
+     * @throws \HeadlessChromium\Exception\CommunicationException
+     * @throws \HeadlessChromium\Exception\NoResponseAvailable
+     */
+    private function sendScrollMessage(array $distances): void
+    {
         $this->page->getSession()->sendMessageSync(new Message('Input.dispatchMouseEvent', [
             'type' => 'mouseWheel',
             'x' => $this->x,
             'y' => $this->y,
-            'deltaX' => $distanceX,
-            'deltaY' => $distanceY,
+            'deltaX' => $distances['x'],
+            'deltaY' => $distances['y'],
         ]));
-
-        // wait until the scroll is done
-        Utils::tryWithTimeout(30000 * 1000, $this->waitForScroll($targetX, $targetY));
-
-        // set new position after move
-        $this->x += $distanceX;
-        $this->y += $distanceY;
-
-        return $this;
     }
 
     /**
@@ -351,32 +394,78 @@ private function getMaximumDistance(int $distance, int $current, int $maximum):
     }
 
     /**
-     * Wait for the browser to process the scroll command.
+     * Wait for the scroll position to settle (stop changing).
      *
-     * Return the number of microseconds to wait before trying again or true in case of success.
+     * Rather than waiting for an exact target position, this polls the live
+     * scroll position and returns once it has been stable for a few consecutive
+     * reads. A settled position is only accepted once we have either observed
+     * actual movement or reached a scroll boundary - this guards against
+     * accepting the start position before the scroll has even begun
+     * (the start-of-scroll race).
      *
-     * @see \HeadlessChromium\Utils::tryWithTimeout
+     * Yields the number of microseconds to wait before trying again or
+     * returns in case of success.
      *
-     * @param int $targetX
-     * @param int $targetY
+     * @see Utils::tryWithTimeout
      *
+     * @throws \HeadlessChromium\Exception\CommunicationException
+     * @throws \HeadlessChromium\Exception\CommunicationException\ResponseHasError
+     * @throws \HeadlessChromium\Exception\NoResponseAvailable
      * @throws \HeadlessChromium\Exception\OperationTimedOut
-     *
-     * @return bool|\Generator
      */
-    private function waitForScroll(int $targetX, int $targetY)
+    private function waitForScrollToSettle(int $distanceY, int $distanceX, int $startX, int $startY): \Generator
     {
-        while (true) {
-            $visibleArea = $this->page->getLayoutMetrics()->getCssVisualViewport();
+        $requiredStableReads = 5;
+        $stableReads = 0;
+        $lastX = $lastY = null;
+        $moved = false;
 
-            if ($visibleArea['pageX'] === $targetX && $visibleArea['pageY'] === $targetY) {
-                return true;
+        while (true) {
+            $pos = $this->getCurrentScrollPosition();
+            $moved = $moved || ($pos['x'] !== $startX || $pos['y'] !== $startY);
+
+            if ($pos['x'] === $lastX && $pos['y'] === $lastY) {
+                ++$stableReads;
+
+                if (
+                    $stableReads >= $requiredStableReads &&
+                    ($moved || $this->isAtScrollBoundary($distanceY, $distanceX, $pos['x'], $pos['y']))
+                ) {
+                    return;
+                }
+            } else {
+                $stableReads = 0;
+                $lastX = $pos['x'];
+                $lastY = $pos['y'];
             }
 
-            yield 1000;
+            yield 16_000; // Time between two frames at 60Hz.
         }
     }
 
+    /**
+     * Whether the page can no longer be scrolled further in the requested
+     * direction (so a non-moving position is a legitimate end state).
+     *
+     * @throws \HeadlessChromium\Exception\CommunicationException
+     * @throws \HeadlessChromium\Exception\CommunicationException\ResponseHasError
+     * @throws \HeadlessChromium\Exception\NoResponseAvailable
+     * @throws \HeadlessChromium\Exception\OperationTimedOut
+     */
+    private function isAtScrollBoundary(int $distanceY, int $distanceX, int $x, int $y): bool
+    {
+        $scrollableArea = $this->page->getLayoutMetrics()->getCssContentSize();
+        $visibleArea = $this->page->getLayoutMetrics()->getCssVisualViewport();
+
+        $maximumX = (int) ($scrollableArea['width'] - $visibleArea['clientWidth']);
+        $maximumY = (int) ($scrollableArea['height'] - $visibleArea['clientHeight']);
+
+        $atBoundaryX = $distanceX > 0 ? $x >= $maximumX : ($distanceX < 0 ? $x <= 0 : true);
+        $atBoundaryY = $distanceY > 0 ? $y >= $maximumY : ($distanceY < 0 ? $y <= 0 : true);
+
+        return $atBoundaryX && $atBoundaryY;
+    }
+
     /**
      * Get the current mouse position.
      *

tests/MouseApiTest.php

@@ -101,6 +101,80 @@ public function testScroll(): void
         self::assertLessThan(10000, $page->mouse()->getPosition()['y']);
     }
 
+    /**
+     * Scrolling works when scrollable area shrinks immediatly after scroll event.
+     *
+     * @throws \HeadlessChromium\Exception\CommunicationException
+     * @throws \HeadlessChromium\Exception\NoResponseAvailable
+     * @throws \HeadlessChromium\Exception\OperationTimedOut
+     */
+    public function testScrollDoesNotTimeOutWhenScrollableAreaShrinks(): void
+    {
+        $page = $this->openSitePage('scrollShrink.html');
+
+        $page->mouse()->scrollDown(4000); // Before patch this threw an OperationTimedOut Exception.
+
+        $windowScrollY = $page->evaluate('window.scrollY')->getReturnValue();
+        $maximumY = $page
+            ->evaluate('document.documentElement.scrollHeight - window.innerHeight')
+            ->getReturnValue();
+
+        // We asked to scroll further than the shrunken page allows, so we end up
+        // exactly at the new (smaller) maximum. The actual regression guarantee
+        // is that scrollDown() returns at all instead of timing out.
+        self::assertSame($maximumY, $windowScrollY);
+    }
+
+    /**
+     * Scrolling works when an overlay pops up and locks scrolling immediatly after scroll event.
+     *
+     * @throws \HeadlessChromium\Exception\CommunicationException
+     * @throws \HeadlessChromium\Exception\NoResponseAvailable
+     * @throws \HeadlessChromium\Exception\OperationTimedOut
+     */
+    public function testScrollDoesNotTimeOutWhenModalLocksScrolling(): void
+    {
+        $page = $this->openSitePage('scrollLock.html');
+
+        $page->mouse()->scrollDown(4000); // Before patch this threw an OperationTimedOut Exception.
+
+        $windowScrollY = $page->evaluate('window.scrollY')->getReturnValue();
+        $maximumY = $page
+            ->evaluate('document.documentElement.scrollHeight - window.innerHeight')
+            ->getReturnValue();
+
+        // Scrolling got locked, so the page can no longer be scrolled at all and
+        // we stay at the top. The guarantee the fix restores is that scrollDown()
+        // returns instead of timing out.
+        self::assertSame(0, $maximumY);
+        self::assertSame(0, $windowScrollY);
+    }
+
+    /**
+     * Scrolling works when the layout shifts (e.g. due to lazy loaded image) during scrolling.
+     *
+     * @throws \HeadlessChromium\Exception\CommunicationException
+     * @throws \HeadlessChromium\Exception\NoResponseAvailable
+     * @throws \HeadlessChromium\Exception\OperationTimedOut
+     */
+    public function testScrollDoesNotTimeOutWhenLayoutShiftsDuringScroll(): void
+    {
+        $page = $this->openSitePage('infiniteScroll.html');
+
+        $page->mouse()->scrollDown(4000); // Before patch this threw an OperationTimedOut Exception.
+
+        $windowScrollY = $page->evaluate('window.scrollY')->getReturnValue();
+        $maximumY = $page
+            ->evaluate('document.documentElement.scrollHeight - window.innerHeight')
+            ->getReturnValue();
+
+        // The page grew (it did not shrink), so we scrolled and ended up at a
+        // valid position within the new bounds rather than timing out. The exact
+        // position is timing-dependent.
+        self::assertGreaterThan(0, $windowScrollY);
+        self::assertLessThanOrEqual($maximumY, $windowScrollY);
+    }
+
     /**
      * @dataProvider providerFindElementWithSingleElement
      *

tests/resources/static-web/infiniteScroll.html

@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>infinite-scroll issue (lazy image above)</title>
+    <style>
+        html, body { margin: 0; padding: 0; font: 16px sans-serif; }
+        #header { height: 80px; background: #1565c0; color: #fff; padding: 8px; }
+        /* The lazy image has no reserved height until it loads (the common
+           cause of layout shift in real feeds). */
+        #lazyimg { display: block; width: 640px; max-width: 100%; margin: 12px; }
+        #feed { height: 12000px; background: linear-gradient(#cfe8ff, #084b97); }
+    </style>
+</head>
+<body>
+<div id="header">feed</div>
+<img id="lazyimg" alt="">
+<div id="feed">feed content</div>
+<script>
+    // On the scroll event we simulate the finished loading of a lazy-loaded image
+    // above the current position. Before patch, this caused an OperationTimedOut
+    // Exception, because it changed the expected scroll position.
+    var loaded = false;
+
+    window.addEventListener('scroll', function () {
+        if (loaded) {
+            return;
+        }
+        loaded = true;
+        // The image finishes loading: assigning the src makes it reflow to its
+        // intrinsic 640x420 size, pushing the feed below it down.
+        var svg = "<svg xmlns='http://www.w3.org/2000/svg' width='640' height='420'>"
+            + "<rect width='100%' height='100%' fill='rgb(120,144,156)'/>"
+            + "<text x='320' y='220' font-size='32' fill='white' text-anchor='middle'"
+            + " font-family='sans-serif'>lazy-loaded image</text></svg>";
+        document.getElementById('lazyimg').src = 'data:image/svg+xml,' + encodeURIComponent(svg);
+    }, { passive: true });
+</script>
+</body>
+</html>

tests/resources/static-web/scrollLock.html

@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>scroll-lock issue</title>
+    <style>
+        html, body { margin: 0; padding: 0; font: 16px sans-serif; }
+        #spacer { height: 5000px; background: linear-gradient(#cfe8ff, #084b97); }
+        #overlay {
+            display: none;
+            position: fixed;
+            top: 0; left: 0; right: 0; bottom: 0;
+            align-items: center;
+            justify-content: center;
+            background: rgba(0, 0, 0, .25);
+            backdrop-filter: blur(8px);
+            -webkit-backdrop-filter: blur(8px);
+        }
+        #dialog {
+            width: 75vw;
+            height: 75vh;
+            box-sizing: border-box;
+            display: flex;
+            flex-direction: column;
+            align-items: center;
+            justify-content: center;
+            gap: 12px;
+            padding: 40px;
+            background: #fff;
+            border-radius: 16px;
+            box-shadow: 0 20px 60px rgba(0, 0, 0, .45);
+            text-align: center;
+        }
+        #dialog h1 { margin: 0; font-size: 28px; color: #b71c1c; }
+        #dialog p { margin: 0; font-size: 18px; color: #333; }
+    </style>
+</head>
+<body>
+<div id="spacer">scroll me</div>
+<div id="overlay">
+    <div id="dialog">
+        <h1>A modal popped up</h1>
+        <p>…and locked scrolling (the page is now position:fixed)</p>
+    </div>
+</div>
+<script>
+    // On the scroll event we immediately show the modal overlay and set
+    // a scroll lock on the body. Before patch, this caused an OperationTimedOut
+    // Exception, because it changes the expected scroll position.
+    var locked = false;
+    window.addEventListener('scroll', function () {
+        if (locked) {
+            return;
+        }
+        locked = true;
+        document.body.style.position = 'fixed';
+        document.body.style.top = '0';
+        document.body.style.left = '0';
+        document.body.style.right = '0';
+        document.documentElement.style.overflow = 'hidden';
+        document.getElementById('overlay').style.display = 'flex';
+    }, { passive: true });
+</script>
+</body>
+</html>