Abstract IPC send out of PasswordGenerationManager

The IPC in question was in
PasswordGenerationManager::SendAccountCreationFormsToRenderer. This is now done
through PasswordManagerDriver.

Most of this patch was developed by vabr@chromium.org.

BUG=335028

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

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

8 files changed, 137 insertions, 108 deletions

diff --git a/chrome/browser/password_manager/content_password_manager_driver.cc b/chrome/browser/password_manager/content_password_manager_driver.cc
index 43e5565..c72167e 100644
--- a/chrome/browser/password_manager/content_password_manager_driver.cc
+++ b/chrome/browser/password_manager/content_password_manager_driver.cc
@@ -6,6 +6,7 @@
 
 #include "components/autofill/content/browser/autofill_driver_impl.h"
 #include "components/autofill/content/common/autofill_messages.h"
+#include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/password_form.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_details.h"
@@ -61,6 +62,13 @@ PasswordManager* ContentPasswordManagerDriver::GetPasswordManager() {
   return &password_manager_;
 }
 
+void ContentPasswordManagerDriver::AccountCreationFormsFound(
+    const std::vector<autofill::FormData>& forms) {
+  content::RenderViewHost* host = web_contents()->GetRenderViewHost();
+  host->Send(new AutofillMsg_AccountCreationFormsDetected(host->GetRoutingID(),
+                                                          forms));
+}
+
 void ContentPasswordManagerDriver::DidNavigateMainFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
diff --git a/chrome/browser/password_manager/content_password_manager_driver.h b/chrome/browser/password_manager/content_password_manager_driver.h
index e88c5f2..5f51cc5 100644
--- a/chrome/browser/password_manager/content_password_manager_driver.h
+++ b/chrome/browser/password_manager/content_password_manager_driver.h
@@ -24,8 +24,8 @@ class WebContents;
 class ContentPasswordManagerDriver : public PasswordManagerDriver,
                                      public content::WebContentsObserver {
  public:
-  explicit ContentPasswordManagerDriver(content::WebContents* web_contents,
-                                        PasswordManagerClient* client);
+  ContentPasswordManagerDriver(content::WebContents* web_contents,
+                               PasswordManagerClient* client);
   virtual ~ContentPasswordManagerDriver();
 
   // PasswordManagerDriver implementation.
@@ -38,6 +38,8 @@ class ContentPasswordManagerDriver : public PasswordManagerDriver,
   virtual autofill::AutofillManager* GetAutofillManager() OVERRIDE;
   virtual void AllowPasswordGenerationForForm(autofill::PasswordForm* form)
       OVERRIDE;
+  virtual void AccountCreationFormsFound(
+      const std::vector<autofill::FormData>& forms) OVERRIDE;
 
   // content::WebContentsObserver overrides.
   virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
diff --git a/chrome/browser/password_manager/password_form_manager_unittest.cc b/chrome/browser/password_manager/password_form_manager_unittest.cc
index c46a30a..ca95dd8 100644
--- a/chrome/browser/password_manager/password_form_manager_unittest.cc
+++ b/chrome/browser/password_manager/password_form_manager_unittest.cc
@@ -37,15 +37,15 @@ class MockPasswordManagerDriver : public PasswordManagerDriver {
   MockPasswordManagerDriver() {}
   virtual ~MockPasswordManagerDriver() {}
 
-  MOCK_METHOD1(FillPasswordForm,
-               void(const autofill::PasswordFormFillData& form_data));
+  MOCK_METHOD1(FillPasswordForm, void(const autofill::PasswordFormFillData&));
   MOCK_METHOD0(DidLastPageLoadEncounterSSLErrors, bool());
   MOCK_METHOD0(IsOffTheRecord, bool());
   MOCK_METHOD0(GetPasswordGenerationManager, PasswordGenerationManager*());
   MOCK_METHOD0(GetPasswordManager, PasswordManager*());
   MOCK_METHOD0(GetAutofillManager, autofill::AutofillManager*());
-  MOCK_METHOD1(AllowPasswordGenerationForForm,
-               void(autofill::PasswordForm* form));
+  MOCK_METHOD1(AllowPasswordGenerationForForm, void(autofill::PasswordForm*));
+  MOCK_METHOD1(AccountCreationFormsFound,
+               void(const std::vector<autofill::FormData>&));
 };
 
 class TestPasswordManagerClient : public PasswordManagerClient {
diff --git a/chrome/browser/password_manager/password_generation_manager.cc b/chrome/browser/password_manager/password_generation_manager.cc
index 2d895d3..e6b87da 100644
--- a/chrome/browser/password_manager/password_generation_manager.cc
+++ b/chrome/browser/password_manager/password_generation_manager.cc
@@ -18,7 +18,6 @@
 #include "components/autofill/core/browser/password_generator.h"
 #include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/password_form.h"
-#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "ui/gfx/rect.h"
@@ -53,10 +52,8 @@ void PasswordGenerationManager::DetectAccountCreationForms(
       }
     }
   }
