Show user credentials chooser bubble.

chrome/browser/ui/passwords/manage_passwords_ui_controller.cc

Index: chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
index 4afd3205d9c9fbdedc93ca8d61d96185b30a93a8..2c5ca00bff5fb168d2958474b360d907cb48d6c5 100644
--- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
+++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/ui/passwords/manage_passwords_icon.h"
 #include "chrome/browser/ui/passwords/password_bubble_experiment.h"
 #include "chrome/common/url_constants.h"
+#include "components/password_manager/content/common/credential_manager_types.h"
 #include "components/password_manager/core/browser/password_store.h"
 #include "content/public/browser/notification_service.h"
 
@@ -64,7 +65,8 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(ManagePasswordsUIController);
 ManagePasswordsUIController::ManagePasswordsUIController(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
-      state_(password_manager::ui::INACTIVE_STATE) {
+      state_(password_manager::ui::INACTIVE_STATE),
+      bubble_shown_(false) {
   password_manager::PasswordStore* password_store =
       GetPasswordStore(web_contents);
   if (password_store)
@@ -74,6 +76,7 @@ ManagePasswordsUIController::ManagePasswordsUIController(
 ManagePasswordsUIController::~ManagePasswordsUIController() {}
 
 void ManagePasswordsUIController::UpdateBubbleAndIconVisibility() {
+  bubble_shown_ = false;
   // If we're not on a "webby" URL (e.g. "chrome://sign-in"), we shouldn't
   // display either the bubble or the icon.
   if (!BrowsingDataHelper::IsWebScheme(
@@ -81,25 +84,41 @@ void ManagePasswordsUIController::UpdateBubbleAndIconVisibility() {
     state_ = password_manager::ui::INACTIVE_STATE;
   }
 
-  #if !defined(OS_ANDROID)
-    Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
-    if (!browser)
-      return;
-    LocationBar* location_bar = browser->window()->GetLocationBar();
-    DCHECK(location_bar);
-    location_bar->UpdateManagePasswordsIconAndBubble();
-  #endif
+#if !defined(OS_ANDROID)
+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents());
+  if (!browser)
+    return;
+  LocationBar* location_bar = browser->window()->GetLocationBar();
+  DCHECK(location_bar);
+  location_bar->UpdateManagePasswordsIconAndBubble();
+#endif
 }
 
 void ManagePasswordsUIController::OnPasswordSubmitted(
     scoped_ptr<PasswordFormManager> form_manager) {
   form_manager_ = form_manager.Pass();
   password_form_map_ = ConstifyMap(form_manager_->best_matches());
-  origin_ = PendingCredentials().origin;
+  origin_ = PendingPassword().origin;
   state_ = password_manager::ui::PENDING_PASSWORD_AND_BUBBLE_STATE;
   UpdateBubbleAndIconVisibility();
 }
 
+bool ManagePasswordsUIController::OnChooseCredentials(
+    ScopedVector<autofill::PasswordForm> credentials,
+    base::Callback<void(const password_manager::CredentialInfo&)> callback){
+  DCHECK(!credentials.empty());
+  form_manager_.reset();
+  origin_ = credentials[0]->origin;
+  new_password_forms_.swap(credentials);
+  // The map is useless because usernames may overlap.

[Mike West, 2014/11/20 08:19:32]

TODO(vasilii)? :)

[vasilii, 2014/11/20 15:08:05]

TODO what? It's useful for other states.

[Mike West, 2014/11/21 11:21:12]

Yeah, I misunderstood this comment. Sorry for the noise.

+  password_form_map_.clear();
+  state_ = password_manager::ui::PENDING_CREDENTIALS_AND_BUBBLE_STATE;
+  UpdateBubbleAndIconVisibility();
+  if (bubble_shown_)

[Mike West, 2014/11/20 08:19:32]

When wouldn't the bubble be shown?

[vasilii, 2014/11/20 15:08:05]

There is a condition in ShowBubbleWithoutUserInteraction() about
input_in_progress(). Another one in ManagePasswordsBubbleView::ShowBubble -
IsShowing().

[Mike West, 2014/11/21 11:21:12]

Hrm. Ok. I hope that case will be impossible to reach for `request()`, but we'll
have to think about it in the future.

For example, if we auto-sign-in a user, we'll want to pop up a toast regardless
of whether they're typing in the omnibox or on the page.

+    credentials_callback_ = callback;

[Mike West, 2014/11/20 08:19:31]

We need to callback to Blink even if the user doesn't pick a credential,
otherwise the Promise will never resolve. Do you handle this case somewhere
else?

Also, don't we need to take ownership of the callback here?

[vasilii, 2014/11/20 15:08:05]

if we return false then this case is handled immediately in
content_credential_manager_dispatcher.cc and we don't need a callback. The
callback is copyable so there is no ownership.

[Mike West, 2014/11/21 11:21:12]

Acknowledged.

+  return bubble_shown_;
+}
+
 void ManagePasswordsUIController::OnAutomaticPasswordSave(
     scoped_ptr<PasswordFormManager> form_manager) {
   form_manager_ = form_manager.Pass();
@@ -157,6 +176,10 @@ void ManagePasswordsUIController::OnLoginsChanged(
         state_ = password_manager::ui::BLACKLIST_STATE;
     }
   }
+  // If we're in PENDING_CREDENTIALS_AND_BUBBLE_STATE just close the bubble.
+  if (current_state ==
+      password_manager::ui::PENDING_CREDENTIALS_AND_BUBBLE_STATE)
+    state_ = password_manager::ui::INACTIVE_STATE;

[Mike West, 2014/11/20 08:19:31]

This seems wrong; if I have another window open, and I save a password there,
the bubble would close here. Why should those behaviors be tied together?

Also: we need to callback into Blink to resolve the Promise with an empty
credential when we close the bubble automagically. I don't see where that's done
here.

[vasilii, 2014/11/20 15:08:05]

You are right. I removed this code. What should we do? Repopulate the bubble?

[Mike West, 2014/11/21 11:21:12]

I'd just add a TODO for now; I don't think we have a good answer. One option
would be to refresh the bubble, another would just be to pretend that nothing
happened. Since the credential is only returned to the page if the user selects
it, I'm not terribly worried about user confusion for the moment.

[vasilii, 2014/11/21 16:45:37]

Done.

   if (current_state != state_)
     UpdateBubbleAndIconVisibility();
 }
@@ -179,6 +202,19 @@ void ManagePasswordsUIController::SavePassword() {
   UpdateBubbleAndIconVisibility();
 }
 
+void ManagePasswordsUIController::ChooseCredential(
+    bool was_chosen,
+    const autofill::PasswordForm& form) {
+  DCHECK_EQ(password_manager::ui::PENDING_CREDENTIALS_AND_BUBBLE_STATE, state_);
+  DCHECK(!credentials_callback_.is_null());
+  password_manager::CredentialInfo info = was_chosen ?
+      password_manager::CredentialInfo(form) :
+      password_manager::CredentialInfo();
+  credentials_callback_.Run(info);
+  state_ = password_manager::ui::INACTIVE_STATE;
+  UpdateBubbleAndIconVisibility();
+}
+
 void ManagePasswordsUIController::SavePasswordInternal() {
   DCHECK(form_manager_.get());
   form_manager_->Save();
@@ -240,7 +276,7 @@ void ManagePasswordsUIController::WasHidden() {
 }
 
 const autofill::PasswordForm& ManagePasswordsUIController::
-    PendingCredentials() const {
+    PendingPassword() const {
   DCHECK(form_manager_);
   return form_manager_->pending_credentials();
 }
@@ -263,6 +299,10 @@ void ManagePasswordsUIController::UpdateIconAndBubbleState(
   }
 }
 
+void ManagePasswordsUIController::OnBubbleShown() {
+  bubble_shown_ = true;
+}
+
 void ManagePasswordsUIController::ShowBubbleWithoutUserInteraction() {
   DCHECK(password_manager::ui::IsAutomaticDisplayState(state_));
 #if !defined(OS_ANDROID)