Update ChangeSavedPassword function.

In the previous version, this function allowed changing the username
and optionally changing the password.
In the new version, this function allows to change only the password
and returns true if the password was successfully changed.


Bug: 377410
Change-Id: I0b555b404b781d8fa35c9eda1aff442141ab9219
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302638
Commit-Queue: Irina Fedorova <irfedorova@google.com>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791170}

chrome/browser/extensions/api/passwords_private/passwords_private_api.cc

@@ -62,12 +62,13 @@ ResponseAction PasswordsPrivateChangeSavedPasswordFunction::Run() {
       api::passwords_private::ChangeSavedPassword::Params::Create(*args_);
   EXTENSION_FUNCTION_VALIDATE(parameters);
 
-  GetDelegate(browser_context())
-      ->ChangeSavedPassword(
-          parameters->id, base::UTF8ToUTF16(parameters->new_username),
-          parameters->new_password ? base::make_optional(base::UTF8ToUTF16(
-                                         *parameters->new_password))
-                                   : base::nullopt);
+  if (!GetDelegate(browser_context())
+           ->ChangeSavedPassword(parameters->id,
+                                 base::UTF8ToUTF16(parameters->new_password))) {
+    return RespondNow(Error(
+        "Could not change the password. Either the password is empty, the user "
+        "is not authenticated or no matching password could be found."));
+  }
 
   return RespondNow(NoArguments());
 }

chrome/browser/extensions/api/passwords_private/passwords_private_apitest.cc

@@ -120,8 +120,18 @@ class PasswordsPrivateApiTest : public ExtensionApiTest {
 
 }  // namespace
 
-IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, ChangeSavedPassword) {
-  EXPECT_TRUE(RunPasswordsSubtest("changeSavedPassword")) << message_;
+IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, ChangeSavedPasswordSucceeds) {
+  EXPECT_TRUE(RunPasswordsSubtest("changeSavedPasswordSucceeds")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest, ChangeSavedPasswordFails) {
+  EXPECT_TRUE(RunPasswordsSubtest("changeSavedPasswordFails")) << message_;
+}
+
+IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest,
+                       ChangeSavedPasswordWithEmptyPasswordFails) {
+  EXPECT_TRUE(RunPasswordsSubtest("changeSavedPasswordWithEmptyPasswordFails"))
+      << message_;
 }
 
 IN_PROC_BROWSER_TEST_F(PasswordsPrivateApiTest,

chrome/browser/extensions/api/passwords_private/passwords_private_delegate.h

@@ -56,14 +56,11 @@ class PasswordsPrivateDelegate : public KeyedService {
       base::OnceCallback<void(const ExceptionEntries&)>;
   virtual void GetPasswordExceptionsList(ExceptionEntriesCallback callback) = 0;
 
-  // Changes the username and password corresponding to |id|.
+  // Changes the password corresponding to |id|.
   // |id|: The id for the password entry being updated.
-  // |new_username|: The new username.
   // |new_password|: The new password.
-  virtual void ChangeSavedPassword(
-      int id,
-      base::string16 new_username,
-      base::Optional<base::string16> new_password) = 0;
+  // Returns whether the change succeeded.
+  virtual bool ChangeSavedPassword(int id, base::string16 new_password) = 0;
 
   // Removes the saved password entries corresponding to the |ids| generated for
   // each entry of the password list. Any invalid id will be ignored.

chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.cc

@@ -202,14 +202,14 @@ void PasswordsPrivateDelegateImpl::GetPasswordExceptionsList(
     get_password_exception_list_callbacks_.push_back(std::move(callback));
 }
 
-void PasswordsPrivateDelegateImpl::ChangeSavedPassword(
+bool PasswordsPrivateDelegateImpl::ChangeSavedPassword(
     int id,
-    base::string16 new_username,
-    base::Optional<base::string16> new_password) {
+    base::string16 new_password) {
   const std::string* sort_key = password_id_generator_.TryGetKey(id);
-  DCHECK(sort_key);
-  password_manager_presenter_->ChangeSavedPassword(
-      *sort_key, std::move(new_username), std::move(new_password));
+
+  return sort_key != nullptr &&
+         password_manager_presenter_->ChangeSavedPassword(
+             *sort_key, std::move(new_password));
 }
 
 void PasswordsPrivateDelegateImpl::RemoveSavedPasswords(

chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h

@@ -49,10 +49,7 @@ class PasswordsPrivateDelegateImpl : public PasswordsPrivateDelegate,
   // PasswordsPrivateDelegate implementation.
   void GetSavedPasswordsList(UiEntriesCallback callback) override;
   void GetPasswordExceptionsList(ExceptionEntriesCallback callback) override;
-  void ChangeSavedPassword(
-      int id,
-      base::string16 new_username,
-      base::Optional<base::string16> new_password) override;
+  bool ChangeSavedPassword(int id, base::string16 new_password) override;
   void RemoveSavedPasswords(const std::vector<int>& ids) override;
   void RemovePasswordExceptions(const std::vector<int>& ids) override;
   void UndoRemoveSavedPasswordOrException() override;

chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl_unittest.cc

@@ -352,8 +352,8 @@ TEST_F(PasswordsPrivateDelegateImplTest, ChangeSavedPassword) {
 
   int sample_form_id = delegate.GetPasswordIdGeneratorForTesting().GenerateId(
       password_manager::CreateSortKey(sample_form));
-  delegate.ChangeSavedPassword(sample_form_id, base::ASCIIToUTF16("new_user"),
-                               base::ASCIIToUTF16("new_pass"));
+  EXPECT_TRUE(delegate.ChangeSavedPassword(sample_form_id,
+                                           base::ASCIIToUTF16("new_pass")));
 
   // Spin the loop to allow PasswordStore tasks posted when changing the
   // password to be completed.
@@ -365,8 +365,6 @@ TEST_F(PasswordsPrivateDelegateImplTest, ChangeSavedPassword) {
       [&](const PasswordsPrivateDelegate::UiEntries& password_list) {
         got_passwords = true;
         ASSERT_EQ(1u, password_list.size());
-        EXPECT_EQ(base::ASCIIToUTF16("new_user"),
-                  base::UTF8ToUTF16(password_list[0].username));
       }));
   EXPECT_TRUE(got_passwords);
 }

chrome/browser/extensions/api/passwords_private/test_passwords_private_delegate.cc

@@ -61,17 +61,14 @@ void TestPasswordsPrivateDelegate::GetPasswordExceptionsList(
   std::move(callback).Run(current_exceptions_);
 }
 
-void TestPasswordsPrivateDelegate::ChangeSavedPassword(
+bool TestPasswordsPrivateDelegate::ChangeSavedPassword(
     int id,
-    base::string16 username,
-    base::Optional<base::string16> password) {
-  if (static_cast<size_t>(id) >= current_entries_.size())
-    return;
+    base::string16 new_password) {
+  if ((static_cast<size_t>(id) >= current_entries_.size()) ||
+      new_password.empty())
+    return false;
 
-  // PasswordUiEntry does not contain a password. Thus we are only updating
-  // the username and the length of the password.
-  current_entries_[id].username = base::UTF16ToUTF8(username);
-  SendSavedPasswordsList();
+  return true;
 }
 
 void TestPasswordsPrivateDelegate::RemoveSavedPasswords(

chrome/browser/extensions/api/passwords_private/test_passwords_private_delegate.h

@@ -23,9 +23,9 @@ class TestPasswordsPrivateDelegate : public PasswordsPrivateDelegate {
   // PasswordsPrivateDelegate implementation.
   void GetSavedPasswordsList(UiEntriesCallback callback) override;
   void GetPasswordExceptionsList(ExceptionEntriesCallback callback) override;
-  void ChangeSavedPassword(int id,
-                           base::string16 username,
-                           base::Optional<base::string16> password) override;
+  // Fake implementation of ChangeSavedPassword. This succeeds if the
+  // current list of entries has the id and if the new password isn't empty.
+  bool ChangeSavedPassword(int id, base::string16 new_password) override;
   void RemoveSavedPasswords(const std::vector<int>& id) override;
   void RemovePasswordExceptions(const std::vector<int>& ids) override;
   // Simplified version of undo logic, only use for testing.

chrome/browser/resources/settings/autofill_page/password_manager_proxy.js

@@ -42,6 +42,15 @@ export class PasswordManagerProxy {
    */
   recordPasswordsPageAccessInSettings() {}
 
+  /**
+   * Changes the password corresponding to |id|.
+   * @param {number} id The id for the password entry being updated.
+   * @param {string} new_password
+   * @return {!Promise<void>} A promise that resolves when the password is
+   *     updated.
+   */
+  changeSavedPassword(id, new_password) {}
+
   /**
    * Should remove the saved password and notify that the list has changed.
    * @param {number} id The id for the password entry being removed.
@@ -362,6 +371,13 @@ export class PasswordManagerImpl {
     chrome.passwordsPrivate.recordPasswordsPageAccessInSettings();
   }
 
+  /** override */
+  changeSavedPassword(id, new_password) {
+    return new Promise(resolve => {
+      chrome.passwordsPrivate.changeSavedPassword(id, new_password, resolve);
+    });
+  }
+
   /** @override */
   removeSavedPassword(id) {
     chrome.passwordsPrivate.removeSavedPassword(id);

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

@@ -321,48 +321,28 @@ const autofill::PasswordForm* PasswordManagerPresenter::GetPasswordException(
   return TryGetPasswordForm(exception_map_, index);
 }
 
-void PasswordManagerPresenter::ChangeSavedPassword(
+bool PasswordManagerPresenter::ChangeSavedPassword(
     const std::string& sort_key,
-    const base::string16& new_username,
-    const base::Optional<base::string16>& new_password) {
+    base::string16 new_password) {
   // Find the equivalence class that needs to be updated.
   auto it = password_map_.find(sort_key);
   if (it == password_map_.end())
-    return;
-
-  const FormVector& old_forms = it->second;
+    return false;
 
-  // If a password was provided, make sure it is not empty.
-  if (new_password && new_password->empty()) {
+  // Make sure new_password is not empty.
+  if (new_password.empty()) {
     DLOG(ERROR) << "The password is empty.";
-    return;
+    return false;
   }
 
-  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;
-        }
-      }
-    }
-  }
+  const FormVector& old_forms = it->second;
 
-  EditSavedPasswords(password_view_->GetProfile(), old_forms, new_username,
-                     new_password);
+  DCHECK(!old_forms.empty());
+  const base::string16& username = old_forms[0]->username_value;
+  EditSavedPasswords(password_view_->GetProfile(), old_forms, username,
+                     std::move(new_password));
+
+  return true;
 }
 
 void PasswordManagerPresenter::RemoveSavedPassword(size_t index) {

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

@@ -76,10 +76,9 @@ class PasswordManagerPresenter
   // Gets the password exception entry at |index|.
   const autofill::PasswordForm* GetPasswordException(size_t index) const;
 
-  // Changes the username and password corresponding to |sort_key|.
-  void ChangeSavedPassword(const std::string& sort_key,
-                           const base::string16& new_username,
-                           const base::Optional<base::string16>& new_password);
+  // Changes the password corresponding to |sort_key|.
+  bool ChangeSavedPassword(const std::string& sort_key,
+                           base::string16 new_password);
 
   // Removes the saved password entries at |index|, or corresponding to
   // |sort_key|, respectively.

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

@@ -60,7 +60,6 @@ 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";
@@ -233,28 +232,26 @@ class PasswordManagerPresenterTest : public testing::Test {
     return form;
   }
 
-  void ChangeSavedPasswordBySortKey(
-      base::StringPiece url,
-      base::StringPiece old_username,
-      base::StringPiece old_password,
-      base::StringPiece new_username,
-      base::Optional<base::StringPiece> new_password) {
+  bool ChangeSavedPasswordBySortKey(base::StringPiece url,
+                                    base::StringPiece username,
+                                    base::StringPiece old_password,
+                                    base::StringPiece new_password) {
     autofill::PasswordForm temp_form;
     temp_form.url = GURL(url);
     temp_form.signon_realm = temp_form.url.GetOrigin().spec();
     temp_form.username_element = base::ASCIIToUTF16("username");
     temp_form.password_element = base::ASCIIToUTF16("password");
-    temp_form.username_value = base::ASCIIToUTF16(old_username);
+    temp_form.username_value = base::ASCIIToUTF16(username);
     temp_form.password_value = base::ASCIIToUTF16(old_password);
 
-    mock_controller_.GetPasswordManagerPresenter()->ChangeSavedPassword(
-        password_manager::CreateSortKey(temp_form),
-        base::ASCIIToUTF16(new_username),
-        new_password ? base::make_optional(base::ASCIIToUTF16(*new_password))
-                     : base::nullopt);
+    bool result =
+        mock_controller_.GetPasswordManagerPresenter()->ChangeSavedPassword(
+            password_manager::CreateSortKey(temp_form),
+            base::ASCIIToUTF16(std::move(new_password)));
     // The password store posts mutation tasks to a background thread, thus we
     // need to spin the message loop here.
     task_environment_.RunUntilIdle();
+    return result;
   }
 
   void UpdatePasswordLists() {
@@ -299,13 +296,14 @@ TEST_F(PasswordManagerPresenterTest,
               ElementsAre(Pair(kUsername, kPassword)));
   testing::Mock::VerifyAndClearExpectations(&GetUIController());
 
-  ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kNewUser, "");
+  EXPECT_FALSE(
+      ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, ""));
   EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
               ElementsAre(Pair(kUsername, kPassword)));
 }
 
 TEST_F(PasswordManagerPresenterTest,
-       ChangeSavedPasswordBySortKey_ChangeUsername) {
+       ChangeSavedPasswordBySortKey_DontRejectSamePassword) {
   AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword);
   EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
   EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
@@ -314,14 +312,14 @@ TEST_F(PasswordManagerPresenterTest,
               ElementsAre(Pair(kUsername, kPassword)));
   testing::Mock::VerifyAndClearExpectations(&GetUIController());
 
-  ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kNewUser,
-                               base::nullopt);
+  EXPECT_TRUE(ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword,
+                                           kPassword));
   EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
-              ElementsAre(Pair(kNewUser, kPassword)));
+              ElementsAre(Pair(kUsername, kPassword)));
 }
 
 TEST_F(PasswordManagerPresenterTest,
-       ChangeSavedPasswordBySortKey_ChangeUsernameAndPassword) {
+       ChangeSavedPasswordBySortKey_ChangePassword) {
   AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword);
   EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(1)));
   EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
