Password bubble: Keep the bubble in sync with the password store.

When passwords for the current site are added, updated, or removed from
the password store, we should add, update, or remove them from the
bubble in order to keep things up to date.

This patch deals with the common case of removing passwords from the
settings page while the password bubble is instantiated on a separate
tab. A future patch will deal with adding or removing the site from the
blacklist.

BUG=330154
R=vabr@chromium.org,markusheintz@chromium.org

Review URL: https://codereview.chromium.org/228593002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@263011 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/passwords/manage_passwords_bubble_model.cc

@@ -24,24 +24,16 @@ ManagePasswordsBubbleModel::ManagePasswordsBubbleModel(
   ManagePasswordsBubbleUIController* manage_passwords_bubble_ui_controller =
       ManagePasswordsBubbleUIController::FromWebContents(web_contents_);
 
-  password_submitted_ =
-      manage_passwords_bubble_ui_controller->password_submitted();
-  if (password_submitted_) {
-    if (manage_passwords_bubble_ui_controller->password_to_be_saved())
-      manage_passwords_bubble_state_ = PASSWORD_TO_BE_SAVED;
-    else
-      manage_passwords_bubble_state_ = MANAGE_PASSWORDS_AFTER_SAVING;
-  } else {
+  if (manage_passwords_bubble_ui_controller->password_to_be_saved())
+    manage_passwords_bubble_state_ = PASSWORD_TO_BE_SAVED;
+  else
     manage_passwords_bubble_state_ = MANAGE_PASSWORDS;
-  }
 
   title_ = l10n_util::GetStringUTF16(
       (manage_passwords_bubble_state_ == PASSWORD_TO_BE_SAVED) ?
           IDS_SAVE_PASSWORD : IDS_MANAGE_PASSWORDS);
-  if (password_submitted_) {
-    pending_credentials_ =
-        manage_passwords_bubble_ui_controller->pending_credentials();
-  }
+  pending_credentials_ =
+      manage_passwords_bubble_ui_controller->pending_credentials();
   best_matches_ = manage_passwords_bubble_ui_controller->best_matches();
   manage_link_ =
       l10n_util::GetStringUTF16(IDS_OPTIONS_PASSWORDS_MANAGE_PASSWORDS_LINK);
@@ -66,7 +58,7 @@ void ManagePasswordsBubbleModel::OnSaveClicked() {
       ManagePasswordsBubbleUIController::FromWebContents(web_contents_);
   manage_passwords_bubble_ui_controller->SavePassword();
   manage_passwords_bubble_ui_controller->unset_password_to_be_saved();
-  manage_passwords_bubble_state_ = MANAGE_PASSWORDS_AFTER_SAVING;
+  manage_passwords_bubble_state_ = MANAGE_PASSWORDS;
 }
 
 void ManagePasswordsBubbleModel::OnManageLinkClicked() {
@@ -89,18 +81,6 @@ void ManagePasswordsBubbleModel::OnPasswordAction(
     password_store->RemoveLogin(password_form);
   else
     password_store->AddLogin(password_form);
-  // This is necessary in case the bubble is instantiated again, we thus do not
-  // display the pending credentials if they were deleted.
-  if (password_form.username_value == pending_credentials_.username_value) {
-    ManagePasswordsBubbleUIController::FromWebContents(web_contents_)
-        ->set_password_submitted(action == ADD_PASSWORD);
-  }
-}
-
-void ManagePasswordsBubbleModel::DeleteFromBestMatches(
-    autofill::PasswordForm password_form) {
-  ManagePasswordsBubbleUIController::FromWebContents(web_contents_)->
-      RemoveFromBestMatches(password_form);
 }
 
 void ManagePasswordsBubbleModel::WebContentsDestroyed(

chrome/browser/ui/passwords/manage_passwords_bubble_model.h

@@ -23,7 +23,6 @@ class ManagePasswordsBubbleModel : public content::WebContentsObserver {
 
   enum ManagePasswordsBubbleState {
     PASSWORD_TO_BE_SAVED,
-    MANAGE_PASSWORDS_AFTER_SAVING,
     MANAGE_PASSWORDS,
     NEVER_SAVE_PASSWORDS,
   };
@@ -48,12 +47,6 @@ class ManagePasswordsBubbleModel : public content::WebContentsObserver {
   void OnPasswordAction(const autofill::PasswordForm& password_form,
                         PasswordAction action);
 
-  // Called by the view code when the ManagePasswordItemView is destroyed and
-  // the user chose to delete the password.
-  // TODO(npentrel): Remove this once best_matches_ are newly made on bubble
-  // opening.
-  void DeleteFromBestMatches(autofill::PasswordForm password_form);
-
   ManagePasswordsBubbleState manage_passwords_bubble_state() {
     return manage_passwords_bubble_state_;
   }
@@ -62,7 +55,6 @@ class ManagePasswordsBubbleModel : public content::WebContentsObserver {
     return manage_passwords_bubble_state() == PASSWORD_TO_BE_SAVED;
   }
 
-  bool password_submitted() { return password_submitted_; }
   const base::string16& title() { return title_; }
   const autofill::PasswordForm& pending_credentials() {
     return pending_credentials_;
@@ -77,7 +69,6 @@ class ManagePasswordsBubbleModel : public content::WebContentsObserver {
 
   content::WebContents* web_contents_;
   ManagePasswordsBubbleState manage_passwords_bubble_state_;
-  bool password_submitted_;
   base::string16 title_;
   autofill::PasswordForm pending_credentials_;
   autofill::PasswordFormMap best_matches_;

chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc

@@ -5,14 +5,27 @@
 #include "chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.h"
 
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/omnibox/location_bar.h"
+#include "components/password_manager/core/browser/password_store.h"
 #include "content/public/browser/notification_service.h"
 
 using autofill::PasswordFormMap;
 using password_manager::PasswordFormManager;
 
+namespace {
+
+password_manager::PasswordStore* GetPasswordStore(
+    content::WebContents* web_contents) {
+  return PasswordStoreFactory::GetForProfile(
+             Profile::FromBrowserContext(web_contents->GetBrowserContext()),
+             Profile::EXPLICIT_ACCESS).get();
+}
+
+} // namespace
+
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(ManagePasswordsBubbleUIController);
 
 ManagePasswordsBubbleUIController::ManagePasswordsBubbleUIController(
@@ -21,8 +34,12 @@ ManagePasswordsBubbleUIController::ManagePasswordsBubbleUIController(
       manage_passwords_icon_to_be_shown_(false),
       password_to_be_saved_(false),
       manage_passwords_bubble_needs_showing_(false),
-      password_submitted_(false),
-      autofill_blocked_(false) {}
+      autofill_blocked_(false) {
+  password_manager::PasswordStore* password_store =
+      GetPasswordStore(web_contents);
+  if (password_store)
+    password_store->AddObserver(this);
+}
 
 ManagePasswordsBubbleUIController::~ManagePasswordsBubbleUIController() {}
 
@@ -41,10 +58,10 @@ void ManagePasswordsBubbleUIController::OnPasswordSubmitted(
     PasswordFormManager* form_manager) {
   form_manager_.reset(form_manager);
   password_form_map_ = form_manager_->best_matches();
+  origin_ = pending_credentials().origin;
   manage_passwords_icon_to_be_shown_ = true;
   password_to_be_saved_ = true;
   manage_passwords_bubble_needs_showing_ = true;
-  password_submitted_ = true;
   autofill_blocked_ = false;
   UpdateBubbleAndIconVisibility();
 }
@@ -52,10 +69,10 @@ void ManagePasswordsBubbleUIController::OnPasswordSubmitted(
 void ManagePasswordsBubbleUIController::OnPasswordAutofilled(
     const PasswordFormMap& password_form_map) {
   password_form_map_ = password_form_map;
+  origin_ = password_form_map_.begin()->second->origin;
   manage_passwords_icon_to_be_shown_ = true;
   password_to_be_saved_ = false;
   manage_passwords_bubble_needs_showing_ = false;
-  password_submitted_ = false;
   autofill_blocked_ = false;
   UpdateBubbleAndIconVisibility();
 }
@@ -64,14 +81,36 @@ void ManagePasswordsBubbleUIController::OnBlacklistBlockedAutofill() {
   manage_passwords_icon_to_be_shown_ = true;
   password_to_be_saved_ = false;
   manage_passwords_bubble_needs_showing_ = false;
-  password_submitted_ = false;
   autofill_blocked_ = true;
   UpdateBubbleAndIconVisibility();
 }
 
-void ManagePasswordsBubbleUIController::RemoveFromBestMatches(
-    autofill::PasswordForm password_form) {
-  password_form_map_.erase(password_form.username_value);
+void ManagePasswordsBubbleUIController::WebContentsDestroyed(
+    content::WebContents* web_contents) {
+  password_manager::PasswordStore* password_store =
+      GetPasswordStore(web_contents);
+  if (password_store)
+    password_store->RemoveObserver(this);
+}
+
+void ManagePasswordsBubbleUIController::OnLoginsChanged(
+    const password_manager::PasswordStoreChangeList& changes) {
+  for (password_manager::PasswordStoreChangeList::const_iterator it =
+           changes.begin();
+       it != changes.end();
+       it++) {
+    const autofill::PasswordForm& changed_form = it->form();
+    if (changed_form.origin != origin_)
+      continue;
+
+    if (it->type() == password_manager::PasswordStoreChange::REMOVE) {
+      password_form_map_.erase(changed_form.username_value);
+    } else {
+      autofill::PasswordForm* new_form =
+          new autofill::PasswordForm(changed_form);
+      password_form_map_[changed_form.username_value] = new_form;
+    }
+  }
 }
 
 void ManagePasswordsBubbleUIController::OnBubbleShown() {
@@ -97,6 +136,5 @@ void ManagePasswordsBubbleUIController::DidNavigateMainFrame(
   manage_passwords_icon_to_be_shown_ = false;
   password_to_be_saved_ = false;
   manage_passwords_bubble_needs_showing_ = false;
-  password_submitted_ = false;
   UpdateBubbleAndIconVisibility();
 }

chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.h

@@ -6,6 +6,8 @@
 #define CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_BUBBLE_UI_CONTROLLER_H_
 
 #include "components/password_manager/core/browser/password_form_manager.h"
+#include "components/password_manager/core/browser/password_store.h"
+#include "components/password_manager/core/browser/password_store_change.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
@@ -17,7 +19,8 @@ class WebContents;
 // Per-tab class to control the Omnibox password icon and bubble.
 class ManagePasswordsBubbleUIController
     : public content::WebContentsObserver,
-      public content::WebContentsUserData<ManagePasswordsBubbleUIController> {
+      public content::WebContentsUserData<ManagePasswordsBubbleUIController>,
+      public password_manager::PasswordStore::Observer {
  public:
   virtual ~ManagePasswordsBubbleUIController();
 
@@ -36,12 +39,9 @@ class ManagePasswordsBubbleUIController
   // Called when a form is _not_ autofilled due to user blacklisting.
   void OnBlacklistBlockedAutofill();
 
-  // TODO(npentrel) This ought to be changed. Best matches should be newly
-  // made when opening the ManagePasswordsBubble because there may have been
-  // changes to the best matches via the settings page. At the moment this also
-  // fails if one deletes a password when they are autofilled, as it still shows
-  // up after logging in and saving a password.
-  void RemoveFromBestMatches(autofill::PasswordForm password_form);
+  // PasswordStore::Observer implementation.
+  virtual void OnLoginsChanged(
+      const password_manager::PasswordStoreChangeList& changes) OVERRIDE;
 
   void SavePassword();
 
@@ -79,14 +79,6 @@ class ManagePasswordsBubbleUIController
     return password_form_map_;
   }
 
-  bool password_submitted() const {
-    return password_submitted_;
-  }
-
-  void set_password_submitted(bool password_submitted) {
-    password_submitted_ = password_submitted;
-  }
-
   bool autofill_blocked() const { return autofill_blocked_; }
   void set_autofill_blocked(bool autofill_blocked) {
     autofill_blocked_ = autofill_blocked;
@@ -107,6 +99,8 @@ class ManagePasswordsBubbleUIController
   virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
+  virtual void WebContentsDestroyed(
+      content::WebContents* web_contents) OVERRIDE;
 
   // Set by OnPasswordSubmitted() when the user submits a form containing login
   // information.  If the user responds to a subsequent "Do you want to save
@@ -121,14 +115,16 @@ class ManagePasswordsBubbleUIController
   bool manage_passwords_icon_to_be_shown_;
   bool password_to_be_saved_;
   bool manage_passwords_bubble_needs_showing_;
-  // Stores whether a new password has been submitted, if so we have
-  // |pending_credentials|.
-  bool password_submitted_;
 
   // Stores whether autofill was blocked due to a user's decision to blacklist
   // the current site ("Never save passwords for this site").
   bool autofill_blocked_;
 
+  // The origin of the form we're currently dealing with; we'll use this to
+  // determine which PasswordStore changes we should care about when updating
+  // |password_form_map_|.
+  GURL origin_;
+
   DISALLOW_COPY_AND_ASSIGN(ManagePasswordsBubbleUIController);
 };
 

chrome/browser/ui/views/passwords/manage_password_item_view.cc

@@ -127,10 +127,7 @@ ManagePasswordItemView::ManagePasswordItemView(
   GetLayoutManager()->Layout(this);
 }
 
-ManagePasswordItemView::~ManagePasswordItemView() {
-  if (delete_password_)
-    manage_passwords_bubble_model_->DeleteFromBestMatches(password_form_);
-}
+ManagePasswordItemView::~ManagePasswordItemView() {}
 
 void ManagePasswordItemView::BuildColumnSet(views::GridLayout* layout,
                                             int column_set_id) {

chrome/browser/ui/views/passwords/manage_passwords_bubble_view.cc

@@ -320,7 +320,6 @@ void ManagePasswordsBubbleView::Init() {
     // TODO(mkwst): Do we really want the "No passwords" case? It would probably
     // be better to only clear the pending password upon navigation, rather than
     // as soon as the bubble closes.
-    int num_items_displayed = 0;
     if (!manage_passwords_bubble_model_->best_matches().empty()) {
       for (autofill::PasswordFormMap::const_iterator i(
                manage_passwords_bubble_model_->best_matches().begin());
@@ -330,14 +329,14 @@ void ManagePasswordsBubbleView::Init() {
             *i->second,
             first_field_width,
             second_field_width,
-            num_items_displayed == 0 ? ManagePasswordItemView::FIRST_ITEM
-                                     : ManagePasswordItemView::SUBSEQUENT_ITEM);
+            i == manage_passwords_bubble_model_->best_matches().begin()
+                ? ManagePasswordItemView::FIRST_ITEM
+                : ManagePasswordItemView::SUBSEQUENT_ITEM);
 
         layout->StartRow(0, SINGLE_VIEW_COLUMN_SET);
         layout->AddView(item);
-        num_items_displayed++;
       }
-    } else if (!manage_passwords_bubble_model_->password_submitted()) {
+    } else {
         views::Label* empty_label = new views::Label(
             l10n_util::GetStringUTF16(IDS_MANAGE_PASSWORDS_NO_PASSWORDS));
         empty_label->SetMultiLine(true);
@@ -346,22 +345,6 @@ void ManagePasswordsBubbleView::Init() {
         layout->AddView(empty_label);
     }
 
-    // If the user just saved a password, it won't be in the 'best matches' list
-    // we just walked through. Display it explicitly.
-    if (manage_passwords_bubble_model_->password_submitted()) {
-      ManagePasswordItemView* item = new ManagePasswordItemView(
-          manage_passwords_bubble_model_,
-          manage_passwords_bubble_model_->pending_credentials(),
-          first_field_width,
-          second_field_width,
-          num_items_displayed ? ManagePasswordItemView::FIRST_ITEM
-                              : ManagePasswordItemView::SUBSEQUENT_ITEM);
-
-      layout->StartRow(0, SINGLE_VIEW_COLUMN_SET);
-      layout->AddView(item);
-      num_items_displayed++;
-    }
-
     // Build a "manage" link and "done" button, and throw them both into a new
     // row
     // containing a double-view columnset.