[Autofill Assistant] Make |GetElementTag| take an Element

This continues the effort to make |WebController| actions use
|ElementFinder::Result| instead of |Selector|.

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

components/autofill_assistant/browser/actions/action_delegate.h

@@ -258,9 +258,10 @@ class ActionDelegate {
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback) = 0;
 
-  // Return the tag of the element given by |selector|.
+  // Return the tag of the |element|. In case of an error, returns an empty
+  // string.
   virtual void GetElementTag(
-      const Selector& selector,
+      const ElementFinder::Result& element,
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback) = 0;
 

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

@@ -15,6 +15,7 @@
 #include "components/autofill_assistant/browser/batch_element_checker.h"
 #include "components/autofill_assistant/browser/client_status.h"
 #include "components/autofill_assistant/browser/field_formatter.h"
+#include "components/autofill_assistant/browser/web/element_finder.h"
 
 namespace autofill_assistant {
 namespace {
@@ -228,7 +229,8 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially(
         action_delegate_, required_field.selector, "",
         required_field.fill_strategy, required_field.delay_in_millisecond,
         base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue,
-                       weak_ptr_factory_.GetWeakPtr(), required_fields_index));
+                       weak_ptr_factory_.GetWeakPtr(), required_fields_index,
+                       /* element= */ nullptr));
     return;
   }
 
