Remove SiteSettingsPrefClient

A couple of notification related prefs are moved to
//components/content_settings. Their registration remains in //chrome
since other embedders don't use them yet.

Bug: 1071603
Change-Id: Ic43ea701619c3764db899b4965db6e414638f016
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2288310
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788878}

chrome/android/chrome_java_sources.gni

@@ -1429,7 +1429,6 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/signin/account_picker/ExistingAccountRowViewBinder.java",
   "java/src/org/chromium/chrome/browser/site_settings/ChromeSiteSettingsClient.java",
   "java/src/org/chromium/chrome/browser/site_settings/ChromeSiteSettingsHelpClient.java",
-  "java/src/org/chromium/chrome/browser/site_settings/ChromeSiteSettingsPrefClient.java",
   "java/src/org/chromium/chrome/browser/site_settings/ChromeWebappSettingsClient.java",
   "java/src/org/chromium/chrome/browser/site_settings/CookieControlsServiceBridge.java",
   "java/src/org/chromium/chrome/browser/site_settings/ManageSpaceActivity.java",

chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

@@ -4,6 +4,8 @@
 
 package org.chromium.chrome.browser.notifications;
 
+import static org.chromium.components.content_settings.PrefNames.NOTIFICATIONS_VIBRATE_ENABLED;
+
 import android.app.Notification;
 import android.app.PendingIntent;
 import android.app.RemoteInput;
@@ -36,7 +38,6 @@ import org.chromium.chrome.browser.ChromeApplication;
 import org.chromium.chrome.browser.browserservices.TrustedWebActivityClient;
 import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
 import org.chromium.chrome.browser.notifications.channels.SiteChannelsManager;
-import org.chromium.chrome.browser.preferences.Pref;
 import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.settings.SettingsLauncher;
 import org.chromium.chrome.browser.settings.SettingsLauncherImpl;
