Integrate ManagePasswordsState into ManagePasswordsUIController.

chrome/browser/ui/passwords/manage_passwords_state.cc

 // Copyright 2015 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_state.h"
 
+#include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "components/password_manager/content/common/credential_manager_types.h"
 #include "components/password_manager/core/browser/browser_save_password_progress_logger.h"
 #include "components/password_manager/core/browser/password_form_manager.h"
-#include "components/password_manager/core/browser/password_manager_client.h"
 
 using password_manager::PasswordFormManager;
 using autofill::PasswordFormMap;
 
 namespace {
 
 // Converts the map with pointers or const pointers to the vector of const
 // pointers.
 template <typename T>
 std::vector<const T*> MapToVector(
@@ skipping 56 matching lines @@
                           Vector* forms) {
   typename Vector::iterator it = std::find_if(
       forms->begin(), forms->end(),
       [&form_to_delete](const autofill::PasswordForm* form) {
     return IsEqualUniqueKey(*form, form_to_delete);
   });
   if (it != forms->end())
     forms->erase(it);
 }
 
+// Inserts |form| to the beginning of |forms| if it's blacklisted or to the end
+// otherwise.
+template <class Vector>
+void InsertFormToVector(const autofill::PasswordForm* form,
+                        Vector* forms) {
+  typename Vector::iterator it = form->blacklisted_by_user ? forms->begin()
+                                                           : forms->end();
+  forms->insert(it, form);
+}
+
 }  // namespace
 
 ManagePasswordsState::ManagePasswordsState()
     : state_(password_manager::ui::INACTIVE_STATE),
-      client_(nullptr) {
+      web_contents_(nullptr) {
 }
 
 ManagePasswordsState::~ManagePasswordsState() {}
 
 void ManagePasswordsState::OnPendingPassword(
       scoped_ptr<password_manager::PasswordFormManager> form_manager) {
   ClearData();
   form_manager_ = form_manager.Pass();
   current_forms_weak_ = MapToVector(form_manager_->best_matches());
   origin_ = form_manager_->pending_credentials().origin;
@@ skipping 49 matching lines @@
     SetState(password_manager::ui::MANAGE_STATE);
   }
 }
 
 void ManagePasswordsState::OnBlacklistBlockedAutofill(
     const autofill::PasswordFormMap& password_form_map) {
   DCHECK(!password_form_map.empty());
   ClearData();
   local_credentials_forms_ = DeepCopyMapToVector(password_form_map);
   origin_ = local_credentials_forms_.front()->origin;
+  DCHECK(local_credentials_forms_.front()->blacklisted_by_user);

[vabr (Chromium), 2015/03/17 09:25:57]

Why is this important?

[vasilii, 2015/03/17 10:49:48]

If you look at the current code in
ManagePasswordsUIController::UnblacklistSite() then you'll see that we always
remove the first form supposing that it's the blacklisted one. It'll be a pity
to remove something else instead.

[vabr (Chromium), 2015/03/17 11:55:13]

Thanks for explaining. I added some further comments on places where I believe
this should be mentioned.

   SetState(password_manager::ui::BLACKLIST_STATE);
 }
 
 void ManagePasswordsState::OnInactive() {
   ClearData();
   origin_ = GURL();
   SetState(password_manager::ui::INACTIVE_STATE);
 }
 
 void ManagePasswordsState::TransitionToState(
     password_manager::ui::State state) {
   DCHECK_NE(password_manager::ui::INACTIVE_STATE, state_);
   DCHECK(state == password_manager::ui::BLACKLIST_STATE ||
          state == password_manager::ui::MANAGE_STATE);
-  if (state_ == password_manager::ui::CREDENTIAL_REQUEST_STATE &&
-      !credentials_callback_.is_null()) {
-    credentials_callback_.Run(password_manager::CredentialInfo());
-    credentials_callback_.Reset();
+  if (state_ == password_manager::ui::CREDENTIAL_REQUEST_STATE) {
+    if (!credentials_callback_.is_null()) {
+      credentials_callback_.Run(password_manager::CredentialInfo());
+      credentials_callback_.Reset();
+    }
+    federated_credentials_forms_.clear();
   }
   SetState(state);
 }
 
 void ManagePasswordsState::ProcessLoginsChanged(
     const password_manager::PasswordStoreChangeList& changes) {
   if (state() == password_manager::ui::INACTIVE_STATE)
     return;
 
   for (const password_manager::PasswordStoreChange& change : changes) {
@@ skipping 24 matching lines @@
   local_credentials_forms_.clear();
   federated_credentials_forms_.clear();
   credentials_callback_.Reset();
 }
 
 void ManagePasswordsState::AddForm(const autofill::PasswordForm& form) {
   if (form.origin != origin_)
     return;
   if (UpdateForm(form))
     return;
-  local_credentials_forms_.push_back(new autofill::PasswordForm(form));
-  if (form_manager_)
-    current_forms_weak_.push_back(local_credentials_forms_.back());
+  if (form_manager_) {
+    local_credentials_forms_.push_back(new autofill::PasswordForm(form));
+    InsertFormToVector(local_credentials_forms_.back(), &current_forms_weak_);
+  } else {
+    InsertFormToVector(new autofill::PasswordForm(form),
+                       &local_credentials_forms_);
+  }
 }
 
 bool ManagePasswordsState::UpdateForm(const autofill::PasswordForm& form) {
   if (form_manager_) {
     // |current_forms_weak_| contains the list of current passwords.
     std::vector<const autofill::PasswordForm*>::iterator it = std::find_if(
         current_forms_weak_.begin(), current_forms_weak_.end(),
         [&form](const autofill::PasswordForm* current_form) {
       return IsEqualUniqueKey(form, *current_form);
     });
@@ skipping 12 matching lines @@
   return false;
 }
 
 void ManagePasswordsState::DeleteForm(const autofill::PasswordForm& form) {
   RemoveFormFromVector(form, &current_forms_weak_);
   RemoveFormFromVector(form, &local_credentials_forms_);
   RemoveFormFromVector(form, &federated_credentials_forms_);
 }
 
 void ManagePasswordsState::SetState(password_manager::ui::State state) {
-  if (client_ && client_->IsLoggingActive()) {
-    password_manager::BrowserSavePasswordProgressLogger logger(client_);
+  password_manager::PasswordManagerClient* client =

[vabr (Chromium), 2015/03/17 09:25:57]

Why the change from getting the client directly to getting it through the
WebContents instead?

[vasilii, 2015/03/17 10:49:48]

I changed it back so the tests set it too.

+      web_contents_ ?
+          ChromePasswordManagerClient::FromWebContents(web_contents_) : nullptr;
+  // |client| might be NULL in tests.
+  if (client && client->IsLoggingActive()) {
+    password_manager::BrowserSavePasswordProgressLogger logger(client);
     logger.LogNumber(
         autofill::SavePasswordProgressLogger::STRING_NEW_UI_STATE,
         state);
   }
   state_ = state;
 }