Password bubble: refactor ManagePasswordsIconView.

chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc

Index: chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc
diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc
index d9516de0fdb8c3cca6668b850f7f317d60548daa..8327f008b8aacda540133c47b28dc6f4806e2704 100644
--- a/chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc
+++ b/chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/omnibox/location_bar.h"
+#include "chrome/browser/ui/passwords/manage_passwords_icon.h"
 #include "chrome/common/url_constants.h"
 #include "components/password_manager/core/browser/password_store.h"
 #include "content/public/browser/notification_service.h"
@@ -58,8 +59,11 @@ void ManagePasswordsBubbleUIController::UpdateBubbleAndIconVisibility() {
 
 void ManagePasswordsBubbleUIController::OnPasswordSubmitted(
     PasswordFormManager* form_manager) {
-  form_manager_.reset(form_manager);
-  password_form_map_ = form_manager_->best_matches();
+  // The |form_manager| might be NULL during testing.
+  if (form_manager) {

[vabr (Chromium), 2014/04/24 12:56:12]

nit: I'm always a bit uneasy about modifying production code just for use in
tests.
I believe a better solution would be to make this method virtual and override it
in ManagePasswordsBubbleUIControllerMock (which would probably need to be
renamed to TestManagePasswordsBubbleUIController) in a way that would allow
passing a NULL form manager. E.g., OnPasswordSubmitted could have the second
half (after if(form_Manager){...}) put into a protected method, and the
overriding test-class OnPasswordSubmitted could call that protected method.

I'm not insisting on this, if you believe this would make things more complex, I
won't fight too long. :)

[Mike West, 2014/04/24 14:27:59]

Naah, you're right. I shouldn't have taken the easy way out. Updated the tests
to create all the necessary bits.

+    form_manager_.reset(form_manager);
+    password_form_map_ = form_manager_->best_matches();
+  }
   origin_ = PendingCredentials().origin;
   manage_passwords_icon_to_be_shown_ = true;
   password_to_be_saved_ = true;
@@ -70,8 +74,11 @@ void ManagePasswordsBubbleUIController::OnPasswordSubmitted(
 
 void ManagePasswordsBubbleUIController::OnPasswordAutofilled(
     const PasswordFormMap& password_form_map) {
-  password_form_map_ = password_form_map;
-  origin_ = password_form_map_.begin()->second->origin;
+  // An empty |password_form_map| might be provided during testing.
+  if (!password_form_map_.empty()) {

[vabr (Chromium), 2014/04/24 12:56:12]

Ditto about putting test-only provisions into production code.

[Mike West, 2014/04/24 14:27:59]

Done.

+    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;
@@ -115,10 +122,6 @@ void ManagePasswordsBubbleUIController::OnLoginsChanged(
   }
 }
 
-void ManagePasswordsBubbleUIController::OnBubbleShown() {
-  unset_manage_passwords_bubble_needs_showing();
-}
-
 void ManagePasswordsBubbleUIController::
     NavigateToPasswordManagerSettingsPage() {
 // TODO(mkwst): chrome_pages.h is compiled out of Android. Need to figure out
@@ -157,3 +160,23 @@ const autofill::PasswordForm& ManagePasswordsBubbleUIController::
   DCHECK(form_manager_);
   return form_manager_->pending_credentials();
 }
+
+void ManagePasswordsBubbleUIController::UpdateIconAndBubbleState(
+    ManagePasswordsIcon* icon) {
+  ManagePasswordsIcon::State state = ManagePasswordsIcon::INACTIVE_STATE;
+
+  if (autofill_blocked_)
+    state = ManagePasswordsIcon::BLACKLISTED_STATE;
+  else if (password_to_be_saved_)
+    state = ManagePasswordsIcon::PENDING_STATE;
+  else if (manage_passwords_icon_to_be_shown_)
+    state = ManagePasswordsIcon::MANAGE_STATE;
+
+  icon->SetState(state);
+
+  if (manage_passwords_bubble_needs_showing_) {
+    DCHECK(state == ManagePasswordsIcon::PENDING_STATE);
+    icon->ShowBubbleWithoutUserInteraction();
+    manage_passwords_bubble_needs_showing_ = false;
+  }
+}