@@ -330,14 +328,14 @@ TEST_F(PasswordManagerPresenterTest,
               ElementsAre(Pair(kUsername, kPassword)));
   testing::Mock::VerifyAndClearExpectations(&GetUIController());
 
-  ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kNewUser,
-                               kNewPass);
+  EXPECT_TRUE(ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword,
+                                           kNewPass));
   EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
-              ElementsAre(Pair(kNewUser, kNewPass)));
+              ElementsAre(Pair(kUsername, kNewPass)));
 }
 
 TEST_F(PasswordManagerPresenterTest,
-       ChangeSavedPasswordBySortKey_RejectSameUsernameForSameRealm) {
+       ChangeSavedPasswordBySortKey_DontRejectSamePasswordForSameRealm) {
   AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword);
   AddPasswordEntry(GURL(kExampleCom), kUsername2, kPassword2);
   EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(2)));
@@ -348,15 +346,15 @@ TEST_F(PasswordManagerPresenterTest,
                                    Pair(kUsername2, kPassword2)));
   testing::Mock::VerifyAndClearExpectations(&GetUIController());
 
-  ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kUsername2,
-                               base::nullopt);
+  EXPECT_TRUE(ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword,
+                                           kPassword2));
   EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
-              UnorderedElementsAre(Pair(kUsername, kPassword),
+              UnorderedElementsAre(Pair(kUsername, kPassword2),
                                    Pair(kUsername2, kPassword2)));
 }
 
 TEST_F(PasswordManagerPresenterTest,
-       ChangeSavedPasswordBySortKey_DontRejectSameUsernameForDifferentRealm) {
+       ChangeSavedPasswordBySortKey_DontRejectSamePasswordForDifferentRealm) {
   AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword);
   AddPasswordEntry(GURL(kExampleOrg), kUsername2, kPassword2);
   EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(2)));
