[PWD Editing Android] Delegate updates the password in the store

PasswordUpdateDelegate has a new method for saving password changes.
It will be used through the PasswordEditing Bridge in the future to save
changes made in PasswordEntryEditor.
Both PasswordManagerPresenter and the PasswordUpdateDelegate use the
editing method from the newly created password_utils file.

Bug: 377410
Change-Id: I1ee09fba8f815db99370c30d3c84acf32d1bdb35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1787507
Commit-Queue: Ivana Zuzic <izuzic@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697736}

chrome/android/java/src/org/chromium/chrome/browser/preferences/password/PasswordEditingBridge.java

@@ -23,6 +23,21 @@ public class PasswordEditingBridge {
         mNativePasswordEditingBridge = nativePasswordEditingBridge;
     }
 
+    /**
+     * The method edits a password form saved in the password store according to changes performed
+     * in PasswordEntryEditor. The delegate holds all the information about the password form that
+     * was loaded in the PasswordEntryEditor, so there's no need to pass the site, the old username
+     * or the old password to this method. Sometimes the form can have no username (for PSL-matched
+     * credentials), but it has to always have a password.
+     *
+     * @param newUsername that will replace the old one if it's given.
+     * @param newPassword that will replace the old one.
+     */
+    public void editSavedPasswordEntry(String newUsername, String newPassword) {
+        PasswordEditingBridgeJni.get().handleEditSavedPasswordEntry(
+                mNativePasswordEditingBridge, PasswordEditingBridge.this, newUsername, newPassword);
+    }
+
     @CalledByNative
     private static PasswordEditingBridge create(long nativePasswordEditingBridge) {
         return new PasswordEditingBridge(nativePasswordEditingBridge);
@@ -51,5 +66,7 @@ public class PasswordEditingBridge {
     @NativeMethods
     interface Natives {
         void destroy(long nativePasswordEditingBridge, PasswordEditingBridge caller);
+        void handleEditSavedPasswordEntry(long nativePasswordEditingBridge,
+                PasswordEditingBridge caller, String newUsername, String newPassword);
     }
 }

chrome/browser/BUILD.gn

@@ -1069,6 +1069,8 @@ jumbo_split_static_library("browser") {
     "password_manager/password_manager_util_win.h",
     "password_manager/password_store_factory.cc",
     "password_manager/password_store_factory.h",
+    "password_manager/password_store_utils.cc",
+    "password_manager/password_store_utils.h",
     "password_manager/reauth_purpose.h",
     "payments/payment_handler_permission_context.cc",
     "payments/payment_handler_permission_context.h",
@@ -2587,7 +2589,7 @@ jumbo_split_static_library("browser") {
       "android/oom_intervention/oom_intervention_tab_helper.h",
       "android/partner_browser_customizations.cc",
       "android/partner_browser_customizations.h",
-      "android/password_change_delegate.h",
+      "android/password_edit_delegate.h",
       "android/password_editing_bridge.cc",
       "android/password_editing_bridge.h",
       "android/password_ui_view_android.cc",

chrome/browser/android/password_change_delegate.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 CHROME_BROWSER_ANDROID_PASSWORD_CHANGE_DELEGATE_H_
-#define CHROME_BROWSER_ANDROID_PASSWORD_CHANGE_DELEGATE_H_
-
-// An interface used by the native side to launch the entry editor and
-// perform a change on a credential record.
-class PasswordChangeDelegate {
- public:
-  virtual ~PasswordChangeDelegate() = default;
-};
-
-#endif  // CHROME_BROWSER_ANDROID_PASSWORD_CHANGE_DELEGATE_H_

chrome/browser/android/password_edit_delegate.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 CHROME_BROWSER_ANDROID_PASSWORD_EDIT_DELEGATE_H_
+#define CHROME_BROWSER_ANDROID_PASSWORD_EDIT_DELEGATE_H_
+
+#include "base/macros.h"
+#include "base/strings/string16.h"
+
+// The delegate, which is created and destroyed together with the bridge, holds
+// all the information about the password form that was loaded and edited in the
+// PasswordEntryEditor.
+class PasswordEditDelegate {
+ public:
+  PasswordEditDelegate() = default;
+  virtual ~PasswordEditDelegate() = default;
+
+  // The method edits a password form held by the delegate. |new_username| and
+  // |new_password| are user input from the PasswordEntryEditor.
+  virtual void EditSavedPassword(const base::string16& new_username,
+                                 const base::string16& new_password) = 0;
+
+  DISALLOW_COPY_AND_ASSIGN(PasswordEditDelegate);
+};
+
+#endif  // CHROME_BROWSER_ANDROID_PASSWORD_EDIT_DELEGATE_H_

chrome/browser/android/password_editing_bridge.cc

@@ -6,7 +6,7 @@
 
 #include "base/android/jni_string.h"
 #include "chrome/android/chrome_jni_headers/PasswordEditingBridge_jni.h"
-#include "chrome/browser/android/password_change_delegate.h"
+#include "chrome/browser/android/password_edit_delegate.h"
 #include "chrome/browser/android/password_update_delegate.h"
 
 using base::android::ConvertUTF16ToJavaString;
@@ -28,13 +28,13 @@ void PasswordEditingBridge::Destroy(JNIEnv* env,
 void PasswordEditingBridge::LaunchPasswordEntryEditor(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& context,
-    password_manager::PasswordStore* store,
+    Profile* profile,
     const autofill::PasswordForm& password_form) {
   // PasswordEditingBridge will destroy itself when the UI is gone on the Java
   // side.
   PasswordEditingBridge* password_editing_bridge = new PasswordEditingBridge();
-  password_editing_bridge->password_change_delegate_ =
-      std::make_unique<PasswordUpdateDelegate>(store, password_form);
+  password_editing_bridge->password_edit_delegate_ =
+      std::make_unique<PasswordUpdateDelegate>(profile, password_form);
   Java_PasswordEditingBridge_showEditingUI(
       base::android::AttachCurrentThread(),
       password_editing_bridge->java_object_, context,
@@ -42,3 +42,13 @@ void PasswordEditingBridge::LaunchPasswordEntryEditor(
       ConvertUTF16ToJavaString(env, password_form.username_value),
       ConvertUTF16ToJavaString(env, password_form.password_value));
 }
+
+void PasswordEditingBridge::HandleEditSavedPasswordEntry(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& object,
+    const JavaParamRef<jstring>& new_username,
+    const JavaParamRef<jstring>& new_password) {
+  password_edit_delegate_->EditSavedPassword(
+      ConvertJavaStringToUTF16(env, new_username),
+      ConvertJavaStringToUTF16(env, new_password));
+}

chrome/browser/android/password_editing_bridge.h

@@ -17,7 +17,8 @@ namespace autofill {
 struct PasswordForm;
 }
 
-class PasswordChangeDelegate;
+class PasswordEditDelegate;
+class Profile;
 
 // A bridge that allows communication between Android UI and the native
 // side. It can be used to launch the password editing activity from the
@@ -36,15 +37,21 @@ class PasswordEditingBridge {
   static void LaunchPasswordEntryEditor(
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& context,
-      password_manager::PasswordStore* store,
+      Profile* profile,
       const autofill::PasswordForm& password_form);
 
+  void HandleEditSavedPasswordEntry(
+      JNIEnv* env,
+      const base::android::JavaParamRef<jobject>& object,
+      const base::android::JavaParamRef<jstring>& new_username,
+      const base::android::JavaParamRef<jstring>& new_password);
+
  private:
   // The corresponding java object.
   base::android::ScopedJavaGlobalRef<jobject> java_object_;
 
   // The delegate belonging to the bridge.
-  std::unique_ptr<PasswordChangeDelegate> password_change_delegate_;
+  std::unique_ptr<PasswordEditDelegate> password_edit_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(PasswordEditingBridge);
 };

chrome/browser/android/password_ui_view_android.cc

@@ -196,10 +196,7 @@ void PasswordUIViewAndroid::HandleShowPasswordEntryEditingView(
     const JavaParamRef<jobject>& context,
     int index) {
   PasswordEditingBridge::LaunchPasswordEntryEditor(
-      env, context,
-      PasswordStoreFactory::GetForProfile(GetProfile(),
-                                          ServiceAccessType::EXPLICIT_ACCESS)
-          .get(),
+      env, context, GetProfile(),
       *password_manager_presenter_.GetPassword(index));
 }
 

chrome/browser/android/password_update_delegate.cc

@@ -4,9 +4,40 @@
 
 #include "chrome/browser/android/password_update_delegate.h"
 
+#include "base/stl_util.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/password_manager/password_store_utils.h"
+#include "components/password_manager/core/browser/password_store.h"
+
 PasswordUpdateDelegate::PasswordUpdateDelegate(
-    password_manager::PasswordStore* store,
+    Profile* profile,
     const autofill::PasswordForm& password_form)
-    : password_form_(password_form) {}
+    : profile_(profile), password_form_(password_form) {}
 
 PasswordUpdateDelegate::~PasswordUpdateDelegate() = default;
+
+void PasswordUpdateDelegate::EditSavedPassword(
+    const base::string16& new_username,
+    const base::string16& new_password) {
+  DCHECK(!new_password.empty()) << "The password is empty.";
+
+  new_username_ = new_username;
+  new_password_ = new_password;
+  PasswordStoreFactory::GetForProfile(profile_,
+                                      ServiceAccessType::EXPLICIT_ACCESS)
+      ->GetLogins(password_manager::PasswordStore::FormDigest(password_form_),
+                  this);
+}
+
+void PasswordUpdateDelegate::OnGetPasswordStoreResults(
+    std::vector<std::unique_ptr<autofill::PasswordForm>> results) {
+  // Banned password credentials and the ones with invalid origin won't be
+  // edited through this delegate.
+  base::EraseIf(results, [](const auto& form) {
+    return form->blacklisted_by_user || form->IsFederatedCredential();
+  });
+
+  EditSavedPasswords(profile_, results, password_form_.username_value,
+                     password_form_.signon_realm, new_username_,
+                     &new_password_);
+}

chrome/browser/android/password_update_delegate.h

@@ -6,24 +6,37 @@
 #define CHROME_BROWSER_ANDROID_PASSWORD_UPDATE_DELEGATE_H_
 
 #include <memory>
+#include <vector>
 
-#include "chrome/browser/android/password_change_delegate.h"
+#include "base/strings/string16.h"
+#include "chrome/browser/android/password_edit_delegate.h"
 #include "chrome/browser/android/password_editing_bridge.h"
 #include "components/autofill/core/common/password_form.h"
-#include "components/password_manager/core/browser/password_store.h"
+#include "components/password_manager/core/browser/password_store_consumer.h"
 
-// An interface used for launching the entry editor and performing a
-// change on a credential record that already exists in the password store.
-class PasswordUpdateDelegate : public PasswordChangeDelegate {
+class Profile;
+
+// An interface used for launching the entry editor and editing a credential
+// record that already exists in the password store.
+class PasswordUpdateDelegate : public PasswordEditDelegate,
+                               public password_manager::PasswordStoreConsumer {
  public:
-  PasswordUpdateDelegate(password_manager::PasswordStore* store,
+  PasswordUpdateDelegate(Profile* profile,
                          const autofill::PasswordForm& password_form);
   ~PasswordUpdateDelegate() override;
 
+  void EditSavedPassword(const base::string16& new_username,
+                         const base::string16& new_password) override;
+
  private:
-  autofill::PasswordForm password_form_;
+  // PasswordStoreConsumer:
+  void OnGetPasswordStoreResults(
+      std::vector<std::unique_ptr<autofill::PasswordForm>> results) override;
 
-  DISALLOW_COPY_AND_ASSIGN(PasswordUpdateDelegate);
+  Profile* profile_ = nullptr;
+  autofill::PasswordForm password_form_;
+  base::string16 new_username_;
+  base::string16 new_password_;
 };
 
 #endif  // CHROME_BROWSER_ANDROID_PASSWORD_UPDATE_DELEGATE_H_

chrome/browser/android/password_update_delegate_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 "chrome/browser/android/password_update_delegate.h"
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/strings/strcat.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/android/password_update_delegate.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/test/base/testing_profile.h"
+#include "components/password_manager/core/browser/password_manager_test_utils.h"
+#include "components/password_manager/core/browser/test_password_store.h"
+#include "content/public/test/browser_task_environment.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+using testing::ElementsAre;
+using testing::Pair;
+
+namespace {
+
+constexpr char kExampleCom[] = "https://example.com/";
+constexpr char kExampleOrg[] = "https://example.org/";
+constexpr char kNewPass[] = "new pass";
+constexpr char kNewUser[] = "new user";
+constexpr char kPassword[] = "pass";
+constexpr char kPassword2[] = "pass2";
+constexpr char kUsername[] = "user";
+constexpr char kUsername2[] = "user2";
+
+std::vector<std::pair<std::string, std::string>> GetUsernamesAndPasswords(
+    const std::vector<autofill::PasswordForm>& forms) {
+  std::vector<std::pair<std::string, std::string>> result;
+  result.reserve(forms.size());
+  for (const auto& form : forms) {
+    result.emplace_back(base::UTF16ToUTF8(form.username_value),
+                        base::UTF16ToUTF8(form.password_value));
+  }
+
+  return result;
+}
+
+autofill::PasswordForm MakeSavedForm(const GURL& origin,
+                                     base::StringPiece username,
+                                     base::StringPiece password) {
+  autofill::PasswordForm form;
+  form.origin = origin;
+  form.signon_realm = origin.GetOrigin().spec();
+  form.username_element = base::ASCIIToUTF16("Email");
+  form.username_value = base::ASCIIToUTF16(username);
+  form.password_element = base::ASCIIToUTF16("Passwd");
+  form.password_value = base::ASCIIToUTF16(password);
+  return form;
+}
+
+}  // namespace
+
+class PasswordUpdateDelegateTest : public testing::Test {
+ protected:
+  PasswordUpdateDelegateTest() {}
+
+  ~PasswordUpdateDelegateTest() override {
+    store_->ShutdownOnUIThread();
+    task_environment_.RunUntilIdle();
+  }
+
+  std::unique_ptr<PasswordUpdateDelegate> CreateTestDelegate(
+      const GURL& origin,
+      base::StringPiece username,
+      base::StringPiece password) {
+    return std::make_unique<PasswordUpdateDelegate>(
+        &profile_, MakeSavedForm(origin, username, password));
+  }
+
+  const std::vector<autofill::PasswordForm>& GetStoredPasswordsForRealm(
+      base::StringPiece signon_realm) {
+    const auto& stored_passwords =
+        static_cast<const password_manager::TestPasswordStore&>(*store_)
+            .stored_passwords();
+    auto for_realm_it = stored_passwords.find(signon_realm);
+    return for_realm_it->second;
+  }
+
+  password_manager::PasswordStore* GetStore() { return store_.get(); }
+
+  void RunUntilIdle() { task_environment_.RunUntilIdle(); }
+
+ private:
+  content::BrowserTaskEnvironment task_environment_;
+  TestingProfile profile_;
+  scoped_refptr<password_manager::PasswordStore> store_ =
+      base::WrapRefCounted(static_cast<password_manager::PasswordStore*>(
+          PasswordStoreFactory::GetInstance()
+              ->SetTestingFactoryAndUse(
+                  &profile_,
+                  base::BindRepeating(&password_manager::BuildPasswordStore<
+                                      content::BrowserContext,
+                                      password_manager::TestPasswordStore>))
+              .get()));
+};
+
+TEST_F(PasswordUpdateDelegateTest, EditSavedPassword_EditPassword) {
+  GetStore()->AddLogin(MakeSavedForm(GURL(kExampleCom), kUsername, kPassword));
+  RunUntilIdle();
+  std::unique_ptr<PasswordUpdateDelegate> password_update_delegate =
+      CreateTestDelegate(GURL(kExampleCom), kUsername, kPassword);
+  password_update_delegate->EditSavedPassword(base::ASCIIToUTF16(kUsername),
+                                              base::ASCIIToUTF16(kNewPass));
+  RunUntilIdle();
+  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
+              ElementsAre(Pair(kUsername, kNewPass)));
+}
+
+TEST_F(PasswordUpdateDelegateTest, EditSavedPassword_EditUsername) {
+  GetStore()->AddLogin(MakeSavedForm(GURL(kExampleCom), kUsername, kPassword));
+  RunUntilIdle();
+  std::unique_ptr<PasswordUpdateDelegate> password_update_delegate =
+      CreateTestDelegate(GURL(kExampleCom), kUsername, kPassword);
+  password_update_delegate->EditSavedPassword(base::ASCIIToUTF16(kNewUser),
+                                              base::ASCIIToUTF16(kPassword));
+  RunUntilIdle();
+  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
+              ElementsAre(Pair(kNewUser, kPassword)));
+}
+
+TEST_F(PasswordUpdateDelegateTest, EditSavedPassword_EditUsernameAndPassword) {
+  GetStore()->AddLogin(MakeSavedForm(GURL(kExampleCom), kUsername, kPassword));
+  RunUntilIdle();
+  std::unique_ptr<PasswordUpdateDelegate> password_update_delegate =
+      CreateTestDelegate(GURL(kExampleCom), kUsername, kPassword);
+  password_update_delegate->EditSavedPassword(base::ASCIIToUTF16(kNewUser),
+                                              base::ASCIIToUTF16(kNewPass));
+  RunUntilIdle();
+  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
+              ElementsAre(Pair(kNewUser, kNewPass)));
+}
+
+TEST_F(PasswordUpdateDelegateTest,
+       EditSavedPassword_RejectSameUsernameForSameRealm) {
+  GetStore()->AddLogin(MakeSavedForm(GURL(kExampleCom), kUsername, kPassword));
+  GetStore()->AddLogin(
+      MakeSavedForm(GURL(kExampleCom), kUsername2, kPassword2));
+  RunUntilIdle();
+  std::unique_ptr<PasswordUpdateDelegate> password_update_delegate =
+      CreateTestDelegate(GURL(kExampleCom), kUsername, kPassword);
+  password_update_delegate->EditSavedPassword(base::ASCIIToUTF16(kUsername2),
+                                              base::ASCIIToUTF16(kPassword));
+  RunUntilIdle();
+  EXPECT_THAT(
+      GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
+      ElementsAre(Pair(kUsername, kPassword), Pair(kUsername2, kPassword2)));
+}
+
+TEST_F(PasswordUpdateDelegateTest,
+       EditSavedPassword_DontRejectSameUsernameForDifferentRealm) {
+  GetStore()->AddLogin(MakeSavedForm(GURL(kExampleCom), kUsername, kPassword));
+  GetStore()->AddLogin(
+      MakeSavedForm(GURL(kExampleOrg), kUsername2, kPassword2));
+  RunUntilIdle();
+  std::unique_ptr<PasswordUpdateDelegate> password_update_delegate =
+      CreateTestDelegate(GURL(kExampleCom), kUsername, kPassword);
+  password_update_delegate->EditSavedPassword(base::ASCIIToUTF16(kUsername2),
+                                              base::ASCIIToUTF16(kPassword2));
+  RunUntilIdle();
+  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
+              ElementsAre(Pair(kUsername2, kPassword2)));
+  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleOrg)),
+              ElementsAre(Pair(kUsername2, kPassword2)));
+}
+
+TEST_F(PasswordUpdateDelegateTest, EditSavedPassword_UpdateDuplicates) {
+  GetStore()->AddLogin(MakeSavedForm(GURL(base::StrCat({kExampleCom, "pathA"})),
+                                     kUsername, kPassword));
+  GetStore()->AddLogin(MakeSavedForm(GURL(base::StrCat({kExampleCom, "pathB"})),
+                                     kUsername, kPassword));
+  RunUntilIdle();
+  std::unique_ptr<PasswordUpdateDelegate> password_update_delegate =
+      CreateTestDelegate(GURL(kExampleCom), kUsername, kPassword);
+  password_update_delegate->EditSavedPassword(base::ASCIIToUTF16(kNewUser),
+                                              base::ASCIIToUTF16(kNewPass));
+  RunUntilIdle();
+  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
+              ElementsAre(Pair(kNewUser, kNewPass), Pair(kNewUser, kNewPass)));
+}