@@ -628,7 +629,7 @@ public class NotificationPlatformBridge {
         // is in vibrate mode, there is no custom pattern, and the vibration default has been
         // disabled. To truly prevent vibration, provide a custom empty pattern.
         boolean vibrateEnabled =
-                PrefServiceBridge.getInstance().getBoolean(Pref.NOTIFICATIONS_VIBRATE_ENABLED);
+                PrefServiceBridge.getInstance().getBoolean(NOTIFICATIONS_VIBRATE_ENABLED);
         if (!vibrateEnabled) {
             vibrationPattern = EMPTY_VIBRATION_PATTERN;
         }

chrome/android/java/src/org/chromium/chrome/browser/site_settings/ChromeSiteSettingsClient.java

@@ -47,7 +47,6 @@ public class ChromeSiteSettingsClient implements SiteSettingsClient {
 
     private final Context mContext;
     private ChromeSiteSettingsHelpClient mChromeSiteSettingsHelpClient;
-    private ChromeSiteSettingsPrefClient mChromeSiteSettingsPrefClient;
     private ChromeWebappSettingsClient mChromeWebappSettingsClient;
     private ManagedPreferenceDelegate mManagedPreferenceDelegate;
 
@@ -81,14 +80,6 @@ public class ChromeSiteSettingsClient implements SiteSettingsClient {
         return mChromeSiteSettingsHelpClient;
     }
 
-    @Override
-    public ChromeSiteSettingsPrefClient getSiteSettingsPrefClient() {
-        if (mChromeSiteSettingsPrefClient == null) {
-            mChromeSiteSettingsPrefClient = new ChromeSiteSettingsPrefClient();
-        }
-        return mChromeSiteSettingsPrefClient;
-    }
-
     @Override
     public WebappSettingsClient getWebappSettingsClient() {
         if (mChromeWebappSettingsClient == null) {

chrome/android/java/src/org/chromium/chrome/browser/site_settings/ChromeSiteSettingsPrefClient.java

-// Copyright 2020 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.
-
-package org.chromium.chrome.browser.site_settings;
-
-import org.chromium.chrome.browser.preferences.Pref;
-import org.chromium.chrome.browser.preferences.PrefServiceBridge;
-import org.chromium.components.browser_ui.site_settings.SiteSettingsPrefClient;
-
-/**
- * A SiteSettingsPrefClient implementation that delegates to Chrome's PrefServiceBridge.
- */
-public class ChromeSiteSettingsPrefClient implements SiteSettingsPrefClient {
-    @Override
-    public boolean getEnableQuietNotificationPermissionUi() {
-        return PrefServiceBridge.getInstance().getBoolean(
-                Pref.ENABLE_QUIET_NOTIFICATION_PERMISSION_UI);
-    }
-
-    @Override
-    public void setEnableQuietNotificationPermissionUi(boolean newValue) {
-        PrefServiceBridge.getInstance().setBoolean(
-                Pref.ENABLE_QUIET_NOTIFICATION_PERMISSION_UI, newValue);
-    }
-
-    @Override
-    public void clearEnableNotificationPermissionUi() {
-        PrefServiceBridge.getInstance().clearPref(Pref.ENABLE_QUIET_NOTIFICATION_PERMISSION_UI);
-    }
-
-    @Override
-    public void setNotificationsVibrateEnabled(boolean newValue) {
-        PrefServiceBridge.getInstance().setBoolean(Pref.NOTIFICATIONS_VIBRATE_ENABLED, newValue);
-    }
-}

chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeTest.java

@@ -6,6 +6,8 @@ package org.chromium.chrome.browser.notifications;
 
 import static org.junit.Assert.assertThat;
 
+import static org.chromium.components.content_settings.PrefNames.NOTIFICATIONS_VIBRATE_ENABLED;
+
 import android.app.Notification;
 import android.app.PendingIntent;
 import android.app.RemoteInput;
@@ -36,7 +38,6 @@ import org.chromium.chrome.R;
 import org.chromium.chrome.browser.engagement.SiteEngagementService;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
 import org.chromium.chrome.browser.permissions.PermissionTestRule;
-import org.chromium.chrome.browser.preferences.Pref;
 import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.tab.Tab;
@@ -444,7 +445,7 @@ public class NotificationPlatformBridgeTest {
         TestThreadUtils.runOnUiThreadBlocking(
                 ()
                         -> PrefServiceBridge.getInstance().setBoolean(
-                                Pref.NOTIFICATIONS_VIBRATE_ENABLED, false));
+                                NOTIFICATIONS_VIBRATE_ENABLED, false));
 
         Notification notification = showAndGetNotification("MyNotification", notificationOptions);
 
@@ -494,7 +495,7 @@ public class NotificationPlatformBridgeTest {
         TestThreadUtils.runOnUiThreadBlocking(
                 ()
                         -> Assert.assertTrue(PrefServiceBridge.getInstance().getBoolean(
-                                Pref.NOTIFICATIONS_VIBRATE_ENABLED)));
+                                NOTIFICATIONS_VIBRATE_ENABLED)));
 
         Notification notification = showAndGetNotification("MyNotification", "{ vibrate: 42 }");
 

chrome/browser/notifications/notification_platform_bridge_android.cc

@@ -25,7 +25,7 @@
 #include "chrome/browser/notifications/platform_notification_service_impl.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/pref_names.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"

chrome/browser/permissions/adaptive_quiet_notification_permission_ui_enabler.cc

@@ -25,6 +25,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/permissions/permission_util.h"
 #include "components/pref_registry/pref_registry_syncable.h"

chrome/browser/permissions/chrome_permission_request_manager_unittest.cc

@@ -24,6 +24,7 @@
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/permissions/features.h"
 #include "components/permissions/notification_permission_ui_selector.h"
 #include "components/permissions/permission_request.h"

chrome/browser/permissions/contextual_notification_permission_ui_selector.cc

@@ -21,7 +21,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/chrome_features.h"
-#include "chrome/common/pref_names.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/permissions/permission_request.h"
 #include "components/prefs/pref_service.h"
 #include "components/safe_browsing/core/db/database_manager.h"

chrome/browser/permissions/contextual_notification_permission_ui_selector_unittest.cc

@@ -20,9 +20,9 @@
 #include "chrome/browser/permissions/quiet_notification_permission_ui_config.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_service.h"
 #include "chrome/common/chrome_features.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/permissions/test/mock_permission_request.h"
 #include "components/prefs/testing_pref_service.h"
 #include "content/public/test/browser_task_environment.h"

chrome/browser/permissions/permission_prompt_android_unittest.cc

@@ -6,8 +6,8 @@
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/infobars/mock_infobar_service.h"
 #include "chrome/common/chrome_features.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/permissions/permission_request_manager.h"
 #include "components/permissions/test/mock_permission_request.h"
 #include "components/prefs/pref_service.h"

chrome/browser/permissions/quiet_notification_permission_ui_state.cc

@@ -6,6 +6,7 @@
 
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 

chrome/browser/ui/content_settings/content_setting_image_model_unittest.cc

@@ -21,13 +21,13 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/browser/tab_specific_content_settings.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
+#include "components/content_settings/core/common/pref_names.h"
 #include "components/permissions/features.h"
 #include "components/permissions/notification_permission_ui_selector.h"
 #include "components/permissions/permission_request.h"

chrome/common/pref_names.cc

@@ -1471,9 +1471,6 @@ const char kLocalDiscoveryNotificationsEnabled[] =
     "local_discovery.notifications_enabled";
 
 #if defined(OS_ANDROID)
-// Enable vibration for web notifications.
-const char kNotificationsVibrateEnabled[] = "notifications.vibrate_enabled";
-
 // Boolean pref indicating whether notification permissions were migrated to
 // notification channels (on Android O+ we use channels to store notification
 // permission, so any existing permissions must be migrated).
@@ -1548,11 +1545,6 @@ const char kHasSeenWelcomePage[] = "browser.has_seen_welcome_page";
 const char kNaviOnboardGroup[] = "browser.navi_onboard_group";
 #endif  // defined(OS_WIN)
 
-// Boolean indicating whether the quiet UX is enabled for notification
-// permission requests.
-const char kEnableQuietNotificationPermissionUi[] =
-    "profile.content_settings.enable_quiet_permission_ui.notifications";
-
 // Boolean indicating whether, as part of the adaptive activation quiet UI dry
 // run experiment, the user has accumulated three notification permission
 // request denies in a row.

chrome/common/pref_names.h

@@ -474,7 +474,6 @@ extern const char kLocalDiscoveryEnabled[];
 extern const char kLocalDiscoveryNotificationsEnabled[];
 
 #if defined(OS_ANDROID)
-extern const char kNotificationsVibrateEnabled[];
 extern const char kMigratedToSiteNotificationChannels[];
 extern const char kClearedBlockedSiteNotificationChannels[];
 extern const char kUsageStatsEnabled[];
@@ -514,7 +513,6 @@ extern const char kNaviOnboardGroup[];
 extern const char kCrashReportingEnabled[];
 #endif  // defined(OS_ANDROID)
 
-extern const char kEnableQuietNotificationPermissionUi[];
 extern const char kQuietNotificationPermissionShouldShowPromo[];
 extern const char kQuietNotificationPermissionPromoWasShown[];
 extern const char kNotificationPermissionActions[];

components/browser_ui/site_settings/android/BUILD.gn

@@ -64,7 +64,6 @@ android_library("java") {
     "java/src/org/chromium/components/browser_ui/site_settings/SiteSettingsClient.java",
     "java/src/org/chromium/components/browser_ui/site_settings/SiteSettingsFeatureList.java",
     "java/src/org/chromium/components/browser_ui/site_settings/SiteSettingsHelpClient.java",
-    "java/src/org/chromium/components/browser_ui/site_settings/SiteSettingsPrefClient.java",
     "java/src/org/chromium/components/browser_ui/site_settings/SiteSettingsPreferenceFragment.java",
     "java/src/org/chromium/components/browser_ui/site_settings/StorageInfo.java",
     "java/src/org/chromium/components/browser_ui/site_settings/TriStateSiteSettingsPreference.java",

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/SingleCategorySettings.java

@@ -8,6 +8,8 @@ import static org.chromium.components.browser_ui.settings.SearchUtils.handleSear
 import static org.chromium.components.browser_ui.site_settings.WebsitePreferenceBridge.SITE_WILDCARD;
 import static org.chromium.components.content_settings.PrefNames.BLOCK_THIRD_PARTY_COOKIES;
 import static org.chromium.components.content_settings.PrefNames.COOKIE_CONTROLS_MODE;
+import static org.chromium.components.content_settings.PrefNames.ENABLE_QUIET_NOTIFICATION_PERMISSION_UI;
+import static org.chromium.components.content_settings.PrefNames.NOTIFICATIONS_VIBRATE_ENABLED;
 
 import android.content.Context;
 import android.content.DialogInterface;
@@ -558,14 +560,13 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
         } else if (THIRD_PARTY_COOKIES_TOGGLE_KEY.equals(preference.getKey())) {
             prefService.setBoolean(BLOCK_THIRD_PARTY_COOKIES, (boolean) newValue);
         } else if (NOTIFICATIONS_VIBRATE_TOGGLE_KEY.equals(preference.getKey())) {
-            getPrefs().setNotificationsVibrateEnabled((boolean) newValue);
+            prefService.setBoolean(NOTIFICATIONS_VIBRATE_ENABLED, (boolean) newValue);
         } else if (NOTIFICATIONS_QUIET_UI_TOGGLE_KEY.equals(preference.getKey())) {
-            boolean boolValue = (boolean) newValue;
-            if (boolValue) {
-                getPrefs().setEnableQuietNotificationPermissionUi(true);
+            if ((boolean) newValue) {
+                prefService.setBoolean(ENABLE_QUIET_NOTIFICATION_PERMISSION_UI, true);
             } else {
                 // Clear the pref so if the default changes later the user will get the new default.
-                getPrefs().clearEnableNotificationPermissionUi();
+                prefService.clearPref(ENABLE_QUIET_NOTIFICATION_PERMISSION_UI);
             }
         }
         return true;
@@ -1173,7 +1174,9 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
                 quiet_ui_pref = (ChromeBaseCheckBoxPreference) getPreferenceScreen().findPreference(
                         NOTIFICATIONS_QUIET_UI_TOGGLE_KEY);
             }
-            quiet_ui_pref.setChecked(getPrefs().getEnableQuietNotificationPermissionUi());
+            PrefService prefService = UserPrefs.get(browserContextHandle);
+            quiet_ui_pref.setChecked(
+                    prefService.getBoolean(ENABLE_QUIET_NOTIFICATION_PERMISSION_UI));
         } else if (quiet_ui_pref != null) {
             // Save a reference to allow re-adding it to the screen.
             mNotificationsQuietUiPref = quiet_ui_pref;
@@ -1190,8 +1193,4 @@ public class SingleCategorySettings extends SiteSettingsPreferenceFragment
             ManagedPreferencesUtils.showManagedByAdministratorToast(getActivity());
         }
     }
