[Autofill Assistant] Fix scrolling for elements within scrollable

containers.

Before this change, when scrolling elements within scrollable container,
the element would be first taken to the desired position within the
screen, *then* scrolled within its container, which could result in the
element not even appearing within the window.

With this change, the element is scrolled into view within its container
before choosing how much to scroll the window vertically. At the end of
the scroll, the top of the element is always at the desired position
within its frame.

This change also switches the browsertest for scrolling to doing
synchronous checks, as we don't use smooth scrolling anymore.

Change-Id: I5bd090988ec8ca5441d15783d2b46ceb3982e57d
Bug: b/129046627
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1636138
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665523}

components/autofill_assistant/browser/web_controller.cc

@@ -53,13 +53,13 @@ const char* const kGetBoundingClientRectAsList =
 
 const char* const kScrollIntoViewScript =
     R"(function(node) {
+    node.scrollIntoViewIfNeeded();
     const rect = node.getBoundingClientRect();
     if (rect.height < window.innerHeight) {
       window.scrollBy({top: rect.top - window.innerHeight * 0.25});
     } else {
       window.scrollBy({top: rect.top});
     }
-    node.scrollIntoViewIfNeeded();
   })";
 
 const char* const kScrollIntoViewIfNeededScript =

components/autofill_assistant/browser/web_controller_browsertest.cc

@@ -412,6 +412,44 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
     std::move(done_callback).Run();
   }
 
+  // Scroll an element into view that's within a container element. This
+  // requires scrolling the container, then the window, to get the element to
+  // the desired y position.
+  void TestScrollIntoView(int initial_window_scroll_y,
+                          int initial_container_scroll_y) {
+    EXPECT_TRUE(content::ExecJs(
+        shell(), base::StringPrintf(
+                     R"(window.scrollTo(0, %d);
+           let container = document.querySelector("#scroll_container");
+           container.scrollTo(0, %d);)",
+                     initial_window_scroll_y, initial_container_scroll_y)));
+
+    Selector selector;
+    selector.selectors.emplace_back("#scroll_item_5");
+
+    FocusElement(selector);
+    base::ListValue eval_result = content::EvalJs(shell(), R"(
+      let item = document.querySelector("#scroll_item_5");
+      let itemRect = item.getBoundingClientRect();
+      let container = document.querySelector("#scroll_container");
+      let containerRect = container.getBoundingClientRect();
+      [itemRect.top, itemRect.bottom, window.innerHeight,
+           containerRect.top, containerRect.bottom])")
+                                      .ExtractList();
+    double top = eval_result.GetList()[0].GetDouble();
+    double bottom = eval_result.GetList()[1].GetDouble();
+    double window_height = eval_result.GetList()[2].GetDouble();
+    double container_top = eval_result.GetList()[3].GetDouble();
+    double container_bottom = eval_result.GetList()[4].GetDouble();
+
+    // Element is at the desired position. (top is relative to the viewport)
+    EXPECT_NEAR(top, window_height * 0.25, 0.5);
+
+    // Element is within the visible portion of its container.
+    EXPECT_GT(bottom, container_top);
+    EXPECT_LT(top, container_bottom);
+  }
+
  protected:
   std::unique_ptr<WebController> web_controller_;
 
@@ -814,47 +852,28 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, FocusElement) {
   selector.selectors.emplace_back("#iframe");
   selector.selectors.emplace_back("#focus");
 
-  // The element is not visible initially.
-  const std::string checkNotVisibleScript = R"(
+  const std::string checkVisibleScript = R"(
       let iframe = document.querySelector("#iframe");
       let div = iframe.contentDocument.querySelector("#focus");
       let iframeRect = iframe.getBoundingClientRect();
       let divRect = div.getBoundingClientRect();
-      iframeRect.y + divRect.y > window.innerHeight;
+      iframeRect.y + divRect.y < window.innerHeight;
   )";
-  EXPECT_EQ(true, content::EvalJs(shell(), checkNotVisibleScript));
+  EXPECT_EQ(false, content::EvalJs(shell(), checkVisibleScript));
   FocusElement(selector);
+  EXPECT_EQ(true, content::EvalJs(shell(), checkVisibleScript));
+}
 
-  // Verify that the scroll moved the div in the iframe into view.
-  const std::string checkVisibleScript = R"(
-    const scrollTimeoutMs = 500;
-    var timer = null;
+IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest,
+                       FocusElementWithScrollIntoViewNeeded) {
+  TestScrollIntoView(/* initial_window_scroll_y= */ 0,
+                     /* initial_container_scroll_y=*/0);
+}
 
-    function check() {
-      let iframe = document.querySelector("#iframe");
-      let div = iframe.contentDocument.querySelector("#focus");
-      let iframeRect = iframe.getBoundingClientRect();
-      let divRect = div.getBoundingClientRect();
-      return iframeRect.y + divRect.y < window.innerHeight;
-    }
-    function onScrollDone() {
-      window.removeEventListener("scroll", onScroll);
-      domAutomationController.send(check());
-    }
-    function onScroll(e) {
-      if (timer != null) {
-        clearTimeout(timer);
-      }
-      timer = setTimeout(onScrollDone, scrollTimeoutMs);
-    }
-    if (check()) {
-      // Scrolling finished before this script started. Just return the result.
-      domAutomationController.send(true);
-    } else {
-      window.addEventListener("scroll", onScroll);
-    }
-  )";
-  EXPECT_EQ(true, content::EvalJsWithManualReply(shell(), checkVisibleScript));
+IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest,
+                       FocusElementWithScrollIntoViewNotNeeded) {
+  TestScrollIntoView(/* initial_window_scroll_y= */ 0,
+                     /* initial_container_scroll_y=*/200);
 }
 
 IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, SelectOption) {

components/test/data/autofill_assistant/autofill_assistant_target_website.html

@@ -278,5 +278,17 @@ found in the LICENSE file.
            flipPosition(this, point_b, point_a);">Moving Button</button>
       <br>
     </div>
+
+    <div style="height:500px"/>
+    <div id="scroll_container" style="width:100px;height:100px;overflow:auto;">
+      <div id="scroll_item_1" style="width:50px;height:50px">1</div>
+      <div id="scroll_item_2" style="width:50px;height:50px">2</div>
+      <!-- elements below this point cannot be seen except by scrolling
+           within the container. -->
+      <div id="scroll_item_3" style="width:50px;height:50px">3</div>
+      <div id="scroll_item_4" style="width:50px;height:50px">4</div>
+      <div id="scroll_item_5" style="width:50px;height:50px">5</div>
+    </div>
+    <div style="height:500px"/>
   </body>
 </html>