chrome/browser/password_manager/password_store_utils.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 "chrome/browser/password_manager/password_store_utils.h"
+
+#include "chrome/browser/password_manager/account_storage/account_password_store_factory.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/autofill/core/common/password_form.h"
+#include "components/password_manager/core/browser/password_store.h"
+
+void EditSavedPasswords(
+    Profile* profile,
+    const base::span<const std::unique_ptr<autofill::PasswordForm>> old_forms,
+    const base::string16& old_username,
+    const std::string& signon_realm,
+    const base::string16& new_username,
+    const base::string16* new_password) {
+  DCHECK(!old_forms.empty());
+
+  const bool username_changed = old_username != new_username;
+
+  // In case the username changed, make sure that there exists no other
+  // credential with the same signon_realm and username.
+  if (username_changed &&
+      std::any_of(old_forms.begin(), old_forms.end(),
+                  [&](const auto& old_form) {
+                    return old_form->signon_realm == signon_realm &&
+                           old_form->username_value == new_username;
+                  })) {
+    // TODO(crbug.com/1002021): We shouldn't fail silently.
+    DLOG(ERROR) << "A credential with the same signon_realm and username "
+                   "already exists.";
+    return;
+  }
+
+  // An updated username implies a change in the primary key, thus we need to
+  // make sure to call the right API. Update every entry in the equivalence
+  // class.
+  for (const auto& old_form : old_forms) {
+    scoped_refptr<password_manager::PasswordStore> store =
+        GetPasswordStore(profile, old_form->IsUsingAccountStore());
+
+    if (!store)
+      continue;
+    autofill::PasswordForm new_form = *old_form;
+    if (new_password)
+      new_form.password_value = *new_password;
+
+    if (username_changed) {
+      new_form.username_value = new_username;
+      store->UpdateLoginWithPrimaryKey(new_form, *old_form);
+    } else {
+      store->UpdateLogin(new_form);
+    }
+  }
+}
+
+scoped_refptr<password_manager::PasswordStore> GetPasswordStore(
+    Profile* profile,
+    bool use_account_store) {
+  if (use_account_store) {
+    return AccountPasswordStoreFactory::GetForProfile(
+        profile, ServiceAccessType::EXPLICIT_ACCESS);
+  }
+  return PasswordStoreFactory::GetForProfile(
+      profile, ServiceAccessType::EXPLICIT_ACCESS);
+}

