Password bubble: Deal correctly with blacklist state changes.

If a user blacklists or unblacklists a site in a separate tab, we need
to correctly update every other tab's state (consider the unlikely case
of logging into the same site 8 times at the same time in separate tabs,
or the slightly less unlikely case of changing status in
chrome://settings/passwords while a login page is visible). This CL updates
the observer logic to account for blacklist entries being added and removed
from the PasswordStore, and updates the UI accordingly.

---------------------------------------------------------------------------
This is a reland of r269244, which was reverted due to LSAN breakage.
Original review at https://codereview.chromium.org/276683003/
---------------------------------------------------------------------------

BUG=261628
TBR=vabr@chromium.org

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

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

chrome/browser/ui/passwords/manage_passwords_ui_controller.cc

@@ -90,6 +90,7 @@ void ManagePasswordsUIController::WebContentsDestroyed() {
 
 void ManagePasswordsUIController::OnLoginsChanged(
     const password_manager::PasswordStoreChangeList& changes) {
+  password_manager::ui::State current_state = state_;
   for (password_manager::PasswordStoreChangeList::const_iterator it =
            changes.begin();
        it != changes.end();
@@ -100,12 +101,20 @@ void ManagePasswordsUIController::OnLoginsChanged(
 
     if (it->type() == password_manager::PasswordStoreChange::REMOVE) {
       password_form_map_.erase(changed_form.username_value);
+      if (changed_form.blacklisted_by_user) {
+        DCHECK(state_ == password_manager::ui::BLACKLIST_STATE);
+        state_ = password_manager::ui::MANAGE_STATE;
+      }
     } else {
-      autofill::PasswordForm* new_form =
-          new autofill::PasswordForm(changed_form);
-      password_form_map_[changed_form.username_value] = new_form;
+      new_password_forms_.push_back(new autofill::PasswordForm(changed_form));
+      password_form_map_[changed_form.username_value] =
+          new_password_forms_.back();
+      if (changed_form.blacklisted_by_user)
+        state_ = password_manager::ui::BLACKLIST_STATE;
     }
   }
+  if (current_state != state_)
+    UpdateBubbleAndIconVisibility();
 }
 
 void ManagePasswordsUIController::

chrome/browser/ui/passwords/manage_passwords_ui_controller.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_UI_CONTROLLER_H_
 #define CHROME_BROWSER_UI_PASSWORDS_MANAGE_PASSWORDS_UI_CONTROLLER_H_
 
+#include "base/memory/scoped_vector.h"
+#include "components/autofill/core/common/password_form.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"
@@ -95,6 +97,11 @@ class ManagePasswordsUIController
   // the value in ManagePasswordsUIControllerMock.
   autofill::PasswordFormMap password_form_map_;
 
+  // We create copies of PasswordForm objects that come in via OnLoginsChanged()
+  // and store them in this vector as well as in |password_form_map_| to ensure
+  // that we destroy them correctly.
+  ScopedVector<autofill::PasswordForm> new_password_forms_;
+
   // The current state of the password manager. Protected so we can manipulate
   // the value in tests.
   password_manager::ui::State state_;

chrome/browser/ui/passwords/manage_passwords_ui_controller_unittest.cc

@@ -102,6 +102,7 @@ TEST_F(ManagePasswordsUIControllerTest, PasswordSubmitted) {
 }
 
 TEST_F(ManagePasswordsUIControllerTest, BlacklistBlockedAutofill) {
+  test_form().blacklisted_by_user = true;
   base::string16 kTestUsername = base::ASCIIToUTF16("test_username");
   autofill::PasswordFormMap map;
   map[kTestUsername] = &test_form();
@@ -131,3 +132,46 @@ TEST_F(ManagePasswordsUIControllerTest, ClickedUnblacklist) {
   controller()->UpdateIconAndBubbleState(&mock);
   EXPECT_EQ(password_manager::ui::MANAGE_STATE, mock.state());
 }
+
+TEST_F(ManagePasswordsUIControllerTest, UnblacklistedElsewhere) {
+  test_form().blacklisted_by_user = true;
+  base::string16 kTestUsername = base::ASCIIToUTF16("test_username");
+  autofill::PasswordFormMap map;
+  map[kTestUsername] = &test_form();
+  controller()->OnBlacklistBlockedAutofill(map);
+
+  password_manager::PasswordStoreChange change(
+      password_manager::PasswordStoreChange::REMOVE, test_form());
+  password_manager::PasswordStoreChangeList list(1, change);
+  controller()->OnLoginsChanged(list);
+
+  EXPECT_EQ(password_manager::ui::MANAGE_STATE, controller()->state());
+  EXPECT_FALSE(controller()->PasswordPendingUserDecision());
+  EXPECT_EQ(test_form().origin, controller()->origin());
+
+  ManagePasswordsIconMock mock;
+  controller()->UpdateIconAndBubbleState(&mock);
+  EXPECT_EQ(password_manager::ui::MANAGE_STATE, mock.state());
+}
+
+TEST_F(ManagePasswordsUIControllerTest, BlacklistedElsewhere) {
+  base::string16 kTestUsername = base::ASCIIToUTF16("test_username");
+  autofill::PasswordFormMap map;
+  map[kTestUsername] = &test_form();
+  controller()->OnPasswordAutofilled(map);
+
+  test_form().blacklisted_by_user = true;
+  password_manager::PasswordStoreChange change(
+      password_manager::PasswordStoreChange::ADD, test_form());
+  password_manager::PasswordStoreChangeList list(1, change);
+  controller()->OnLoginsChanged(list);
+
+  EXPECT_EQ(password_manager::ui::BLACKLIST_STATE, controller()->state());
+  EXPECT_FALSE(controller()->PasswordPendingUserDecision());
+  EXPECT_EQ(test_form().origin, controller()->origin());
+
+  ManagePasswordsIconMock mock;
+  controller()->UpdateIconAndBubbleState(&mock);
+  EXPECT_EQ(password_manager::ui::BLACKLIST_STATE, mock.state());
+}
+