[Autofill Assistant] Add new web-action to get attribute

This CL adds a new web-action to get the attribute of an element.
This is required for extending the |GetElementStatusAction| to
also handle the text content of an element.

The attributes can be nested.

Bug: b/169924567
Change-Id: Id5ff86d907d34d8cbf8e56aa8dfd377fe8f934bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463233Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: Luca Hunkeler <hluca@google.com>
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#816111}

components/autofill_assistant/browser/actions/action_delegate.h

@@ -231,6 +231,15 @@ class ActionDelegate {
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback) = 0;
 
+  // Get the value of a nested |attribute| from an |element| and return the
+  // result through |callback|. If the lookup fails, the value will be empty.
+  // An empty result does not mean an error.
+  virtual void GetStringAttribute(
+      const std::vector<std::string>& attributes,
+      const ElementFinder::Result& element,
+      base::OnceCallback<void(const ClientStatus&, const std::string&)>
+          callback) = 0;
+
   // Set the |value| of field |element| and return the result through
   // |callback|. If |simulate_key_presses| is true, the value will be set by
   // clicking the field and then simulating key presses, otherwise the `value`

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -189,6 +189,12 @@ class MockActionDelegate : public ActionDelegate {
                     base::OnceCallback<void(const ClientStatus&,
                                             const std::string&)>& callback));
 
+  MOCK_METHOD3(GetStringAttribute,
+               void(const std::vector<std::string>& attributes,
+                    const ElementFinder::Result& element,
+                    base::OnceCallback<void(const ClientStatus&,
+                                            const std::string&)> callback));
+
   void SetFieldValue(const std::string& value,
                      KeyboardValueFillStrategy fill_strategy,
                      int key_press_delay_in_millisecond,

components/autofill_assistant/browser/script_executor.cc

@@ -583,6 +583,15 @@ void ScriptExecutor::GetFieldValue(
   delegate_->GetWebController()->GetFieldValue(selector, std::move(callback));
 }
 
+void ScriptExecutor::GetStringAttribute(
+    const std::vector<std::string>& attributes,
+    const ElementFinder::Result& element,
+    base::OnceCallback<void(const ClientStatus&, const std::string&)>
+        callback) {
+  delegate_->GetWebController()->GetStringAttribute(element, attributes,
+                                                    std::move(callback));
+}
+
 void ScriptExecutor::SetFieldValue(
     const std::string& value,
     KeyboardValueFillStrategy fill_strategy,

components/autofill_assistant/browser/script_executor.h

@@ -185,6 +185,11 @@ class ScriptExecutor : public ActionDelegate,
       const Selector& selector,
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback) override;
+  void GetStringAttribute(
+      const std::vector<std::string>& attributes,
+      const ElementFinder::Result& element,
+      base::OnceCallback<void(const ClientStatus&, const std::string&)>
+          callback) override;
   void SetFieldValue(
       const std::string& value,
       KeyboardValueFillStrategy fill_strategy,

components/autofill_assistant/browser/web/mock_web_controller.h

@@ -77,6 +77,19 @@ class MockWebController : public WebController {
                     base::OnceCallback<void(const ClientStatus&,
                                             const std::string&)>& callback));
 
+  void GetStringAttribute(
+      const ElementFinder::Result& element,
+      const std::vector<std::string>& attributes,
+      base::OnceCallback<void(const ClientStatus&, const std::string&)>
+          callback) override {
+    OnGetStringAttribute(element, attributes, callback);
+  }
+  MOCK_METHOD3(OnGetStringAttribute,
+               void(const ElementFinder::Result& element,
+                    const std::vector<std::string>& attributes,
+                    base::OnceCallback<void(const ClientStatus&,
+                                            const std::string&)>& callback));
+
   void GetVisualViewport(
       base::OnceCallback<void(bool, const RectF&)> callback) override {
     OnGetVisualViewport(callback);

components/autofill_assistant/browser/web/web_controller.cc

@@ -112,6 +112,18 @@ const char* const kHighlightElementScript =
 const char* const kGetValueAttributeScript =
     "function () { return this.value; }";
 
+// Javascript code to retrieve the nested |attribute| of a node.
+// The function intentionally has no "has value" check, such that a bad access
+// will return an error.
+const char* const kGetElementAttributeScript =
+    R"(function (attributes) {
+        let it = this;
+        for (let i = 0; i < attributes.length; ++i) {
+          it = it[attributes[i]];
+        }
+        return it;
+      })";
+
 // Javascript code to select the current value.
 const char* const kSelectFieldValue = "function() { this.select(); }";
 
