[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.

This is the M-76 merge for http://crrev/c/1636138.

(cherry picked from commit b7517fe4d094c623e4c152ac4df1b24fde087f8c)

Bug: 973031
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-Original-Commit-Position: refs/heads/master@{#665523}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1657888
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#279}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}

diff --git a/components/autofill_assistant/browser/web_controller.cc b/components/autofill_assistant/browser/web_controller.cc
index ac0356b8..03aca28 100644
--- a/components/autofill_assistant/browser/web_controller.cc
+++ b/components/autofill_assistant/browser/web_controller.cc

@@ -53,13 +53,13 @@
 
 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 =

diff --git a/components/autofill_assistant/browser/web_controller_browsertest.cc b/components/autofill_assistant/browser/web_controller_browsertest.cc
index 3c36d69..4253e4e 100644
--- a/components/autofill_assistant/browser/web_controller_browsertest.cc
+++ b/components/autofill_assistant/browser/web_controller_browsertest.cc

@@ -412,6 +412,44 @@
     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 @@
   selector.selectors.emplace_back("#iframe");
   selector.selectors.emplace_back("#focus");
 
-  // The element is not visible initially.
-  const std::string checkNotVisibleScript = 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;
-  )";
-  EXPECT_EQ(true, content::EvalJs(shell(), checkNotVisibleScript));
-  FocusElement(selector);
-
-  // Verify that the scroll moved the div in the iframe into view.
   const std::string checkVisibleScript = R"(
-    const scrollTimeoutMs = 500;
-    var timer = null;
-
-    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);
-    }
+      iframeRect.y + divRect.y < window.innerHeight;
   )";
-  EXPECT_EQ(true, content::EvalJsWithManualReply(shell(), checkVisibleScript));
+  EXPECT_EQ(false, content::EvalJs(shell(), checkVisibleScript));
+  FocusElement(selector);
+  EXPECT_EQ(true, content::EvalJs(shell(), checkVisibleScript));
+}
+
+IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest,
+                       FocusElementWithScrollIntoViewNeeded) {
+  TestScrollIntoView(/* initial_window_scroll_y= */ 0,
+                     /* initial_container_scroll_y=*/0);
+}
+
+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) {

diff --git a/components/test/data/autofill_assistant/autofill_assistant_target_website.html b/components/test/data/autofill_assistant/autofill_assistant_target_website.html
index 5046b319..ee4ca7d 100644
--- a/components/test/data/autofill_assistant/autofill_assistant_target_website.html
+++ b/components/test/data/autofill_assistant/autofill_assistant_target_website.html

@@ -278,5 +278,17 @@
            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>