-  if (!account_creation_forms.empty() && IsGenerationEnabled()) {
-    SendAccountCreationFormsToRenderer(web_contents_->GetRenderViewHost(),
-                                       account_creation_forms);
-  }
+  if (!account_creation_forms.empty() && IsGenerationEnabled())
+    driver_->AccountCreationFormsFound(account_creation_forms);
 }
 
 // In order for password generation to be enabled, we need to make sure:
@@ -76,13 +73,6 @@ bool PasswordGenerationManager::IsGenerationEnabled() const {
   return true;
 }
 
-void PasswordGenerationManager::SendAccountCreationFormsToRenderer(
-    content::RenderViewHost* host,
-    const std::vector<autofill::FormData>& forms) {
-  host->Send(new AutofillMsg_AccountCreationFormsDetected(
-      host->GetRoutingID(), forms));
-}
-
 gfx::RectF PasswordGenerationManager::GetBoundsInScreenSpace(
     const gfx::RectF& bounds) {
   gfx::Rect client_area;
diff --git a/chrome/browser/password_manager/password_generation_manager.h b/chrome/browser/password_manager/password_generation_manager.h
index df21285..47f1c71 100644
--- a/chrome/browser/password_manager/password_generation_manager.h
+++ b/chrome/browser/password_manager/password_generation_manager.h
@@ -85,12 +85,6 @@ class PasswordGenerationManager {
   // Determines current state of password generation
   bool IsGenerationEnabled() const;
 
-  // Sends a message to the renderer specifying form(s) that we should enable
-  // password generation on. This is a separate function to aid in testing.
-  virtual void SendAccountCreationFormsToRenderer(
-      content::RenderViewHost* host,
-      const std::vector<autofill::FormData>& forms);
-
   // Given |bounds| in the renderers coordinate system, return the same bounds
   // in the screens coordinate system.
   gfx::RectF GetBoundsInScreenSpace(const gfx::RectF& bounds);
diff --git a/chrome/browser/password_manager/password_generation_manager_unittest.cc b/chrome/browser/password_manager/password_generation_manager_unittest.cc
index 33c33bc..c2c8646 100644
--- a/chrome/browser/password_manager/password_generation_manager_unittest.cc
+++ b/chrome/browser/password_manager/password_generation_manager_unittest.cc
@@ -6,11 +6,9 @@
 
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/password_manager/chrome_password_manager_client.h"
 #include "chrome/browser/password_manager/password_generation_manager.h"
 #include "chrome/browser/password_manager/password_manager.h"
-#include "chrome/browser/sync/profile_sync_service.h"
-#include "chrome/browser/sync/profile_sync_service_factory.h"
+#include "chrome/browser/password_manager/password_manager_client.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
@@ -27,115 +25,147 @@ using base::ASCIIToUTF16;
 
 namespace {
 
-// Unlike the base AutofillMetrics, exposes copy and assignment constructors,
-// which are handy for briefer test code.  The AutofillMetrics class is
-// stateless, so this is safe.
-class TestAutofillMetrics : public autofill::AutofillMetrics {
- public:
-  TestAutofillMetrics() {}
-  virtual ~TestAutofillMetrics() {}
-};
-
-}  // anonymous namespace
-
-class TestPasswordGenerationManager : public PasswordGenerationManager {
+class TestPasswordManagerDriver : public PasswordManagerDriver {
  public:
-  explicit TestPasswordGenerationManager(content::WebContents* contents)
-      : PasswordGenerationManager(
-            contents,
-            ChromePasswordManagerClient::FromWebContents(contents)) {}
-  virtual ~TestPasswordGenerationManager() {}
-
-  virtual void SendAccountCreationFormsToRenderer(
-      content::RenderViewHost* host,
+  TestPasswordManagerDriver(content::WebContents* web_contents,
+                            PasswordManagerClient* client)
+      : password_manager_(client),
+        password_generation_manager_(web_contents, client),
+        is_off_the_record_(false) {}
+  virtual ~TestPasswordManagerDriver() {}
+
+  // PasswordManagerDriver implementation.
+  virtual void FillPasswordForm(const autofill::PasswordFormFillData& form_data)
+      OVERRIDE {}
+  virtual bool DidLastPageLoadEncounterSSLErrors() OVERRIDE { return false; }
+  virtual bool IsOffTheRecord() OVERRIDE { return is_off_the_record_; }
+  virtual PasswordGenerationManager* GetPasswordGenerationManager() OVERRIDE {
+    return &password_generation_manager_;
+  }
+  virtual PasswordManager* GetPasswordManager() OVERRIDE {
+    return &password_manager_;
+  }
+  virtual autofill::AutofillManager* GetAutofillManager() OVERRIDE {
+    return NULL;
+  }
+  virtual void AllowPasswordGenerationForForm(autofill::PasswordForm* form)
+      OVERRIDE {}
+  virtual void AccountCreationFormsFound(
       const std::vector<autofill::FormData>& forms) OVERRIDE {
-    sent_account_creation_forms_.insert(
-        sent_account_creation_forms_.begin(), forms.begin(), forms.end());
+    found_account_creation_forms_.insert(
+        found_account_creation_forms_.begin(), forms.begin(), forms.end());
   }
 
-  const std::vector<autofill::FormData>& GetSentAccountCreationForms() {
-    return sent_account_creation_forms_;
+  const std::vector<autofill::FormData>& GetFoundAccountCreationForms() {
+    return found_account_creation_forms_;
+  }
+  void set_is_off_the_record(bool is_off_the_record) {
+    is_off_the_record_ = is_off_the_record;
   }
 
-  void ClearSentAccountCreationForms() {
-    sent_account_creation_forms_.clear();
+ private:
+  PasswordManager password_manager_;
+  PasswordGenerationManager password_generation_manager_;
+  std::vector<autofill::FormData> found_account_creation_forms_;
+  bool is_off_the_record_;
+};
+
+class TestPasswordManagerClient : public PasswordManagerClient {
+ public:
+  explicit TestPasswordManagerClient(content::WebContents* web_contents,
+                                     Profile* profile)
+      : profile_(profile),
+        driver_(web_contents, this),
+        is_sync_enabled_(false) {}
+
+  virtual void PromptUserToSavePassword(PasswordFormManager* form_to_save)
+      OVERRIDE {}
+  virtual PasswordStore* GetPasswordStore() OVERRIDE { return NULL; }
+  virtual PrefService* GetPrefs() OVERRIDE { return profile_->GetPrefs(); }
+  virtual PasswordManagerDriver* GetDriver() OVERRIDE { return &driver_; }
+  virtual void AuthenticateAutofillAndFillForm(
+      scoped_ptr<autofill::PasswordFormFillData> fill_data) OVERRIDE {}
+  virtual bool IsPasswordSyncEnabled() OVERRIDE { return is_sync_enabled_; }
+
+  void set_is_password_sync_enabled(bool enabled) {
+    is_sync_enabled_ = enabled;
   }
 
  private:
-  std::vector<autofill::FormData> sent_account_creation_forms_;
+  Profile* profile_;
+  TestPasswordManagerDriver driver_;
+  bool is_sync_enabled_;
+};
 
-  DISALLOW_COPY_AND_ASSIGN(TestPasswordGenerationManager);
+// Unlike the base AutofillMetrics, exposes copy and assignment constructors,
+// which are handy for briefer test code.  The AutofillMetrics class is
+// stateless, so this is safe.
+class TestAutofillMetrics : public autofill::AutofillMetrics {
+ public:
+  TestAutofillMetrics() {}
+  virtual ~TestAutofillMetrics() {}
 };
 
+}  // anonymous namespace
+
 class PasswordGenerationManagerTest : public ChromeRenderViewHostTestHarness {
  protected:
   virtual void SetUp() OVERRIDE {
     SetThreadBundleOptions(content::TestBrowserThreadBundle::REAL_IO_THREAD);
     ChromeRenderViewHostTestHarness::SetUp();
 
-    ChromePasswordManagerClient::CreateForWebContents(web_contents());
-    password_generation_manager_.reset(
-        new TestPasswordGenerationManager(web_contents()));
+    client_.reset(new TestPasswordManagerClient(web_contents(), profile()));
   }
 
   virtual void TearDown() OVERRIDE {
+    client_.reset();
     ChromeRenderViewHostTestHarness::TearDown();
   }
 
+  PasswordGenerationManager* GetGenerationManager() {
+    return client_->GetDriver()->GetPasswordGenerationManager();
+  }
+
+  TestPasswordManagerDriver* GetTestDriver() {
+    return static_cast<TestPasswordManagerDriver*>(client_->GetDriver());
+  }
+
   bool IsGenerationEnabled() {
-    return password_generation_manager_->IsGenerationEnabled();
+    return GetGenerationManager()->IsGenerationEnabled();
   }
 
   void DetectAccountCreationForms(
       const std::vector<autofill::FormStructure*>& forms) {
-    password_generation_manager_->DetectAccountCreationForms(forms);
+    GetGenerationManager()->DetectAccountCreationForms(forms);
   }
 
-  scoped_ptr<TestPasswordGenerationManager> password_generation_manager_;
-};
-
-class IncognitoPasswordGenerationManagerTest :
-    public PasswordGenerationManagerTest {
- public:
-  virtual content::BrowserContext* CreateBrowserContext() OVERRIDE {
-    // Create an incognito profile.
-    TestingProfile::Builder builder;
-    builder.SetIncognito();
-    scoped_ptr<TestingProfile> profile = builder.Build();
-    return profile.release();
-  }
+  scoped_ptr<TestPasswordManagerClient> client_;
 };
 
 TEST_F(PasswordGenerationManagerTest, IsGenerationEnabled) {
-  PrefService* prefs = profile()->GetPrefs();
-
-  // Enable syncing. Generation should be enabled.
-  prefs->SetBoolean(prefs::kSyncKeepEverythingSynced, false);
-  ProfileSyncService* sync_service = ProfileSyncServiceFactory::GetForProfile(
-      profile());
-  sync_service->SetSyncSetupCompleted();
-  syncer::ModelTypeSet preferred_set;
-  preferred_set.Put(syncer::PASSWORDS);
-  sync_service->ChangePreferredDataTypes(preferred_set);
+  // Enabling the PasswordManager and password sync should cause generation to
+  // be enabled.
+  PrefService* prefs = client_->GetPrefs();
+  prefs->SetBoolean(prefs::kPasswordManagerEnabled, true);
+  client_->set_is_password_sync_enabled(true);
   EXPECT_TRUE(IsGenerationEnabled());
 
-  // Change syncing preferences to not include passwords. Generation should
-  // be disabled.
-  preferred_set.Put(syncer::EXTENSIONS);
-  preferred_set.Remove(syncer::PASSWORDS);
-  sync_service->ChangePreferredDataTypes(preferred_set);
+  // Disabling password syncing should cause generation to be disabled.
+  client_->set_is_password_sync_enabled(false);
   EXPECT_FALSE(IsGenerationEnabled());
 
-  // Disable syncing. Generation should also be disabled.
-  sync_service->DisableForUser();
+  // Disabling the PasswordManager should cause generation to be disabled even
+  // if syncing is enabled.
+  prefs->SetBoolean(prefs::kPasswordManagerEnabled, false);
+  client_->set_is_password_sync_enabled(true);
   EXPECT_FALSE(IsGenerationEnabled());
 }
 
 TEST_F(PasswordGenerationManagerTest, DetectAccountCreationForms) {
   // Setup so that IsGenerationEnabled() returns true.
-  ProfileSyncService* sync_service = ProfileSyncServiceFactory::GetForProfile(
-      profile());
-  sync_service->SetSyncSetupCompleted();
+  PrefService* prefs = client_->GetPrefs();
+  prefs->SetBoolean(prefs::kPasswordManagerEnabled, true);
+  client_->set_is_password_sync_enabled(true);
 
   autofill::FormData login_form;
   login_form.origin = GURL("http://www.yahoo.com/login/");
@@ -179,23 +209,19 @@ TEST_F(PasswordGenerationManagerTest, DetectAccountCreationForms) {
       TestAutofillMetrics());
 
   DetectAccountCreationForms(forms);
-  EXPECT_EQ(1u,
-            password_generation_manager_->GetSentAccountCreationForms().size());
-  EXPECT_EQ(
-      GURL("http://accounts.yahoo.com/"),
-      password_generation_manager_->GetSentAccountCreationForms()[0].origin);
+  EXPECT_EQ(1u, GetTestDriver()->GetFoundAccountCreationForms().size());
+  EXPECT_EQ(GURL("http://accounts.yahoo.com/"),
+            GetTestDriver()->GetFoundAccountCreationForms()[0].origin);
 }
 
-TEST_F(IncognitoPasswordGenerationManagerTest,
-       UpdatePasswordSyncStateIncognito) {
-  // Disable password manager by going incognito. Even though syncing is
-  // enabled, generation should still be disabled.
-  PrefService* prefs = profile()->GetPrefs();
-
-  // Allow this test to control what should get synced.
-  prefs->SetBoolean(prefs::kSyncKeepEverythingSynced, false);
+TEST_F(PasswordGenerationManagerTest, UpdatePasswordSyncStateIncognito) {
+  // Disable password manager by going incognito. Even though password
+  // syncing is enabled, generation should still
+  // be disabled.
+  GetTestDriver()->set_is_off_the_record(true);
+  PrefService* prefs = client_->GetPrefs();
+  prefs->SetBoolean(prefs::kPasswordManagerEnabled, true);
+  client_->set_is_password_sync_enabled(true);
 
-  browser_sync::SyncPrefs sync_prefs(profile()->GetPrefs());
-  sync_prefs.SetSyncSetupCompleted();
   EXPECT_FALSE(IsGenerationEnabled());
 }
diff --git a/chrome/browser/password_manager/password_manager_driver.h b/chrome/browser/password_manager/password_manager_driver.h
index 74cc845..3cd772e 100644
--- a/chrome/browser/password_manager/password_manager_driver.h
+++ b/chrome/browser/password_manager/password_manager_driver.h
@@ -5,11 +5,14 @@
 #ifndef CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_DRIVER_H_
 #define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_DRIVER_H_
 
+#include <vector>
+
 class PasswordGenerationManager;
 class PasswordManager;
 
 namespace autofill {
 class AutofillManager;
+struct FormData;
 struct PasswordForm;
 struct PasswordFormFillData;
 }  // namespace autofill
@@ -44,6 +47,10 @@ class PasswordManagerDriver {
   // Informs the driver that |form| can be used for password generation.
   virtual void AllowPasswordGenerationForForm(autofill::PasswordForm* form) = 0;
 
+  // Notifies the driver that account creation |forms| were found.
+  virtual void AccountCreationFormsFound(
+      const std::vector<autofill::FormData>& forms) = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(PasswordManagerDriver);
 };
diff --git a/chrome/browser/password_manager/password_manager_unittest.cc b/chrome/browser/password_manager/password_manager_unittest.cc
index ec6a0d6..c1c236f 100644
--- a/chrome/browser/password_manager/password_manager_unittest.cc
+++ b/chrome/browser/password_manager/password_manager_unittest.cc
@@ -66,6 +66,8 @@ class MockPasswordManagerDriver : public PasswordManagerDriver {
   MOCK_METHOD0(GetAutofillManager, autofill::AutofillManager*());
   MOCK_METHOD1(AllowPasswordGenerationForForm,
                void(autofill::PasswordForm* form));
+  MOCK_METHOD1(AccountCreationFormsFound,
+               void(const std::vector<autofill::FormData>&));
 };
 
 ACTION_P(InvokeConsumer, forms) {