@@ -368,10 +366,10 @@ TEST_F(PasswordManagerPresenterTest,
               ElementsAre(Pair(kUsername2, kPassword2)));
   testing::Mock::VerifyAndClearExpectations(&GetUIController());
 
-  ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kUsername2,
-                               base::nullopt);
+  EXPECT_TRUE(ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword,
+                                           kPassword2));
   EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
-              ElementsAre(Pair(kUsername2, kPassword)));
+              ElementsAre(Pair(kUsername, kPassword2)));
   EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleOrg)),
               ElementsAre(Pair(kUsername2, kPassword2)));
 }
@@ -390,38 +388,11 @@ TEST_F(PasswordManagerPresenterTest,
                                    Pair(kUsername, kPassword)));
   testing::Mock::VerifyAndClearExpectations(&GetUIController());
 
-  ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kNewUser,
-                               kNewPass);
-  EXPECT_THAT(
-      GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
-      UnorderedElementsAre(Pair(kNewUser, kNewPass), Pair(kNewUser, kNewPass)));
-}
-
-TEST_F(PasswordManagerPresenterTest,
-       ChangeSavedPasswordBySortKey_EditUsernameForTheRightCredential) {
-  AddPasswordEntry(GURL(kExampleCom), kUsername, kPassword);
-  AddPasswordEntry(GURL(kExampleCom), kUsername2, kPassword);
-  AddPasswordEntry(GURL(kExampleOrg), kUsername, kPassword);
-  AddPasswordEntry(GURL(kExampleOrg), kUsername2, kPassword);
-  EXPECT_CALL(GetUIController(), SetPasswordList(SizeIs(4)));
-  EXPECT_CALL(GetUIController(), SetPasswordExceptionList(IsEmpty()));
-  UpdatePasswordLists();
-  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
-              UnorderedElementsAre(Pair(kUsername, kPassword),
-                                   Pair(kUsername2, kPassword)));
-  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleOrg)),
-              UnorderedElementsAre(Pair(kUsername, kPassword),
-                                   Pair(kUsername2, kPassword)));
-  testing::Mock::VerifyAndClearExpectations(&GetUIController());
-
-  ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kNewUser,
-                               kPassword);
+  EXPECT_TRUE(ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword,
+                                           kNewPass));
   EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
