[Autofill Assistant] Adds action to use login credentials.

This CL extends SetFormFieldValueAction such that it can also fill a selector with a previously stored username or password. To this end, the action is refactored to allow for better code reuse and readability. Also, the CL adds unit tests (not just for the new functionality, but for the entire action).


Bug: b/128833341
Change-Id: I43e57bee5681047f1df954040f62265e0ab44f28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1728560
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Reviewed-by: Stephane Zermatten <szermatt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689877}

components/autofill_assistant/browser/BUILD.gn

@@ -179,6 +179,7 @@ source_set("unit_tests") {
     "actions/mock_action_delegate.h",
     "actions/popup_message_action_unittest.cc",
     "actions/prompt_action_unittest.cc",
+    "actions/set_form_field_value_action_unittest.cc",
     "actions/wait_for_document_action_unittest.cc",
     "actions/wait_for_dom_action_unittest.cc",
     "batch_element_checker_unittest.cc",
@@ -193,6 +194,8 @@ source_set("unit_tests") {
     "mock_service.h",
     "mock_web_controller.cc",
     "mock_web_controller.h",
+    "mock_website_login_fetcher.cc",
+    "mock_website_login_fetcher.h",
     "protocol_utils_unittest.cc",
     "retry_timer_unittest.cc",
     "script_executor_unittest.cc",

components/autofill_assistant/browser/actions/mock_action_delegate.h

@@ -146,7 +146,7 @@ class MockActionDelegate : public ActionDelegate {
                      int delay_in_millisecond,
                      base::OnceCallback<void(const ClientStatus&)> callback) {
     OnSetFieldValue(selector, value, callback);
-    OnSetFieldValue(selector, value, delay_in_millisecond, delay_in_millisecond,
+    OnSetFieldValue(selector, value, simulate_key_presses, delay_in_millisecond,
                     callback);
   }
 
@@ -168,11 +168,20 @@ class MockActionDelegate : public ActionDelegate {
                     const std::string& value,
                     base::OnceCallback<void(const ClientStatus&)> callback));
 
-  MOCK_METHOD4(SendKeyboardInput,
+  void SendKeyboardInput(
+      const Selector& selector,
+      const std::vector<UChar32>& codepoints,
+      int delay_in_millisecond,
+      base::OnceCallback<void(const ClientStatus&)> callback) {
+    OnSendKeyboardInput(selector, codepoints, delay_in_millisecond, callback);
+  }
+
+  MOCK_METHOD4(OnSendKeyboardInput,
                void(const Selector& selector,
                     const std::vector<UChar32>& codepoints,
                     int delay_in_millisecond,
-                    base::OnceCallback<void(const ClientStatus&)> callback));
+                    base::OnceCallback<void(const ClientStatus&)>& callback));
+
   MOCK_METHOD2(GetOuterHtml,
                void(const Selector& selector,
                     base::OnceCallback<void(const ClientStatus&,

components/autofill_assistant/browser/actions/set_form_field_value_action.h

@@ -8,9 +8,11 @@
 #include <string>
 #include <vector>
 
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "components/autofill_assistant/browser/actions/action.h"
+#include "components/autofill_assistant/browser/string_conversions_util.h"
 
 namespace autofill_assistant {
 
@@ -22,29 +24,48 @@ class SetFormFieldValueAction : public Action {
   ~SetFormFieldValueAction() override;
 
  private:
+  // A field input as extracted from the proto, but already checked for
+  // validity.
+  struct FieldInput {
+    explicit FieldInput(std::unique_ptr<std::vector<UChar32>> keyboard_input);
+    explicit FieldInput(std::string value);
+    explicit FieldInput(bool use_password);
+    FieldInput(FieldInput&& other);
+    ~FieldInput();
+
+    // The keys to press if either |keycode| or |keyboard_input| is set, else
+    // nullptr.
+    std::unique_ptr<std::vector<UChar32>> keyboard_input = nullptr;
+    // True if the value should be retrieved from the login details in client
+    // memory.
+    bool use_password = false;
+    // The string to input (for all other cases).
+    std::string value;
+  };
+
   // Overrides Action:
   void InternalProcessAction(ProcessActionCallback callback) override;
 
-  void OnWaitForElement(ProcessActionCallback callback,
-                        const Selector& selector,
-                        bool element_found);
+  void OnWaitForElement(bool element_found);
 
-  void OnGetFieldValue(ProcessActionCallback callback,
-                       const Selector& selector,
-                       int next,
+  void OnGetFieldValue(int field_index,
+                       const std::string& requested_value,
                        bool status,
-                       const std::string& value);
+                       const std::string& actual_value);
 
-  void OnSetFieldValue(ProcessActionCallback callback,
-                       const Selector& selector,
-                       int next,
-                       const ClientStatus& status);
+  void OnSetFieldValue(int next, const ClientStatus& status);
 
-  void OnSetFieldValueAndCheckFallback(ProcessActionCallback callback,
-                                       const Selector& selector,
-                                       int next,
+  void OnSetFieldValueAndCheckFallback(int field_index,
+                                       const std::string& requested_value,
                                        const ClientStatus& status);
 
+  void OnGetPassword(int field_index, bool success, std::string password);
+
+  void EndAction(const ClientStatus& status);
+
+  Selector selector_;
+  std::vector<FieldInput> field_inputs_;
+  ProcessActionCallback process_action_callback_;
   base::WeakPtrFactory<SetFormFieldValueAction> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SetFormFieldValueAction);

components/autofill_assistant/browser/actions/set_form_field_value_action_unittest.cc

+// Copyright 2019 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/set_form_field_value_action.h"
+
+#include <string>
+#include <utility>
+
+#include "base/test/gmock_callback_support.h"
+#include "base/test/mock_callback.h"
+#include "components/autofill_assistant/browser/actions/mock_action_delegate.h"
+#include "components/autofill_assistant/browser/client_memory.h"
+#include "components/autofill_assistant/browser/client_status.h"
+#include "components/autofill_assistant/browser/mock_website_login_fetcher.h"
+#include "components/autofill_assistant/browser/string_conversions_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace {
+const char kFakeUrl[] = "https://www.example.com";
+const char kFakeSelector[] = "#some_selector";
+const char kFakeUsername[] = "user@example.com";
+const char kFakePassword[] = "example_password";
+}  // namespace
+
+namespace autofill_assistant {
+namespace {
+
+using ::base::test::RunOnceCallback;
+using ::testing::_;
+using ::testing::InSequence;
+using ::testing::Pointee;
+using ::testing::Property;
+using ::testing::Return;
+
+class SetFormFieldValueActionTest : public testing::Test {
+ public:
+  void SetUp() override {
+    set_form_field_proto_ = proto_.mutable_set_form_value();
+    set_form_field_proto_->mutable_element()->add_selectors(kFakeSelector);
+    set_form_field_proto_->mutable_element()->set_visibility_requirement(
+        MUST_BE_VISIBLE);
+    ON_CALL(mock_action_delegate_, GetClientMemory)
+        .WillByDefault(Return(&client_memory_));
+    ON_CALL(mock_action_delegate_, GetWebsiteLoginFetcher)
+        .WillByDefault(Return(&mock_website_login_fetcher_));
+    ON_CALL(mock_action_delegate_, OnShortWaitForElement(_, _))
+        .WillByDefault(RunOnceCallback<1>(true));
+    ON_CALL(mock_action_delegate_, OnSetFieldValue(_, _, _, _, _))
+        .WillByDefault(RunOnceCallback<4>(OkClientStatus()));
+
+    ON_CALL(mock_website_login_fetcher_, OnGetLoginsForUrl(_, _))
+        .WillByDefault(
+            RunOnceCallback<1>(std::vector<WebsiteLoginFetcher::Login>{
+                WebsiteLoginFetcher::Login(GURL(kFakeUrl), kFakeUsername)}));
+    ON_CALL(mock_website_login_fetcher_, OnGetPasswordForLogin(_, _))
+        .WillByDefault(RunOnceCallback<1>(true, kFakePassword));
+    client_memory_.set_selected_login({GURL(kFakeUrl), kFakeUsername});
+    fake_selector_ = Selector({kFakeSelector}).MustBeVisible();
+  }
+
+ protected:
+  Selector fake_selector_;
+  MockActionDelegate mock_action_delegate_;
+  MockWebsiteLoginFetcher mock_website_login_fetcher_;
+  base::MockCallback<Action::ProcessActionCallback> callback_;
+  ActionProto proto_;
+  SetFormFieldValueProto* set_form_field_proto_;
+  ClientMemory client_memory_;
+};
+
+TEST_F(SetFormFieldValueActionTest, RequestedUsernameButNoLoginInClientMemory) {
+  ClientMemory empty_client_memory;
+  ON_CALL(mock_action_delegate_, GetClientMemory)
+      .WillByDefault(Return(&empty_client_memory));
+  auto* value = set_form_field_proto_->add_value();
+  value->set_use_username(true);
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  EXPECT_CALL(callback_, Run(Pointee(Property(&ProcessedActionProto::status,
+                                              PRECONDITION_FAILED))));
+  action.ProcessAction(callback_.Get());
+}
+
+TEST_F(SetFormFieldValueActionTest, RequestedPasswordButNoLoginInClientMemory) {
+  ClientMemory empty_client_memory;
+  ON_CALL(mock_action_delegate_, GetClientMemory)
+      .WillByDefault(Return(&empty_client_memory));
+  auto* value = set_form_field_proto_->add_value();
+  value->set_use_password(true);
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  EXPECT_CALL(callback_, Run(Pointee(Property(&ProcessedActionProto::status,
+                                              PRECONDITION_FAILED))));
+  action.ProcessAction(callback_.Get());
+}
+
+TEST_F(SetFormFieldValueActionTest, RequestedPasswordButPasswordNotAvailable) {
+  ON_CALL(mock_website_login_fetcher_, OnGetPasswordForLogin(_, _))
+      .WillByDefault(RunOnceCallback<1>(false, std::string()));
+  auto* value = set_form_field_proto_->add_value();
+  value->set_use_password(true);
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  EXPECT_CALL(callback_, Run(Pointee(Property(&ProcessedActionProto::status,
+                                              AUTOFILL_INFO_NOT_AVAILABLE))));
+  action.ProcessAction(callback_.Get());
+}
+
+TEST_F(SetFormFieldValueActionTest, NonAsciiKeycode) {
+  auto* value = set_form_field_proto_->add_value();
+  value->set_keycode(UTF8ToUnicode("𠜎")[0]);
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, INVALID_ACTION))));
+  action.ProcessAction(callback_.Get());
+}
+
+TEST_F(SetFormFieldValueActionTest, Username) {
+  auto* value = set_form_field_proto_->add_value();
+  value->set_use_username(true);
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  ON_CALL(mock_action_delegate_, OnGetFieldValue(_, _))
+      .WillByDefault(RunOnceCallback<1>(true, kFakeUsername));
+  EXPECT_CALL(mock_action_delegate_,
+              OnSetFieldValue(fake_selector_, kFakeUsername, _, _, _))
+      .WillOnce(RunOnceCallback<4>(OkClientStatus()));
+
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
+  action.ProcessAction(callback_.Get());
+}
+
+TEST_F(SetFormFieldValueActionTest, Password) {
+  auto* value = set_form_field_proto_->add_value();
+  value->set_use_password(true);
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  ON_CALL(mock_action_delegate_, OnGetFieldValue(_, _))
+      .WillByDefault(RunOnceCallback<1>(true, kFakePassword));
+  EXPECT_CALL(mock_action_delegate_,
+              OnSetFieldValue(fake_selector_, kFakePassword, _, _, _))
+      .WillOnce(RunOnceCallback<4>(OkClientStatus()));
+
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
+  action.ProcessAction(callback_.Get());
+}
+
+TEST_F(SetFormFieldValueActionTest, Keycode) {
+  auto* value = set_form_field_proto_->add_value();
+  value->set_keycode(13);  // carriage return
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  EXPECT_CALL(mock_action_delegate_,
+              OnSendKeyboardInput(fake_selector_, std::vector<int>{13}, _, _))
+      .WillOnce(RunOnceCallback<3>(OkClientStatus()));
+
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
+  action.ProcessAction(callback_.Get());
+}
+
+TEST_F(SetFormFieldValueActionTest, KeyboardInput) {
+  auto* value = set_form_field_proto_->add_value();
+  std::string keyboard_input = "SomeQuery𠜎\r";
+  value->set_keyboard_input(keyboard_input);
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  EXPECT_CALL(
+      mock_action_delegate_,
+      OnSendKeyboardInput(fake_selector_, UTF8ToUnicode(keyboard_input), _, _))
+      .WillOnce(RunOnceCallback<3>(OkClientStatus()));
+
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
+  action.ProcessAction(callback_.Get());
+}
+
+TEST_F(SetFormFieldValueActionTest, Text) {
+  auto* value = set_form_field_proto_->add_value();
+  value->set_text("SomeText𠜎");
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+  ON_CALL(mock_action_delegate_, OnGetFieldValue(_, _))
+      .WillByDefault(RunOnceCallback<1>(true, "SomeText𠜎"));
+  EXPECT_CALL(mock_action_delegate_,
+              OnSetFieldValue(fake_selector_, "SomeText𠜎", _, _, _))
+      .WillOnce(RunOnceCallback<4>(OkClientStatus()));
+
+  EXPECT_CALL(
+      callback_,
+      Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
+  action.ProcessAction(callback_.Get());
+}
+
+// Test that automatic fallback to simulate keystrokes works.
+TEST_F(SetFormFieldValueActionTest, Fallback) {
+  auto* value = set_form_field_proto_->add_value();
+  value->set_text("123");
+  SetFormFieldValueAction action(&mock_action_delegate_, proto_);
+
+  ON_CALL(mock_action_delegate_, OnGetFieldValue(_, _))
+      .WillByDefault(RunOnceCallback<1>(true, ""));
+
+  {
+    InSequence seq;
+    EXPECT_CALL(mock_action_delegate_,
+                OnSetFieldValue(fake_selector_, "123",
+                                /* simulate_key_presses = */ false, _, _));
+    EXPECT_CALL(mock_action_delegate_,
+                OnSetFieldValue(fake_selector_, "123",
+                                /* simulate_key_presses = */ true, _, _));
+  }
+
+  EXPECT_CALL(callback_,
+              Run(Pointee(AllOf(
+                  Property(&ProcessedActionProto::status, ACTION_APPLIED),
+                  Property(&ProcessedActionProto::set_form_field_value_result,
+                           Property(&SetFormFieldValueProto::Result::
+                                        fallback_to_simulate_key_presses,
+                                    true))))));
+
+  action.ProcessAction(callback_.Get());
+}
+
+}  // namespace
+}  // namespace autofill_assistant
\ No newline at end of file

components/autofill_assistant/browser/client_status.cc

@@ -122,6 +122,10 @@ std::ostream& operator<<(std::ostream& out,
       out << "NAVIGATION_ERROR";
       break;
 
+    case ProcessedActionStatusProto::AUTOFILL_INFO_NOT_AVAILABLE:
+      out << "AUTOFILL_INFO_NOT_AVAILABLE";
+      break;
+
       // Intentionally no default case to make compilation fail if a new value
       // was added to the enum but not to this list.
   }

components/autofill_assistant/browser/mock_website_login_fetcher.cc

+// Copyright 2019 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/mock_website_login_fetcher.h"
+
+namespace autofill_assistant {
+
+MockWebsiteLoginFetcher::MockWebsiteLoginFetcher() {}
+MockWebsiteLoginFetcher::~MockWebsiteLoginFetcher() {}
+
+}  // namespace autofill_assistant

components/autofill_assistant/browser/mock_website_login_fetcher.h

+// Copyright 2019 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.
+
+#ifndef COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_MOCK_WEBSITE_LOGIN_FETCHER_H_
+#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_MOCK_WEBSITE_LOGIN_FETCHER_H_
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "components/autofill_assistant/browser/website_login_fetcher.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace autofill_assistant {
+
+// Mock login fetcher for unit tests.
+class MockWebsiteLoginFetcher : public WebsiteLoginFetcher {
+ public:
+  MockWebsiteLoginFetcher();
+  ~MockWebsiteLoginFetcher() override;
+
+  void GetLoginsForUrl(
+      const GURL& url,
+      base::OnceCallback<void(std::vector<Login>)> callback) override {
+    OnGetLoginsForUrl(url, callback);
+  }
+
+  MOCK_METHOD2(OnGetLoginsForUrl,
+               void(const GURL& domain,
+                    base::OnceCallback<void(std::vector<Login>)>&));
+
+  void GetPasswordForLogin(
+      const Login& login,
+      base::OnceCallback<void(bool, std::string)> callback) override {
+    OnGetPasswordForLogin(login, callback);
+  }
+
+  MOCK_METHOD2(OnGetPasswordForLogin,
+               void(const Login& login,
+                    base::OnceCallback<void(bool, std::string)>&));
+
+  DISALLOW_COPY_AND_ASSIGN(MockWebsiteLoginFetcher);
+};
+
+}  // namespace autofill_assistant
+
+#endif  // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_MOCK_WEBSITE_LOGIN_FETCHER_H_

components/autofill_assistant/browser/service.proto

@@ -650,6 +650,10 @@ enum ProcessedActionStatusProto {
 
   // A selector included into the current action is invalid.
   INVALID_SELECTOR = 20;
+
+  // The requested autofill info (e.g., Chrome password manager login) was not
+  // available. It might have been recently deleted.
+  AUTOFILL_INFO_NOT_AVAILABLE = 21;
 }
 
 // The pseudo type values come from
@@ -855,7 +859,8 @@ message UseAddressProto {
   repeated RequiredField required_fields = 6;
 }
 
-// Fill a form with a credit card if there is, otherwise fail this action.
+// Fill a form with a credit card if there is one stored in client memory,
+// otherwise fail this action.
 message UseCreditCardProto {
   // Message used to indicate what form fields should be filled with what
   // information.
@@ -1323,6 +1328,12 @@ message SetFormFieldValueProto {
       // layout. This can also be used for keyboard control sequences such
       // as "\r" or "\t".
       string keyboard_input = 3;
+      // Use the username from the Chrome password manager login previously
+      // selected in a GetPaymentInformationAction.
+      bool use_username = 4;
+      // Use the password from the Chrome password manager login previously
+      // selected in a GetPaymentInformationAction.
+      bool use_password = 5;
     }
   }