[PwdSettings] Use the settings service to check prefs

On Android, with UPM, we need to check whether the updater bridge exists/can be created to decide which pref to check, so the prefs should not be checked directly, but via the service which takes care of this.

Bug: 1289700

Change-Id: I034728fe83b971dad041969b7c8c5aeae631c5cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3620376
Commit-Queue: Ioana Pandele <ioanap@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001538}

chrome/browser/password_manager/chrome_password_manager_client.cc

@@ -29,6 +29,7 @@
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/password_manager/account_password_store_factory.h"
 #include "chrome/browser/password_manager/field_info_manager_factory.h"
+#include "chrome/browser/password_manager/password_manager_settings_service_factory.h"
 #include "chrome/browser/password_manager/password_reuse_manager_factory.h"
 #include "chrome/browser/password_manager/password_scripts_fetcher_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
@@ -71,6 +72,8 @@
 #include "components/password_manager/core/browser/password_form_manager_for_ui.h"
 #include "components/password_manager/core/browser/password_manager_constants.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
+#include "components/password_manager/core/browser/password_manager_setting.h"
+#include "components/password_manager/core/browser/password_manager_settings_service.h"
 #include "components/password_manager/core/browser/password_manager_util.h"
 #include "components/password_manager/core/browser/password_requirements_service.h"
 #include "components/password_manager/core/browser/password_scripts_fetcher.h"
@@ -176,6 +179,7 @@ using password_manager::PasswordForm;
 using password_manager::PasswordManagerClientHelper;
 using password_manager::PasswordManagerDriver;
 using password_manager::PasswordManagerMetricsRecorder;
+using password_manager::PasswordManagerSetting;
 using password_manager::metrics_util::PasswordType;
 using sessions::SerializedNavigationEntry;
 
@@ -264,8 +268,10 @@ bool ChromePasswordManagerClient::IsSavingAndFillingEnabled(
     // page, and there is no API to access (or dismiss) UI bubbles/infobars.
     return false;
   }
-  return password_manager_util::IsSavingPasswordsEnabled(GetPrefs(),
-                                                         GetSyncService()) &&
+  PasswordManagerSettingsService* settings_service =
+      PasswordManagerSettingsServiceFactory::GetForProfile(profile_);
+  return settings_service->IsSettingEnabled(
+             PasswordManagerSetting::kOfferToSavePasswords) &&
          !IsIncognito() && IsFillingEnabled(url);
 }
 
@@ -288,6 +294,13 @@ bool ChromePasswordManagerClient::IsFillingFallbackEnabled(
   return IsFillingEnabled(url) && !profile->IsGuestSession();
 }
 