-              UnorderedElementsAre(Pair(kNewUser, kPassword),
-                                   Pair(kUsername2, kPassword)));
-  EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleOrg)),
-              UnorderedElementsAre(Pair(kUsername, kPassword),
-                                   Pair(kUsername2, kPassword)));
+              UnorderedElementsAre(Pair(kUsername, kNewPass),
+                                   Pair(kUsername, kNewPass)));
 }
 
 TEST_F(PasswordManagerPresenterTest,
@@ -441,8 +412,8 @@ TEST_F(PasswordManagerPresenterTest,
                                    Pair(kUsername2, kPassword)));
   testing::Mock::VerifyAndClearExpectations(&GetUIController());
 
-  ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword, kUsername,
-                               kNewPass);
+  EXPECT_TRUE(ChangeSavedPasswordBySortKey(kExampleCom, kUsername, kPassword,
+                                           kNewPass));
   EXPECT_THAT(GetUsernamesAndPasswords(GetStoredPasswordsForRealm(kExampleCom)),
               UnorderedElementsAre(Pair(kUsername, kNewPass),
                                    Pair(kUsername2, kPassword)));

chrome/common/extensions/api/passwords_private.idl

@@ -202,12 +202,14 @@ namespace passwordsPrivate {
     // Settings WebUI.
     static void recordPasswordsPageAccessInSettings();
 
-    // Changes the username and password corresponding to |id|.
+    // Changes the password corresponding to |id|.
+    // Invokes |callback| or raises an error depending on whether the operation
+    // succeeded.
     // |id|: The id for the password entry being updated.
-    // |new_username|: The new username.
     // |new_password|: The new password.
-    static void changeSavedPassword(long id, DOMString new_username,
-                                    optional DOMString new_password);
+    // |callback|: The callback that gets invoked in the end.
+    static void changeSavedPassword(long id, DOMString new_password,
+                                    optional VoidCallback callback);
 
     // Removes the saved password corresponding to |id|. If no saved password
     // for this pair exists, this function is a no-op. |id|: The id for the

chrome/test/data/extensions/api_test/passwords_private/test.js

@@ -10,26 +10,31 @@
 const COMPROMISE_TIME = 158322960000;
 
 var availableTests = [
-  function changeSavedPassword() {
-    var numCalls = 0;
-    var callback = function(savedPasswordsList) {
-      numCalls++;
-      if (numCalls == 1) {
-        chrome.passwordsPrivate.changeSavedPassword(0, 'new_user');
-      } else if (numCalls == 2) {
-        chrome.test.assertEq('new_user', savedPasswordsList[0].username);
-        chrome.passwordsPrivate.changeSavedPassword(
-            0, 'another_user', 'new_pass');
-      } else if (numCalls == 3) {
-        chrome.test.assertEq('another_user', savedPasswordsList[0].username);
-        chrome.test.succeed();
-      } else {
-        chrome.test.fail();
-      }
-    };
+  function changeSavedPasswordSucceeds() {
+    chrome.passwordsPrivate.changeSavedPassword(0, 'new_pass', () => {
+      chrome.test.assertNoLastError();
+      chrome.test.succeed();
+    });
+  },
 
-    chrome.passwordsPrivate.onSavedPasswordsListChanged.addListener(callback);
-    chrome.passwordsPrivate.getSavedPasswordList(callback);
+  function changeSavedPasswordFails() {
+    chrome.passwordsPrivate.changeSavedPassword(-1, 'new_pass', () => {
+      chrome.test.assertLastError(
+          'Could not change the password. Either the password is empty, ' +
+          'the user is not authenticated or no matching password could be ' +
+          'found.');
+      chrome.test.succeed();
+    });
+  },
+
+  function changeSavedPasswordWithEmptyPasswordFails() {
+    chrome.passwordsPrivate.changeSavedPassword(0, '', () => {
+      chrome.test.assertLastError(
+          'Could not change the password. Either the password is empty, ' +
+          'the user is not authenticated or no matching password could be ' +
+          'found.');
+      chrome.test.succeed();
+    });
   },
 
   function removeAndUndoRemoveSavedPassword() {

chrome/test/data/webui/settings/test_password_manager_proxy.js

@@ -59,6 +59,7 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
       'movePasswordToAccount',
       'removeException',
       'removeExceptions',
+      'changeSavedPassword',
     ]);
 
     /** @private {!PasswordManagerExpectations} */
@@ -297,6 +298,12 @@ export class TestPasswordManagerProxy extends TestBrowserProxy {
     this.methodCalled('recordPasswordCheckReferrer', referrer);
   }
 
+  /** override */
+  changeSavedPassword(id, new_password) {
+    this.methodCalled('changeSavedPassword', {id, new_password});
+    return Promise.resolve();
+  }
+
   /** override */
   addPasswordsFileExportProgressListener() {}
 

third_party/closure_compiler/externs/passwords_private.js

@@ -131,13 +131,15 @@ chrome.passwordsPrivate.PasswordCheckStatus;
 chrome.passwordsPrivate.recordPasswordsPageAccessInSettings = function() {};
 
 /**
- * Changes the username and password corresponding to |id|.
+ * Changes the password corresponding to |id|. Invokes |callback| or raises an
+ * error depending on whether the operation succeeded.
  * @param {number} id The id for the password entry being updated.
- * @param {string} new_username The new username.
- * @param {string=} new_password The new password.
+ * @param {string} new_password The new password.
+ * @param {function(): void=} callback The callback that gets invoked in the
+ *     end.
  */
 chrome.passwordsPrivate.changeSavedPassword = function(
-    id, new_username, new_password) {};
+    id, new_password, callback) {};
 
 /**
  * Removes the saved password corresponding to |id|. If no saved password for
@@ -147,28 +149,32 @@ chrome.passwordsPrivate.changeSavedPassword = function(
 chrome.passwordsPrivate.removeSavedPassword = function(id) {};
 
 /**
- * Removes the saved passwords corresponding to |ids|. If no saved password
- * exists for a certain id, that id is ignored.
- * @param {Array<number>} ids The ids for the password entries being removed.
+ * Removes the saved password corresponding to |ids|. If no saved password
+ * exists for a certain id, that id is ignored. Undoing this operation via
+ * undoRemoveSavedPasswordOrException will restore all the removed passwords in
+ * the batch.
+ * @param {!Array<number>} ids
  */
 chrome.passwordsPrivate.removeSavedPasswords = function(ids) {};
 
 /**
- * Removes the saved password exception corresponding to |id|. If no
- * exception with this id exists, this function is a no-op.
+ * Removes the saved password exception corresponding to |id|. If no exception
+ * with this id exists, this function is a no-op.
  * @param {number} id The id for the exception url entry being removed.
  */
 chrome.passwordsPrivate.removePasswordException = function(id) {};
 
 /**
- * Removes the saved password exceptions corresponding to |ids|. If no
- * exception exists for a certain id, that id is ignored.
- * @param {Array<number>} ids The ids for the exception entries being removed.
+ * Removes the saved password exceptions corresponding to |ids|. If no exception
+ * exists for a certain id, that id is ignored. Undoing this operation via
+ * undoRemoveSavedPasswordOrException will restore all the removed exceptions in
+ * the batch.
+ * @param {!Array<number>} ids
  */
 chrome.passwordsPrivate.removePasswordExceptions = function(ids) {};
 
 /**
- * Undoes the last removal of a saved password or exception.
+ * Undoes the last removal of saved password(s) or exception(s).
  */
 chrome.passwordsPrivate.undoRemoveSavedPasswordOrException = function() {};
 
@@ -201,10 +207,9 @@ chrome.passwordsPrivate.getPasswordExceptionList = function(callback) {};
 
 /**
  * Moves a password currently stored on the device to being stored in the
- * signed-in, non-syncing Google Account. The result is a no-op if any of
- * these is true: |id| is invalid; |id| corresponds to a password already
- * stored in the account; or the user is not using the account-scoped password
- * storage.
+ * signed-in, non-syncing Google Account. The result is a no-op if any of these
+ * is true: |id| is invalid; |id| corresponds to a password already stored in
+ * the account; or the user is not using the account-scoped password storage.
  * @param {number} id The id for the password entry being moved.
  */
 chrome.passwordsPrivate.movePasswordToAccount = function(id) {};
@@ -218,8 +223,8 @@ chrome.passwordsPrivate.importPasswords = function() {};
  * Triggers the Password Manager password export functionality. Completion Will
  * be signaled by the onPasswordsFileExportProgress event. |callback| will be
  * called when the request is started or rejected. If rejected
- * <code>chrome.runtime.lastError</code> will be set to 'in-progress' or
- * 'reauth-failed'.
+ * $(ref:runtime.lastError) will be set to <code>'in-progress'</code> or
+ * <code>'reauth-failed'</code>.
  * @param {function(): void} callback
  */
 chrome.passwordsPrivate.exportPasswords = function(callback) {};