[Autofill Assistant] Move |ClickOrTap| chain to util

This finally moves the action chain used for |ClickOrTap| (and as a
result also |SendKeyboardInput|) to the util.

The chain for |SetFieldValue| still remains, as extracting that one
requires a bit more work to be done.

Bug: b/158153191
Change-Id: I255ed476f1e55f4ac8dd5aececf49a9cf8143b0a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2276266
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#785705}

components/autofill_assistant/browser/BUILD.gn

@@ -249,6 +249,7 @@ source_set("unit_tests") {
   sources = [
     "actions/action_test_utils.cc",
     "actions/action_test_utils.h",
+    "actions/click_action_unittest.cc",
     "actions/collect_user_data_action_unittest.cc",
     "actions/configure_bottom_sheet_action_unittest.cc",
     "actions/fallback_handler/required_fields_fallback_handler_unittest.cc",

components/autofill_assistant/browser/actions/action_delegate.h

@@ -106,6 +106,16 @@ class ActionDelegate {
       ClickType click_type,
       base::OnceCallback<void(const ClientStatus&)> callback) = 0;
 
+  // Wait for the |element|'s document to become interactive.
+  virtual void WaitForDocumentToBecomeInteractive(
+      const ElementFinder::Result& element,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
+
+  // Scroll the |element| into view.
+  virtual void ScrollIntoView(
+      const ElementFinder::Result& element,
+      base::OnceCallback<void(const ClientStatus&)> callback) = 0;
+
   // Have the UI enter the prompt mode and make the given actions available.
   //
   // While a prompt is in progress, the UI looks the same as it does between

components/autofill_assistant/browser/actions/action_delegate_util.cc

@@ -22,6 +22,49 @@ void RetainElementAndExecuteCallback(
   std::move(callback).Run(status);
 }
 
+void OnScrollIntoViewForClickOrTap(
+    ActionDelegate* delegate,
+    ClickType click_type,
+    const ElementFinder::Result& element,
+    base::OnceCallback<void(const ClientStatus&)> callback,
+    const ClientStatus& scroll_status) {
+  if (!scroll_status.ok()) {
+    VLOG(1) << __func__ << " Failed to scroll to the element.";
+    std::move(callback).Run(scroll_status);
+    return;
+  }
+
+  delegate->ClickOrTapElement(element, click_type, std::move(callback));
+}
+
+void OnWaitForDocumentToBecomeInteractiveForClickOrTap(
+    ActionDelegate* delegate,
+    ClickType click_type,
+    const ElementFinder::Result& element,
+    base::OnceCallback<void(const ClientStatus&)> callback,
+    const ClientStatus& wait_status) {
+  if (!wait_status.ok()) {
+    VLOG(1) << __func__
+            << " Waiting for document to become interactive timed out.";
+    std::move(callback).Run(wait_status);
+    return;
+  }
+
+  delegate->ScrollIntoView(
+      element, base::BindOnce(&OnScrollIntoViewForClickOrTap, delegate,
+                              click_type, element, std::move(callback)));
+}
+
+void PerformClickOrTap(ActionDelegate* delegate,
+                       ClickType click_type,
+                       const ElementFinder::Result& element,
+                       base::OnceCallback<void(const ClientStatus&)> callback) {
+  delegate->WaitForDocumentToBecomeInteractive(
+      element,
+      base::BindOnce(&OnWaitForDocumentToBecomeInteractiveForClickOrTap,
+                     delegate, click_type, element, std::move(callback)));
+}
+
 void OnFindElementForClickOrTap(
     ActionDelegate* delegate,
     ClickType click_type,
@@ -34,10 +77,37 @@ void OnFindElementForClickOrTap(
     return;
   }
 
-  delegate->ClickOrTapElement(
-      *element, click_type,
-      base::BindOnce(&RetainElementAndExecuteCallback, std::move(element),
-                     std::move(callback)));
+  PerformClickOrTap(delegate, click_type, *element,
+                    base::BindOnce(&RetainElementAndExecuteCallback,
+                                   std::move(element), std::move(callback)));
+}
+
+void OnClickOrTapForSendKeyboardInput(
+    ActionDelegate* delegate,
+    const std::vector<UChar32> codepoints,
+    int delay_in_millis,
+    const ElementFinder::Result& element,
+    base::OnceCallback<void(const ClientStatus&)> callback,
+    const ClientStatus& click_status) {
+  if (!click_status.ok()) {
+    std::move(callback).Run(click_status);
+    return;
+  }
+
+  delegate->SendKeyboardInput(element, codepoints, delay_in_millis,
+                              std::move(callback));
+}
+
+void PerformSendKeyboardInput(
+    ActionDelegate* delegate,
+    const std::vector<UChar32> codepoints,
+    int delay_in_millis,
+    const ElementFinder::Result& element,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
+  PerformClickOrTap(
+      delegate, ClickType::CLICK, element,
+      base::BindOnce(&OnClickOrTapForSendKeyboardInput, delegate, codepoints,
+                     delay_in_millis, element, std::move(callback)));
 }
 
 void OnFindElementForSendKeyboardInput(
@@ -54,8 +124,8 @@ void OnFindElementForSendKeyboardInput(
     return;
   }
 
-  delegate->SendKeyboardInput(
-      *element, codepoints, delay_in_millis,
+  PerformSendKeyboardInput(
+      delegate, codepoints, delay_in_millis, *element,
       base::BindOnce(&RetainElementAndExecuteCallback, std::move(element),
                      std::move(callback)));
 }
@@ -74,6 +144,10 @@ void OnFindElementForSetFieldValue(
     return;
   }
 
+  // TODO(b/158153191): This should reuse the callback chains in the util
+  //  instead of relying on the |WebController| to properly implement it.
+  //  This requires to extract more methods and some internal logic of
+  //  |SetFieldValue|.
   delegate->SetFieldValue(
       *element, value, fill_strategy, key_press_delay_in_millisecond,
       base::BindOnce(&RetainElementAndExecuteCallback, std::move(element),

components/autofill_assistant/browser/actions/click_action_unittest.cc

@@ -0,0 +1,82 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/autofill_assistant/browser/actions/click_action.h"
+
+#include "base/test/gmock_callback_support.h"
+#include "base/test/mock_callback.h"
+#include "components/autofill_assistant/browser/actions/action_test_utils.h"
+#include "components/autofill_assistant/browser/actions/mock_action_delegate.h"
+#include "components/autofill_assistant/browser/selector.h"
+#include "components/autofill_assistant/browser/service.pb.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace autofill_assistant {
+namespace {
+
+using ::base::test::RunOnceCallback;
+using ::testing::_;
+using ::testing::InSequence;
+using ::testing::Pointee;
+using ::testing::Property;
+
+class ClickActionTest : public testing::Test {
+ public:
+  ClickActionTest() {}
+
+  void SetUp() override {}
+
+ protected:
+  void Run() {
+    ActionProto action_proto;
+    *action_proto.mutable_click() = proto_;
+    ClickAction action(&mock_action_delegate_, action_proto);
+    action.ProcessAction(callback_.Get());
+  }
+
+  MockActionDelegate mock_action_delegate_;
+  base::MockCallback<Action::ProcessActionCallback> callback_;
+  ClickProto proto_;
+};
+
+TEST_F(ClickActionTest, EmptySelectorFails) {
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, INVALID_SELECTOR))));
+  Run();
+}
+
+TEST_F(ClickActionTest, CheckExpectedCallChain) {
+  InSequence sequence;
+
+  Selector selector({"#click"});
+  *proto_.mutable_element_to_click() = selector.proto;
+  proto_.set_click_type(ClickType::CLICK);
+
+  Selector expected_selector = selector;
+  expected_selector.MustBeVisible();
+  EXPECT_CALL(mock_action_delegate_,
+              OnShortWaitForElement(expected_selector, _))
+      .WillOnce(RunOnceCallback<1>(OkClientStatus()));
+  auto expected_element =
+      test_util::MockFindElement(mock_action_delegate_, expected_selector);
+  EXPECT_CALL(mock_action_delegate_, WaitForDocumentToBecomeInteractive(
+                                         EqualsElement(expected_element), _))
+      .WillOnce(RunOnceCallback<1>(OkClientStatus()));
+  EXPECT_CALL(mock_action_delegate_,
+              ScrollIntoView(EqualsElement(expected_element), _))
+      .WillOnce(RunOnceCallback<1>(OkClientStatus()));
+  EXPECT_CALL(
+      mock_action_delegate_,
+      ClickOrTapElement(EqualsElement(expected_element), ClickType::CLICK, _))
+      .WillOnce(RunOnceCallback<2>(OkClientStatus()));
+
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
+  Run();
+}
+
+}  // namespace
+}  // namespace autofill_assistant

components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler_unittest.cc

@@ -28,7 +28,6 @@ using ::base::test::RunOnceCallback;
 using ::testing::_;
 using ::testing::Expectation;
 using ::testing::Invoke;
-using ::testing::WithArgs;
 
 RequiredField CreateRequiredField(const std::string& value_expression,
                                   const std::vector<std::string>& selector) {
@@ -50,6 +49,10 @@ class RequiredFieldsFallbackHandlerTest : public testing::Test {
         .WillByDefault(RunOnceCallback<1>(OkClientStatus(), "INPUT"));
     ON_CALL(mock_action_delegate_, OnSetFieldValue(_, _, _))
         .WillByDefault(RunOnceCallback<2>(OkClientStatus()));
+    ON_CALL(mock_action_delegate_, WaitForDocumentToBecomeInteractive(_, _))
+        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
+    ON_CALL(mock_action_delegate_, ScrollIntoView(_, _))
+        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
   }
 
  protected:

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -70,6 +70,12 @@ class MockActionDelegate : public ActionDelegate {
                void(const ElementFinder::Result& element,
                     ClickType click_type,
                     base::OnceCallback<void(const ClientStatus&)> callback));
+  MOCK_METHOD2(WaitForDocumentToBecomeInteractive,
+               void(const ElementFinder::Result& element,
+                    base::OnceCallback<void(const ClientStatus&)> callback));
+  MOCK_METHOD2(ScrollIntoView,
+               void(const ElementFinder::Result& element,
+                    base::OnceCallback<void(const ClientStatus&)> callback));
 
   MOCK_METHOD4(Prompt,
                void(std::unique_ptr<std::vector<UserAction>> user_actions,

components/autofill_assistant/browser/actions/set_form_field_value_action_unittest.cc

@@ -57,6 +57,12 @@ class SetFormFieldValueActionTest : public testing::Test {
           auto element_result = std::make_unique<ElementFinder::Result>();
           std::move(callback).Run(OkClientStatus(), std::move(element_result));
         }));
+    ON_CALL(mock_action_delegate_, WaitForDocumentToBecomeInteractive(_, _))
+        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
+    ON_CALL(mock_action_delegate_, ScrollIntoView(_, _))
+        .WillByDefault(RunOnceCallback<1>(OkClientStatus()));
+    ON_CALL(mock_action_delegate_, ClickOrTapElement(_, _, _))
+        .WillByDefault(RunOnceCallback<2>(OkClientStatus()));
 
     ON_CALL(mock_website_login_manager_, OnGetLoginsForUrl(_, _))
         .WillByDefault(

components/autofill_assistant/browser/script_executor.cc

@@ -229,6 +229,19 @@ void ScriptExecutor::FindElement(const Selector& selector,
                                              std::move(callback));
 }
 
+void ScriptExecutor::WaitForDocumentToBecomeInteractive(
+    const ElementFinder::Result& element,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
+  delegate_->GetWebController()->WaitForDocumentToBecomeInteractive(
+      element, std::move(callback));
+}
+
+void ScriptExecutor::ScrollIntoView(
+    const ElementFinder::Result& element,
+    base::OnceCallback<void(const ClientStatus&)> callback) {
+  delegate_->GetWebController()->ScrollIntoView(element, std::move(callback));
+}
+
 void ScriptExecutor::ClickOrTapElement(
     const ElementFinder::Result& element,
     ClickType click_type,

components/autofill_assistant/browser/script_executor.h

@@ -119,6 +119,12 @@ class ScriptExecutor : public ActionDelegate,
   std::string GetBubbleMessage() override;
   void FindElement(const Selector& selector,
                    ElementFinder::Callback callback) override;
+  void WaitForDocumentToBecomeInteractive(
+      const ElementFinder::Result& element,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
+  void ScrollIntoView(
+      const ElementFinder::Result& element,
+      base::OnceCallback<void(const ClientStatus&)> callback) override;
   void ClickOrTapElement(
       const ElementFinder::Result& element,
       ClickType click_type,