Only Register() desktop-only content settings types on desktop platforms

components/content_settings/core/browser/content_settings_registry.cc

Index: components/content_settings/core/browser/content_settings_registry.cc
diff --git a/components/content_settings/core/browser/content_settings_registry.cc b/components/content_settings/core/browser/content_settings_registry.cc
index 9c66ce35e919c8408d78ab6480a839dcc8e1c523..e8a0c1a17ea5bde1107a5752a064038db83d3abf 100644
--- a/components/content_settings/core/browser/content_settings_registry.cc
+++ b/components/content_settings/core/browser/content_settings_registry.cc
@@ -125,14 +125,6 @@ void ContentSettingsRegistry::Init() {
            WebsiteSettingsInfo::REQUESTING_DOMAIN_ONLY_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
-  Register(CONTENT_SETTINGS_TYPE_IMAGES, "images", CONTENT_SETTING_ALLOW,
-           WebsiteSettingsInfo::SYNCABLE,
-           WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme,
-                              kExtensionScheme),
-           ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK),
-           WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
-           ContentSettingsInfo::INHERIT_IN_INCOGNITO);
-
   Register(CONTENT_SETTINGS_TYPE_JAVASCRIPT, "javascript",
            CONTENT_SETTING_ALLOW, WebsiteSettingsInfo::SYNCABLE,
            WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme,
@@ -141,16 +133,6 @@ void ContentSettingsRegistry::Init() {
            WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
-  Register(CONTENT_SETTINGS_TYPE_PLUGINS, "plugins",
-           CONTENT_SETTING_DETECT_IMPORTANT_CONTENT,
-           WebsiteSettingsInfo::SYNCABLE,
-           WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
-           ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
-                         CONTENT_SETTING_ASK,
-                         CONTENT_SETTING_DETECT_IMPORTANT_CONTENT),
-           WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
-           ContentSettingsInfo::INHERIT_IN_INCOGNITO);
-
   Register(CONTENT_SETTINGS_TYPE_POPUPS, "popups", CONTENT_SETTING_BLOCK,
            WebsiteSettingsInfo::SYNCABLE,
            WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme,
@@ -184,14 +166,6 @@ void ContentSettingsRegistry::Init() {
            WebsiteSettingsInfo::REQUESTING_ORIGIN_AND_TOP_LEVEL_ORIGIN_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
-  Register(CONTENT_SETTINGS_TYPE_MOUSELOCK, "mouselock", CONTENT_SETTING_ASK,
-           WebsiteSettingsInfo::SYNCABLE,
-           WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
-           ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
-                         CONTENT_SETTING_ASK),
-           WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
-           ContentSettingsInfo::INHERIT_IN_INCOGNITO);
-
   Register(CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC, "media-stream-mic",
            CONTENT_SETTING_ASK, WebsiteSettingsInfo::UNSYNCABLE,
            WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
@@ -208,13 +182,13 @@ void ContentSettingsRegistry::Init() {
            WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
-  Register(CONTENT_SETTINGS_TYPE_PPAPI_BROKER, "ppapi-broker",
-           CONTENT_SETTING_ASK, WebsiteSettingsInfo::UNSYNCABLE,
-           WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
-           ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
-                         CONTENT_SETTING_ASK),
-           WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
-           ContentSettingsInfo::INHERIT_IN_INCOGNITO);
+  RegisterDesktop(CONTENT_SETTINGS_TYPE_PPAPI_BROKER, "ppapi-broker",

[msramek, 2016/05/17 18:58:27]

nit: Move this down as well?

+                  CONTENT_SETTING_ASK, WebsiteSettingsInfo::UNSYNCABLE,
+                  WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
+                  ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
+                                CONTENT_SETTING_ASK),
+                  WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
+                  ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
   Register(CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, "automatic-downloads",
            CONTENT_SETTING_ASK, WebsiteSettingsInfo::SYNCABLE,
@@ -271,28 +245,55 @@ void ContentSettingsRegistry::Init() {
            WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
-  // Content settings that aren't used to store any data. TODO(raymes): use a
-  // different mechanism rather than content settings to represent these.
-  // Since nothing is stored in them, there is no real point in them being a
-  // content setting.
-  Register(CONTENT_SETTINGS_TYPE_PROTOCOL_HANDLERS, "protocol-handler",
-           CONTENT_SETTING_DEFAULT, WebsiteSettingsInfo::UNSYNCABLE,
-           WhitelistedSchemes(), ValidSettings(),
-           WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
-           ContentSettingsInfo::INHERIT_IN_INCOGNITO);
-
-  Register(CONTENT_SETTINGS_TYPE_MIXEDSCRIPT, "mixed-script",
-           CONTENT_SETTING_DEFAULT, WebsiteSettingsInfo::UNSYNCABLE,
-           WhitelistedSchemes(), ValidSettings(),
-           WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
-           ContentSettingsInfo::INHERIT_IN_INCOGNITO);
-
   Register(CONTENT_SETTINGS_TYPE_BLUETOOTH_GUARD, "bluetooth-guard",
            CONTENT_SETTING_ASK, WebsiteSettingsInfo::UNSYNCABLE,
            WhitelistedSchemes(),
            ValidSettings(CONTENT_SETTING_ASK, CONTENT_SETTING_BLOCK),
            WebsiteSettingsInfo::REQUESTING_ORIGIN_AND_TOP_LEVEL_ORIGIN_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
+
+  // Desktop-only content settings.
+  RegisterDesktop(CONTENT_SETTINGS_TYPE_IMAGES, "images", CONTENT_SETTING_ALLOW,
+                  WebsiteSettingsInfo::SYNCABLE,
+                  WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme,
+                                     kExtensionScheme),
+                  ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK),
+                  WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
+                  ContentSettingsInfo::INHERIT_IN_INCOGNITO);
+
+  RegisterDesktop(CONTENT_SETTINGS_TYPE_PLUGINS, "plugins",
+                  CONTENT_SETTING_DETECT_IMPORTANT_CONTENT,
+                  WebsiteSettingsInfo::SYNCABLE,
+                  WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
+                  ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
+                                CONTENT_SETTING_ASK,
+                                CONTENT_SETTING_DETECT_IMPORTANT_CONTENT),
+                  WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
+                  ContentSettingsInfo::INHERIT_IN_INCOGNITO);
+
+  RegisterDesktop(CONTENT_SETTINGS_TYPE_MOUSELOCK, "mouselock",
+                  CONTENT_SETTING_ASK, WebsiteSettingsInfo::SYNCABLE,
+                  WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
+                  ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
+                                CONTENT_SETTING_ASK),
+                  WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
+                  ContentSettingsInfo::INHERIT_IN_INCOGNITO);
+
+  // Content settings that aren't used to store any data. TODO(raymes): use a
+  // different mechanism rather than content settings to represent these.
+  // Since nothing is stored in them, there is no real point in them being a
+  // content setting.
+  RegisterDesktop(CONTENT_SETTINGS_TYPE_PROTOCOL_HANDLERS, "protocol-handler",
+                  CONTENT_SETTING_DEFAULT, WebsiteSettingsInfo::UNSYNCABLE,
+                  WhitelistedSchemes(), ValidSettings(),
+                  WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
+                  ContentSettingsInfo::INHERIT_IN_INCOGNITO);
+
+  RegisterDesktop(CONTENT_SETTINGS_TYPE_MIXEDSCRIPT, "mixed-script",
+                  CONTENT_SETTING_DEFAULT, WebsiteSettingsInfo::UNSYNCABLE,
+                  WhitelistedSchemes(), ValidSettings(),
+                  WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
+                  ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 }
 
 void ContentSettingsRegistry::Register(
@@ -324,4 +325,19 @@ void ContentSettingsRegistry::Register(
                               valid_settings, incognito_behavior));
 }
 
+void ContentSettingsRegistry::RegisterDesktop(

[msramek, 2016/05/17 18:58:26]

Since we do basically everything else via enum, maybe this should follow the
pattern? ALL_PLATFORMS, DESKTOP_ONLY, MOBILE_ONLY (this probably doesn't exist
yet) or something like that...

That will also force people to think about what they need instead of just using
Register() and not noticing RegisterDesktop(). And it will make it easier to
define more types if necessary (e.g. Blink-related settings would be defined on
Desktop and Android but not iOS).

+    ContentSettingsType type,
+    const std::string& name,
+    ContentSetting initial_default_value,
+    WebsiteSettingsInfo::SyncStatus sync_status,
+    const std::vector<std::string>& whitelisted_schemes,
+    const std::set<ContentSetting>& valid_settings,
+    WebsiteSettingsInfo::ScopingType scoping_type,
+    ContentSettingsInfo::IncognitoBehavior incognito_behavior) {
+#if !defined(OS_ANDROID)

[msramek, 2016/05/17 18:58:27]

Hmm, but what about iOS? :)

[msramek, 2016/05/17 19:12:11]

OK, after a quick search it appears that we have supported cookies and popups
default settings at some point in time, I'm not sure which we do right now (I
don't have an iPhone to test).

It seems that exceptions are not used though (which also means there is no UI)
and default settings are hidden in helper methods like this one:

https://code.google.com/p/chromium/codesearch#chromium/src/components/content...

+  Register(type, name, initial_default_value, sync_status, whitelisted_schemes,
+           valid_settings, scoping_type, incognito_behavior);
+#endif
+}
+
 }  // namespace content_settings