@@ -126,6 +138,8 @@ const char* const kSetValueAttributeScript =
        })";
 
 // Javascript code to set an attribute of a node to a given value.
+// The function intentionally has no "has value" check, such that a bad access
+// will return an error.
 const char* const kSetAttributeScript =
     R"(function (attribute, value) {
          let receiver = this;
@@ -288,11 +302,25 @@ void WebController::OnJavaScriptResult(
   ClientStatus status =
       CheckJavaScriptResult(reply_status, result.get(), __FILE__, __LINE__);
   if (!status.ok()) {
-    VLOG(1) << __func__ << " Failed JavaScript.";
+    VLOG(1) << __func__ << " Failed JavaScript with status: " << status;
   }
   std::move(callback).Run(status);
 }
 
+void WebController::OnJavaScriptResultForString(
+    base::OnceCallback<void(const ClientStatus&, const std::string&)> callback,
+    const DevtoolsClient::ReplyStatus& reply_status,
+    std::unique_ptr<runtime::CallFunctionOnResult> result) {
+  std::string value;
+  ClientStatus status =
+      CheckJavaScriptResult(reply_status, result.get(), __FILE__, __LINE__);
+  if (!status.ok()) {
+    VLOG(1) << __func__ << "Failed JavaScript with status: " << status;
+  }
+  SafeGetStringValue(result->GetResult(), &value);
+  std::move(callback).Run(status, value);
+}
+
 void WebController::ScrollIntoView(
     const ElementFinder::Result& element,
     base::OnceCallback<void(const ClientStatus&)> callback) {
@@ -902,7 +930,6 @@ void WebController::OnFindElementForGetFieldValue(
     base::OnceCallback<void(const ClientStatus&, const std::string&)> callback,
     const ClientStatus& status,
     std::unique_ptr<ElementFinder::Result> element_result) {
-  const std::string object_id = element_result->object_id;
   if (!status.ok()) {
     std::move(callback).Run(status, "");
     return;
@@ -910,28 +937,44 @@ void WebController::OnFindElementForGetFieldValue(
 
   devtools_client_->GetRuntime()->CallFunctionOn(
       runtime::CallFunctionOnParams::Builder()
-          .SetObjectId(object_id)
+          .SetObjectId(element_result->object_id)
           .SetFunctionDeclaration(std::string(kGetValueAttributeScript))
           .SetReturnByValue(true)
           .Build(),
       element_result->node_frame_id,
-      base::BindOnce(&WebController::OnGetValueAttribute,
+      base::BindOnce(&WebController::OnJavaScriptResultForString,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void WebController::OnGetValueAttribute(
-    base::OnceCallback<void(const ClientStatus& element_status,
-                            const std::string&)> callback,
-    const DevtoolsClient::ReplyStatus& reply_status,
-    std::unique_ptr<runtime::CallFunctionOnResult> result) {
-  std::string value;
-  ClientStatus status =
-      CheckJavaScriptResult(reply_status, result.get(), __FILE__, __LINE__);
-  // Read the result returned from Javascript code.
-  VLOG_IF(1, !status.ok()) << __func__
-                           << "Failed to get attribute value: " << status;
-  SafeGetStringValue(result->GetResult(), &value);
-  std::move(callback).Run(status, value);
+void WebController::GetStringAttribute(
+    const ElementFinder::Result& element,
+    const std::vector<std::string>& attributes,
+    base::OnceCallback<void(const ClientStatus&, const std::string&)>
+        callback) {
+  VLOG(3) << __func__ << " attributes=[" << base::JoinString(attributes, ",")
+          << "]";
+
+  if (attributes.empty()) {
+    std::move(callback).Run(UnexpectedErrorStatus(__FILE__, __LINE__), "");
+    return;
+  }
+  base::Value::ListStorage attribute_values;
+  for (const std::string& attribute : attributes) {
+    attribute_values.emplace_back(base::Value(attribute));
+  }
+
+  std::vector<std::unique_ptr<runtime::CallArgument>> arguments;
+  AddRuntimeCallArgument(attribute_values, &arguments);
+  devtools_client_->GetRuntime()->CallFunctionOn(
+      runtime::CallFunctionOnParams::Builder()
+          .SetObjectId(element.object_id)
+          .SetArguments(std::move(arguments))
+          .SetFunctionDeclaration(std::string(kGetElementAttributeScript))
+          .SetReturnByValue(true)
+          .Build(),
+      element.node_frame_id,
+      base::BindOnce(&WebController::OnJavaScriptResultForString,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
 void WebController::SetFieldValue(
@@ -1193,17 +1236,16 @@ void WebController::SetAttribute(
     const std::vector<std::string>& attributes,
     const std::string& value,
     base::OnceCallback<void(const ClientStatus&)> callback) {
-#ifdef NDEBUG
-  VLOG(3) << __func__ << " attributes=(redacted), value=(redacted)";
-#else
   DVLOG(3) << __func__ << " attributes=[" << base::JoinString(attributes, ",")
            << "], value=" << value;
-#endif
 
-  DCHECK_GT(attributes.size(), 0u);
+  if (attributes.empty()) {
+    std::move(callback).Run(UnexpectedErrorStatus(__FILE__, __LINE__));
+    return;
+  }
   base::Value::ListStorage attribute_values;
-  for (const std::string& string : attributes) {
-    attribute_values.emplace_back(base::Value(string));
+  for (const std::string& attribute : attributes) {
+    attribute_values.emplace_back(base::Value(attribute));
   }
 
   std::vector<std::unique_ptr<runtime::CallArgument>> arguments;
@@ -1336,27 +1378,10 @@ void WebController::GetOuterHtml(
           .SetReturnByValue(true)
           .Build(),
       element.node_frame_id,
-      base::BindOnce(&WebController::OnGetOuterHtml,
+      base::BindOnce(&WebController::OnJavaScriptResultForString,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void WebController::OnGetOuterHtml(
-    base::OnceCallback<void(const ClientStatus&, const std::string&)> callback,
-    const DevtoolsClient::ReplyStatus& reply_status,
-    std::unique_ptr<runtime::CallFunctionOnResult> result) {
-  ClientStatus status =
-      CheckJavaScriptResult(reply_status, result.get(), __FILE__, __LINE__);
-  if (!status.ok()) {
-    VLOG(2) << __func__ << " Failed to get HTML content for GetOuterHtml";
-    std::move(callback).Run(status, "");
-    return;
-  }
-
-  std::string value;
-  SafeGetStringValue(result->GetResult(), &value);
-  std::move(callback).Run(OkClientStatus(), value);
-}
-
 void WebController::GetElementTag(
     const ElementFinder::Result& element,
     base::OnceCallback<void(const ClientStatus&, const std::string&)>
@@ -1368,26 +1393,10 @@ void WebController::GetElementTag(
           .SetReturnByValue(true)
           .Build(),
       element.node_frame_id,
-      base::BindOnce(&WebController::OnGetElementTag,
+      base::BindOnce(&WebController::OnJavaScriptResultForString,
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
-void WebController::OnGetElementTag(
-    base::OnceCallback<void(const ClientStatus&, const std::string&)> callback,
-    const DevtoolsClient::ReplyStatus& reply_status,
-    std::unique_ptr<runtime::CallFunctionOnResult> result) {
-  ClientStatus status =
-      CheckJavaScriptResult(reply_status, result.get(), __FILE__, __LINE__);
-  if (!status.ok()) {
-    VLOG(2) << __func__ << " Failed to get element tag for GetElementTag";
-    std::move(callback).Run(status, "");
-    return;
-  }
-  std::string value;
-  SafeGetStringValue(result->GetResult(), &value);
-  std::move(callback).Run(OkClientStatus(), value);
-}
-
 void WebController::InternalWaitForDocumentToBecomeInteractive(
     int remaining_rounds,
     const std::string& object_id,

components/autofill_assistant/browser/web/web_controller.h

@@ -151,6 +151,15 @@ class WebController {
       base::OnceCallback<void(const ClientStatus&, const std::string&)>
           callback);
 
+  // Get the value of a nested |attribute| from an |element| and return the
+  // result through |callback|. If the lookup fails, the value will be empty.
+  // An empty result does not mean an error.
+  virtual void GetStringAttribute(
+      const ElementFinder::Result& element,
+      const std::vector<std::string>& attributes,
+      base::OnceCallback<void(const ClientStatus&, const std::string&)>
+          callback);
+
   // Set the |value| of field |element| and return the result through
   // |callback|. The strategy used to fill the value is defined by
   // |fill_strategy|, see the proto for further explanation.
@@ -281,6 +290,11 @@ class WebController {
       base::OnceCallback<void(const ClientStatus&)> callback,
       const DevtoolsClient::ReplyStatus& reply_status,
       std::unique_ptr<runtime::CallFunctionOnResult> result);
+  void OnJavaScriptResultForString(
+      base::OnceCallback<void(const ClientStatus&, const std::string&)>
+          callback,
+      const DevtoolsClient::ReplyStatus& reply_status,
+      std::unique_ptr<runtime::CallFunctionOnResult> result);
   void OnWaitForDocumentToBecomeInteractive(
       base::OnceCallback<void(const ClientStatus&)> callback,
       bool result);
@@ -360,11 +374,6 @@ class WebController {
           callback,
       const ClientStatus& status,
       std::unique_ptr<ElementFinder::Result> element_result);
-  void OnGetValueAttribute(
-      base::OnceCallback<void(const ClientStatus&, const std::string&)>
-          callback,
-      const DevtoolsClient::ReplyStatus& reply_status,
-      std::unique_ptr<runtime::CallFunctionOnResult> result);
   void OnClearFieldForSetFieldValue(
       const ElementFinder::Result& element,
       const std::vector<UChar32>& codepoints,
@@ -420,14 +429,6 @@ class WebController {
       const ElementFinder::Result& element,
       const std::string& value,
       base::OnceCallback<void(const ClientStatus&)> callback);
-  void OnGetOuterHtml(base::OnceCallback<void(const ClientStatus&,
-                                              const std::string&)> callback,
-                      const DevtoolsClient::ReplyStatus& reply_status,
-                      std::unique_ptr<runtime::CallFunctionOnResult> result);
-  void OnGetElementTag(base::OnceCallback<void(const ClientStatus&,
-                                               const std::string&)> callback,
-                       const DevtoolsClient::ReplyStatus& reply_status,
-                       std::unique_ptr<runtime::CallFunctionOnResult> result);
   void OnFindElementForPosition(
       base::OnceCallback<void(bool, const RectF&)> callback,
       const ClientStatus& status,

components/autofill_assistant/browser/web/web_controller_browsertest.cc

@@ -148,6 +148,19 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
     std::move(done_callback).Run();
   }
 
+  void ElementRetainingStringCallback(
+      std::unique_ptr<ElementFinder::Result> element,
+      base::OnceClosure done_callback,
+      ClientStatus* result_output,
+      std::string* result,
+      const ClientStatus& status,
+      const std::string& value) {
+    EXPECT_TRUE(element != nullptr);
+    *result_output = status;
+    result->assign(value);
+    std::move(done_callback).Run();
+  }
+
   void ClickOrTapElement(const Selector& selector, ClickType click_type) {
     base::RunLoop run_loop;
     ClientStatus result_output;
@@ -356,6 +369,7 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
             html_output));
 
     run_loop.Run();
+    EXPECT_EQ(ACTION_APPLIED, result.proto_status());
     return result;
   }
 
@@ -369,22 +383,10 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
     ASSERT_TRUE(element_result != nullptr);
     web_controller_->GetOuterHtml(
         *element_result,
-        base::BindOnce(&WebControllerBrowserTest::OnGetOuterHtml,
-                       base::Unretained(this), std::move(done_callback),
-                       result_output, html_output, std::move(element_result)));
-  }
-
-  void OnGetOuterHtml(base::OnceClosure done_callback,
-                      ClientStatus* successful_output,
-                      std::string* html_output,
-                      std::unique_ptr<ElementFinder::Result> element,
-                      const ClientStatus& status,
-                      const std::string& html) {
-    EXPECT_EQ(ACTION_APPLIED, status.proto_status());
-    EXPECT_TRUE(element != nullptr);
-    *successful_output = status;
-    *html_output = html;
-    std::move(done_callback).Run();
+        base::BindOnce(
+            &WebControllerBrowserTest::ElementRetainingStringCallback,
+            base::Unretained(this), std::move(element_result),
+            std::move(done_callback), result_output, html_output));
   }
 
   ClientStatus GetElementTag(const Selector& selector,
@@ -400,6 +402,7 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
             element_tag_output));
 
     run_loop.Run();
+    EXPECT_EQ(ACTION_APPLIED, result.proto_status());
     return result;
   }
 
@@ -413,23 +416,10 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
     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();
+        base::BindOnce(
+            &WebControllerBrowserTest::ElementRetainingStringCallback,
+            base::Unretained(this), std::move(element_result),
+            std::move(done_callback), result_output, element_tag_output));
   }
 
   void FindElement(const Selector& selector,
@@ -491,6 +481,40 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
     EXPECT_FALSE(result.object_id.empty());
   }
 
+  ClientStatus GetStringAttribute(const Selector& selector,
+                                  const std::vector<std::string>& attributes,
+                                  std::string* value) {
+    base::RunLoop run_loop;
+    ClientStatus result;
+
+    web_controller_->FindElement(
+        selector, /* strict= */ true,
+        base::BindOnce(
+            &WebControllerBrowserTest::FindGetStringAttributeElementCallback,
+            base::Unretained(this), attributes, run_loop.QuitClosure(), &result,
+            value));
+
+    run_loop.Run();
+    return result;
+  }
+
+  void FindGetStringAttributeElementCallback(
+      const std::vector<std::string>& attributes,
+      base::OnceClosure done_callback,
+      ClientStatus* result_output,
+      std::string* value,
+      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_->GetStringAttribute(
+        *element_result, attributes,
+        base::BindOnce(
+            &WebControllerBrowserTest::ElementRetainingStringCallback,
+            base::Unretained(this), std::move(element_result),
+            std::move(done_callback), result_output, value));
+  }
+
   void GetFieldsValue(const std::vector<Selector>& selectors,
                       const std::vector<std::string>& expected_values) {
     base::RunLoop run_loop;
@@ -553,20 +577,11 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
     web_controller_->SetFieldValue(
         *element_result, value, fill_strategy,
         /* key_press_delay_in_millisecond= */ 0,
-        base::BindOnce(&WebControllerBrowserTest::SetFieldValueCallback,
+        base::BindOnce(&WebControllerBrowserTest::ElementRetainingCallback,
                        base::Unretained(this), std::move(element_result),
                        std::move(done_callback), result_output));
   }
 
-  void SetFieldValueCallback(std::unique_ptr<ElementFinder::Result> element,
-                             base::OnceClosure done_callback,
-                             ClientStatus* result_output,
-                             const ClientStatus& status) {
-    EXPECT_TRUE(element != nullptr);
-    *result_output = status;
-    std::move(done_callback).Run();
-  }
-
   ClientStatus SendKeyboardInput(const Selector& selector,
                                  const std::vector<UChar32>& codepoints,
                                  int delay_in_milli) {
@@ -2054,4 +2069,25 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest,
   GetFieldsValue({selector}, {"email@example.com"});
 }
 
+IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, GetStringAttribute) {
+  std::string value;
+
+  std::vector<std::string> inner_text_attribute = {"innerText"};
+  ASSERT_EQ(ACTION_APPLIED, GetStringAttribute(Selector({"#testOuterHtml p"}),
+                                               inner_text_attribute, &value)
+                                .proto_status());
+  EXPECT_EQ("Paragraph", value);
+
+  std::vector<std::string> option_label_attribute = {"options", "2", "label"};
+  ASSERT_EQ(ACTION_APPLIED, GetStringAttribute(Selector({"#select"}),
+                                               option_label_attribute, &value)
+                                .proto_status());
+  EXPECT_EQ("Three", value);
+
+  std::vector<std::string> bad_access = {"none", "none"};
+  ASSERT_EQ(UNEXPECTED_JS_ERROR,
+            GetStringAttribute(Selector({"#button"}), bad_access, &value)
+                .proto_status());
+}
+
 }  // namespace autofill_assistant