[AutofillAssistant] Presave generated password

This CL intorduces presaving of generated password.

Bug: 1064264

Change-Id: I7fd18722a2eecb883c0f62bbc7d0aaed764e37b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2153109
Commit-Queue: Milica Selakovic <selakovic@google.com>
Reviewed-by: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#762818}

components/autofill_assistant/browser/actions/generate_password_for_form_field_action.cc

@@ -53,13 +53,35 @@ void GeneratePasswordForFormFieldAction::OnGetFormAndFieldDataForGeneration(
   if (!status.ok()) {
     EndAction(status);
   }
+
+  if (!delegate_->GetUserData()->selected_login_.has_value()) {
+    VLOG(1) << "GeneratePasswordForFormFieldAction: requested login details "
+               "not available in client memory.";
+    EndAction(ClientStatus(PRECONDITION_FAILED));
+    return;
+  }
+
   uint64_t max_length = field_data.max_length;
   std::string password = delegate_->GetWebsiteLoginFetcher()->GeneratePassword(
       autofill::CalculateFormSignature(form_data),
       autofill::CalculateFieldSignatureForField(field_data), max_length);
+
   delegate_->WriteUserData(base::BindOnce(
       &GeneratePasswordForFormFieldAction::StoreGeneratedPasswordToUserData,
       weak_ptr_factory_.GetWeakPtr(), memory_key, password));
+
+  // Presaving stores a generated password with empty username for the cases
+  // when Chrome misses or misclassifies a successful submission. Thus, even if
+  // a site saves/updates the password and Chrome doesn't, the generated
+  // password will be in the password store.
+  // Ideally, a generated password should be presaved after form filling.
+  // Otherwise, if filling fails and submission cannot happen for sure, the
+  // presaved password is pointless.
+  delegate_->GetWebsiteLoginFetcher()->PresaveGeneratedPassword(
+      *delegate_->GetUserData()->selected_login_, password, form_data,
+      base::BindOnce(&GeneratePasswordForFormFieldAction::EndAction,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     ClientStatus(ACTION_APPLIED)));
 }
 
 void GeneratePasswordForFormFieldAction::StoreGeneratedPasswordToUserData(
@@ -69,8 +91,6 @@ void GeneratePasswordForFormFieldAction::StoreGeneratedPasswordToUserData(
     UserData::FieldChange* field_change) {
   DCHECK(user_data);
   user_data->additional_values_[memory_key] = SimpleValue(generated_password);
-
-  EndAction(ClientStatus(ACTION_APPLIED));
 }
 
 void GeneratePasswordForFormFieldAction::EndAction(const ClientStatus& status) {

components/autofill_assistant/browser/actions/generate_password_for_form_field_action_unittest.cc

@@ -15,7 +15,9 @@
 #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 kGeneratedPassword[] = "m-W2b-_.7Fu9A.A";
 const char kMemoryKeyForGeneratedPassword[] = "memory-key-for-generation";
 }  // namespace
@@ -32,41 +34,47 @@ using ::testing::Return;
 class GeneratePasswordForFormFieldActionTest : public testing::Test {
  public:
   void SetUp() override {
-    generate_password_proto_ =
-        proto_.mutable_generate_password_for_form_field();
-    generate_password_proto_->mutable_element()->add_selectors(kFakeSelector);
-    generate_password_proto_->mutable_element()->set_visibility_requirement(
-        MUST_BE_VISIBLE);
     ON_CALL(mock_action_delegate_, WriteUserData)
         .WillByDefault(
             RunOnceCallback<0>(&user_data_, /* field_change = */ nullptr));
     ON_CALL(mock_action_delegate_, GetWebsiteLoginFetcher)
         .WillByDefault(Return(&mock_website_login_fetcher_));
 
+    ON_CALL(mock_action_delegate_, GetUserData)
+        .WillByDefault(Return(&user_data_));
+
     ON_CALL(mock_website_login_fetcher_, GetGeneratedPassword())
         .WillByDefault(Return(kGeneratedPassword));
 
-    fake_selector_ = Selector({kFakeSelector}).MustBeVisible();
+    user_data_.selected_login_ =
+        base::make_optional<WebsiteLoginFetcher::Login>(GURL(kFakeUrl),
+                                                        kFakeUsername);
   }
 
  protected:
-  Selector fake_selector_;
   MockActionDelegate mock_action_delegate_;
   MockWebsiteLoginFetcher mock_website_login_fetcher_;
   base::MockCallback<Action::ProcessActionCallback> callback_;
   ActionProto proto_;
-  GeneratePasswordForFormFieldProto* generate_password_proto_;
   UserData user_data_;
 };
 
 TEST_F(GeneratePasswordForFormFieldActionTest, GeneratedPassword) {
-  generate_password_proto_->set_memory_key(kMemoryKeyForGeneratedPassword);
+  GeneratePasswordForFormFieldProto* generate_password_proto =
+      proto_.mutable_generate_password_for_form_field();
+  generate_password_proto->mutable_element()->add_selectors(kFakeSelector);
+  generate_password_proto->mutable_element()->set_visibility_requirement(
+      MUST_BE_VISIBLE);
+  generate_password_proto->set_memory_key(kMemoryKeyForGeneratedPassword);
+
+  Selector fake_selector = Selector({kFakeSelector}).MustBeVisible();
 
   GeneratePasswordForFormFieldAction action(&mock_action_delegate_, proto_);
 
-  EXPECT_CALL(
-      callback_,
-      Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
+  EXPECT_CALL(mock_website_login_fetcher_,
+              OnPresaveGeneratedPassword(_, kGeneratedPassword, _, _))
+      .Times(1);
+
   action.ProcessAction(callback_.Get());
   EXPECT_EQ(kGeneratedPassword,
             user_data_.additional_values_[kMemoryKeyForGeneratedPassword]

components/autofill_assistant/browser/mock_website_login_fetcher.h

@@ -45,6 +45,24 @@ class MockWebsiteLoginFetcher : public WebsiteLoginFetcher {
   }
 
   MOCK_METHOD0(GetGeneratedPassword, std::string());
+
+  void PresaveGeneratedPassword(const Login& login,
+                                const std::string& password,
+                                const autofill::FormData& form_data,
+                                base::OnceCallback<void()> callback) override {
+    OnPresaveGeneratedPassword(login, password, form_data, callback);
+  }
+
+  MOCK_METHOD4(OnPresaveGeneratedPassword,
+               void(const Login& login,
+                    const std::string& password,
+                    const autofill::FormData& form_data,
+                    base::OnceCallback<void()>&));
+
+  void CommitGeneratedPassword() override { OnCommitGeneratedPassword(); }
+
+  MOCK_METHOD0(OnCommitGeneratedPassword, void());
+
   DISALLOW_COPY_AND_ASSIGN(MockWebsiteLoginFetcher);
 };
 

components/autofill_assistant/browser/service.proto

@@ -1604,7 +1604,8 @@ message SetFormFieldValueProto {
 
 // Asks the password manager to generate a suitable password for |element|. The
 // generated password can be filled in subsequent SetFormFieldValueProto
-// actions.
+// actions. Generated password will be presaved to password store,
+// login details need to be available in the client memory for this action.
 message GeneratePasswordForFormFieldProto {
   // A reference to the form element for which to generate a password.
   optional ElementReferenceProto element = 1;

components/autofill_assistant/browser/website_login_fetcher.cc

@@ -13,4 +13,4 @@ WebsiteLoginFetcher::Login::Login(const GURL& _origin,
 WebsiteLoginFetcher::Login::Login(const Login& other) = default;
 WebsiteLoginFetcher::Login::~Login() = default;
 
-}  // namespace autofill_assistant
\ No newline at end of file
+}  // namespace autofill_assistant

components/autofill_assistant/browser/website_login_fetcher.h

@@ -11,6 +11,7 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/signatures_util.h"
 #include "url/gurl.h"
 
@@ -52,6 +53,17 @@ class WebsiteLoginFetcher {
                                        autofill::FieldSignature field_signature,
                                        uint64_t max_length) = 0;
 
+  // Presaves generated passwod for the form. Password will be saved after
+  // successful form submission.
+  virtual void PresaveGeneratedPassword(
+      const Login& login,
+      const std::string& password,
+      const autofill::FormData& form_data,
+      base::OnceCallback<void()> callback) = 0;
+
+  // Commits the presaved passwod to the store.
+  virtual void CommitGeneratedPassword() = 0;
+
   DISALLOW_COPY_AND_ASSIGN(WebsiteLoginFetcher);
 };
 

components/autofill_assistant/browser/website_login_fetcher_impl.cc

@@ -8,13 +8,35 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
 #include "components/password_manager/core/browser/form_fetcher_impl.h"
+#include "components/password_manager/core/browser/password_form_metrics_recorder.h"
 #include "components/password_manager/core/browser/password_generation_frame_helper.h"
+#include "components/password_manager/core/browser/password_manager_client.h"
 #include "components/password_manager/core/browser/password_manager_driver.h"
+#include "components/password_manager/core/browser/password_save_manager_impl.h"
+#include "components/password_manager/core/browser/votes_uploader.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace autofill_assistant {
 
+namespace {
+
+// Creates a |PasswordForm| with minimal initialization (origin, username,
+// password).
+autofill::PasswordForm CreatePasswordForm(
+    const WebsiteLoginFetcher::Login& login,
+    const std::string& password) {
+  autofill::PasswordForm form;
+  form.signon_realm = login.origin.spec();
+  form.origin = login.origin.GetOrigin();
+  form.username_value = base::UTF8ToUTF16(login.username);
+  form.password_value = base::UTF8ToUTF16(password);
+
+  return form;
+}
+
+}  // namespace
+
 // Represents a pending form fetcher request which will notify the
 // |WebsiteLoginFetcherImpl| when finished.
 class WebsiteLoginFetcherImpl::PendingRequest
@@ -137,8 +159,76 @@ class WebsiteLoginFetcherImpl::PendingFetchPasswordRequest
   base::OnceCallback<void(bool, std::string)> callback_;
 };
 
+// A request to update store with new password for a login.
+class WebsiteLoginFetcherImpl::UpdatePasswordRequest
+    : public password_manager::FormFetcher::Consumer {
+ public:
+  UpdatePasswordRequest(const Login& login,
+                        const std::string& password,
+                        const autofill::FormData& form_data,
+                        password_manager::PasswordManagerClient* client,
+                        base::OnceCallback<void()> presaving_completed_callback)
+      : password_form_(CreatePasswordForm(login, password)),
+        form_data_(form_data),
+        client_(client),
+        presaving_completed_callback_(std::move(presaving_completed_callback)),
+        password_save_manager_(password_manager::PasswordSaveManagerImpl::
+                                   CreatePasswordSaveManagerImpl(client)),
+        metrics_recorder_(
+            base::MakeRefCounted<password_manager::PasswordFormMetricsRecorder>(
+                client->IsMainFrameSecure(),
+                client->GetUkmSourceId())),
+        votes_uploader_(client, true /* is_possible_change_password_form */) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+    password_manager::PasswordStore::FormDigest digest(
+        autofill::PasswordForm::Scheme::kHtml, login.origin.spec(), GURL());
+    form_fetcher_ = std::make_unique<password_manager::FormFetcherImpl>(
+        digest, client, true);
+  }
+
+  // Password will be presaved when |form_fetcher_| completes fetching.
+  void FetchAndPresave() {
+    form_fetcher_->Fetch();
+    form_fetcher_->AddConsumer(this);
+  }
+
+  void CommitGeneratedPassword() {
+    password_save_manager_->Save(form_data_ /* observed_form */,
+                                 password_form_);
+  }
+
+  // password_manager::FormFetcher::Consumer
+  void OnFetchCompleted() override {
+    password_save_manager_->Init(client_, form_fetcher_.get(),
+                                 metrics_recorder_, &votes_uploader_);
+    password_save_manager_->PresaveGeneratedPassword(password_form_);
+    password_save_manager_->CreatePendingCredentials(
+        password_form_, form_data_ /* observed_form */,
+        form_data_ /* submitted_form */, false /* is_http_auth */,
+        false /* is_credential_api_save */);
+
+    if (presaving_completed_callback_) {
+      std::move(presaving_completed_callback_).Run();
+    }
+  }
+
+ private:
+  const autofill::PasswordForm password_form_;
+  const autofill::FormData form_data_;
+  password_manager::PasswordManagerClient* const client_ = nullptr;
+  // This callback will execute when presaving is completed.
+  base::OnceCallback<void()> presaving_completed_callback_;
+  const std::unique_ptr<password_manager::PasswordSaveManager>
+      password_save_manager_;
+  scoped_refptr<password_manager::PasswordFormMetricsRecorder>
+      metrics_recorder_;
+  password_manager::VotesUploader votes_uploader_;
+  std::unique_ptr<password_manager::FormFetcher> form_fetcher_;
+};
+
 WebsiteLoginFetcherImpl::WebsiteLoginFetcherImpl(
-    const password_manager::PasswordManagerClient* client,
+    password_manager::PasswordManagerClient* client,
     password_manager::PasswordManagerDriver* driver)
     : client_(client), driver_(driver), weak_ptr_factory_(this) {}
 
@@ -180,6 +270,26 @@ std::string WebsiteLoginFetcherImpl::GeneratePassword(
           max_length));
 }
 
