Password entries in settings being indexed to avoid ambiguity

Password entries shown in settings are stored with many other pieces of
informations besides username and url. However, since most of the
lookups coming from settings would take into account only these two
pieces of data. We are now using an index based on the order that the
list is generated to refer to elements when querying their password
text or just removing them.

R=hcarmona@chromium.org, jdoerrie@chromium.org, stevenjb@chromium.org, tbarzic@chromium.org

Bug: 764667
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib5bafd10a7bed25e27c0ffc00b24f7a9e1f77d48
Reviewed-on: https://chromium-review.googlesource.com/666617Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503420}

AUTHORS

@@ -146,6 +146,7 @@ Chris Vasselli <clindsay@gmail.com>
 Chris Szurgot <szurgotc@amazon.com>
 Christophe Dumez <ch.dumez@samsung.com>
 Christopher Dale <chrelad@gmail.com>
+Claudio DeSouza <claudiomdsjr@gmail.com>
 Clemens Fruhwirth <clemens@endorphin.org>
 Clement Scheelfeldt Skau <clementskau@gmail.com>
 Clinton Staley <clintstaley@chromium.org>

chrome/browser/android/password_ui_view_android.cc

@@ -36,11 +36,8 @@ Profile* PasswordUIViewAndroid::GetProfile() {
   return ProfileManager::GetLastUsedProfile();
 }
 
-void PasswordUIViewAndroid::ShowPassword(
-    size_t index,
-    const std::string& origin_url,
-    const std::string& username,
-    const base::string16& password_value) {
+void PasswordUIViewAndroid::ShowPassword(size_t index,
+                                         const base::string16& password_value) {
   NOTIMPLEMENTED();
 }
 

chrome/browser/android/password_ui_view_android.h

@@ -30,11 +30,8 @@ class PasswordUIViewAndroid : public PasswordUIView {
 
   // PasswordUIView implementation.
   Profile* GetProfile() override;
-  void ShowPassword(
-      size_t index,
-      const std::string& origin_url,
-      const std::string& username,
-      const base::string16& password_value) override;
+  void ShowPassword(size_t index,
+                    const base::string16& password_value) override;
   void SetPasswordList(
       const std::vector<std::unique_ptr<autofill::PasswordForm>>& password_list)
       override;

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

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_api.h"
 
+#include <memory>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
@@ -33,8 +35,8 @@ ExtensionFunction::ResponseAction
   PasswordsPrivateDelegate* delegate =
       PasswordsPrivateDelegateFactory::GetForBrowserContext(browser_context(),
                                                             true /* create */);
-  delegate->RemoveSavedPassword(parameters->login_pair.urls.origin,
-                                parameters->login_pair.username);
+
+  delegate->RemoveSavedPassword(parameters->index);
 
   return RespondNow(NoArguments());
 }
@@ -56,7 +58,7 @@ ExtensionFunction::ResponseAction
   PasswordsPrivateDelegate* delegate =
       PasswordsPrivateDelegateFactory::GetForBrowserContext(browser_context(),
                                                             true /* create */);
-  delegate->RemovePasswordException(parameters->exception_url);
+  delegate->RemovePasswordException(parameters->index);
 
   return RespondNow(NoArguments());
 }
@@ -78,10 +80,7 @@ ExtensionFunction::ResponseAction
   PasswordsPrivateDelegate* delegate =
       PasswordsPrivateDelegateFactory::GetForBrowserContext(browser_context(),
                                                             true /* create */);
-
-  delegate->RequestShowPassword(parameters->login_pair.urls.origin,
-                                parameters->login_pair.username,
-                                GetSenderWebContents());
+  delegate->RequestShowPassword(parameters->index, GetSenderWebContents());
 
   // No response given from this API function; instead, listeners wait for the
   // chrome.passwordsPrivate.onPlaintextPasswordRetrieved event to fire.

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

@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/observer_list.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
@@ -39,6 +40,7 @@ api::passwords_private::PasswordUiEntry CreateEntry(size_t num) {
   entry.login_pair.urls.link = entry.login_pair.urls.origin;
   entry.login_pair.username = "testName" + std::to_string(num);
   entry.num_characters_in_password = kNumCharactersInPassword;
+  entry.index = num;
   return entry;
 }
 
@@ -47,6 +49,7 @@ api::passwords_private::ExceptionEntry CreateException(size_t num) {
   exception.urls.shown = "exception" + std::to_string(num) + ".com";
   exception.urls.origin = "http://" + exception.urls.shown + "/login";
   exception.urls.link = exception.urls.origin;
+  exception.index = num;
   return exception;
 }
 
@@ -88,8 +91,7 @@ class TestDelegate : public PasswordsPrivateDelegate {
     callback.Run(current_exceptions_);
   }
 