-
-    private SiteSettingsPrefClient getPrefs() {
-        return getSiteSettingsClient().getSiteSettingsPrefClient();
-    }
 }

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/SiteSettingsClient.java

@@ -37,13 +37,6 @@ public interface SiteSettingsClient {
      */
     SiteSettingsHelpClient getSiteSettingsHelpClient();
 
-    /**
-     * @return The SiteSettingsPrefClient that should be used to access native prefs when rendering
-     *     the Site Settings UI.
-     */
-    // TODO(crbug.com/1071603): Remove this once PrefServiceBridge is componentized.
-    SiteSettingsPrefClient getSiteSettingsPrefClient();
-
     /**
      * @return The WebappSettingsClient that should be used when showing the Site Settings UI.
      */

components/browser_ui/site_settings/android/java/src/org/chromium/components/browser_ui/site_settings/SiteSettingsPrefClient.java

-// Copyright 2020 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.
-
-package org.chromium.components.browser_ui.site_settings;
-
-/**
- * An interface that allows the Site Settings UI to read and write native prefs.
- *
- * This interface was introduced because there's currently no way for a component to access native
- * prefs from Java, and should be removed once that functionality exists.
- */
-// TODO(crbug.com/1071603): Remove this once PrefServiceBridge is componentized.
-public interface SiteSettingsPrefClient {
-    boolean getEnableQuietNotificationPermissionUi();
-    void setEnableQuietNotificationPermissionUi(boolean newValue);
-    void clearEnableNotificationPermissionUi();
-
-    void setNotificationsVibrateEnabled(boolean newValue);
-}

components/content_settings/core/common/pref_names.cc

@@ -94,4 +94,14 @@ const char kManagedWebUsbBlockedForUrls[] =
 const char kManagedLegacyCookieAccessAllowedForDomains[] =
     "profile.managed_legacy_cookie_access_allowed_for_domains";
 
+// Boolean indicating whether the quiet UX is enabled for notification
+// permission requests.
+const char kEnableQuietNotificationPermissionUi[] =
+    "profile.content_settings.enable_quiet_permission_ui.notifications";
+
+#if defined(OS_ANDROID)
+// Enable vibration for web notifications.
+const char kNotificationsVibrateEnabled[] = "notifications.vibrate_enabled";
+#endif
+
 }  // namespace prefs

