Password bubble: refactor ManagePasswordsIconView.

chrome/browser/ui/passwords/manage_passwords_bubble_ui_controller.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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/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"
 
 using autofill::PasswordFormMap;
 using password_manager::PasswordFormManager;
 
 namespace {
 
 password_manager::PasswordStore* GetPasswordStore(
@@ skipping 28 matching lines @@
     if (!browser)
       return;
     LocationBar* location_bar = browser->window()->GetLocationBar();
     DCHECK(location_bar);
     location_bar->UpdateManagePasswordsIconAndBubble();
   #endif
 }
 
 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;
   manage_passwords_bubble_needs_showing_ = true;
   autofill_blocked_ = false;
   UpdateBubbleAndIconVisibility();
 }
 
 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;
   autofill_blocked_ = false;
   UpdateBubbleAndIconVisibility();
 }
 
 void ManagePasswordsBubbleUIController::OnBlacklistBlockedAutofill() {
   manage_passwords_icon_to_be_shown_ = true;
   password_to_be_saved_ = false;
@@ skipping 23 matching lines @@
     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() {
-  unset_manage_passwords_bubble_needs_showing();
-}
-
 void ManagePasswordsBubbleUIController::
     NavigateToPasswordManagerSettingsPage() {
 // TODO(mkwst): chrome_pages.h is compiled out of Android. Need to figure out
 // how this navigation should work there.
 #if !defined(OS_ANDROID)
   chrome::ShowSettingsSubPage(
       chrome::FindBrowserWithWebContents(web_contents()),
       chrome::kPasswordManagerSubPage);
 #endif
 }
@@ skipping 18 matching lines @@
   password_to_be_saved_ = false;
   manage_passwords_bubble_needs_showing_ = false;
   UpdateBubbleAndIconVisibility();
 }
 
 const autofill::PasswordForm& ManagePasswordsBubbleUIController::
     PendingCredentials() const {
   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;
+  }
+}