-  void RemoveSavedPassword(const std::string& origin,
-                           const std::string& username) override {
+  void RemoveSavedPassword(size_t index) override {
     if (current_entries_.empty())
       return;
 
@@ -99,8 +101,8 @@ class TestDelegate : public PasswordsPrivateDelegate {
     SendSavedPasswordsList();
   }
 
-  void RemovePasswordException(const std::string& exception_url) override {
-    if (current_exceptions_.empty())
+  void RemovePasswordException(size_t index) override {
+    if (index >= current_exceptions_.size())
       return;
 
     // Since this is just mock data, remove the first entry regardless of
@@ -109,15 +111,16 @@ class TestDelegate : public PasswordsPrivateDelegate {
     SendPasswordExceptionsList();
   }
 
-  void RequestShowPassword(const std::string& origin,
-                           const std::string& username,
+  void RequestShowPassword(size_t index,
                            content::WebContents* web_contents) override {
     // Return a mocked password value.
     std::string plaintext_password(kPlaintextPassword);
     PasswordsPrivateEventRouter* router =
         PasswordsPrivateEventRouterFactory::GetForProfile(profile_);
     if (router) {
-      router->OnPlaintextPasswordFetched(origin, username, plaintext_password);
+      if (index >= current_entries_.size())
+        return;
+      router->OnPlaintextPasswordFetched(index, plaintext_password);
     }
   }
 

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

@@ -50,29 +50,22 @@ class PasswordsPrivateDelegate : public KeyedService {
   virtual void GetPasswordExceptionsList(
       const ExceptionEntriesCallback& callback) = 0;
 
-  // Removes the saved password entry corresponding to |origin_url| and
-  // |username|.
-  // |origin_url| The origin for the URL where the password is used; should be
-  //     obtained using CreateUrlCollectionFromForm().origin.
-  // |username| The username used in conjunction with the saved password.
-  virtual void RemoveSavedPassword(
-      const std::string& origin_url, const std::string& username) = 0;
+  // Removes the saved password entry corresponding to the |index| generated for
+  // each entry of the password list.
+  // |index| the index created when going over the list of saved passwords.
+  virtual void RemoveSavedPassword(size_t index) = 0;
 
-  // Removes the saved password exception entry corresponding to
-  // |exception_url|.
-  // |exception_url| The URL corresponding to the exception to remove; should
-  //     be obtained using CreateUrlCollectionFromForm().origin.
-  virtual void RemovePasswordException(const std::string& exception_url) = 0;
+  // Removes the saved password exception entry corresponding set in the
+  // given |index|
+  // |index| The index for the exception url entry being removed.
+  virtual void RemovePasswordException(size_t index) = 0;
 
-  // Requests the plain text password for entry corresponding to |origin_url|
-  // and |username|.
-  // |origin_url| The origin for the URL where the password is used; should be
-  //     obtained using CreateUrlCollectionFromForm().origin.
-  // |username| The username used in conjunction with the saved password.
-  // |native_window| The Chrome host window; will be used to show an OS-level
-  //     authentication dialog if necessary.
-  virtual void RequestShowPassword(const std::string& origin_url,
-                                   const std::string& username,
+  // Requests the plain text password for entry corresponding to the |index|
+  // generated for each entry of the password list.
+  // |index| the index created when going over the list of saved passwords.
+  // |web_contents| The web content object used as the UI; will be used to show
+  //     an OS-level authentication dialog if necessary.
+  virtual void RequestShowPassword(size_t index,
                                    content::WebContents* web_contents) = 0;
 };
 

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

@@ -4,6 +4,9 @@
 
 #include "chrome/browser/extensions/api/passwords_private/passwords_private_delegate_impl.h"
 
+#include <utility>
+
+#include "base/numerics/safe_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -21,16 +24,6 @@
 #include "content/public/browser/web_contents.h"
 #include "ui/base/l10n/l10n_util.h"
 
-namespace {
-
-std::string LoginPairToMapKey(const std::string& origin_url,
-                              const std::string& username) {
-  // Concatenate origin URL and username to form a unique key.
-  return origin_url + ',' + username;
-}
-
-}  // namespace
-
 namespace extensions {
 
 PasswordsPrivateDelegateImpl::PasswordsPrivateDelegateImpl(Profile* profile)
@@ -76,66 +69,38 @@ void PasswordsPrivateDelegateImpl::GetPasswordExceptionsList(
     get_password_exception_list_callbacks_.push_back(callback);
 }
 
-void PasswordsPrivateDelegateImpl::RemoveSavedPassword(
-    const std::string& origin_url, const std::string& username) {
-  ExecuteFunction(base::Bind(
-      &PasswordsPrivateDelegateImpl::RemoveSavedPasswordInternal,
-      base::Unretained(this),
-      origin_url,
-      username));
+void PasswordsPrivateDelegateImpl::RemoveSavedPassword(size_t index) {
+  ExecuteFunction(
+      base::Bind(&PasswordsPrivateDelegateImpl::RemoveSavedPasswordInternal,
+                 base::Unretained(this), index));
 }
 
-void PasswordsPrivateDelegateImpl::RemoveSavedPasswordInternal(
-    const std::string& origin_url, const std::string& username) {
-  std::string key = LoginPairToMapKey(origin_url, username);
-  if (login_pair_to_index_map_.find(key) == login_pair_to_index_map_.end()) {
-    // If the URL/username pair does not exist in the map, do nothing.
-    return;
-  }
-
-  password_manager_presenter_->RemoveSavedPassword(
-      login_pair_to_index_map_[key]);
+void PasswordsPrivateDelegateImpl::RemoveSavedPasswordInternal(size_t index) {
+  password_manager_presenter_->RemoveSavedPassword(index);
 }
 
-void PasswordsPrivateDelegateImpl::RemovePasswordException(
-    const std::string& exception_url) {
-  ExecuteFunction(base::Bind(
-      &PasswordsPrivateDelegateImpl::RemovePasswordExceptionInternal,
-      base::Unretained(this),
-      exception_url));
+void PasswordsPrivateDelegateImpl::RemovePasswordException(size_t index) {
+  ExecuteFunction(
+      base::Bind(&PasswordsPrivateDelegateImpl::RemovePasswordExceptionInternal,
+                 base::Unretained(this), index));
 }
 
 void PasswordsPrivateDelegateImpl::RemovePasswordExceptionInternal(
-    const std::string& exception_url) {
-  if (exception_url_to_index_map_.find(exception_url) ==
-      exception_url_to_index_map_.end()) {
-    // If the exception URL does not exist in the map, do nothing.
-    return;
-  }
-
-  password_manager_presenter_->RemovePasswordException(
-      exception_url_to_index_map_[exception_url]);
+    size_t index) {
+  password_manager_presenter_->RemovePasswordException(index);
 }
 
 void PasswordsPrivateDelegateImpl::RequestShowPassword(
-    const std::string& origin_url,
-    const std::string& username,
+    size_t index,
     content::WebContents* web_contents) {
   ExecuteFunction(
       base::Bind(&PasswordsPrivateDelegateImpl::RequestShowPasswordInternal,
-                 base::Unretained(this), origin_url, username, web_contents));
+                 base::Unretained(this), index, web_contents));
 }
 
 void PasswordsPrivateDelegateImpl::RequestShowPasswordInternal(
-    const std::string& origin_url,
-    const std::string& username,
+    size_t index,
     content::WebContents* web_contents) {
-  std::string key = LoginPairToMapKey(origin_url, username);
-  if (login_pair_to_index_map_.find(key) == login_pair_to_index_map_.end()) {
-    // If the URL/username pair does not exist in the map, do nothing.
-    return;
-  }
-
   // Save |web_contents| so that the call to RequestShowPassword() below
   // can use this value by calling GetNativeWindow(). Note: This is safe because
   // GetNativeWindow() will only be called immediately from
@@ -145,8 +110,7 @@ void PasswordsPrivateDelegateImpl::RequestShowPasswordInternal(
   web_contents_ = web_contents;
 
   // Request the password. When it is retrieved, ShowPassword() will be called.
-  password_manager_presenter_->RequestShowPassword(
-      login_pair_to_index_map_[key]);
+  password_manager_presenter_->RequestShowPassword(index);
 }
 
 Profile* PasswordsPrivateDelegateImpl::GetProfile() {
@@ -155,36 +119,29 @@ Profile* PasswordsPrivateDelegateImpl::GetProfile() {
 
 void PasswordsPrivateDelegateImpl::ShowPassword(
     size_t index,
-    const std::string& origin_url,
-    const std::string& username,
     const base::string16& password_value) {
   PasswordsPrivateEventRouter* router =
       PasswordsPrivateEventRouterFactory::GetForProfile(profile_);
   if (router) {
-    router->OnPlaintextPasswordFetched(origin_url, username,
+    router->OnPlaintextPasswordFetched(index,
                                        base::UTF16ToUTF8(password_value));
   }
 }
 
 void PasswordsPrivateDelegateImpl::SetPasswordList(
     const std::vector<std::unique_ptr<autofill::PasswordForm>>& password_list) {
-  // Rebuild |login_pair_to_index_map_| so that it reflects the contents of the
-  // new list.
-  // Also, create a list of PasswordUiEntry objects to send to observers.
-  login_pair_to_index_map_.clear();
+  // Create a list of PasswordUiEntry objects to send to observers.
   current_entries_.clear();
 
   for (size_t i = 0; i < password_list.size(); i++) {
     const auto& form = password_list[i];
     api::passwords_private::UrlCollection urls =
         CreateUrlCollectionFromForm(*form);
-    std::string key =
-        LoginPairToMapKey(urls.origin, base::UTF16ToUTF8(form->username_value));
-    login_pair_to_index_map_[key] = i;
 
     api::passwords_private::PasswordUiEntry entry;
     entry.login_pair.urls = std::move(urls);
     entry.login_pair.username = base::UTF16ToUTF8(form->username_value);
+    entry.index = base::checked_cast<int>(i);
     entry.num_characters_in_password = form->password_value.length();
 
     if (!form->federation_origin.unique()) {
@@ -212,20 +169,17 @@ void PasswordsPrivateDelegateImpl::SetPasswordList(
 void PasswordsPrivateDelegateImpl::SetPasswordExceptionList(
     const std::vector<std::unique_ptr<autofill::PasswordForm>>&
         password_exception_list) {
-  // Rebuild |exception_url_to_index_map_| so that it reflects the contents of
-  // the new list.
-  // Also, create a list of exceptions to send to observers.
-  exception_url_to_index_map_.clear();
+  // Creates a list of exceptions to send to observers.
   current_exceptions_.clear();
 
   for (size_t i = 0; i < password_exception_list.size(); i++) {
     const auto& form = password_exception_list[i];
     api::passwords_private::UrlCollection urls =
         CreateUrlCollectionFromForm(*form);
-    exception_url_to_index_map_[urls.origin] = i;
 
     api::passwords_private::ExceptionEntry current_exception_entry;
     current_exception_entry.urls = std::move(urls);
+    current_exception_entry.index = base::checked_cast<int>(i);
     current_exceptions_.push_back(std::move(current_exception_entry));
   }
 

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

@@ -7,7 +7,6 @@
 
 #include <stddef.h>
 
-#include <map>
 #include <memory>
 #include <string>
 #include <vector>
@@ -44,19 +43,15 @@ class PasswordsPrivateDelegateImpl : public PasswordsPrivateDelegate,
   void SendPasswordExceptionsList() override;
   void GetPasswordExceptionsList(
       const ExceptionEntriesCallback& callback) override;
-  void RemoveSavedPassword(
-      const std::string& origin_url, const std::string& username) override;
-  void RemovePasswordException(const std::string& exception_url) override;
-  void RequestShowPassword(const std::string& origin_url,
-                           const std::string& username,
+  void RemoveSavedPassword(size_t index) override;
+  void RemovePasswordException(size_t index) override;
+  void RequestShowPassword(size_t index,
                            content::WebContents* web_contents) override;
 
   // PasswordUIView implementation.
   Profile* GetProfile() override;
   void ShowPassword(
       size_t index,
-      const std::string& origin_url,
-      const std::string& username,
       const base::string16& plaintext_password) override;
   void SetPasswordList(
       const std::vector<std::unique_ptr<autofill::PasswordForm>>& password_list)
@@ -81,11 +76,9 @@ class PasswordsPrivateDelegateImpl : public PasswordsPrivateDelegate,
   // has been initialized or by deferring it until initialization has completed.
   void ExecuteFunction(const base::Closure& callback);
 
-  void RemoveSavedPasswordInternal(
-      const std::string& origin_url, const std::string& username);
-  void RemovePasswordExceptionInternal(const std::string& exception_url);
-  void RequestShowPasswordInternal(const std::string& origin_url,
-                                   const std::string& username,
+  void RemoveSavedPasswordInternal(size_t index);
+  void RemovePasswordExceptionInternal(size_t index);
+  void RequestShowPasswordInternal(size_t index,
                                    content::WebContents* web_contents);
 
   // Not owned by this class.
@@ -118,14 +111,6 @@ class PasswordsPrivateDelegateImpl : public PasswordsPrivateDelegate,
   // NativeWindow for the window where the API was called.
   content::WebContents* web_contents_;
 
-  // Map from origin URL and username to the index of |password_list_| at which
-  // the corresponding entry resides.
-  std::map<std::string, size_t> login_pair_to_index_map_;
-
-  // Map from password exception URL to the index of |password_exception_list_|
-  // at which the correponding entry resides.
-  std::map<std::string, size_t> exception_url_to_index_map_;
-
   DISALLOW_COPY_AND_ASSIGN(PasswordsPrivateDelegateImpl);
 };
 

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

@@ -70,12 +70,10 @@ void PasswordsPrivateEventRouter::SendPasswordExceptionListToListeners() {
 }
 
 void PasswordsPrivateEventRouter::OnPlaintextPasswordFetched(
-        const std::string& origin_url,
-        const std::string& username,
-        const std::string& plaintext_password) {
+    size_t index,
+    const std::string& plaintext_password) {
   api::passwords_private::PlaintextPasswordEventParameters params;
-  params.login_pair.urls.origin = origin_url;
-  params.login_pair.username = username;
+  params.index = index;
   params.plaintext_password = plaintext_password;
 
   std::unique_ptr<base::ListValue> event_value(new base::ListValue);

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

@@ -36,13 +36,10 @@ class PasswordsPrivateEventRouter : public KeyedService {
       const std::vector<api::passwords_private::ExceptionEntry>& exceptions);
 
   // Notifies listeners after fetching a plain-text password.
-  // |origin_url| The origin which the password is saved for.
-  // |username| The username that this password belongs to.
+  // |index| the index for the password entry being shown.
   // |plaintext_password| The human-readable password.
-  void OnPlaintextPasswordFetched(
-      const std::string& origin_url,
-      const std::string& username,
-      const std::string& plaintext_password);
+  void OnPlaintextPasswordFetched(size_t index,
+                                  const std::string& plaintext_password);
 
  protected:
   explicit PasswordsPrivateEventRouter(content::BrowserContext* context);

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

@@ -18,6 +18,7 @@ namespace extensions {
 // well as a URL that is linked to.
 api::passwords_private::UrlCollection CreateUrlCollectionFromForm(
     const autofill::PasswordForm& form);
-}
+
+}  // namespace extensions
 
 #endif  // CHROME_BROWSER_EXTENSIONS_API_PASSWORDS_PRIVATE_PASSWORDS_PRIVATE_UTILS_H_

chrome/browser/resources/settings/passwords_and_forms_page/passwords_section.js

@@ -33,10 +33,10 @@ class PasswordManager {
 
   /**
    * Should remove the saved password and notify that the list has changed.
-   * @param {!PasswordManager.LoginPair} loginPair The saved password that
-   *     should be removed from the list. No-op if |loginPair| is not found.
+   * @param {number} index The index for the password entry being removed.
+   *     No-op if |index| is not in the list.
    */
-  removeSavedPassword(loginPair) {}
+  removeSavedPassword(index) {}
 
   /**
    * Add an observer to the list of password exceptions.
@@ -58,18 +58,18 @@ class PasswordManager {
 
   /**
    * Should remove the password exception and notify that the list has changed.
-   * @param {string} exception The exception that should be removed from the
-   *     list. No-op if |exception| is not in the list.
+   * @param {number} index The index for the exception url entry being removed.
+   *     No-op if |index| is not in the list.
    */
-  removeException(exception) {}
+  removeException(index) {}
 
   /**
    * Gets the saved password for a given login pair.
-   * @param {!PasswordManager.LoginPair} loginPair The saved password that
-   *     should be retrieved.
+   * @param {number} index The index for password entry that should be
+   *     retrieved. No-op if |index| is not in the list.
    * @param {function(!PasswordManager.PlaintextPasswordEvent):void} callback
    */
-  getPlaintextPassword(loginPair, callback) {}
+  getPlaintextPassword(index, callback) {}
 }
 
 /** @typedef {chrome.passwordsPrivate.PasswordUiEntry} */
@@ -106,8 +106,8 @@ class PasswordManagerImpl {
   }
 
   /** @override */
-  removeSavedPassword(loginPair) {
-    chrome.passwordsPrivate.removeSavedPassword(loginPair);
+  removeSavedPassword(index) {
+    chrome.passwordsPrivate.removeSavedPassword(index);
   }
 
   /** @override */
@@ -128,23 +128,22 @@ class PasswordManagerImpl {
   }
 
   /** @override */
-  removeException(exception) {
-    chrome.passwordsPrivate.removePasswordException(exception);
+  removeException(index) {
+    chrome.passwordsPrivate.removePasswordException(index);
   }
 
   /** @override */
-  getPlaintextPassword(loginPair, callback) {
+  getPlaintextPassword(index, callback) {
     var listener = function(reply) {
       // Only handle the reply for our loginPair request.
-      if (reply.loginPair.urls.origin == loginPair.urls.origin &&
-          reply.loginPair.username == loginPair.username) {
+      if (reply.index == index) {
         chrome.passwordsPrivate.onPlaintextPasswordRetrieved.removeListener(
             listener);
         callback(reply);
       }
     };
     chrome.passwordsPrivate.onPlaintextPasswordRetrieved.addListener(listener);
-    chrome.passwordsPrivate.requestPlaintextPassword(loginPair);
+    chrome.passwordsPrivate.requestPlaintextPassword(index);
   }
 }
 
@@ -326,7 +325,7 @@ Polymer({
    * @private
    */
   onMenuRemovePasswordTap_: function() {
-    this.passwordManager_.removeSavedPassword(this.activePassword.loginPair);
+    this.passwordManager_.removeSavedPassword(this.activePassword.index);
     /** @type {CrActionMenuElement} */ (this.$.menu).close();
   },
 
@@ -336,7 +335,7 @@ Polymer({
    * @private
    */
   onRemoveExceptionButtonTap_: function(e) {
-    this.passwordManager_.removeException(e.model.item.urls.origin);
+    this.passwordManager_.removeException(e.model.item.index);
   },
 
   /**
@@ -372,8 +371,7 @@ Polymer({
    */
   showPassword_: function(event) {
     this.passwordManager_.getPlaintextPassword(
-        /** @type {!PasswordManager.LoginPair} */ (event.detail.item.loginPair),
-        item => {
+        /** @type {!number} */ (event.detail.item.index), item => {
           event.detail.password = item.plaintextPassword;
         });
   },

chrome/browser/ui/passwords/password_manager_presenter.cc

@@ -251,11 +251,7 @@ void PasswordManagerPresenter::RequestShowPassword(size_t index) {
   // Call back the front end to reveal the password.
   std::string origin_url =
       extensions::CreateUrlCollectionFromForm(*password_list_[index]).origin;
-  password_view_->ShowPassword(
-      index,
-      origin_url,
-      base::UTF16ToUTF8(password_list_[index]->username_value),
-      password_list_[index]->password_value);
+  password_view_->ShowPassword(index, password_list_[index]->password_value);
   UMA_HISTOGRAM_ENUMERATION(
       "PasswordManager.AccessPasswordInSettings",
       password_manager::metrics_util::ACCESS_PASSWORD_VIEWED,

chrome/browser/ui/passwords/password_manager_presenter_unittest.cc

@@ -47,8 +47,7 @@ class MockPasswordUIView : public PasswordUIView {
 #if !defined(OS_ANDROID)
   gfx::NativeWindow GetNativeWindow() const override;
 #endif
-  MOCK_METHOD4(ShowPassword, void(
-      size_t, const std::string&, const std::string&, const base::string16&));
+  MOCK_METHOD2(ShowPassword, void(size_t, const base::string16&));
   MOCK_METHOD1(
       SetPasswordList,
       void(const std::vector<std::unique_ptr<autofill::PasswordForm>>&));

chrome/browser/ui/passwords/password_ui_view.h

@@ -32,13 +32,8 @@ class PasswordUIView {
 
   // Reveals the password for the saved password entry.
   // |index| the index of the saved password entry.
-  // |origin_url| the URL of the saved password entry; obtained via
-  //     CreateUrlCollectionFromForm().origin.
-  // |username| the username of the saved password entry.
   // |password_value| the value of saved password entry at |index|.
   virtual void ShowPassword(size_t index,
-                            const std::string& origin_url,
-                            const std::string& username,
                             const base::string16& password_value) = 0;
 
   // Updates the list of passwords in the UI.

chrome/common/extensions/api/passwords_private.idl

@@ -35,12 +35,15 @@ namespace passwordsPrivate {
 
     // Text shown if the password was obtained via a federated identity.
     DOMString? federationText;
+
+    // An index to refer back to a unique password entry record.
+    long index;
   };
 
   // Dictionary passed to listeners for the onPlaintextPasswordRetrieved event.
   dictionary PlaintextPasswordEventParameters {
-    // The LoginPair associated with the retrieved password.
-    LoginPair loginPair;
+    // An index to refer back to a unique password entry record.
+    long index;
 
     // The password in plaintext.
     DOMString plaintextPassword;
@@ -48,6 +51,9 @@ namespace passwordsPrivate {
 
   dictionary ExceptionEntry {
     UrlCollection urls;
+
+    // An index to refer back to a unique exception entry record.
+    long index;
   };
 
   callback PasswordListCallback = void(PasswordUiEntry[] entries);
@@ -57,24 +63,23 @@ namespace passwordsPrivate {
     // Removes the saved password corresponding to |loginPair|. If no saved
     // password for this pair exists, this function is a no-op.
     //
-    // |loginPair|: The LoginPair corresponding to the entry to remove.
-    static void removeSavedPassword(LoginPair loginPair);
+    // |index|: The index for the password entry being removed.
+    static void removeSavedPassword(long index);
 
     // Removes the saved password exception corresponding to |exceptionUrl|. If
     // no exception with this URL exists, this function is a no-op.
     //
-    // |exceptionUrl|: The URL corresponding to the exception to remove.
-    static void removePasswordException(DOMString exceptionUrl);
+    // |index|: The index for the exception url entry being removed.
+    static void removePasswordException(long index);
 
-    // Returns the plaintext password corresponding to |loginPair|. Note that on
+    // Returns the plaintext password corresponding to |index|. Note that on
     // some operating systems, this call may result in an OS-level
     // reauthentication. Once the password has been fetched, it will be returned
     // via the onPlaintextPasswordRetrieved event.
     // TODO(hcarmona): Investigate using a callback for consistency.
     //
-    // |loginPair|: The LoginPair corresponding to the entry whose password
-    //     is to be returned.
-    static void requestPlaintextPassword(LoginPair loginPair);
+    // |index|: The index for the password entry being being retrieved.
+    static void requestPlaintextPassword(long index);
 
     // Returns the list of saved passwords.
     // |callback|: Called with the list of saved passwords.
@@ -101,7 +106,7 @@ namespace passwordsPrivate {
     // Fired when a plaintext password has been fetched in response to a call to
     // chrome.passwordsPrivate.requestPlaintextPassword().
     //
-    // |loginPair|: The LoginPair whose password was found.
+    // |index|: The index for the password entry being being retrieved.
     // |plaintextPassword|: The plaintext password which was retrieved.
     static void onPlaintextPasswordRetrieved(
         PlaintextPasswordEventParameters dict);

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

@@ -15,14 +15,8 @@ var availableTests = [
 
       if (numCalls == 1) {
         numSavedPasswords = savedPasswordsList.length;
-        chrome.passwordsPrivate.removeSavedPassword({
-          urls: {
-            origin: savedPasswordsList[0].loginPair.urls.origin,
-            shown: savedPasswordsList[0].loginPair.urls.shown,
-            link: savedPasswordsList[0].loginPair.urls.link,
-          },
-          username: savedPasswordsList[0].loginPair.username
-        });
+        chrome.passwordsPrivate.removeSavedPassword(
+            savedPasswordsList[0].index);
       } else if (numCalls == 2) {
         chrome.test.assertEq(
             savedPasswordsList.length, numSavedPasswords - 1);
@@ -45,7 +39,7 @@ var availableTests = [
       if (numCalls == 1) {
         numPasswordExceptions = passwordExceptionsList.length;
         chrome.passwordsPrivate.removePasswordException(
-            passwordExceptionsList[0].urls.origin);
+            passwordExceptionsList[0].index);
       } else if (numCalls == 2) {
         chrome.test.assertEq(
             passwordExceptionsList.length, numPasswordExceptions - 1);
@@ -67,14 +61,7 @@ var availableTests = [
     };
 
     chrome.passwordsPrivate.onPlaintextPasswordRetrieved.addListener(callback);
-    chrome.passwordsPrivate.requestPlaintextPassword({
-      urls: {
-        origin: 'http://www.test.com',
-        shown: 'www.test.com',
-        link: 'http://www.test.com',
-      },
-      username: 'test@test.com'
-    });
+    chrome.passwordsPrivate.requestPlaintextPassword(0);
   },
 
   function getSavedPasswordList() {
@@ -88,6 +75,7 @@ var availableTests = [
         chrome.test.assertTrue(!!entry.loginPair.urls.origin);
         chrome.test.assertTrue(!!entry.loginPair.urls.shown);
         chrome.test.assertTrue(!!entry.loginPair.urls.link);
+        chrome.test.assertEq(entry.index, i);
       }
 
       // Ensure that the callback is invoked.
@@ -107,6 +95,7 @@ var availableTests = [
         chrome.test.assertTrue(!!exception.urls.origin);
         chrome.test.assertTrue(!!exception.urls.shown);
         chrome.test.assertTrue(!!exception.urls.link);
+        chrome.test.assertEq(exception.index, i);
       }
 
       // Ensure that the callback is invoked.

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

@@ -21,6 +21,8 @@ FakeDataMaker.passwordEntry = function(url, username, passwordLength) {
   url = url || FakeDataMaker.patternMaker_('www.xxxxxx.com', 16);
   username = username || FakeDataMaker.patternMaker_('user_xxxxx', 16);
   passwordLength = passwordLength || Math.floor(Math.random() * 15) + 3;
+  entryIndex = -1;
+  exceptionIndex = -1;
 
   return {
     loginPair: {
@@ -32,6 +34,7 @@ FakeDataMaker.passwordEntry = function(url, username, passwordLength) {
       username: username,
     },
     numCharactersInPassword: passwordLength,
+    index: ++entryIndex,
   };
 };
 
@@ -47,7 +50,8 @@ FakeDataMaker.exceptionEntry = function(url) {
       origin: 'http://' + url + '/login',
       shown: url,
       link: 'http://' + url + '/login',
-    }
+    },
+    index: ++exceptionIndex,
   };
 };
 
@@ -196,11 +200,11 @@ TestPasswordManager.prototype = {
   },
 
   /** @override */
-  removeSavedPassword: function(loginPair) {
+  removeSavedPassword: function(index) {
     this.actual_.removed.passwords++;
 
     if (this.onRemoveSavedPassword)
-      this.onRemoveSavedPassword(loginPair);
+      this.onRemoveSavedPassword(index);
   },
 
   /** @override */
@@ -221,15 +225,15 @@ TestPasswordManager.prototype = {
   },
 
   /** @override */
-  removeException: function(exception) {
+  removeException: function(index) {
     this.actual_.removed.exceptions++;
 
     if (this.onRemoveException)
-      this.onRemoveException(exception);
+      this.onRemoveException(index);
   },
 
   /** @override */
-  getPlaintextPassword: function(loginPair, callback) {
+  getPlaintextPassword: function(index, callback) {
     this.actual_.requested.plaintextPassword++;
     this.lastCallback.getPlaintextPassword = callback;
   },

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

@@ -289,10 +289,9 @@ TEST_F('SettingsPasswordSectionBrowserTest', 'uiTests', function() {
       assert(firstNode);
       var firstPassword = passwordList[0];
 
-      passwordManager.onRemoveSavedPassword = function(detail) {
+      passwordManager.onRemoveSavedPassword = function(index) {
         // Verify that the event matches the expected value.
-        assertEquals(firstPassword.loginPair.urls.origin, detail.urls.origin);
-        assertEquals(firstPassword.loginPair.username, detail.username);
+        assertEquals(firstPassword.index, index);
 
         // Clean up after self.
         passwordManager.onRemoveSavedPassword = null;
@@ -436,19 +435,19 @@ TEST_F('SettingsPasswordSectionBrowserTest', 'uiTests', function() {
           getDomRepeatChildren(passwordsSection.$.passwordExceptionsList);
 
       // The index of the button currently being checked.
-      var index = 0;
+      var item = 0;
 
       var clickRemoveButton = function() {
         MockInteractions.tap(
-            exceptions[index].querySelector('#removeExceptionButton'));
+            exceptions[item].querySelector('#removeExceptionButton'));
       };
 
-      passwordManager.onRemoveException = function(detail) {
+      passwordManager.onRemoveException = function(index) {
         // Verify that the event matches the expected value.
-        assertTrue(index < exceptionList.length);
-        assertEquals(exceptionList[index].urls.origin, detail);
+        assertTrue(item < exceptionList.length);
+        assertEquals(index, exceptionList[item].index);
 
-        if (++index < exceptionList.length) {
+        if (++item < exceptionList.length) {
           clickRemoveButton();  // Click 'remove' on all passwords, one by one.
         } else {
           // Clean up after self.

third_party/closure_compiler/externs/passwords_private.js

@@ -39,7 +39,8 @@ chrome.passwordsPrivate.LoginPair;
  * @typedef {{
  *   loginPair: !chrome.passwordsPrivate.LoginPair,
  *   numCharactersInPassword: number,
- *   federationText: (string|undefined)
+ *   federationText: (string|undefined),
+ *   index: number
  * }}
  * @see https://developer.chrome.com/extensions/passwordsPrivate#type-PasswordUiEntry
  */
@@ -47,7 +48,7 @@ chrome.passwordsPrivate.PasswordUiEntry;
 
 /**
  * @typedef {{
- *   loginPair: !chrome.passwordsPrivate.LoginPair,
+ *   index: number,
  *   plaintextPassword: string
  * }}
  * @see https://developer.chrome.com/extensions/passwordsPrivate#type-PlaintextPasswordEventParameters
@@ -56,7 +57,8 @@ chrome.passwordsPrivate.PlaintextPasswordEventParameters;
 
 /**
  * @typedef {{
- *   urls: !chrome.passwordsPrivate.UrlCollection
+ *   urls: !chrome.passwordsPrivate.UrlCollection,
+ *   index: number
  * }}
  * @see https://developer.chrome.com/extensions/passwordsPrivate#type-ExceptionEntry
  */
@@ -65,32 +67,29 @@ chrome.passwordsPrivate.ExceptionEntry;
 /**
  * Removes the saved password corresponding to |loginPair|. If no saved password
  * for this pair exists, this function is a no-op.
- * @param {!chrome.passwordsPrivate.LoginPair} loginPair The LoginPair
- *     corresponding to the entry to remove.
+ * @param {number} index The index for the password entry being removed.
  * @see https://developer.chrome.com/extensions/passwordsPrivate#method-removeSavedPassword
  */
-chrome.passwordsPrivate.removeSavedPassword = function(loginPair) {};
+chrome.passwordsPrivate.removeSavedPassword = function(index) {};
 
 /**
  * Removes the saved password exception corresponding to |exceptionUrl|. If no
  * exception with this URL exists, this function is a no-op.
- * @param {string} exceptionUrl The URL corresponding to the exception to
- *     remove.
+ * @param {number} index The index for the exception url entry being removed.
  * @see https://developer.chrome.com/extensions/passwordsPrivate#method-removePasswordException
  */
-chrome.passwordsPrivate.removePasswordException = function(exceptionUrl) {};
+chrome.passwordsPrivate.removePasswordException = function(index) {};
 
 /**
- * Returns the plaintext password corresponding to |loginPair|. Note that on
- * some operating systems, this call may result in an OS-level reauthentication.
- * Once the password has been fetched, it will be returned via the
+ * Returns the plaintext password corresponding to |index|. Note that on some
+ * operating systems, this call may result in an OS-level reauthentication. Once
+ * the password has been fetched, it will be returned via the
  * onPlaintextPasswordRetrieved event. TODO(hcarmona): Investigate using a
  * callback for consistency.
- * @param {!chrome.passwordsPrivate.LoginPair} loginPair The LoginPair
- *     corresponding to the entry whose password     is to be returned.
+ * @param {number} index The index for the password entry being being retrieved.
  * @see https://developer.chrome.com/extensions/passwordsPrivate#method-requestPlaintextPassword
  */
-chrome.passwordsPrivate.requestPlaintextPassword = function(loginPair) {};
+chrome.passwordsPrivate.requestPlaintextPassword = function(index) {};
 
 /**
  * Returns the list of saved passwords.