components/content_settings/core/common/pref_names.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_CONTENT_SETTINGS_CORE_COMMON_PREF_NAMES_H_
 #define COMPONENTS_CONTENT_SETTINGS_CORE_COMMON_PREF_NAMES_H_
 
+#include "build/build_config.h"
 
 namespace prefs {
 
@@ -55,6 +56,12 @@ extern const char kManagedWebUsbAskForUrls[];
 extern const char kManagedWebUsbBlockedForUrls[];
 extern const char kManagedLegacyCookieAccessAllowedForDomains[];
 
+extern const char kEnableQuietNotificationPermissionUi[];
+
+#if defined(OS_ANDROID)
+extern const char kNotificationsVibrateEnabled[];
+#endif
+
 }  // namespace prefs
 
 #endif  // COMPONENTS_CONTENT_SETTINGS_CORE_COMMON_PREF_NAMES_H_

weblayer/browser/java/org/chromium/weblayer_private/WebLayerSiteSettingsClient.java

@@ -16,7 +16,6 @@ import org.chromium.components.browser_ui.settings.ManagedPreferenceDelegate;
 import org.chromium.components.browser_ui.site_settings.SiteSettingsCategory.Type;
 import org.chromium.components.browser_ui.site_settings.SiteSettingsClient;
 import org.chromium.components.browser_ui.site_settings.SiteSettingsHelpClient;
