Check whether the users have blacklisted the site (i.e. "never save passwords") before we show the password generation bubble.

BUG=125413
TEST=PasswordGenerationTest,PasswordManagerTest,PasswordFormManagerTest


Review URL: https://chromiumcodereview.appspot.com/10545094

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

5 files changed, 166 insertions, 19 deletions

diff --git a/chrome/browser/password_manager/password_form_manager.cc b/chrome/browser/password_manager/password_form_manager.cc
index 13c942d..b9cc0bc 100644
--- a/chrome/browser/password_manager/password_form_manager.cc
+++ b/chrome/browser/password_manager/password_form_manager.cc
@@ -13,6 +13,8 @@
 #include "chrome/browser/password_manager/password_store.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/autofill_messages.h"
+#include "content/public/browser/render_view_host.h"
 #include "webkit/forms/password_form_dom_manager.h"
 
 using base::Time;
@@ -21,6 +23,7 @@ using webkit::forms::PasswordFormMap;
 
 PasswordFormManager::PasswordFormManager(Profile* profile,
                                          PasswordManager* password_manager,
+                                         content::RenderViewHost* host,
                                          const PasswordForm& observed_form,
                                          bool ssl_valid)
     : best_matches_deleter_(&best_matches_),
@@ -32,6 +35,7 @@ PasswordFormManager::PasswordFormManager(Profile* profile,
       preferred_match_(NULL),
       state_(PRE_MATCHING_PHASE),
       profile_(profile),
+      host_(host),
       manager_action_(kManagerActionNone),
       user_action_(kUserActionNone),
       submit_result_(kSubmitResultNotSubmitted) {
@@ -306,6 +310,9 @@ void PasswordFormManager::OnRequestDone(int handle,
     return;
   }
 
+  // If not blacklisted, send a message to allow password generation.
+  SendNotBlacklistedToRenderer();
+
   // Proceed to autofill.
   // Note that we provide the choices but don't actually prefill a value if
   // either: (1) we are in Incognito mode, or (2) the ACTION paths don't match.
@@ -329,6 +336,10 @@ void PasswordFormManager::OnPasswordStoreRequestDone(
 
   if (result.empty()) {
     state_ = POST_MATCHING_PHASE;
+    // No result means that we visit this site the first time so we don't need
+    // to check whether this site is blacklisted or not. Just send a message
+    // to allow password generation.
+    SendNotBlacklistedToRenderer();
     return;
   }
 
@@ -494,3 +505,8 @@ void PasswordFormManager::SubmitPassed() {
 void PasswordFormManager::SubmitFailed() {
   submit_result_ = kSubmitResultFailed;
 }
+
+void PasswordFormManager::SendNotBlacklistedToRenderer() {
+  host_->Send(new AutofillMsg_FormNotBlacklisted(host_->GetRoutingID(),
+                                                 observed_form_));
+}
diff --git a/chrome/browser/password_manager/password_form_manager.h b/chrome/browser/password_manager/password_form_manager.h
index 10c0d77..8de86a1 100644
--- a/chrome/browser/password_manager/password_form_manager.h
+++ b/chrome/browser/password_manager/password_form_manager.h
@@ -15,6 +15,10 @@
 #include "chrome/browser/password_manager/password_store_consumer.h"
 #include "webkit/forms/password_form.h"
 
+namespace content {
+class RenderViewHost;
+}  // namespace content
+
 class PasswordManager;
 class PasswordStore;
 class Profile;
@@ -31,6 +35,7 @@ class PasswordFormManager : public PasswordStoreConsumer {
   //           used to filter login results from database.
   PasswordFormManager(Profile* profile,
                       PasswordManager* password_manager,
+                      content::RenderViewHost* host,
                       const webkit::forms::PasswordForm& observed_form,
                       bool ssl_valid);
   virtual ~PasswordFormManager();
@@ -178,6 +183,11 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // UMA.
   int GetActionsTaken();
 
+  // Informs the renderer that the user has not blacklisted observed_form_ by
+  // choosing "never save passwords for this site". This is used by the password
+  // generation manager to deside whether to show the password generation icon.
+  virtual void SendNotBlacklistedToRenderer();
+
   // Set of PasswordForms from the DB that best match the form
   // being managed by this. Use a map instead of vector, because we most
   // frequently require lookups by username value in IsNewLogin.
@@ -233,6 +243,9 @@ class PasswordFormManager : public PasswordStoreConsumer {
   // The profile from which we get the PasswordStore.
   Profile* profile_;
 
+  // Render view host for sending messages to the corresponding renderer.
+  content::RenderViewHost* host_;
+
   // These three fields record the "ActionsTaken" by the browser and
   // the user with this form, and the result. They are combined and
   // recorded in UMA when the manager is destroyed.
diff --git a/chrome/browser/password_manager/password_form_manager_unittest.cc b/chrome/browser/password_manager/password_form_manager_unittest.cc
index 90b3772..09af894 100644
--- a/chrome/browser/password_manager/password_form_manager_unittest.cc
+++ b/chrome/browser/password_manager/password_form_manager_unittest.cc
@@ -1,19 +1,69 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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 "testing/gtest/include/gtest/gtest.h"
 
+#include "base/memory/scoped_ptr.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/password_manager/password_form_manager.h"
 #include "chrome/browser/password_manager/password_manager.h"
+#include "chrome/browser/password_manager/password_manager_delegate.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/test/base/testing_profile.h"
 #include "webkit/forms/password_form.h"
 
 using webkit::forms::PasswordForm;
 
+class TestPasswordManagerDelegate : public PasswordManagerDelegate {
+ public:
+  explicit TestPasswordManagerDelegate(Profile* profile) : profile_(profile) {}
+
+  virtual void FillPasswordForm(
+      const webkit::forms::PasswordFormFillData& form_data) OVERRIDE {}
+  virtual void AddSavePasswordInfoBarIfPermitted(
+      PasswordFormManager* form_to_save) OVERRIDE {}
+  virtual Profile* GetProfile() OVERRIDE { return profile_; }
+  virtual bool DidLastPageLoadEncounterSSLErrors() OVERRIDE { return false; }
+
+ private:
+  Profile* profile_;
+};
+
+class TestPasswordManager : public PasswordManager {
+ public:
+  explicit TestPasswordManager(PasswordManagerDelegate* delegate)
+    : PasswordManager(NULL, delegate) {}
+
+  virtual void Autofill(
+      const webkit::forms::PasswordForm& form_for_autofill,
+      const webkit::forms::PasswordFormMap& best_matches,
+      const webkit::forms::PasswordForm& preferred_match,
+      bool wait_for_username) const OVERRIDE {}
+};
+
+class TestPasswordFormManager : public PasswordFormManager {
+ public:
+  TestPasswordFormManager(Profile* profile,
+                          PasswordManager* manager,
+                          const webkit::forms::PasswordForm& observed_form,
+                          bool ssl_valid)
+    : PasswordFormManager(profile, manager, NULL, observed_form, ssl_valid),
+      num_sent_messages_(0) {}
+
+  virtual void SendNotBlacklistedToRenderer() OVERRIDE {
+    ++num_sent_messages_;
+  }
+
+  size_t num_sent_messages() {
+    return num_sent_messages_;
+  }
+
+ private:
+  size_t num_sent_messages_;
+};
+
 class PasswordFormManagerTest : public testing::Test {
  public:
   PasswordFormManagerTest() {
@@ -55,6 +105,23 @@ class PasswordFormManagerTest : public testing::Test {
     p->preferred_match_ = match;
   }
 
+  void SimulateFetchMatchingLoginsFromPasswordStore(
+      PasswordFormManager* manager,
+      int handle) {
+    // Just need to update the internal states.
+    manager->state_ = PasswordFormManager::MATCHING_PHASE;
+    manager->pending_login_query_ = handle;
+  }
+
+  void SimulateResponseFromPasswordStore(
+      PasswordFormManager* manager,
+      int handle,
+      const std::vector<PasswordForm*>& result) {
+    // Simply call the callback method when request done. This will transfer
+    // the ownership of the objects in |result| to the |manager|.
+    manager->OnPasswordStoreRequestDone(handle, result);
+  }
+
   bool IgnoredResult(PasswordFormManager* p, PasswordForm* form) {
     return p->IgnoreResult(*form);
   }
@@ -63,6 +130,12 @@ class PasswordFormManagerTest : public testing::Test {
 
   PasswordForm* observed_form() { return &observed_form_; }
   PasswordForm* saved_match() { return &saved_match_; }
+  PasswordForm* CreateSavedMatch(bool blacklisted) {
+    // Owned by the caller of this method.
+    PasswordForm* match = new PasswordForm(saved_match_);
+    match->blacklisted_by_user = blacklisted;
+    return match;
+  }
 
  private:
   PasswordForm observed_form_;
@@ -72,7 +145,7 @@ class PasswordFormManagerTest : public testing::Test {
 
 TEST_F(PasswordFormManagerTest, TestNewLogin) {
   PasswordFormManager* manager = new PasswordFormManager(
-      profile(), NULL, *observed_form(), false);
+      profile(), NULL, NULL, *observed_form(), false);
   SimulateMatchingPhase(manager, false);
   // User submits credentials for the observed form.
   PasswordForm credentials = *observed_form();
@@ -127,7 +200,7 @@ TEST_F(PasswordFormManagerTest, TestUpdatePassword) {
   // Create a PasswordFormManager with observed_form, as if we just
   // saw this form and need to find matching logins.
   PasswordFormManager* manager = new PasswordFormManager(
-      profile(), NULL, *observed_form(), false);
+      profile(), NULL, NULL, *observed_form(), false);
   SimulateMatchingPhase(manager, true);
 
   // User submits credentials for the observed form using a username previously
@@ -163,7 +236,7 @@ TEST_F(PasswordFormManagerTest, TestUpdatePassword) {
 
 TEST_F(PasswordFormManagerTest, TestIgnoreResult) {
   PasswordFormManager* manager = new PasswordFormManager(
-      profile(), NULL, *observed_form(), false);
+      profile(), NULL, NULL, *observed_form(), false);
   // Make sure we don't match a PasswordForm if it was originally saved on
   // an SSL-valid page and we are now on a page with invalid certificate.
   saved_match()->ssl_valid = true;
@@ -181,7 +254,7 @@ TEST_F(PasswordFormManagerTest, TestIgnoreResult) {
 
 TEST_F(PasswordFormManagerTest, TestEmptyAction) {
   scoped_ptr<PasswordFormManager> manager(new PasswordFormManager(
-      profile(), NULL, *observed_form(), false));
+      profile(), NULL, NULL, *observed_form(), false));
 
   saved_match()->action = GURL();
   SimulateMatchingPhase(manager.get(), true);
@@ -205,27 +278,27 @@ TEST_F(PasswordFormManagerTest, TestValidForms) {
   credentials.password_value = saved_match()->password_value;
 
   // Form with both username_element and password_element.
-  PasswordFormManager manager1(profile(), NULL, credentials, false);
+  PasswordFormManager manager1(profile(), NULL, NULL, credentials, false);
   SimulateMatchingPhase(&manager1, false);
   EXPECT_TRUE(manager1.HasValidPasswordForm());
 
   // Form without a username_element but with a password_element.
   credentials.username_element.clear();
-  PasswordFormManager manager2(profile(), NULL, credentials, false);
+  PasswordFormManager manager2(profile(), NULL, NULL, credentials, false);
   SimulateMatchingPhase(&manager2, false);
   EXPECT_FALSE(manager2.HasValidPasswordForm());
 
   // Form without a password_element but with a username_element.
   credentials.username_element = saved_match()->username_element;
   credentials.password_element.clear();
-  PasswordFormManager manager3(profile(), NULL, credentials, false);
+  PasswordFormManager manager3(profile(), NULL, NULL, credentials, false);
   SimulateMatchingPhase(&manager3, false);
   EXPECT_FALSE(manager3.HasValidPasswordForm());
 
   // Form with neither a password_element nor a username_element.
   credentials.username_element.clear();
   credentials.password_element.clear();
-  PasswordFormManager manager4(profile(), NULL, credentials, false);
+  PasswordFormManager manager4(profile(), NULL, NULL, credentials, false);
   SimulateMatchingPhase(&manager4, false);
   EXPECT_FALSE(manager4.HasValidPasswordForm());
 }
@@ -238,27 +311,64 @@ TEST_F(PasswordFormManagerTest, TestValidFormsBasic) {
   credentials.password_value = saved_match()->password_value;
 
   // Form with both username_element and password_element.
-  PasswordFormManager manager1(profile(), NULL, credentials, false);
+  PasswordFormManager manager1(profile(), NULL, NULL, credentials, false);
   SimulateMatchingPhase(&manager1, false);
   EXPECT_TRUE(manager1.HasValidPasswordForm());
 
   // Form without a username_element but with a password_element.
   credentials.username_element.clear();
-  PasswordFormManager manager2(profile(), NULL, credentials, false);
+  PasswordFormManager manager2(profile(), NULL, NULL, credentials, false);
   SimulateMatchingPhase(&manager2, false);
   EXPECT_TRUE(manager2.HasValidPasswordForm());
 
   // Form without a password_element but with a username_element.
   credentials.username_element = saved_match()->username_element;
   credentials.password_element.clear();
-  PasswordFormManager manager3(profile(), NULL, credentials, false);
+  PasswordFormManager manager3(profile(), NULL, NULL, credentials, false);
   SimulateMatchingPhase(&manager3, false);
   EXPECT_TRUE(manager3.HasValidPasswordForm());
 
   // Form with neither a password_element nor a username_element.
   credentials.username_element.clear();
   credentials.password_element.clear();
-  PasswordFormManager manager4(profile(), NULL, credentials, false);
+  PasswordFormManager manager4(profile(), NULL, NULL, credentials, false);
   SimulateMatchingPhase(&manager4, false);
   EXPECT_TRUE(manager4.HasValidPasswordForm());
 }
+
+TEST_F(PasswordFormManagerTest, TestSendNotBlacklistedMessage) {
+  // A dumb password manager.
+  TestPasswordManagerDelegate delegate(profile());
+  TestPasswordManager password_manager(&delegate);
+
+  // First time sign up attempt; No login result is found from password store;
+  // We should send the not blacklisted message.
+  scoped_ptr<TestPasswordFormManager> manager(new TestPasswordFormManager(
+      profile(), &password_manager, *observed_form(), false));
+  SimulateFetchMatchingLoginsFromPasswordStore(manager.get(), 1);
+  std::vector<PasswordForm*> result;
+  SimulateResponseFromPasswordStore(manager.get(), 1, result);
+  EXPECT_EQ(1u, manager->num_sent_messages());
+
+  // Sign up attempt to previously visited sites; Login result is found from
+  // password store, and is not blacklisted; We should send the not blacklisted
+  // message.
+  manager.reset(new TestPasswordFormManager(
+      profile(), &password_manager, *observed_form(), false));
+  SimulateFetchMatchingLoginsFromPasswordStore(manager.get(), 2);
+  // We need add heap allocated objects to result.
+  result.push_back(CreateSavedMatch(false));
+  SimulateResponseFromPasswordStore(manager.get(), 2, result);
+  EXPECT_EQ(1u, manager->num_sent_messages());
+
+  // Sign up attempt to previously visited sites; Login result is found from
+  // password store, but is blacklisted; We should not send the not blacklisted
+  // message.
+  manager.reset(new TestPasswordFormManager(
+      profile(), &password_manager, *observed_form(), false));
+  SimulateFetchMatchingLoginsFromPasswordStore(manager.get(), 3);
+  result.clear();
+  result.push_back(CreateSavedMatch(true));
+  SimulateResponseFromPasswordStore(manager.get(), 3, result);
+  EXPECT_EQ(0u, manager->num_sent_messages());
+}
diff --git a/chrome/browser/password_manager/password_manager.cc b/chrome/browser/password_manager/password_manager.cc
index f83ce24..46c8322 100644
--- a/chrome/browser/password_manager/password_manager.cc
+++ b/chrome/browser/password_manager/password_manager.cc
@@ -13,6 +13,7 @@
 #include "chrome/common/autofill_messages.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/user_metrics.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/frame_navigate_params.h"
 #include "grit/generated_resources.h"
 
@@ -89,7 +90,11 @@ void PasswordManager::SetFormHasGeneratedPassword(const PasswordForm& form) {
   bool ssl_valid = (form.origin.SchemeIsSecure() &&
                     !delegate_->DidLastPageLoadEncounterSSLErrors());
   PasswordFormManager* manager =
-      new PasswordFormManager(delegate_->GetProfile(), this, form, ssl_valid);
+      new PasswordFormManager(delegate_->GetProfile(),
+                              this,
+                              web_contents()->GetRenderViewHost(),
+                              form,
+                              ssl_valid);
   pending_login_managers_.push_back(manager);
   manager->SetHasGeneratedPassword();
   // TODO(gcasto): Add UMA stats to track this.
@@ -193,7 +198,10 @@ void PasswordManager::OnPasswordFormsParsed(
        iter != forms.end(); ++iter) {
     bool ssl_valid = iter->origin.SchemeIsSecure() && !had_ssl_error;
     PasswordFormManager* manager =
-        new PasswordFormManager(delegate_->GetProfile(), this, *iter,
+        new PasswordFormManager(delegate_->GetProfile(),
+                                this,
+                                web_contents()->GetRenderViewHost(),
+                                *iter,
                                 ssl_valid);
     pending_login_managers_.push_back(manager);
     manager->FetchMatchingLoginsFromPasswordStore();
diff --git a/chrome/browser/password_manager/password_manager.h b/chrome/browser/password_manager/password_manager.h
index afeab33..06d292c 100644
--- a/chrome/browser/password_manager/password_manager.h
+++ b/chrome/browser/password_manager/password_manager.h
@@ -45,10 +45,10 @@ class PasswordManager : public LoginModel,
 
   // Called by a PasswordFormManager when it decides a form can be autofilled
   // on the page.
-  void Autofill(const webkit::forms::PasswordForm& form_for_autofill,
-                const webkit::forms::PasswordFormMap& best_matches,
-                const webkit::forms::PasswordForm& preferred_match,
-                bool wait_for_username) const;
+  virtual void Autofill(const webkit::forms::PasswordForm& form_for_autofill,
+                        const webkit::forms::PasswordFormMap& best_matches,
+                        const webkit::forms::PasswordForm& preferred_match,
+                        bool wait_for_username) const;
 
   // LoginModel implementation.
   virtual void SetObserver(LoginModelObserver* observer) OVERRIDE;