[Autofill Assistant] Fixed SetFieldValue() to support UTF-8 text.

Also in this CL:
 - refactoring of parts of the backend to remove redundant and potentially error-prone code
 - modified unit tests for web_controller to test for UTF-8 compliance of SetFieldValue()

Change-Id: Ib7dd2429159a527ec81f325622e8bf6f97c5e5ab
Reviewed-on: https://chromium-review.googlesource.com/c/1360691
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614300}

components/autofill_assistant/browser/actions/action_delegate.h

@@ -156,10 +156,10 @@ class ActionDelegate {
                             const std::string& value,
                             base::OnceCallback<void(bool)> callback) = 0;
 
-  // Sets the keyboard focus to |selector| and inputs the specified text.
+  // Sets the keyboard focus to |selector| and inputs the specified text parts.
   // Returns the result through |callback|.
   virtual void SendKeyboardInput(const Selector& selector,
-                                 const std::string& text,
+                                 const std::vector<std::string>& text_parts,
                                  base::OnceCallback<void(bool)> callback) = 0;
 
   // Return the outerHTML of an element given by |selector|.

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -142,7 +142,7 @@ class MockActionDelegate : public ActionDelegate {
 
   MOCK_METHOD3(SendKeyboardInput,
                void(const Selector& selector,
-                    const std::string& text,
+                    const std::vector<std::string>& text_parts,
                     base::OnceCallback<void(bool)> callback));
   MOCK_METHOD2(
       GetOuterHtml,

components/autofill_assistant/browser/actions/set_form_field_value_action.cc

@@ -78,7 +78,7 @@ void SetFormFieldValueAction::OnSetFieldValue(ActionDelegate* delegate,
       // You should use the `keyboard_input' field instead.
       if (key_field.keycode() < 128) {  // US-ASCII
         delegate->SendKeyboardInput(
-            selector, std::string(1, char(key_field.keycode())),
+            selector, {std::string(1, char(key_field.keycode()))},
             base::BindOnce(&SetFormFieldValueAction::OnSetFieldValue,
                            weak_ptr_factory_.GetWeakPtr(), delegate,
                            std::move(callback), /* next = */ next + 1));
@@ -93,7 +93,7 @@ void SetFormFieldValueAction::OnSetFieldValue(ActionDelegate* delegate,
       break;
     case SetFormFieldValueProto_KeyPress::kKeyboardInput:
       delegate->SendKeyboardInput(
-          selector, key_field.keyboard_input(),
+          selector, {key_field.keyboard_input()},
           base::BindOnce(&SetFormFieldValueAction::OnSetFieldValue,
                          weak_ptr_factory_.GetWeakPtr(), delegate,
                          std::move(callback), /* next = */ next + 1));

components/autofill_assistant/browser/script_executor.cc

@@ -235,9 +235,9 @@ void ScriptExecutor::SetAttribute(const Selector& selector,
 
 void ScriptExecutor::SendKeyboardInput(
     const Selector& selector,
-    const std::string& text,
+    const std::vector<std::string>& text_parts,
     base::OnceCallback<void(bool)> callback) {
-  delegate_->GetWebController()->SendKeyboardInput(selector, text,
+  delegate_->GetWebController()->SendKeyboardInput(selector, text_parts,
                                                    std::move(callback));
 }
 

components/autofill_assistant/browser/script_executor.h

@@ -135,7 +135,7 @@ class ScriptExecutor : public ActionDelegate {
                     const std::string& value,
                     base::OnceCallback<void(bool)> callback) override;
   void SendKeyboardInput(const Selector& selector,
-                         const std::string& text,
+                         const std::vector<std::string>& text_parts,
                          base::OnceCallback<void(bool)> callback) override;
   void GetOuterHtml(
       const Selector& selector,

components/autofill_assistant/browser/web_controller.cc

@@ -11,8 +11,10 @@
 
 #include "base/bind_helpers.h"
 #include "base/callback.h"
+#include "base/i18n/char_iterator.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
 #include "build/build_config.h"
 #include "components/autofill/content/browser/content_autofill_driver.h"
@@ -1036,17 +1038,31 @@ void WebController::SetFieldValue(const Selector& selector,
                                   bool simulate_key_presses,
                                   base::OnceCallback<void(bool)> callback) {
   if (simulate_key_presses) {
+    std::vector<std::string> utf8_chars;
+    base::i18n::UTF8CharIterator iter(&value);
+    while (!iter.end()) {
+      wchar_t wide_char = iter.get();
+      std::string utf8_char;
+      if (!base::WideToUTF8(&wide_char, 1, &utf8_char)) {
+        DLOG(ERROR) << "Failed to convert character to UTF-8: " << wide_char;
+        OnResult(false, std::move(callback));
+        return;
+      }
+
+      utf8_chars.push_back(utf8_char);
+      iter.Advance();
+    }
+
     // We first clear the field value, and then simulate the key presses.
     // TODO(crbug.com/806868): Disable keyboard during this action and then
     // reset to previous state.
     InternalSetFieldValue(
         selector, "",
-        base::BindOnce(&WebController::OnClearFieldForDispatchKeyEvent,
-                       weak_ptr_factory_.GetWeakPtr(), selector, value,
+        base::BindOnce(&WebController::OnClearFieldForSendKeyboardInput,
+                       weak_ptr_factory_.GetWeakPtr(), selector, utf8_chars,
                        std::move(callback)));
     return;
   }
-
   InternalSetFieldValue(selector, value, std::move(callback));
 }
 
@@ -1061,112 +1077,59 @@ void WebController::InternalSetFieldValue(
                              std::move(callback)));
 }
 
-void WebController::OnClearFieldForDispatchKeyEvent(
+void WebController::OnClearFieldForSendKeyboardInput(
     const Selector& selector,
-    const std::string& value,
+    const std::vector<std::string>& utf8_chars,
     base::OnceCallback<void(bool)> callback,
     bool clear_status) {
   if (!clear_status) {
     OnResult(false, std::move(callback));
     return;
   }
-
-  // TODO(crbug.com/806868): Substitute mouse click with touch tap.
-  //
-  // Note that 'KeyDown' will not be handled by the element immediately after
-  // touch tap. Add ~1 second delay before 'DispatchKeyDownEvent' in
-  // 'OnClickOrTapElementForDispatchKeyEvent' solved the problem, needs more
-  // investigation for this timing issue. One possible reason is that events
-  // from different devices are not guarranteed to be handled in order (needs a
-  // way to make sure previous events have been handled).
-  ClickElement(selector,
-               base::BindOnce(&WebController::OnClickElementForDispatchKeyEvent,
-                              weak_ptr_factory_.GetWeakPtr(), value,
-                              std::move(callback)));
-}
-
-void WebController::OnClickElementForDispatchKeyEvent(
-    const std::string& value,
-    base::OnceCallback<void(bool)> callback,
-    bool click_status) {
-  if (!click_status) {
-    OnResult(/* result= */ false, std::move(callback));
-    return;
-  }
-  DispatchKeyDownEvent(value, 0, std::move(callback));
+  SendKeyboardInput(selector, utf8_chars, std::move(callback));
 }
 
 void WebController::OnClickElementForSendKeyboardInput(
-    const std::string& text,
+    const std::vector<std::string>& utf8_chars,
     base::OnceCallback<void(bool)> callback,
     bool click_status) {
   if (!click_status) {
     OnResult(false, std::move(callback));
     return;
   }
-  DispatchKeyboardTextDownEvent(text, std::move(callback));
+  DispatchKeyboardTextDownEvent(utf8_chars, 0, std::move(callback));
 }
 
-void WebController::DispatchKeyDownEvent(
-    const std::string& value,
+void WebController::DispatchKeyboardTextDownEvent(
+    const std::vector<std::string>& utf8_chars,
     size_t index,
     base::OnceCallback<void(bool)> callback) {
-  if (index >= value.size()) {
+  if (index >= utf8_chars.size()) {
     OnResult(true, std::move(callback));
     return;
   }
 
-  devtools_client_->GetInput()->DispatchKeyEvent(
-      input::DispatchKeyEventParams::Builder()
-          .SetType(input::DispatchKeyEventType::KEY_DOWN)
-          .SetText(std::string(1, value[index]))
-          .Build(),
-      base::BindOnce(&WebController::DispatchKeyUpEvent,
-                     weak_ptr_factory_.GetWeakPtr(), value, index,
-                     std::move(callback)));
-}
-
-void WebController::DispatchKeyUpEvent(
-    const std::string& value,
-    size_t index,
-    base::OnceCallback<void(bool)> callback) {
-  DCHECK_LT(index, value.size());
-  devtools_client_->GetInput()->DispatchKeyEvent(
-      input::DispatchKeyEventParams::Builder()
-          .SetType(input::DispatchKeyEventType::KEY_UP)
-          .SetText(std::string(1, value[index]))
-          .Build(),
-      base::BindOnce(&WebController::OnDispatchKeyUpEvent,
-                     weak_ptr_factory_.GetWeakPtr(), value, index,
-                     std::move(callback)));
-}
-
-void WebController::OnDispatchKeyUpEvent(
-    const std::string& value,
-    size_t index,
-    base::OnceCallback<void(bool)> callback) {
-  DispatchKeyDownEvent(value, index + 1, std::move(callback));
-}
-
-void WebController::DispatchKeyboardTextDownEvent(
-    const std::string& text,
-    base::OnceCallback<void(bool)> callback) {
   devtools_client_->GetInput()->DispatchKeyEvent(
       CreateKeyEventParamsFromText(
-          autofill_assistant::input::DispatchKeyEventType::KEY_DOWN, text),
+          autofill_assistant::input::DispatchKeyEventType::KEY_DOWN,
+          utf8_chars[index]),
       base::BindOnce(&WebController::DispatchKeyboardTextUpEvent,
-                     weak_ptr_factory_.GetWeakPtr(), text,
+                     weak_ptr_factory_.GetWeakPtr(), utf8_chars, index,
                      std::move(callback)));
 }
 
 void WebController::DispatchKeyboardTextUpEvent(
-    const std::string& text,
+    const std::vector<std::string>& utf8_chars,
+    size_t index,
     base::OnceCallback<void(bool)> callback) {
+  DCHECK_LT(index, utf8_chars.size());
   devtools_client_->GetInput()->DispatchKeyEvent(
       CreateKeyEventParamsFromText(
-          autofill_assistant::input::DispatchKeyEventType::KEY_UP, text),
-      base::BindOnce(&WebController::OnDispatchKeyboardTextUpEvent,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+          autofill_assistant::input::DispatchKeyEventType::KEY_UP,
+          utf8_chars[index]),
+      base::BindOnce(&WebController::DispatchKeyboardTextDownEvent,
+                     weak_ptr_factory_.GetWeakPtr(), utf8_chars, index + 1,
+                     std::move(callback)));
 }
 
 auto WebController::CreateKeyEventParamsFromText(
@@ -1177,11 +1140,6 @@ auto WebController::CreateKeyEventParamsFromText(
   return params;
 }
 
-void WebController::OnDispatchKeyboardTextUpEvent(
-    base::OnceCallback<void(bool)> callback) {
-  OnResult(/* result= */ true, std::move(callback));
-}
-
 void WebController::OnPressKeyboard(
     int key_code,
     base::OnceCallback<void(bool)> callback,
@@ -1274,26 +1232,27 @@ void WebController::OnSetAttribute(
   OnResult(result && !result->HasExceptionDetails(), std::move(callback));
 }
 
-void WebController::SendKeyboardInput(const Selector& selector,
-                                      const std::string& text,
-                                      base::OnceCallback<void(bool)> callback) {
+void WebController::SendKeyboardInput(
+    const Selector& selector,
+    const std::vector<std::string>& utf8_chars,
+    base::OnceCallback<void(bool)> callback) {
   DCHECK(!selector.empty());
   FindElement(selector,
               /* strict_mode= */ true,
               base::BindOnce(&WebController::OnFindElementForSendKeyboardInput,
-                             weak_ptr_factory_.GetWeakPtr(), selector, text,
-                             std::move(callback)));
+                             weak_ptr_factory_.GetWeakPtr(), selector,
+                             utf8_chars, std::move(callback)));
 }
 
 void WebController::OnFindElementForSendKeyboardInput(
     const Selector& selector,
-    const std::string& text,
+    const std::vector<std::string>& utf8_chars,
     base::OnceCallback<void(bool)> callback,
     std::unique_ptr<FindElementResult> element_result) {
-  ClickElement(selector,
-               base::BindOnce(
-                   &WebController::OnClickElementForSendKeyboardInput,
-                   weak_ptr_factory_.GetWeakPtr(), text, std::move(callback)));
+  ClickElement(selector, base::BindOnce(
+                             &WebController::OnClickElementForSendKeyboardInput,
+                             weak_ptr_factory_.GetWeakPtr(), utf8_chars,
+                             std::move(callback)));
 }
 
 void WebController::GetOuterHtml(

components/autofill_assistant/browser/web_controller.h

@@ -112,10 +112,11 @@ class WebController {
                             const std::string& value,
                             base::OnceCallback<void(bool)> callback);
 
-  // Sets the keyboard focus to |selector| and inputs the specified text.
+  // Sets the keyboard focus to |selector| and inputs the specified UTF-8
+  // characters in the specified order.
   // Returns the result through |callback|.
   virtual void SendKeyboardInput(const Selector& selector,
-                                 const std::string& text,
+                                 const std::vector<std::string>& utf8_chars,
                                  base::OnceCallback<void(bool)> callback);
 
   // Return the outerHTML of |selector|.
@@ -381,31 +382,20 @@ class WebController {
   void InternalSetFieldValue(const Selector& selector,
                              const std::string& value,
                              base::OnceCallback<void(bool)> callback);
-  void OnClearFieldForDispatchKeyEvent(const Selector& selector,
-                                       const std::string& value,
-                                       base::OnceCallback<void(bool)> callback,
-                                       bool clear_status);
-  void OnClickElementForDispatchKeyEvent(
-      const std::string& value,
+  void OnClearFieldForSendKeyboardInput(
+      const Selector& selector,
+      const std::vector<std::string>& utf8_chars,
       base::OnceCallback<void(bool)> callback,
-      bool click_status);
+      bool clear_status);
   void OnClickElementForSendKeyboardInput(
-      const std::string& text,
+      const std::vector<std::string>& utf8_chars,
       base::OnceCallback<void(bool)> callback,
       bool click_status);
-  void DispatchKeyDownEvent(const std::string& value,
-                            size_t index,
-                            base::OnceCallback<void(bool)> callback);
-  void DispatchKeyUpEvent(const std::string& value,
-                          size_t index,
-                          base::OnceCallback<void(bool)> callback);
-  void OnDispatchKeyUpEvent(const std::string& value,
-                            size_t index,
-                            base::OnceCallback<void(bool)> callback);
-  void OnDispatchKeyboardTextUpEvent(base::OnceCallback<void(bool)> callback);
-  void DispatchKeyboardTextDownEvent(const std::string& text,
+  void DispatchKeyboardTextDownEvent(const std::vector<std::string>& utf8_chars,
+                                     size_t index,
                                      base::OnceCallback<void(bool)> callback);
-  void DispatchKeyboardTextUpEvent(const std::string& text,
+  void DispatchKeyboardTextUpEvent(const std::vector<std::string>& utf8_chars,
+                                   size_t index,
                                    base::OnceCallback<void(bool)> callback);
   void OnFindElementForSetAttribute(
       const std::vector<std::string>& attribute,
@@ -416,7 +406,7 @@ class WebController {
                       std::unique_ptr<runtime::CallFunctionOnResult> result);
   void OnFindElementForSendKeyboardInput(
       const Selector& selector,
-      const std::string& text,
+      const std::vector<std::string>& utf8_chars,
       base::OnceCallback<void(bool)> callback,
       std::unique_ptr<FindElementResult> element_result);
   void OnPressKeyboard(int key_code,

components/autofill_assistant/browser/web_controller_browsertest.cc

@@ -292,11 +292,12 @@ class WebControllerBrowserTest : public content::ContentBrowserTest,
     std::move(done_callback).Run();
   }
 
-  bool SendKeyboardInput(const Selector& selector, const std::string& text) {
+  bool SendKeyboardInput(const Selector& selector,
+                         const std::vector<std::string>& text_parts) {
     base::RunLoop run_loop;
     bool result;
     web_controller_->SendKeyboardInput(
-        selector, text,
+        selector, text_parts,
         base::BindOnce(&WebControllerBrowserTest::OnSendKeyboardInput,
                        base::Unretained(this), run_loop.QuitClosure(),
                        &result));
@@ -658,10 +659,10 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, GetAndSetFieldValue) {
   a_selector.selectors.clear();
   a_selector.selectors.emplace_back("#uppercase_input");
   selectors.emplace_back(a_selector);
-  EXPECT_TRUE(
-      SetFieldValue(a_selector, "baz", /* simulate_key_presses= */ true));
+  EXPECT_TRUE(SetFieldValue(a_selector, /* Zürich */ "Z\xc3\xbcrich",
+                            /* simulate_key_presses= */ true));
   expected_values.clear();
-  expected_values.emplace_back("BAZ");
+  expected_values.emplace_back(/* ZÜRICH */ "Z\xc3\x9cRICH");
   GetFieldsValue(selectors, expected_values);
 
   selectors.clear();
@@ -677,17 +678,15 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, GetAndSetFieldValue) {
 }
 
 IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, SendKeyboardInput) {
-  std::vector<std::string> input = {
-      "Z", /* letter u with diaeresis */ "\xc3\xbc", "r", "i", "c", "h", "\r"};
+  std::vector<std::string> text_parts = {
+      "Z", /* ü */ "\xc3\xbc", "r", "i", "c", "h", "\r"};
   std::vector<std::string> expected_values = {"Z\xc3\xbcrich"};
 
   std::vector<Selector> selectors;
   Selector a_selector;
   a_selector.selectors.emplace_back("#input6");
   selectors.emplace_back(a_selector);
-  for (const auto& text : input) {
-    EXPECT_TRUE(SendKeyboardInput(a_selector, text));
-  }
+  EXPECT_TRUE(SendKeyboardInput(a_selector, text_parts));
   GetFieldsValue(selectors, expected_values);
 }