-import org.chromium.components.browser_ui.site_settings.SiteSettingsPrefClient;
 import org.chromium.components.browser_ui.site_settings.WebappSettingsClient;
 import org.chromium.components.content_settings.ContentSettingsType;
 import org.chromium.components.embedder_support.browser_context.BrowserContextHandle;
@@ -28,9 +27,8 @@ import java.util.Set;
 /**
  * A SiteSettingsClient instance that contains WebLayer-specific Site Settings logic.
  */
-public class WebLayerSiteSettingsClient
-        implements SiteSettingsClient, ManagedPreferenceDelegate, SiteSettingsHelpClient,
-                   SiteSettingsPrefClient, WebappSettingsClient {
+public class WebLayerSiteSettingsClient implements SiteSettingsClient, ManagedPreferenceDelegate,
+                                                   SiteSettingsHelpClient, WebappSettingsClient {
     private final BrowserContextHandle mBrowserContextHandle;
 
     public WebLayerSiteSettingsClient(BrowserContextHandle browserContextHandle) {
@@ -54,11 +52,6 @@ public class WebLayerSiteSettingsClient
         return this;
     }
 
-    @Override
-    public SiteSettingsPrefClient getSiteSettingsPrefClient() {
-        return this;
-    }
-
     @Override
     public WebappSettingsClient getWebappSettingsClient() {
         return this;
@@ -147,22 +140,6 @@ public class WebLayerSiteSettingsClient
     @Override
     public void launchProtectedContentHelpAndFeedbackActivity(Activity currentActivity) {}
 
-    // SiteSettingsPrefClient implementation:
-
-    // The quiet notification UI is a Chrome-specific feature for now.
-    @Override
-    public boolean getEnableQuietNotificationPermissionUi() {
-        return false;
-    }
-    @Override
-    public void setEnableQuietNotificationPermissionUi(boolean newValue) {}
-    @Override
-    public void clearEnableNotificationPermissionUi() {}
-
-    // WebLayer doesn't support notifications yet.
-    @Override
-    public void setNotificationsVibrateEnabled(boolean newValue) {}
-
     // WebappSettingsClient implementation:
     // A no-op since WebLayer doesn't support webapps.