[Autofill Assistant] Fix tap operation and add browser tests

Bug: 806868
Change-Id: I9730373e60f28b65fb515f9de42da0f6505db594
Reviewed-on: https://chromium-review.googlesource.com/c/1324239Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Commit-Queue: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606824}

components/autofill_assistant/browser/web_controller.cc

@@ -175,36 +175,39 @@ void WebController::OnFindElementForClickOrTap(
     base::OnceCallback<void(bool)> callback,
     bool is_a_click,
     std::unique_ptr<FindElementResult> result) {
-  if (result->object_id.empty()) {
+  // Found element must belong to a frame.
+  if (!result->container_frame_host || result->object_id.empty()) {
     DLOG(ERROR) << "Failed to find the element to click or tap.";
     OnResult(false, std::move(callback));
     return;
   }
 
-  ClickOrTapObject(result->object_id, is_a_click, std::move(callback));
+  ClickOrTapElement(std::move(result), is_a_click, std::move(callback));
 }
 
-void WebController::ClickOrTapObject(const std::string& object_id,
-                                     bool is_a_click,
-                                     base::OnceCallback<void(bool)> callback) {
+void WebController::ClickOrTapElement(
+    std::unique_ptr<FindElementResult> target_element,
+    bool is_a_click,
+    base::OnceCallback<void(bool)> callback) {
+  std::string element_object_id = target_element->object_id;
   std::vector<std::unique_ptr<runtime::CallArgument>> argument;
   argument.emplace_back(
-      runtime::CallArgument::Builder().SetObjectId(object_id).Build());
+      runtime::CallArgument::Builder().SetObjectId(element_object_id).Build());
   devtools_client_->GetRuntime()->CallFunctionOn(
       runtime::CallFunctionOnParams::Builder()
-          .SetObjectId(object_id)
+          .SetObjectId(element_object_id)
           .SetArguments(std::move(argument))
           .SetFunctionDeclaration(std::string(kScrollIntoViewScript))
           .SetReturnByValue(true)
           .Build(),
       base::BindOnce(&WebController::OnScrollIntoView,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback),
-                     object_id, is_a_click));
+                     weak_ptr_factory_.GetWeakPtr(), std::move(target_element),
+                     std::move(callback), is_a_click));
 }
 
 void WebController::OnScrollIntoView(
+    std::unique_ptr<FindElementResult> target_element,
     base::OnceCallback<void(bool)> callback,
-    std::string object_id,
     bool is_a_click,
     std::unique_ptr<runtime::CallFunctionOnResult> result) {
   if (!result || result->HasExceptionDetails()) {
@@ -213,6 +216,28 @@ void WebController::OnScrollIntoView(
     return;
   }
 
+  // Wait for a roundtrips through the renderer and compositor pipeline,
+  // otherwise touch event may be dropped because of missing handler.
+  // Note that mouse left button will always be send to the renderer, but it is
+  // slightly better to wait for the changes, like scroll, to be visualized in
+  // compositor as real interaction.
+  target_element->container_frame_host->InsertVisualStateCallback(
+      base::BindOnce(&WebController::OnVisualStateCallback,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback),
+                     target_element->object_id, is_a_click));
+}
+
+void WebController::OnVisualStateCallback(
+    base::OnceCallback<void(bool)> callback,
+    std::string object_id,
+    bool is_a_click,
+    bool state) {
+  if (!state) {
+    DLOG(ERROR) << "Wait for visual state failed unexpectedly.";
+    OnResult(false, std::move(callback));
+    return;
+  }
+
   devtools_client_->GetDOM()->GetBoxModel(
       dom::GetBoxModelParams::Builder().SetObjectId(object_id).Build(),
       base::BindOnce(&WebController::OnGetBoxModelForClickOrTap,

components/autofill_assistant/browser/web_controller.h

@@ -72,8 +72,6 @@ class WebController {
   // Perform a touch tap on the element given by |selectors| and return the
   // result through callback. CSS selectors in |selectors| are ordered from top
   // frame to the frame contains the element and the element.
-  //
-  // TODO(crbug.com/806868): Add WebControllerBrowserTest for this interface.
   virtual void TapElement(const std::vector<std::string>& selectors,
                           base::OnceCallback<void(bool)> callback);
 
@@ -202,13 +200,17 @@ class WebController {
                                   std::unique_ptr<FindElementResult> result);
   void OnFindElementForTap(base::OnceCallback<void(bool)> callback,
                            std::unique_ptr<FindElementResult> result);
-  void ClickOrTapObject(const std::string& object_id,
-                        bool is_a_click,
-                        base::OnceCallback<void(bool)> callback);
-  void OnScrollIntoView(base::OnceCallback<void(bool)> callback,
-                        std::string object_id,
+  void ClickOrTapElement(std::unique_ptr<FindElementResult> target_element,
+                         bool is_a_click,
+                         base::OnceCallback<void(bool)> callback);
+  void OnScrollIntoView(std::unique_ptr<FindElementResult> target_element,
+                        base::OnceCallback<void(bool)> callback,
                         bool is_a_click,
                         std::unique_ptr<runtime::CallFunctionOnResult> result);
+  void OnVisualStateCallback(base::OnceCallback<void(bool)> callback,
+                             std::string object_id,
+                             bool is_a_click,
+                             bool state);
   void OnGetBoxModelForClickOrTap(
       base::OnceCallback<void(bool)> callback,
       bool is_a_click,

components/autofill_assistant/browser/web_controller_browsertest.cc

@@ -76,6 +76,20 @@ class WebControllerBrowserTest : public content::ContentBrowserTest {
     done_callback.Run();
   }
 
+  void TapElement(const std::vector<std::string>& selectors) {
+    base::RunLoop run_loop;
+    web_controller_->TapElement(
+        selectors,
+        base::BindOnce(&WebControllerBrowserTest::ClickElementCallback,
+                       base::Unretained(this), run_loop.QuitClosure()));
+    run_loop.Run();
+  }
+
+  void TapElementCallback(const base::Closure& done_callback, bool result) {
+    ASSERT_TRUE(result);
+    done_callback.Run();
+  }
+
   void WaitForElementRemove(const std::vector<std::string>& selectors) {
     base::RunLoop run_loop;
     web_controller_->ElementCheck(
@@ -388,6 +402,23 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest,
   WaitForElementRemove(selectors);
 }
 
+IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, TapElement) {
+  std::vector<std::string> selectors;
+  selectors.emplace_back("#touch_area");
+  TapElement(selectors);
+
+  WaitForElementRemove(selectors);
+}
+
+IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, TapElementInIFrame) {
+  std::vector<std::string> selectors;
+  selectors.emplace_back("#iframe");
+  selectors.emplace_back("#touch_area");
+  TapElement(selectors);
+
+  WaitForElementRemove(selectors);
+}
+
 IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, FindElement) {
   std::vector<std::string> selectors;
   selectors.emplace_back("#button");

components/test/data/autofill_assistant/autofill_assistant_target_website.html

@@ -16,6 +16,11 @@ found in the LICENSE file.
         var button = document.getElementById("button");
         button.parentNode.removeChild(button);
       }
+
+      var removeTouchArea = function() {
+        var touch_area = document.getElementById("touch_area");
+        touch_area.parentNode.removeChild(touch_area);
+      }
     </script>
 
     <style>
@@ -41,6 +46,11 @@ found in the LICENSE file.
       <br>
     </div>
 
+    <div>
+      <p id="touch_area" ontouchend="removeTouchArea()">Touchable Area</p>
+      <br>
+    </div>
+
     <div>
       <select id="select">
         <option value="one">One</option>

components/test/data/autofill_assistant/autofill_assistant_target_website_iframe_one.html

@@ -24,6 +24,11 @@ found in the LICENSE file.
         var button = document.getElementById("button");
         button.parentNode.removeChild(button);
       }
+
+      var removeTouchArea = function() {
+        var touch_area = document.getElementById("touch_area");
+        touch_area.parentNode.removeChild(touch_area);
+      }
     </script>
 
     <style>
@@ -62,6 +67,11 @@ found in the LICENSE file.
       <button id="button" type="button">Test Button</button><br>
     </div>
 
+    <div>
+      <p id="touch_area" ontouchend="removeTouchArea()">Touchable Area</p>
+      <br>
+    </div>
+
     <hr>
     <div id="shadowsection"></div>