+bool ChromePasswordManagerClient::IsAutoSignInEnabled() const {
+  PasswordManagerSettingsService* settings_service =
+      PasswordManagerSettingsServiceFactory::GetForProfile(profile_);
+  return settings_service->IsSettingEnabled(
+      PasswordManagerSetting::kAutoSignIn);
+}
+
 bool ChromePasswordManagerClient::PromptUserToSaveOrUpdatePassword(
     std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save,
     bool update_password) {

chrome/browser/password_manager/chrome_password_manager_client.h

@@ -106,6 +106,7 @@ class ChromePasswordManagerClient
   bool IsSavingAndFillingEnabled(const GURL& url) const override;
   bool IsFillingEnabled(const GURL& url) const override;
   bool IsFillingFallbackEnabled(const GURL& url) const override;
+  bool IsAutoSignInEnabled() const override;
   bool PromptUserToSaveOrUpdatePassword(
       std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save,
       bool is_update) override;

chrome/browser/password_manager/chrome_password_manager_client_unittest.cc

@@ -22,6 +22,7 @@
 #include "chrome/browser/autofill/mock_manual_filling_view.h"
 #include "chrome/browser/autofill/mock_password_accessory_controller.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
+#include "chrome/browser/password_manager/password_manager_settings_service_factory.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/user_interaction_observer.h"
@@ -40,16 +41,14 @@
 #include "components/password_manager/content/browser/password_manager_log_router_factory.h"
 #include "components/password_manager/core/browser/credential_cache.h"
 #include "components/password_manager/core/browser/credentials_filter.h"
+#include "components/password_manager/core/browser/mock_password_manager_settings_service.h"
 #include "components/password_manager/core/browser/password_form.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/password_manager/core/browser/password_manager_test_utils.h"
 #include "components/password_manager/core/browser/stub_password_manager_client.h"
 #include "components/password_manager/core/common/credential_manager_types.h"
 #include "components/password_manager/core/common/password_manager_features.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "components/prefs/pref_service.h"
-#include "components/prefs/testing_pref_service.h"
 #include "components/safe_browsing/buildflags.h"
 #include "components/safe_browsing/core/common/features.h"
 #include "components/sessions/content/content_record_password_state.h"
@@ -95,6 +94,7 @@ using content::BrowserContext;
 using content::WebContents;
 using password_manager::PasswordForm;
 using password_manager::PasswordManagerClient;
+using password_manager::PasswordManagerSetting;
 using sessions::GetPasswordStateFromNavigation;
 using sessions::SerializedNavigationEntry;
 using testing::_;
@@ -225,10 +225,6 @@ class ChromePasswordManagerClientTest : public ChromeRenderViewHostTestHarness {
   void SetUp() override;
   void TearDown() override;
 
-  sync_preferences::TestingPrefServiceSyncable* prefs() {
-    return profile()->GetTestingPrefService();
-  }
-
   // Caller does not own the returned pointer.
   syncer::TestSyncService* SetupBasicTestSync() {
     syncer::TestSyncService* sync_service =
@@ -238,6 +234,14 @@ class ChromePasswordManagerClientTest : public ChromeRenderViewHostTestHarness {
     return sync_service;
   }
 
+  void SetupSettingsServiceFactory() {
+    PasswordManagerSettingsServiceFactory::GetInstance()->SetTestingFactory(
+        profile(), base::BindRepeating([](content::BrowserContext* context)
+                                           -> std::unique_ptr<KeyedService> {
+          return std::make_unique<MockPasswordManagerSettingsService>();
+        }));
+  }
+
   // Make a navigation entry that will accept an annotation.
   void SetupNavigationForAnnotation() {
     syncer::TestSyncService* sync_service = SetupBasicTestSync();
@@ -256,7 +260,6 @@ class ChromePasswordManagerClientTest : public ChromeRenderViewHostTestHarness {
 
   FakePasswordAutofillAgent fake_agent_;
 
-  TestingPrefServiceSimple prefs_;
   bool metrics_enabled_ = false;
 
   base::test::ScopedFeatureList scoped_feature_list_;
@@ -272,11 +275,11 @@ void ChromePasswordManagerClientTest::SetUp() {
       base::BindRepeating(&FakePasswordAutofillAgent::BindReceiver,
                           base::Unretained(&fake_agent_)));
 
-  prefs_.registry()->RegisterBooleanPref(
-      password_manager::prefs::kCredentialsEnableService, true);
   ChromePasswordManagerClient::CreateForWebContentsWithAutofillClient(
       web_contents(), nullptr);
 
+  SetupSettingsServiceFactory();
+
   // Connect our bool for testing.
   ChromeMetricsServiceAccessor::SetMetricsAndCrashReportingForTesting(
       &metrics_enabled_);
@@ -360,16 +363,30 @@ TEST_F(ChromePasswordManagerClientTest, GetPasswordSyncState) {
 }
 
 TEST_F(ChromePasswordManagerClientTest,
-       SavingDependsOnManagerEnabledPreference) {
-  // Test that saving passwords depends on the password manager enabled
-  // preference.
+       SavingPasswordsTrueDeterminedByService) {
+  // Test that saving passwords depends on querying the settings service.
   ChromePasswordManagerClient* client = GetClient();
-  prefs()->SetUserPref(password_manager::prefs::kCredentialsEnableService,
-                       std::make_unique<base::Value>(true));
+  MockPasswordManagerSettingsService* settings_service =
+      static_cast<MockPasswordManagerSettingsService*>(
+          PasswordManagerSettingsServiceFactory::GetForProfile(profile()));
+  ON_CALL(*settings_service,
+          IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
+      .WillByDefault(Return(true));
   const GURL kUrlOn("https://accounts.google.com");
   EXPECT_TRUE(client->IsSavingAndFillingEnabled(kUrlOn));
-  prefs()->SetUserPref(password_manager::prefs::kCredentialsEnableService,
-                       std::make_unique<base::Value>(false));
+}
+
+TEST_F(ChromePasswordManagerClientTest,
+       SavingPasswordsFalseDeterminedByService) {
+  // Test that saving passwords depends on querying the settings service.
+  ChromePasswordManagerClient* client = GetClient();
+  MockPasswordManagerSettingsService* settings_service =
+      static_cast<MockPasswordManagerSettingsService*>(
+          PasswordManagerSettingsServiceFactory::GetForProfile(profile()));
+  EXPECT_CALL(*settings_service,
+              IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
+      .WillOnce(Return(false));
+  const GURL kUrlOn("https://accounts.google.com");
   EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
 }
 
@@ -379,6 +396,9 @@ TEST_F(ChromePasswordManagerClientTest, SavingAndFillingEnabledConditionsTest) {
           web_contents()->GetBrowserContext(), nullptr));
   std::unique_ptr<MockChromePasswordManagerClient> client(
       new MockChromePasswordManagerClient(test_web_contents.get()));
+  MockPasswordManagerSettingsService* settings_service =
+      static_cast<MockPasswordManagerSettingsService*>(
+          PasswordManagerSettingsServiceFactory::GetForProfile(profile()));
   // Functionality disabled if there is an SSL error.
   EXPECT_CALL(*client, GetMainFrameCertStatus())
       .WillRepeatedly(Return(net::CERT_STATUS_AUTHORITY_INVALID));
@@ -387,35 +407,45 @@ TEST_F(ChromePasswordManagerClientTest, SavingAndFillingEnabledConditionsTest) {
   EXPECT_FALSE(client->IsFillingEnabled(kUrlOn));
   EXPECT_FALSE(client->IsFillingFallbackEnabled(kUrlOn));
 
+  // Disable password saving.
+  ON_CALL(*settings_service,
+          IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
+      .WillByDefault(Return(false));
+
   // Functionality disabled if there are SSL errors and the manager itself is
   // disabled.
-  prefs()->SetUserPref(password_manager::prefs::kCredentialsEnableService,
-                       std::make_unique<base::Value>(false));
   EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
   EXPECT_FALSE(client->IsFillingEnabled(kUrlOn));
   EXPECT_FALSE(client->IsFillingFallbackEnabled(kUrlOn));
 
   // Saving disabled if there are no SSL errors, but the manager itself is
   // disabled.
   EXPECT_CALL(*client, GetMainFrameCertStatus()).WillRepeatedly(Return(0));
-  prefs()->SetUserPref(password_manager::prefs::kCredentialsEnableService,
-                       std::make_unique<base::Value>(false));
   EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
   EXPECT_TRUE(client->IsFillingEnabled(kUrlOn));
   EXPECT_TRUE(client->IsFillingFallbackEnabled(kUrlOn));
 
+  // Enable password saving.
+  ON_CALL(*settings_service,
+          IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
+      .WillByDefault(Return(true));
+
   // Functionality enabled if there are no SSL errors and the manager is
   // enabled.
   EXPECT_CALL(*client, GetMainFrameCertStatus()).WillRepeatedly(Return(0));
-  prefs()->SetUserPref(password_manager::prefs::kCredentialsEnableService,
-                       std::make_unique<base::Value>(true));
   EXPECT_TRUE(client->IsSavingAndFillingEnabled(kUrlOn));
   EXPECT_TRUE(client->IsFillingEnabled(kUrlOn));
   EXPECT_TRUE(client->IsFillingFallbackEnabled(kUrlOn));
 }
 
 TEST_F(ChromePasswordManagerClientTest,
        SavingAndFillingDisabledConditionsInOffTheRecord) {
+  MockPasswordManagerSettingsService* settings_service =
+      static_cast<MockPasswordManagerSettingsService*>(
+          PasswordManagerSettingsServiceFactory::GetForProfile(profile()));
+  ON_CALL(*settings_service,
+          IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
+      .WillByDefault(Return(true));
   std::unique_ptr<WebContents> incognito_web_contents(
       content::WebContentsTester::CreateTestWebContents(
           profile()->GetPrimaryOTRProfile(/*create_if_needed=*/true), nullptr));
@@ -429,14 +459,6 @@ TEST_F(ChromePasswordManagerClientTest,
   EXPECT_TRUE(client->IsFillingEnabled(kUrlOn));
   EXPECT_TRUE(client->IsFillingFallbackEnabled(kUrlOn));
 
-  // Saving disabled in Incognito mode also when manager itself is
-  // enabled.
-  prefs()->SetUserPref(password_manager::prefs::kCredentialsEnableService,
-                       std::make_unique<base::Value>(true));
-  EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
-  EXPECT_TRUE(client->IsFillingEnabled(kUrlOn));
-  EXPECT_TRUE(client->IsFillingFallbackEnabled(kUrlOn));
-
   // In guest mode saving is disabled, filling is enabled but there is in fact
   // nothing to fill, manual filling is disabled.
   profile()->SetGuestSession(true);
@@ -449,6 +471,33 @@ TEST_F(ChromePasswordManagerClientTest,
   EXPECT_FALSE(client->IsFillingFallbackEnabled(kUrlOn));
 }
 
+TEST_F(ChromePasswordManagerClientTest, AutoSignInEnabledDeterminedByService) {
+  // Test that auto sign in being allowed depends on querying the settings
+  // service.
+  ChromePasswordManagerClient* client = GetClient();
+  MockPasswordManagerSettingsService* settings_service =
+      static_cast<MockPasswordManagerSettingsService*>(
+          PasswordManagerSettingsServiceFactory::GetForProfile(profile()));
+  ON_CALL(*settings_service,
+          IsSettingEnabled(PasswordManagerSetting::kAutoSignIn))
+      .WillByDefault(Return(true));
+  EXPECT_TRUE(client->IsAutoSignInEnabled());
+}
+
+TEST_F(ChromePasswordManagerClientTest,
+       AutoSignInDisableddDeterminedByService) {
+  // Test that auto sign in being disallowed depends on querying the settings
+  // service.
+  ChromePasswordManagerClient* client = GetClient();
+  MockPasswordManagerSettingsService* settings_service =
+      static_cast<MockPasswordManagerSettingsService*>(
+          PasswordManagerSettingsServiceFactory::GetForProfile(profile()));
+  ON_CALL(*settings_service,
+          IsSettingEnabled(PasswordManagerSetting::kAutoSignIn))
+      .WillByDefault(Return(false));
+  EXPECT_FALSE(client->IsAutoSignInEnabled());
+}
+
 class ChromePasswordManagerClientAutomatedTest
     : public ChromePasswordManagerClientTest,
       public testing::WithParamInterface<bool> {
@@ -469,6 +518,14 @@ TEST_P(ChromePasswordManagerClientAutomatedTest, SavingDependsOnAutomation) {
   // Test that saving passwords UI is disabled for automated tests,
   // and enabled for non-automated tests.
   ChromePasswordManagerClient* client = GetClient();
+  MockPasswordManagerSettingsService* settings_service =
+      static_cast<MockPasswordManagerSettingsService*>(
+          PasswordManagerSettingsServiceFactory::GetForProfile(profile()));
+  // If saving isn't allowed it shouldn't be due to the setting, so make
+  // sure that is enabled.
+  ON_CALL(*settings_service,
+          IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
+      .WillByDefault(Return(true));
   const GURL kUrlOn("https://accounts.google.com");
   EXPECT_NE(client->IsSavingAndFillingEnabled(kUrlOn), GetParam());
 }
@@ -536,6 +593,14 @@ TEST_P(ChromePasswordManagerClientSchemeTest,
   auto* it = std::find_if(
       std::begin(kSchemeTestCases), std::end(kSchemeTestCases),
       [](auto test_case) { return strcmp(test_case.scheme, GetParam()) == 0; });
+  // If saving isn't allowed it shouldn't be due to the setting, so make
+  // sure that is enabled.
+  MockPasswordManagerSettingsService* settings_service =
+      static_cast<MockPasswordManagerSettingsService*>(
+          PasswordManagerSettingsServiceFactory::GetForProfile(profile()));
+  ON_CALL(*settings_service,
+          IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
+      .WillByDefault(Return(true));
   ASSERT_FALSE(it == std::end(kSchemeTestCases));
   EXPECT_EQ(it->password_manager_works,
             GetClient()->IsSavingAndFillingEnabled(url));
@@ -862,6 +927,12 @@ void ChromePasswordManagerClientAndroidTest::SetUp() {
       base::BindRepeating(&password_manager::BuildPasswordStoreInterface<
                           content::BrowserContext,
                           password_manager::MockPasswordStoreInterface>));
+  PasswordManagerSettingsServiceFactory::GetInstance()->SetTestingFactory(
+      GetBrowserContext(),
+      base::BindRepeating([](content::BrowserContext* context)
+                              -> std::unique_ptr<KeyedService> {
+        return std::make_unique<MockPasswordManagerSettingsService>();
+      }));
 }
 
 std::unique_ptr<password_manager::ContentPasswordManagerDriver>

chrome/browser/password_manager/password_manager_settings_service_factory.cc

@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/sync_service_factory.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/password_manager/core/browser/password_manager_settings_service.h"
 #include "components/password_manager/core/common/password_manager_features.h"
 #if BUILDFLAG(IS_ANDROID)
 #include "chrome/browser/password_manager/android/password_manager_settings_service_android_impl.h"

chrome/browser/ui/autofill/chrome_autofill_client.cc

@@ -23,6 +23,7 @@
 #include "chrome/browser/autofill/strike_database_factory.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/password_manager/chrome_password_manager_client.h"
+#include "chrome/browser/password_manager/password_manager_settings_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
@@ -61,6 +62,8 @@
 #include "components/autofill_assistant/browser/public/runtime_manager.h"
 #include "components/password_manager/content/browser/content_password_manager_driver.h"
 #include "components/password_manager/core/browser/password_manager_metrics_util.h"
+#include "components/password_manager/core/browser/password_manager_setting.h"
+#include "components/password_manager/core/browser/password_manager_settings_service.h"
 #include "components/password_manager/core/browser/password_manager_util.h"
 #include "components/password_manager/core/browser/password_requirements_service.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
@@ -867,8 +870,10 @@ bool ChromeAutofillClient::IsAutocompleteEnabled() {
 }
 
 bool ChromeAutofillClient::IsPasswordManagerEnabled() {
-  return password_manager_util::IsSavingPasswordsEnabled(GetPrefs(),
-                                                         GetSyncService());
+  PasswordManagerSettingsService* settings_service =
+      PasswordManagerSettingsServiceFactory::GetForProfile(GetProfile());
+  return settings_service->IsSettingEnabled(
+      password_manager::PasswordManagerSetting::kOfferToSavePasswords);
 }
 
 void ChromeAutofillClient::PropagateAutofillPredictions(

chrome/test/BUILD.gn

@@ -5618,6 +5618,7 @@ test("unit_tests") {
     "//components/page_load_metrics/common:test_support",
     "//components/paint_preview/common/mojom",
     "//components/password_manager/content/browser",
+    "//components/password_manager/core/browser:test_support",
     "//components/payments/content",
     "//components/payments/content:test_support",
     "//components/payments/content:utils",

components/password_manager/core/browser/BUILD.gn

@@ -536,6 +536,8 @@ static_library("test_support") {
     "mock_password_feature_manager.h",
     "mock_password_form_manager_for_ui.cc",
     "mock_password_form_manager_for_ui.h",
+    "mock_password_manager_settings_service.cc",
+    "mock_password_manager_settings_service.h",
     "mock_password_reuse_manager.cc",
     "mock_password_reuse_manager.h",
     "mock_password_store_backend.cc",

components/password_manager/core/browser/credential_manager_impl.cc

@@ -163,9 +163,7 @@ void CredentialManagerImpl::Get(CredentialMediationRequirement mediation,
 }
 
 bool CredentialManagerImpl::IsZeroClickAllowed() const {
-  return password_manager_util::IsAutoSignInEnabled(
-             client_->GetPrefs(), client_->GetSyncService()) &&
-         !client_->IsIncognito();
+  return client_->IsAutoSignInEnabled() && !client_->IsIncognito();
 }
 
 PasswordFormDigest CredentialManagerImpl::GetSynthesizedFormForOrigin() const {