chrome/browser/password_manager/password_store_utils.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.
+
+// This file contains utilities related to password store.
+
+#ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_UTILS_H_
+#define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_UTILS_H_
+
+#include "base/containers/span.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/optional.h"
+#include "base/strings/string16.h"
+
+namespace autofill {
+struct PasswordForm;
+}
+
+namespace password_manager {
+class PasswordStore;
+}
+
+class Profile;
+
+// Changes a credential record in password store. If new_password is null it
+// isn't changed, but if it is non-null it can't be empty.
+void EditSavedPasswords(
+    Profile* profile,
+    const base::span<const std::unique_ptr<autofill::PasswordForm>> old_forms,
+    const base::string16& old_username,
+    const std::string& signon_realm,
+    const base::string16& new_username,
+    const base::string16* new_password);
+
+// Returns the password store associated with the currently active profile.
+scoped_refptr<password_manager::PasswordStore> GetPasswordStore(
+    Profile* profile,
+    bool use_account_store);
+
+#endif  // CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_STORE_UTILS_H_

chrome/browser/ui/passwords/settings/password_manager_presenter.cc

@@ -15,6 +15,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
+#include "base/stl_util.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
@@ -22,6 +23,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/password_manager/account_storage/account_password_store_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
+#include "chrome/browser/password_manager/password_store_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
@@ -236,7 +238,40 @@ void PasswordManagerPresenter::ChangeSavedPassword(
   if (it == password_map_.end())
     return;
 
-  ChangeSavedPasswords(it->second, new_username, new_password);
+  const FormVector& old_forms = it->second;
+
+  // If a password was provided, make sure it is not empty.
+  if (new_password && new_password->empty()) {
+    DLOG(ERROR) << "The password is empty.";
+    return;
+  }
+
+  const std::string& signon_realm = old_forms[0]->signon_realm;
+  const base::string16& old_username = old_forms[0]->username_value;
+
+  // TODO(crbug.com/377410): Clean up this check for duplicates because a
+  // very similar one is in password_store_utils in EditSavedPasswords already.
+
+  // In case the username
+  // changed, make sure that there exists no other credential with the same
+  // signon_realm and username.
+  const bool username_changed = old_username != new_username;
+  if (username_changed) {
+    for (const auto& sort_key_passwords_pair : password_map_) {
+      for (const auto& password : sort_key_passwords_pair.second) {
+        if (password->signon_realm == signon_realm &&
+            password->username_value == new_username) {
+          DLOG(ERROR) << "A credential with the same signon_realm and username "
+                         "already exists.";
+          return;
+        }
+      }
+    }
+  }
+
+  EditSavedPasswords(password_view_->GetProfile(), old_forms, old_username,
+                     signon_realm, new_username,
+                     base::OptionalOrNullptr(new_password));
 }
 
 void PasswordManagerPresenter::RemoveSavedPassword(size_t index) {
@@ -327,54 +362,6 @@ void PasswordManagerPresenter::RemoveLogin(const autofill::PasswordForm& form) {
   store->RemoveLogin(form);
 }
 
-void PasswordManagerPresenter::ChangeSavedPasswords(
-    const FormVector& old_forms,
-    const base::string16& new_username,
-    const base::Optional<base::string16>& new_password) {
-  // If a password was provided, make sure it is not empty.
-  if (new_password && new_password->empty()) {
-    DLOG(ERROR) << "The password is empty.";
-    return;
-  }
-
-  DCHECK(!old_forms.empty());
-  const std::string& signon_realm = old_forms[0]->signon_realm;
-  const base::string16& old_username = old_forms[0]->username_value;
-
-  const bool username_changed = old_username != new_username;
-  // In case the username changed, make sure that there exists no other
-  // credential with the same signon_realm and username.
-  if (username_changed) {
-    for (const auto& sort_key_passwords_pair : password_map_) {
-      for (const auto& password : sort_key_passwords_pair.second) {
-        if (password->signon_realm == signon_realm &&
-            password->username_value == new_username) {
-          DLOG(ERROR) << "A credential with the same signon_realm and username "
-                         "already exists.";
-          return;
-        }
-      }
-    }
-  }
-
-  // An updated username implies a change in the primary key, thus we need to
-  // make sure to call the right API. Update every entry in the equivalence
-  // class.
-  for (const auto& old_form : old_forms) {
-    PasswordStore* store = GetPasswordStore(old_form->IsUsingAccountStore());
-    if (!store)
-      continue;
-
-    autofill::PasswordForm new_form = *old_form;
-    new_form.username_value = new_username;
-    if (new_password)
-      new_form.password_value = *new_password;
-
-    username_changed ? store->UpdateLoginWithPrimaryKey(new_form, *old_form)
-                     : store->UpdateLogin(new_form);
-  }
-}
-
 bool PasswordManagerPresenter::TryRemovePasswordEntries(
     PasswordFormMap* form_map,
     size_t index) {

chrome/browser/ui/passwords/settings/password_manager_presenter.h

@@ -110,12 +110,6 @@ class PasswordManagerPresenter
       std::map<std::string,
                std::vector<std::unique_ptr<autofill::PasswordForm>>>;
 
-  // Implementation used in both |ChangeSavedPassword()| methods.
-  void ChangeSavedPasswords(
-      const std::vector<std::unique_ptr<autofill::PasswordForm>>& old_forms,
-      const base::string16& new_username,
-      const base::Optional<base::string16>& new_password);
-
   // Attempts to remove the entries corresponding to |index| from |form_map|.
   // This will also add a corresponding undo operation to |undo_manager_|.
   // Returns whether removing the entry succeeded.

chrome/test/BUILD.gn

@@ -2848,6 +2848,7 @@ test("unit_tests") {
     "../browser/android/oom_intervention/near_oom_monitor_unittest.cc",
     "../browser/android/oom_intervention/oom_intervention_decider_unittest.cc",
     "../browser/android/password_ui_view_android_unittest.cc",
+    "../browser/android/password_update_delegate_unittest.cc",
     "../browser/android/preferences/pref_service_bridge_unittest.cc",
     "../browser/android/preferences/prefs_unittest.cc",
     "../browser/android/shortcut_info_unittest.cc",