Only Register() platform specific content settings types on different platforms

components/content_settings/core/browser/website_settings_registry_unittest.cc

Index: components/content_settings/core/browser/website_settings_registry_unittest.cc
diff --git a/components/content_settings/core/browser/website_settings_registry_unittest.cc b/components/content_settings/core/browser/website_settings_registry_unittest.cc
index d79ba248162047ec980015f087185e0aedd29bb6..3c077d09bdba3a178bfc0e787bd62a0837a29fca 100644
--- a/components/content_settings/core/browser/website_settings_registry_unittest.cc
+++ b/components/content_settings/core/browser/website_settings_registry_unittest.cc
@@ -24,6 +24,7 @@ class WebsiteSettingsRegistryTest : public testing::Test {
   WebsiteSettingsRegistry registry_;
 };
 
+#if !defined(OS_IOS)
 TEST_F(WebsiteSettingsRegistryTest, Get) {

[msramek, 2016/05/24 14:29:24]

I think the spirit of this test is not to test specifically whether APP_BANNER
is registered, but whether Get() works properly. Why not change APP_BANNER to
COOKIES to run the test everywhere?

Then we could add one more test to test specifically the platform-dependent
behavior, e.g. GetPlatformDependent that would test something that is not on
iOS, and something that is not on mobile.

[raymes, 2016/05/25 03:36:07]

+1

[lshang, 2016/05/25 10:38:16]

Done.

Cookies is ContentSettingsInfo, it don't get registered in
WebsiteSettingsRegistry. But yeah I agree with your idea and thanks for the
reminding! I added a GetPlatformDependent test in ContentSettingsRegistryTest to
test platform-dependent behavior.

For the 5 types in WebsiteSettingsInfo, auto-select-certificate is unsure
whether or not it is used on iOS, so I make it apply to all platforms until we
get some evidence. The other 4 types(ssl-cert-decision, app-banner,
site-engagement and usb-chooser-data) are confirmed to be not used. I checked in
.gypi file, they are under non-ios sources. So I changed app-banner to
auto-select-certificate in these tests.

   // CONTENT_SETTINGS_TYPE_APP_BANNER should be registered.
   const WebsiteSettingsInfo* info =
@@ -32,23 +33,28 @@ TEST_F(WebsiteSettingsRegistryTest, Get) {
   EXPECT_EQ(CONTENT_SETTINGS_TYPE_APP_BANNER, info->type());
   EXPECT_EQ("app-banner", info->name());
 }
+#endif
 
 TEST_F(WebsiteSettingsRegistryTest, GetByName) {
   // Random string shouldn't be registered.
   EXPECT_FALSE(registry()->GetByName("abc"));
+  const WebsiteSettingsInfo* info;
 
+#if !defined(OS_IOS)

[msramek, 2016/05/24 14:29:24]

Ditto here.

[lshang, 2016/05/25 10:38:17]

Done.

   // "app-banner" should be registered.
-  const WebsiteSettingsInfo* info = registry()->GetByName("app-banner");
+  info = registry()->GetByName("app-banner");
   ASSERT_TRUE(info);
   EXPECT_EQ(CONTENT_SETTINGS_TYPE_APP_BANNER, info->type());
   EXPECT_EQ("app-banner", info->name());
   EXPECT_EQ(registry()->Get(CONTENT_SETTINGS_TYPE_APP_BANNER), info);
+#endif
 
   // Register a new setting.
   registry()->Register(static_cast<ContentSettingsType>(10), "test", nullptr,
                        WebsiteSettingsInfo::UNSYNCABLE,
                        WebsiteSettingsInfo::LOSSY,
                        WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
+                       WebsiteSettingsInfo::ALL_PLATFORMS,
                        WebsiteSettingsInfo::INHERIT_IN_INCOGNITO);
   info = registry()->GetByName("test");
   ASSERT_TRUE(info);
@@ -58,9 +64,10 @@ TEST_F(WebsiteSettingsRegistryTest, GetByName) {
 }
 
 TEST_F(WebsiteSettingsRegistryTest, Properties) {
+  const WebsiteSettingsInfo* info;
+#if !defined(OS_IOS)

[msramek, 2016/05/24 14:29:24]

Ditto.

[lshang, 2016/05/25 10:38:16]

Done.

   // "app-banner" should be registered.
-  const WebsiteSettingsInfo* info =
-      registry()->Get(CONTENT_SETTINGS_TYPE_APP_BANNER);
+  info = registry()->Get(CONTENT_SETTINGS_TYPE_APP_BANNER);
   ASSERT_TRUE(info);
   EXPECT_EQ("profile.content_settings.exceptions.app_banner",
             info->pref_name());
@@ -68,13 +75,14 @@ TEST_F(WebsiteSettingsRegistryTest, Properties) {
             info->default_value_pref_name());
   ASSERT_FALSE(info->initial_default_value());
   EXPECT_EQ(PrefRegistry::LOSSY_PREF, info->GetPrefRegistrationFlags());
-
+#endif
   // Register a new setting.
   registry()->Register(static_cast<ContentSettingsType>(10), "test",
                        base::WrapUnique(new base::FundamentalValue(999)),
                        WebsiteSettingsInfo::SYNCABLE,
                        WebsiteSettingsInfo::LOSSY,
                        WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
+                       WebsiteSettingsInfo::ALL_PLATFORMS,
                        WebsiteSettingsInfo::INHERIT_IN_INCOGNITO);
   info = registry()->Get(static_cast<ContentSettingsType>(10));
   ASSERT_TRUE(info);
@@ -84,9 +92,13 @@ TEST_F(WebsiteSettingsRegistryTest, Properties) {
   int setting;
   ASSERT_TRUE(info->initial_default_value()->GetAsInteger(&setting));
   EXPECT_EQ(999, setting);
+#if defined(OS_IOS)
+  EXPECT_EQ(PrefRegistry::LOSSY_PREF, info->GetPrefRegistrationFlags());
+#else
   EXPECT_EQ(PrefRegistry::LOSSY_PREF |
                 user_prefs::PrefRegistrySyncable::SYNCABLE_PREF,
             info->GetPrefRegistrationFlags());
+#endif
   EXPECT_EQ(WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
             info->scoping_type());
   EXPECT_EQ(WebsiteSettingsInfo::INHERIT_IN_INCOGNITO,
@@ -99,6 +111,7 @@ TEST_F(WebsiteSettingsRegistryTest, Iteration) {
                        WebsiteSettingsInfo::SYNCABLE,
                        WebsiteSettingsInfo::LOSSY,
                        WebsiteSettingsInfo::TOP_LEVEL_DOMAIN_ONLY_SCOPE,
+                       WebsiteSettingsInfo::ALL_PLATFORMS,
                        WebsiteSettingsInfo::INHERIT_IN_INCOGNITO);
 
   bool found = false;