@@ -242,7 +244,6 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially(
   }
 
   if (required_field.fallback_click_element.has_value()) {
-    DVLOG(3) << "Clicking on " << required_field.selector;
     ClickType click_type = required_field.click_type;
     if (click_type == ClickType::NOT_SET) {
       // default: TAP
@@ -257,17 +258,40 @@ void RequiredFieldsFallbackHandler::SetFallbackFieldValuesSequentially(
     return;
   }
 
-  DVLOG(3) << "Getting element tag for " << required_field.selector;
-  action_delegate_->GetElementTag(
+  action_delegate_->FindElement(
       required_field.selector,
-      base::BindOnce(&RequiredFieldsFallbackHandler::OnGetFallbackFieldTag,
+      base::BindOnce(&RequiredFieldsFallbackHandler::OnFindElement,
                      weak_ptr_factory_.GetWeakPtr(), fallback_value.value(),
                      required_fields_index));
 }
 
-void RequiredFieldsFallbackHandler::OnGetFallbackFieldTag(
+void RequiredFieldsFallbackHandler::OnFindElement(
+    const std::string& value,
+    size_t required_fields_index,
+    const ClientStatus& element_status,
+    std::unique_ptr<ElementFinder::Result> element_result) {
+  if (!element_status.ok()) {
+    FillStatusDetailsWithError(required_fields_[required_fields_index],
+                               element_status.proto_status(), &client_status_);
+
+    // Fallback failed: we stop the script without checking the other fields.
+    std::move(status_update_callback_)
+        .Run(ClientStatus(AUTOFILL_INCOMPLETE), client_status_);
+    return;
+  }
+
+  action_delegate_->GetElementTag(
+      *element_result,
+      base::BindOnce(
+          &RequiredFieldsFallbackHandler::OnGetFallbackFieldElementTag,
+          weak_ptr_factory_.GetWeakPtr(), value, required_fields_index,
+          std::move(element_result)));
+}
+
+void RequiredFieldsFallbackHandler::OnGetFallbackFieldElementTag(
     const std::string& value,
     size_t required_fields_index,
+    std::unique_ptr<ElementFinder::Result> element,
     const ClientStatus& element_tag_status,
     const std::string& element_tag) {
   if (!element_tag_status.ok()) {
@@ -287,18 +311,20 @@ void RequiredFieldsFallbackHandler::OnGetFallbackFieldTag(
       select_strategy = LABEL_STARTS_WITH;
     }
 
-    ActionDelegateUtil::SelectOption(
-        action_delegate_, required_field.selector, value, select_strategy,
+    action_delegate_->SelectOption(
+        *element, value, select_strategy,
         base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue,
-                       weak_ptr_factory_.GetWeakPtr(), required_fields_index));
+                       weak_ptr_factory_.GetWeakPtr(), required_fields_index,
+                       std::move(element)));
     return;
   }
 
-  ActionDelegateUtil::SetFieldValue(
-      action_delegate_, required_field.selector, value,
-      required_field.fill_strategy, required_field.delay_in_millisecond,
+  action_delegate_->SetFieldValue(
+      *element, value, required_field.fill_strategy,
+      required_field.delay_in_millisecond,
       base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue,
-                     weak_ptr_factory_.GetWeakPtr(), required_fields_index));
+                     weak_ptr_factory_.GetWeakPtr(), required_fields_index,
+                     std::move(element)));
 }
 
 void RequiredFieldsFallbackHandler::OnClickOrTapFallbackElement(
@@ -320,7 +346,6 @@ void RequiredFieldsFallbackHandler::OnClickOrTapFallbackElement(
   Selector value_selector = required_field.fallback_click_element.value();
   value_selector.MatchingInnerText(value).MustBeVisible();
 
-  DVLOG(3) << "Finding option for " << required_field.selector;
   action_delegate_->ShortWaitForElement(
       value_selector,
       base::BindOnce(&RequiredFieldsFallbackHandler::OnShortWaitForElement,
@@ -344,7 +369,6 @@ void RequiredFieldsFallbackHandler::OnShortWaitForElement(
     return;
   }
 
-  DVLOG(3) << "Clicking option for " << required_field.selector;
   ClickType click_type = required_field.click_type;
   if (click_type == ClickType::NOT_SET) {
     // default: TAP
@@ -353,11 +377,13 @@ void RequiredFieldsFallbackHandler::OnShortWaitForElement(
   ActionDelegateUtil::ClickOrTapElement(
       action_delegate_, selector_to_click, click_type,
       base::BindOnce(&RequiredFieldsFallbackHandler::OnSetFallbackFieldValue,
-                     weak_ptr_factory_.GetWeakPtr(), required_fields_index));
+                     weak_ptr_factory_.GetWeakPtr(), required_fields_index,
+                     /* element= */ nullptr));
 }
 
 void RequiredFieldsFallbackHandler::OnSetFallbackFieldValue(
     size_t required_fields_index,
+    std::unique_ptr<ElementFinder::Result> element,
     const ClientStatus& set_field_status) {
   if (!set_field_status.ok()) {
     VLOG(1) << "Error setting value for required_field: "

components/autofill_assistant/browser/actions/fallback_handler/required_fields_fallback_handler.h

@@ -18,6 +18,7 @@
 #include "components/autofill_assistant/browser/actions/action.h"
 #include "components/autofill_assistant/browser/actions/fallback_handler/required_field.h"
 #include "components/autofill_assistant/browser/batch_element_checker.h"
+#include "components/autofill_assistant/browser/web/element_finder.h"
 
 namespace autofill_assistant {
 class ClientStatus;
@@ -64,11 +65,20 @@ class RequiredFieldsFallbackHandler {
   // Sets fallback field values for empty fields.
   void SetFallbackFieldValuesSequentially(size_t required_fields_index);
 
+  // Called after attempting to find one of the elements to execute a fallback
+  // action on.
+  void OnFindElement(const std::string& value,
+                     size_t required_fields_index,
+                     const ClientStatus& element_status,
+                     std::unique_ptr<ElementFinder::Result> element_result);
+
   // Called after retrieving tag name from a field.
-  void OnGetFallbackFieldTag(const std::string& value,
-                             size_t required_fields_index,
-                             const ClientStatus& element_tag_status,
-                             const std::string& element_tag);
+  void OnGetFallbackFieldElementTag(
+      const std::string& value,
+      size_t required_fields_index,
+      std::unique_ptr<ElementFinder::Result> element,
+      const ClientStatus& element_tag_status,
+      const std::string& element_tag);
 
   // Called after clicking a fallback element.
   void OnClickOrTapFallbackElement(const std::string& value,
@@ -82,6 +92,7 @@ class RequiredFieldsFallbackHandler {
   // Called after trying to set form values without Autofill in case of
   // fallback after failed validation.
   void OnSetFallbackFieldValue(size_t required_fields_index,
+                               std::unique_ptr<ElementFinder::Result> element,
                                const ClientStatus& status);
 
   ClientStatus client_status_;

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

@@ -479,16 +479,17 @@ TEST_F(RequiredFieldsFallbackHandlerTest,
 
 TEST_F(RequiredFieldsFallbackHandlerTest, UsesSelectOptionForDropdowns) {
   Selector expected_selector({"#year"});
+  const ElementFinder::Result& expected_element =
+      test_util::MockFindElement(mock_action_delegate_, expected_selector);
   EXPECT_CALL(mock_web_controller_, OnGetFieldValue(expected_selector, _))
       .WillOnce(RunOnceCallback<1>(OkClientStatus(), ""));
-  EXPECT_CALL(mock_action_delegate_, GetElementTag(expected_selector, _))
+  EXPECT_CALL(mock_action_delegate_,
+              GetElementTag(EqualsElement(expected_element), _))
       .WillOnce(RunOnceCallback<1>(OkClientStatus(), "SELECT"));
   Expectation select_option =
-      EXPECT_CALL(
-          mock_action_delegate_,
-          SelectOption(EqualsElement(test_util::MockFindElement(
-                           mock_action_delegate_, expected_selector)),
-                       "2050", DropdownSelectStrategy::LABEL_STARTS_WITH, _))
+      EXPECT_CALL(mock_action_delegate_,
+                  SelectOption(EqualsElement(expected_element), "2050",
+                               DropdownSelectStrategy::LABEL_STARTS_WITH, _))
           .WillOnce(RunOnceCallback<3>(OkClientStatus()));
   EXPECT_CALL(mock_web_controller_, OnGetFieldValue(expected_selector, _))
       .After(select_option)

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -229,7 +229,7 @@ class MockActionDelegate : public ActionDelegate {
                                             const std::string&)> callback));
 
   MOCK_METHOD2(GetElementTag,
-               void(const Selector& selector,
+               void(const ElementFinder::Result& element,
                     base::OnceCallback<void(const ClientStatus&,
                                             const std::string&)> callback));
 

components/autofill_assistant/browser/script_executor.cc

@@ -613,10 +613,10 @@ void ScriptExecutor::GetOuterHtml(
 }
 
 void ScriptExecutor::GetElementTag(
-    const Selector& selector,
+    const ElementFinder::Result& element,
     base::OnceCallback<void(const ClientStatus&, const std::string&)>
         callback) {
-  delegate_->GetWebController()->GetElementTag(selector, std::move(callback));
+  delegate_->GetWebController()->GetElementTag(element, std::move(callback));
 }
 
 void ScriptExecutor::ExpectNavigation() {

components/autofill_assistant/browser/script_executor.h

@@ -205,7 +205,7 @@ class ScriptExecutor : public ActionDelegate,
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback) override;
   void GetElementTag(
-      const Selector& selector,
+      const ElementFinder::Result& element,
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback) override;
   void ExpectNavigation() override;

components/autofill_assistant/browser/web/web_controller.cc

@@ -1481,34 +1481,16 @@ void WebController::OnGetOuterHtml(
 }
 
 void WebController::GetElementTag(
-    const Selector& selector,
+    const ElementFinder::Result& element,
     base::OnceCallback<void(const ClientStatus&, const std::string&)>
         callback) {
-  VLOG(3) << __func__ << " " << selector;
-  FindElement(
-      selector,
-      /* strict_mode= */ true,
-      base::BindOnce(&WebController::OnFindElementForGetElementTag,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
-}
-
-void WebController::OnFindElementForGetElementTag(
-    base::OnceCallback<void(const ClientStatus&, const std::string&)> callback,
-    const ClientStatus& status,
-    std::unique_ptr<ElementFinder::Result> element_result) {
-  if (!status.ok()) {
-    VLOG(2) << __func__ << " Failed to find element for GetElementTag";
-    std::move(callback).Run(status, "");
-    return;
-  }
-
   devtools_client_->GetRuntime()->CallFunctionOn(
       runtime::CallFunctionOnParams::Builder()
-          .SetObjectId(element_result->object_id)
+          .SetObjectId(element.object_id)
           .SetFunctionDeclaration(std::string(kGetElementTagScript))
           .SetReturnByValue(true)
           .Build(),
-      element_result->node_frame_id,
+      element.node_frame_id,
       base::BindOnce(&WebController::OnGetElementTag,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }

components/autofill_assistant/browser/web/web_controller.h

@@ -185,9 +185,10 @@ class WebController {
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback);
 
-  // Return the tag of |selector|.
+  // Return the tag of the |element|. In case of an error, will return an empty
+  // string.
   virtual void GetElementTag(
-      const Selector& selector,
+      const ElementFinder::Result& element,
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback);
 
@@ -459,11 +460,6 @@ class WebController {
                                               const std::string&)> callback,
                       const DevtoolsClient::ReplyStatus& reply_status,
                       std::unique_ptr<runtime::CallFunctionOnResult> result);
-  void OnFindElementForGetElementTag(
-      base::OnceCallback<void(const ClientStatus&, const std::string&)>
-          callback,
-      const ClientStatus& status,
-      std::unique_ptr<ElementFinder::Result> element_result);
   void OnGetElementTag(base::OnceCallback<void(const ClientStatus&,
                                                const std::string&)> callback,
                        const DevtoolsClient::ReplyStatus& reply_status,

components/autofill_assistant/browser/web/web_controller_browsertest.cc

@@ -329,20 +329,42 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
                              std::string* element_tag_output) {
     base::RunLoop run_loop;
     ClientStatus result;
-    web_controller_->GetElementTag(
-        selector, base::BindOnce(&WebControllerBrowserTest::OnGetElementTag,
-                                 base::Unretained(this), run_loop.QuitClosure(),
-                                 &result, element_tag_output));
+
+    web_controller_->FindElement(
+        selector, /* strict= */ true,
+        base::BindOnce(
+            &WebControllerBrowserTest::FindGetElementTagElementCallback,
+            base::Unretained(this), run_loop.QuitClosure(), &result,
+            element_tag_output));
+
     run_loop.Run();
     return result;
   }
 
+  void FindGetElementTagElementCallback(
+      base::OnceClosure done_callback,
+      ClientStatus* result_output,
+      std::string* element_tag_output,
+      const ClientStatus& element_status,
+      std::unique_ptr<ElementFinder::Result> element_result) {
+    EXPECT_EQ(ACTION_APPLIED, element_status.proto_status());
+    ASSERT_TRUE(element_result != nullptr);
+    web_controller_->GetElementTag(
+        *element_result,
+        base::BindOnce(&WebControllerBrowserTest::OnGetElementTag,
+                       base::Unretained(this), std::move(done_callback),
+                       result_output, element_tag_output,
+                       std::move(element_result)));
+  }
+
   void OnGetElementTag(base::OnceClosure done_callback,
                        ClientStatus* successful_output,
                        std::string* element_tag_output,
+                       std::unique_ptr<ElementFinder::Result> element,
                        const ClientStatus& status,
                        const std::string& element_tag) {
     EXPECT_EQ(ACTION_APPLIED, status.proto_status());
+    ASSERT_TRUE(element != nullptr);
     *successful_output = status;
     *element_tag_output = element_tag;
     std::move(done_callback).Run();