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

components/content_settings/core/browser/content_settings_registry.cc

 // Copyright 2015 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.
 
 #include "components/content_settings/core/browser/content_settings_registry.h"
 
 #include <utility>
 
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 107 matching lines @@
   // generate preference names and should never be changed!
 
   Register(CONTENT_SETTINGS_TYPE_COOKIES, "cookies", CONTENT_SETTING_ALLOW,
            WebsiteSettingsInfo::SYNCABLE,
            WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
                          CONTENT_SETTING_SESSION_ONLY),
            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,
                               kExtensionScheme),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK),
            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,
                               kExtensionScheme),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK),
            WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
   Register(CONTENT_SETTINGS_TYPE_GEOLOCATION, "geolocation",
            CONTENT_SETTING_ASK, WebsiteSettingsInfo::UNSYNCABLE,
@@ skipping 13 matching lines @@
            // implements additional incognito exceptions.
            ContentSettingsInfo::INHERIT_IN_INCOGNITO_EXCEPT_ALLOW);
 
   Register(CONTENT_SETTINGS_TYPE_FULLSCREEN, "fullscreen", CONTENT_SETTING_ASK,
            WebsiteSettingsInfo::SYNCABLE,
            WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_ASK),
            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),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
                          CONTENT_SETTING_ASK),
            WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
   Register(CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA, "media-stream-camera",
            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_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,
            WhitelistedSchemes(kChromeUIScheme, kChromeDevToolsScheme,
                               kExtensionScheme),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK,
                          CONTENT_SETTING_ASK),
            WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
@@ skipping 36 matching lines @@
            WebsiteSettingsInfo::REQUESTING_ORIGIN_ONLY_SCOPE,
            ContentSettingsInfo::INHERIT_IN_INCOGNITO);
 
   Register(CONTENT_SETTINGS_TYPE_BACKGROUND_SYNC, "background-sync",
            CONTENT_SETTING_ALLOW, WebsiteSettingsInfo::UNSYNCABLE,
            WhitelistedSchemes(),
            ValidSettings(CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK),
            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(
     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,
@@ skipping 11 matching lines @@
       website_settings_registry_->Register(
           type, name, std::move(default_value), sync_status,
           WebsiteSettingsInfo::NOT_LOSSY, scoping_type,
           WebsiteSettingsInfo::INHERIT_IN_INCOGNITO);
   DCHECK(!ContainsKey(content_settings_info_, type));
   content_settings_info_[type] = make_scoped_ptr(
       new ContentSettingsInfo(website_settings_info, whitelisted_schemes,
                               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