Expose PasswordManagerDriver as a public interface to core Password code.

This CL eliminates the forwarding that PasswordManagerDelegateImpl was
previously doing to ContentPasswordManagerDriver. Instead,
PasswordManagerDelegate has a new GetDriver() method that returns the
PasswordManagerDriver that the core PasswordManager code should use. Tests are
updated to reflect this change.

BUG=335096

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

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

chrome/browser/password_manager/password_form_manager_unittest.cc

@@ -11,6 +11,7 @@
 #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/password_manager/password_manager_driver.h"
 #include "chrome/browser/password_manager/password_store.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/password_manager/test_password_store.h"
@@ -26,19 +27,27 @@ using ::testing::Eq;
 
 namespace {
 
-class TestPasswordManagerDelegate : public PasswordManagerDelegate {
+class TestPasswordManagerDriver : public PasswordManagerDriver {
  public:
-  explicit TestPasswordManagerDelegate(Profile* profile) : profile_(profile) {}
+  TestPasswordManagerDriver() {}
 
   virtual void FillPasswordForm(
       const autofill::PasswordFormFillData& form_data) OVERRIDE {}
+  virtual bool DidLastPageLoadEncounterSSLErrors() OVERRIDE { return false; }
+};
+
+class TestPasswordManagerDelegate : public PasswordManagerDelegate {
+ public:
+  explicit TestPasswordManagerDelegate(Profile* profile) : profile_(profile) {}
+
   virtual void AddSavePasswordInfoBarIfPermitted(
       PasswordFormManager* form_to_save) OVERRIDE {}
   virtual Profile* GetProfile() OVERRIDE { return profile_; }
-  virtual bool DidLastPageLoadEncounterSSLErrors() OVERRIDE { return false; }
+  virtual PasswordManagerDriver* GetDriver() OVERRIDE { return &driver_; }
 
  private:
   Profile* profile_;
+  TestPasswordManagerDriver driver_;
 };
 
 class TestPasswordManager : public PasswordManager {

chrome/browser/password_manager/password_manager.cc

@@ -13,6 +13,7 @@
 #include "base/threading/platform_thread.h"
 #include "chrome/browser/password_manager/password_form_manager.h"
 #include "chrome/browser/password_manager/password_manager_delegate.h"
+#include "chrome/browser/password_manager/password_manager_driver.h"
 #include "chrome/browser/password_manager/password_manager_metrics_util.h"
 #include "chrome/browser/password_manager/password_manager_util.h"
 #include "chrome/browser/profiles/profile.h"
@@ -112,8 +113,10 @@ void PasswordManager::CreateForWebContentsAndDelegate(
 PasswordManager::PasswordManager(WebContents* web_contents,
                                  PasswordManagerDelegate* delegate)
     : content::WebContentsObserver(web_contents),
-      delegate_(delegate) {
+      delegate_(delegate),
+      driver_(delegate->GetDriver()) {
   DCHECK(delegate_);
+  DCHECK(driver_);
   password_manager_enabled_.Init(prefs::kPasswordManagerEnabled,
                                  delegate_->GetProfile()->GetPrefs());
 
@@ -138,7 +141,7 @@ void PasswordManager::SetFormHasGeneratedPassword(const PasswordForm& form) {
   // not the common case, and should only happen when there is a bug in our
   // ability to detect forms.
   bool ssl_valid = (form.origin.SchemeIsSecure() &&
-                    !delegate_->DidLastPageLoadEncounterSSLErrors());
+                    !driver_->DidLastPageLoadEncounterSSLErrors());
   PasswordFormManager* manager =
       new PasswordFormManager(delegate_->GetProfile(),
                               this,
@@ -234,7 +237,7 @@ void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {
 
   PasswordForm provisionally_saved_form(form);
   provisionally_saved_form.ssl_valid = form.origin.SchemeIsSecure() &&
-      !delegate_->DidLastPageLoadEncounterSSLErrors();
+      !driver_->DidLastPageLoadEncounterSSLErrors();
   provisionally_saved_form.preferred = true;
   PasswordFormManager::OtherPossibleUsernamesAction action =
       PasswordFormManager::IGNORE_OTHER_POSSIBLE_USERNAMES;
@@ -309,7 +312,7 @@ void PasswordManager::OnPasswordFormSubmitted(
 void PasswordManager::OnPasswordFormsParsed(
     const std::vector<PasswordForm>& forms) {
   // Ask the SSLManager for current security.
-  bool had_ssl_error = delegate_->DidLastPageLoadEncounterSSLErrors();
+  bool had_ssl_error = driver_->DidLastPageLoadEncounterSSLErrors();
 
   for (std::vector<PasswordForm>::const_iterator iter = forms.begin();
        iter != forms.end(); ++iter) {
@@ -452,7 +455,7 @@ void PasswordManager::Autofill(
                                wait_for_username,
                                OtherPossibleUsernamesEnabled(),
                                &fill_data);
-      delegate_->FillPasswordForm(fill_data);
+      driver_->FillPasswordForm(fill_data);
       break;
     }
     default:

chrome/browser/password_manager/password_manager.h

@@ -21,6 +21,7 @@
 #include "content/public/browser/web_contents_user_data.h"
 
 class PasswordManagerDelegate;
+class PasswordManagerDriver;
 class PasswordManagerTest;
 class PasswordFormManager;
 class PrefRegistrySimple;
@@ -162,10 +163,12 @@ class PasswordManager : public LoginModel,
   // time a user submits a login form and gets to the next page.
   scoped_ptr<PasswordFormManager> provisional_save_manager_;
 
-  // Our delegate for carrying out external operations.  This is typically the
-  // containing WebContents.
+  // The embedder-level client. Must outlive this class.
   PasswordManagerDelegate* const delegate_;
 
+  // The platform-level driver. Must outlive this class.
+  PasswordManagerDriver* const driver_;
+
   // Set to false to disable the password manager (will no longer ask if you
   // want to save passwords but will continue to fill passwords).
   BooleanPrefMember password_manager_enabled_;

chrome/browser/password_manager/password_manager_delegate.h

@@ -6,12 +6,9 @@
 #define CHROME_BROWSER_PASSWORD_MANAGER_PASSWORD_MANAGER_DELEGATE_H_
 
 class PasswordFormManager;
+class PasswordManagerDriver;
 class Profile;
 
-namespace autofill {
-struct PasswordFormFillData;
-}  // namespace autofill
-
 // An abstraction of operations in the external environment (WebContents)
 // that the PasswordManager depends on.  This allows for more targeted
 // unit testing.
@@ -20,12 +17,6 @@ class PasswordManagerDelegate {
   PasswordManagerDelegate() {}
   virtual ~PasswordManagerDelegate() {}
 
-  // Fill forms matching |form_data| in |web_contents|.  By default, goes
-  // through the RenderViewHost to FillPasswordForm.  Tests can override this
-  // to sever the dependency on the entire rendering stack.
-  virtual void FillPasswordForm(
-      const autofill::PasswordFormFillData& form_data) = 0;
-
   // A mechanism to show an infobar in the current tab at our request.
   // The infobar may not show in some circumstances, such as when the one-click
   // sign in infobar is or will be shown.
@@ -35,9 +26,8 @@ class PasswordManagerDelegate {
   // Get the profile for which we are managing passwords.
   virtual Profile* GetProfile() = 0;
 
-  // If any SSL certificate errors were encountered as a result of the last
-  // page load.
-  virtual bool DidLastPageLoadEncounterSSLErrors() = 0;
+  // Returns the PasswordManagerDriver instance associated with this instance.
+  virtual PasswordManagerDriver* GetDriver() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PasswordManagerDelegate);

chrome/browser/password_manager/password_manager_delegate_impl.cc

@@ -215,11 +215,6 @@ PasswordManagerDelegateImpl::PasswordManagerDelegateImpl(
 PasswordManagerDelegateImpl::~PasswordManagerDelegateImpl() {
 }
 
-void PasswordManagerDelegateImpl::FillPasswordForm(
-    const autofill::PasswordFormFillData& form_data) {
-  driver_.FillPasswordForm(form_data);
-}
-
 void PasswordManagerDelegateImpl::AddSavePasswordInfoBarIfPermitted(
     PasswordFormManager* form_to_save) {
   std::string uma_histogram_suffix(
@@ -234,6 +229,6 @@ Profile* PasswordManagerDelegateImpl::GetProfile() {
   return Profile::FromBrowserContext(web_contents_->GetBrowserContext());
 }
 
-bool PasswordManagerDelegateImpl::DidLastPageLoadEncounterSSLErrors() {
-  return driver_.DidLastPageLoadEncounterSSLErrors();
+PasswordManagerDriver* PasswordManagerDelegateImpl::GetDriver() {
+  return &driver_;
 }

chrome/browser/password_manager/password_manager_delegate_impl.h

@@ -22,12 +22,10 @@ class PasswordManagerDelegateImpl
   virtual ~PasswordManagerDelegateImpl();
 
   // PasswordManagerDelegate implementation.
-  virtual void FillPasswordForm(
-      const autofill::PasswordFormFillData& form_data) OVERRIDE;
   virtual void AddSavePasswordInfoBarIfPermitted(
       PasswordFormManager* form_to_save) OVERRIDE;
   virtual Profile* GetProfile() OVERRIDE;
-  virtual bool DidLastPageLoadEncounterSSLErrors() OVERRIDE;
+  virtual PasswordManagerDriver* GetDriver() OVERRIDE;
 
  private:
   explicit PasswordManagerDelegateImpl(content::WebContents* web_contents);

chrome/browser/password_manager/password_manager_unittest.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/password_manager/mock_password_store.h"
 #include "chrome/browser/password_manager/password_manager.h"
 #include "chrome/browser/password_manager/password_manager_delegate.h"
+#include "chrome/browser/password_manager/password_manager_driver.h"
 #include "chrome/browser/password_manager/password_store.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/common/pref_names.h"
@@ -35,9 +36,14 @@ namespace {
 
 class MockPasswordManagerDelegate : public PasswordManagerDelegate {
  public:
-  MOCK_METHOD1(FillPasswordForm, void(const autofill::PasswordFormFillData&));
   MOCK_METHOD1(AddSavePasswordInfoBarIfPermitted, void(PasswordFormManager*));
   MOCK_METHOD0(GetProfile, Profile*());
+  MOCK_METHOD0(GetDriver, PasswordManagerDriver*());
+};
+
+class MockPasswordManagerDriver : public PasswordManagerDriver {
+ public:
+  MOCK_METHOD1(FillPasswordForm, void(const autofill::PasswordFormFillData&));
   MOCK_METHOD0(DidLastPageLoadEncounterSSLErrors, bool());
 };
 
@@ -83,9 +89,10 @@ class PasswordManagerTest : public ChromeRenderViewHostTestHarness {
             profile(), MockPasswordStore::Build).get());
 
     EXPECT_CALL(delegate_, GetProfile()).WillRepeatedly(Return(profile()));
+    EXPECT_CALL(delegate_, GetDriver()).WillRepeatedly(Return(&driver_));
     manager_ = TestPasswordManager::CreateForWebContentsAndDelegate(
         web_contents(), &delegate_);
-    EXPECT_CALL(delegate_, DidLastPageLoadEncounterSSLErrors())
+    EXPECT_CALL(driver_, DidLastPageLoadEncounterSSLErrors())
         .WillRepeatedly(Return(false));
   }
 
@@ -183,6 +190,7 @@ class PasswordManagerTest : public ChromeRenderViewHostTestHarness {
   scoped_refptr<MockPasswordStore> store_;
   TestPasswordManager* manager_;
   MockPasswordManagerDelegate delegate_;  // Owned by manager_.
+  MockPasswordManagerDriver driver_;
   PasswordForm submitted_form_;
 };
 
@@ -200,7 +208,7 @@ MATCHER_P(FormMatches, form, "") {
 TEST_F(PasswordManagerTest, FormSubmitEmptyStore) {
   // Test that observing a newly submitted form shows the save password bar.
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -232,7 +240,7 @@ TEST_F(PasswordManagerTest, GeneratedPasswordFormSubmitEmptyStore) {
   // This test is the same FormSubmitEmptyStore, except that it simulates the
   // user generating the password through the browser.
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -266,7 +274,7 @@ TEST_F(PasswordManagerTest, FormSubmitNoGoodMatch) {
   PasswordForm* existing_different = new PasswordForm(MakeSimpleForm());
   existing_different->username_value = ASCIIToUTF16("google2");
   result.push_back(existing_different);
-  EXPECT_CALL(delegate_, FillPasswordForm(_));
+  EXPECT_CALL(driver_, FillPasswordForm(_));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
 
@@ -296,7 +304,7 @@ TEST_F(PasswordManagerTest, FormSubmitNoGoodMatch) {
 
 TEST_F(PasswordManagerTest, FormSeenThenLeftPage) {
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -317,7 +325,7 @@ TEST_F(PasswordManagerTest, FormSubmitAfterNavigateSubframe) {
   // Test that navigating a subframe does not prevent us from showing the save
   // password infobar.
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -353,7 +361,7 @@ TEST_F(PasswordManagerTest, FormSubmitAfterNavigateSubframe) {
 // This test verifies a fix for http://crbug.com/236673
 TEST_F(PasswordManagerTest, FormSubmitWithFormOnPreviousPage) {
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillRepeatedly(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   PasswordForm first_form(MakeSimpleForm());
@@ -405,7 +413,7 @@ TEST_F(PasswordManagerTest, FormSubmitWithFormOnPreviousPage) {
 
 TEST_F(PasswordManagerTest, FormSubmitFailedLogin) {
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillRepeatedly(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -426,7 +434,7 @@ TEST_F(PasswordManagerTest, FormSubmitInvisibleLogin) {
   // Tests fix of issue 28911: if the login form reappears on the subsequent
   // page, but is invisible, it shouldn't count as a failed login.
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillRepeatedly(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -459,7 +467,7 @@ TEST_F(PasswordManagerTest, InitiallyInvisibleForm) {
   std::vector<PasswordForm*> result;
   PasswordForm* existing = new PasswordForm(MakeSimpleForm());
   result.push_back(existing);
-  EXPECT_CALL(delegate_, FillPasswordForm(_));
+  EXPECT_CALL(driver_, FillPasswordForm(_));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillRepeatedly(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -494,7 +502,7 @@ TEST_F(PasswordManagerTest, FillPasswordsOnDisabledManager) {
   TestingPrefServiceSyncable* prefService = profile()->GetTestingPrefService();
   prefService->SetUserPref(prefs::kPasswordManagerEnabled,
                            base::Value::CreateBooleanValue(false));
-  EXPECT_CALL(delegate_, FillPasswordForm(_));
+  EXPECT_CALL(driver_, FillPasswordForm(_));
   EXPECT_CALL(*store_.get(),
               GetLogins(_, testing::Eq(PasswordStore::DISALLOW_PROMPT), _))
       .WillRepeatedly(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
@@ -508,7 +516,7 @@ TEST_F(PasswordManagerTest, FormNotSavedAutocompleteOff) {
   // Test password form with non-generated password will not be saved if
   // autocomplete=off.
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -536,7 +544,7 @@ TEST_F(PasswordManagerTest, GeneratedPasswordFormSavedAutocompleteOff) {
   // Test password form with generated password will still be saved if
   // autocomplete=off.
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(Exactly(0));
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(Exactly(0));
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillOnce(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;
@@ -576,7 +584,7 @@ TEST_F(PasswordManagerTest, PasswordFormReappearance) {
   // is at least one visible password form in the next page that
   // means that our previous login attempt failed.
   std::vector<PasswordForm*> result;  // Empty password store.
-  EXPECT_CALL(delegate_, FillPasswordForm(_)).Times(0);
+  EXPECT_CALL(driver_, FillPasswordForm(_)).Times(0);
   EXPECT_CALL(*store_.get(), GetLogins(_, _, _))
       .WillRepeatedly(DoAll(WithArg<2>(InvokeConsumer(result)), Return()));
   std::vector<PasswordForm> observed;