+void WebsiteLoginFetcherImpl::PresaveGeneratedPassword(
+    const Login& login,
+    const std::string& password,
+    const autofill::FormData& form_data,
+    base::OnceCallback<void()> callback) {
+  DCHECK(!update_password_request_);
+  update_password_request_ = std::make_unique<UpdatePasswordRequest>(
+      login, password, form_data, client_, std::move(callback));
+
+  update_password_request_->FetchAndPresave();
+}
+
+void WebsiteLoginFetcherImpl::CommitGeneratedPassword() {
+  DCHECK(update_password_request_);
+
+  update_password_request_->CommitGeneratedPassword();
+
+  update_password_request_.reset();
+}
+
 void WebsiteLoginFetcherImpl::OnRequestFinished(const PendingRequest* request) {
   base::EraseIf(pending_requests_, [request](const auto& candidate_request) {
     return candidate_request.get() == request;

components/autofill_assistant/browser/website_login_fetcher_impl.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
+#include "components/autofill/core/common/password_form.h"
 #include "components/autofill_assistant/browser/website_login_fetcher.h"
 
 namespace password_manager {
@@ -20,7 +21,7 @@ namespace autofill_assistant {
 // wraps access to the Chrome password manager.
 class WebsiteLoginFetcherImpl : public WebsiteLoginFetcher {
  public:
-  WebsiteLoginFetcherImpl(const password_manager::PasswordManagerClient* client,
+  WebsiteLoginFetcherImpl(password_manager::PasswordManagerClient* client,
                           password_manager::PasswordManagerDriver* driver);
   ~WebsiteLoginFetcherImpl() override;
 
@@ -35,16 +36,29 @@ class WebsiteLoginFetcherImpl : public WebsiteLoginFetcher {
                                autofill::FieldSignature field_signature,
                                uint64_t max_length) override;
 
+  void PresaveGeneratedPassword(const Login& login,
+                                const std::string& password,
+                                const autofill::FormData& form_data,
+                                base::OnceCallback<void()> callback) override;
+
+  void CommitGeneratedPassword() override;
+
  private:
   class PendingRequest;
   class PendingFetchLoginsRequest;
   class PendingFetchPasswordRequest;
+  class UpdatePasswordRequest;
 
   void OnRequestFinished(const PendingRequest* request);
 
-  const password_manager::PasswordManagerClient* client_;
+  password_manager::PasswordManagerClient* const client_;
+
+  password_manager::PasswordManagerDriver* const driver_;
 
-  password_manager::PasswordManagerDriver* driver_;
+  // Update password request will be created in PresaveGeneratedPassword and
+  // released in CommitGeneratedPassword after committing presaved password to
+  // password store.
+  std::unique_ptr<UpdatePasswordRequest> update_password_request_;
 
   // Fetch requests owned by the password manager, released when they are
   // finished.