[Password Manager] Improve signature of PasswordManagerClient::IsPasswordSyncEnabled

Currently, IsPasswordSyncEnabled needs to know if the caller is interested in syncing passwords with a custom passphrase or without, and returns a Boolean depending on whether the expected syncing occurs or not.

This CL gets rid of the need to pass the expectation, and changes the Boolean return value to say: no password sync, p.s. without a custom passphrase, or p.s. with a custom passphrase. It also changes the name of the method and the associated enum to make sense with the new signature.

This is a cosmetic change, but will make a subsequent CL easier to digest.

R=melandory@chromium.org

BUG=400674,486739

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

Cr-Commit-Position: refs/heads/master@{#329637}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -82,8 +82,8 @@ void ReportMetrics(bool password_manager_enabled,
   // May be null in tests.
   if (store) {
     store->ReportMetrics(client->GetSyncUsername(),
-                         client->IsPasswordSyncEnabled(
-                             password_manager::ONLY_CUSTOM_PASSPHRASE));
+                         client->GetPasswordSyncState() ==
+                             password_manager::SYNCING_WITH_CUSTOM_PASSPHRASE);
   }
   UMA_HISTOGRAM_BOOLEAN("PasswordManager.Enabled", password_manager_enabled);
 }
@@ -305,20 +305,18 @@ ChromePasswordManagerClient::GetPasswordStore() const {
              profile_, ServiceAccessType::EXPLICIT_ACCESS).get();
 }
 
-bool ChromePasswordManagerClient::IsPasswordSyncEnabled(
-    password_manager::CustomPassphraseState state) const {
+password_manager::PasswordSyncState
+ChromePasswordManagerClient::GetPasswordSyncState() const {
   ProfileSyncService* sync_service =
       ProfileSyncServiceFactory::GetForProfile(profile_);
   if (sync_service && sync_service->HasSyncSetupCompleted() &&
       sync_service->SyncActive() &&
       sync_service->GetActiveDataTypes().Has(syncer::PASSWORDS)) {
-    if (sync_service->IsUsingSecondaryPassphrase()) {
-      return state == password_manager::ONLY_CUSTOM_PASSPHRASE;
-    } else {
-      return state == password_manager::WITHOUT_CUSTOM_PASSPHRASE;
-    }
+    return sync_service->IsUsingSecondaryPassphrase()
+               ? password_manager::SYNCING_WITH_CUSTOM_PASSPHRASE
+               : password_manager::SYNCING_NORMAL_ENCRYPTION;
   }
-  return false;
+  return password_manager::NOT_SYNCING_PASSWORDS;
 }
 
 void ChromePasswordManagerClient::OnLogRouterAvailabilityChanged(

chrome/browser/password_manager/chrome_password_manager_client.h

@@ -69,8 +69,7 @@ class ChromePasswordManagerClient
       const autofill::PasswordFormMap& best_matches) const override;
   PrefService* GetPrefs() override;
   password_manager::PasswordStore* GetPasswordStore() const override;
-  bool IsPasswordSyncEnabled(
-      password_manager::CustomPassphraseState state) const override;
+  password_manager::PasswordSyncState GetPasswordSyncState() const override;
   void OnLogRouterAvailabilityChanged(bool router_can_be_used) override;
   void LogSavePasswordProgress(const std::string& text) const override;
   bool IsLoggingActive() const override;

chrome/browser/password_manager/chrome_password_manager_client_unittest.cc

@@ -357,7 +357,7 @@ TEST_F(ChromePasswordManagerClientTest,
   EXPECT_TRUE(client->IsPasswordManagementEnabledForCurrentPage());
 }
 
-TEST_F(ChromePasswordManagerClientTest, IsPasswordSyncEnabled) {
+TEST_F(ChromePasswordManagerClientTest, GetPasswordSyncState) {
   ChromePasswordManagerClient* client = GetClient();
 
   ProfileSyncServiceMock* mock_sync_service =
@@ -376,39 +376,31 @@ TEST_F(ChromePasswordManagerClientTest, IsPasswordSyncEnabled) {
       .WillRepeatedly(Return(false));
 
   // Passwords are syncing and custom passphrase isn't used.
-  EXPECT_FALSE(
-      client->IsPasswordSyncEnabled(password_manager::ONLY_CUSTOM_PASSPHRASE));
-  EXPECT_TRUE(client->IsPasswordSyncEnabled(
-      password_manager::WITHOUT_CUSTOM_PASSPHRASE));
+  EXPECT_EQ(password_manager::SYNCING_NORMAL_ENCRYPTION,
+            client->GetPasswordSyncState());
 
   // Again, using a custom passphrase.
   EXPECT_CALL(*mock_sync_service, IsUsingSecondaryPassphrase())
       .WillRepeatedly(Return(true));
 
-  EXPECT_TRUE(
-      client->IsPasswordSyncEnabled(password_manager::ONLY_CUSTOM_PASSPHRASE));
-  EXPECT_FALSE(client->IsPasswordSyncEnabled(
-      password_manager::WITHOUT_CUSTOM_PASSPHRASE));
+  EXPECT_EQ(password_manager::SYNCING_WITH_CUSTOM_PASSPHRASE,
+            client->GetPasswordSyncState());
 
-  // Always return false if we aren't syncing passwords.
+  // Report correctly if we aren't syncing passwords.
   active_types.Remove(syncer::PASSWORDS);
   active_types.Put(syncer::BOOKMARKS);
   EXPECT_CALL(*mock_sync_service, GetActiveDataTypes())
       .WillRepeatedly(Return(active_types));
 
-  EXPECT_FALSE(
-      client->IsPasswordSyncEnabled(password_manager::ONLY_CUSTOM_PASSPHRASE));
-  EXPECT_FALSE(client->IsPasswordSyncEnabled(
-      password_manager::WITHOUT_CUSTOM_PASSPHRASE));
+  EXPECT_EQ(password_manager::NOT_SYNCING_PASSWORDS,
+            client->GetPasswordSyncState());
 
   // Again, without a custom passphrase.
   EXPECT_CALL(*mock_sync_service, IsUsingSecondaryPassphrase())
       .WillRepeatedly(Return(false));
 
-  EXPECT_FALSE(
-      client->IsPasswordSyncEnabled(password_manager::ONLY_CUSTOM_PASSPHRASE));
-  EXPECT_FALSE(client->IsPasswordSyncEnabled(
-      password_manager::WITHOUT_CUSTOM_PASSPHRASE));
+  EXPECT_EQ(password_manager::NOT_SYNCING_PASSWORDS,
+            client->GetPasswordSyncState());
 }
 
 TEST_F(ChromePasswordManagerClientTest, IsOffTheRecordTest) {

components/password_manager/core/browser/password_generation_manager.cc

@@ -57,8 +57,8 @@ bool PasswordGenerationManager::IsGenerationEnabled() const {
   }
 
   // Don't consider sync enabled if the user has a custom passphrase. See
-  // crbug.com/358998 for more details.
-  if (!client_->IsPasswordSyncEnabled(WITHOUT_CUSTOM_PASSPHRASE)) {
+  // http://crbug.com/358998 for more details.
+  if (client_->GetPasswordSyncState() != SYNCING_NORMAL_ENCRYPTION) {
     VLOG(2) << "Generation disabled because passwords are not being synced or"
              << " custom passphrase is used.";
     return false;

components/password_manager/core/browser/password_generation_manager_unittest.cc

@@ -66,7 +66,7 @@ class TestPasswordManagerDriver : public StubPasswordManagerDriver {
 
 class MockPasswordManagerClient : public StubPasswordManagerClient {
  public:
-  MOCK_CONST_METHOD1(IsPasswordSyncEnabled, bool(CustomPassphraseState));
+  MOCK_CONST_METHOD0(GetPasswordSyncState, PasswordSyncState());
   MOCK_CONST_METHOD0(IsSavingEnabledForCurrentPage, bool());
   MOCK_CONST_METHOD0(IsOffTheRecord, bool());
 
@@ -122,24 +122,32 @@ class PasswordGenerationManagerTest : public testing::Test {
 
 TEST_F(PasswordGenerationManagerTest, IsGenerationEnabled) {
   // Enabling the PasswordManager and password sync should cause generation to
-  // be enabled.
+  // be enabled, unless the sync is with a custom passphrase.
   EXPECT_CALL(*client_, IsSavingEnabledForCurrentPage())
       .WillRepeatedly(testing::Return(true));
-  EXPECT_CALL(*client_, IsPasswordSyncEnabled(_))
-      .WillRepeatedly(testing::Return(true));
+  EXPECT_CALL(*client_, GetPasswordSyncState())
+      .WillRepeatedly(testing::Return(SYNCING_NORMAL_ENCRYPTION));
   EXPECT_TRUE(IsGenerationEnabled());
 
+  EXPECT_CALL(*client_, GetPasswordSyncState())
+      .WillRepeatedly(testing::Return(SYNCING_WITH_CUSTOM_PASSPHRASE));
+  EXPECT_FALSE(IsGenerationEnabled());
+
   // Disabling password syncing should cause generation to be disabled.
-  EXPECT_CALL(*client_, IsPasswordSyncEnabled(_))
-      .WillRepeatedly(testing::Return(false));
+  EXPECT_CALL(*client_, GetPasswordSyncState())
+      .WillRepeatedly(testing::Return(NOT_SYNCING_PASSWORDS));
   EXPECT_FALSE(IsGenerationEnabled());
 
   // Disabling the PasswordManager should cause generation to be disabled even
   // if syncing is enabled.
   EXPECT_CALL(*client_, IsSavingEnabledForCurrentPage())
       .WillRepeatedly(testing::Return(false));
-  EXPECT_CALL(*client_, IsPasswordSyncEnabled(_))
-      .WillRepeatedly(testing::Return(true));
+  EXPECT_CALL(*client_, GetPasswordSyncState())
+      .WillRepeatedly(testing::Return(SYNCING_NORMAL_ENCRYPTION));
+  EXPECT_FALSE(IsGenerationEnabled());
+
+  EXPECT_CALL(*client_, GetPasswordSyncState())
+      .WillRepeatedly(testing::Return(SYNCING_WITH_CUSTOM_PASSPHRASE));
   EXPECT_FALSE(IsGenerationEnabled());
 }
 
@@ -147,8 +155,8 @@ TEST_F(PasswordGenerationManagerTest, DetectAccountCreationForms) {
   // Setup so that IsGenerationEnabled() returns true.
   EXPECT_CALL(*client_, IsSavingEnabledForCurrentPage())
       .WillRepeatedly(testing::Return(true));
-  EXPECT_CALL(*client_, IsPasswordSyncEnabled(_))
-      .WillRepeatedly(testing::Return(true));
+  EXPECT_CALL(*client_, GetPasswordSyncState())
+      .WillRepeatedly(testing::Return(SYNCING_NORMAL_ENCRYPTION));
 
   autofill::FormData login_form;
   login_form.origin = GURL("http://www.yahoo.com/login/");
@@ -201,8 +209,8 @@ TEST_F(PasswordGenerationManagerTest, UpdatePasswordSyncStateIncognito) {
   EXPECT_CALL(*client_, IsOffTheRecord()).WillRepeatedly(testing::Return(true));
   PrefService* prefs = client_->GetPrefs();
   prefs->SetBoolean(prefs::kPasswordManagerSavingEnabled, true);
-  EXPECT_CALL(*client_, IsPasswordSyncEnabled(_))
-      .WillRepeatedly(testing::Return(true));
+  EXPECT_CALL(*client_, GetPasswordSyncState())
+      .WillRepeatedly(testing::Return(SYNCING_NORMAL_ENCRYPTION));
 
   EXPECT_FALSE(IsGenerationEnabled());
 }

components/password_manager/core/browser/password_manager_client.cc

@@ -29,9 +29,8 @@ void PasswordManagerClient::PasswordAutofillWasBlocked(
     const autofill::PasswordFormMap& best_matches) const {
 }
 
-bool PasswordManagerClient::IsPasswordSyncEnabled(
-    CustomPassphraseState state) const {
-  return false;
+PasswordSyncState PasswordManagerClient::GetPasswordSyncState() const {
+  return NOT_SYNCING_PASSWORDS;
 }
 
 void PasswordManagerClient::OnLogRouterAvailabilityChanged(

components/password_manager/core/browser/password_manager_client.h

@@ -24,9 +24,10 @@ class PasswordManager;
 class PasswordManagerDriver;
 class PasswordStore;
 
-enum CustomPassphraseState {
-  WITHOUT_CUSTOM_PASSPHRASE,
-  ONLY_CUSTOM_PASSPHRASE
+enum PasswordSyncState {
+  NOT_SYNCING_PASSWORDS,
+  SYNCING_NORMAL_ENCRYPTION,
+  SYNCING_WITH_CUSTOM_PASSPHRASE
 };
 
 enum class CredentialSourceType {
@@ -125,10 +126,9 @@ class PasswordManagerClient {
   // Returns the PasswordStore associated with this instance.
   virtual PasswordStore* GetPasswordStore() const = 0;
 
-  // Returns true if password sync is enabled in the embedder. Return value for
-  // custom passphrase users depends on |state|. The default implementation
-  // always returns false.
-  virtual bool IsPasswordSyncEnabled(CustomPassphraseState state) const;
+  // Reports whether and how passwords are synced in the embedder. The default
+  // implementation always returns NOT_SYNCING_PASSWORDS.
+  virtual PasswordSyncState GetPasswordSyncState() const;
 
   // Only for clients which registered with a LogRouter: If called with
   // |router_can_be_used| set to false, the client may no longer use the