Set custom-scoped patterns in HostContentSettingsMapTest to keep test coverage

chrome/browser/content_settings/host_content_settings_map_unittest.cc

 // Copyright (c) 2012 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 <memory>
 #include <string>
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
@@ skipping 283 matching lines @@
 }
 
 TEST_F(HostContentSettingsMapTest, Patterns) {
   TestingProfile profile;
   HostContentSettingsMap* host_content_settings_map =
       HostContentSettingsMapFactory::GetForProfile(&profile);
 
   GURL host1("http://example.com/");
   GURL host2("http://www.example.com/");
   GURL host3("http://example.org/");
+  ContentSettingsPattern pattern1 =
+      ContentSettingsPattern::FromString("[*.]example.com");
+  ContentSettingsPattern pattern2 =
+      ContentSettingsPattern::FromString("example.org");
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetContentSetting(
                 host1, host1, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
-  host_content_settings_map->SetContentSettingDefaultScope(
-      host1, GURL(), CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
-      CONTENT_SETTING_BLOCK);
+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern1, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_COOKIES, std::string(), CONTENT_SETTING_BLOCK);
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
                 host1, host1, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
                 host2, host2, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
   EXPECT_EQ(CONTENT_SETTING_ALLOW,
             host_content_settings_map->GetContentSetting(
                 host3, host3, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
-  host_content_settings_map->SetContentSettingDefaultScope(
-      host3, GURL(), CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
-      CONTENT_SETTING_BLOCK);
+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern2, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_COOKIES, std::string(), CONTENT_SETTING_BLOCK);
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
                 host3, host3, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
 }
 
+TEST_F(HostContentSettingsMapTest, Origins) {
+  TestingProfile profile;
+  HostContentSettingsMap* host_content_settings_map =
+      HostContentSettingsMapFactory::GetForProfile(&profile);
+
+  GURL host1("http://example.com/");
+  GURL host2("http://www.example.com/");
+  GURL host3("http://example.org/");
+  ContentSettingsPattern pattern =
+      ContentSettingsPattern::FromString("example.com");

[raymes, 2016/06/23 01:15:19]

How about we use FromURLNoWildcard here (or SetDefaultScope on an origin-scoped
setting)

[lshang, 2016/06/23 04:29:42]

Done.

+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetContentSetting(
+                host1, host1, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_COOKIES, std::string(), CONTENT_SETTING_BLOCK);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            host_content_settings_map->GetContentSetting(
+                host1, host1, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  // Changing a setting for one origin doesn't affect subdomains.

[raymes, 2016/06/23 01:15:19]

Maybe move this (and the comment below) as comments for the test?

[lshang, 2016/06/23 04:29:42]

Done.

+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetContentSetting(
+                host2, host2, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  // Changing a setting for one origin doesn't affect other origins.
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetContentSetting(
+                host3, host3, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+}
+
 TEST_F(HostContentSettingsMapTest, Observer) {
   TestingProfile profile;
   HostContentSettingsMap* host_content_settings_map =
       HostContentSettingsMapFactory::GetForProfile(&profile);
   MockSettingsObserver observer(host_content_settings_map);
 
   GURL host("http://example.com/");
   ContentSettingsPattern primary_pattern =
       ContentSettingsPattern::FromString("[*.]example.com");
   ContentSettingsPattern secondary_pattern =
@@ skipping 232 matching lines @@
             host_content_settings_map->GetContentSetting(
                 host_ending_with_dot, host_ending_with_dot,
                 CONTENT_SETTINGS_TYPE_AUTOPLAY, std::string()));
 }
 
 TEST_F(HostContentSettingsMapTest, NestedSettings) {
   TestingProfile profile;
   HostContentSettingsMap* host_content_settings_map =
       HostContentSettingsMapFactory::GetForProfile(&profile);
 
-  GURL host("http://a.b.example.com/");
   GURL host1("http://example.com/");
   GURL host2("http://b.example.com/");
+  GURL host3("http://a.b.example.com/");
+  ContentSettingsPattern pattern1 =
+      ContentSettingsPattern::FromString("[*.]example.com");
+  ContentSettingsPattern pattern2 =
+      ContentSettingsPattern::FromString("[*.]b.example.com");
+  ContentSettingsPattern pattern3 =
+      ContentSettingsPattern::FromString("a.b.example.com");
 
-  host_content_settings_map->SetContentSettingDefaultScope(
-      host1, GURL(), CONTENT_SETTINGS_TYPE_POPUPS, std::string(),
-      CONTENT_SETTING_BLOCK);
+  // Test nested patterns for one type.
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetDefaultContentSetting(
+                CONTENT_SETTINGS_TYPE_COOKIES, NULL));

[raymes, 2016/06/23 01:15:18]

nullptr

[lshang, 2016/06/23 04:29:42]

Done.

+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern1, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_COOKIES, std::string(), CONTENT_SETTING_BLOCK);
+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern2, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_COOKIES, std::string(), CONTENT_SETTING_ALLOW);
+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern3, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_COOKIES, std::string(), CONTENT_SETTING_BLOCK);
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            host_content_settings_map->GetContentSetting(
+                host1, host1, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetContentSetting(
+                host2, host2, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            host_content_settings_map->GetContentSetting(
+                host3, host3, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  GURL host4("http://a.example.com/");

[raymes, 2016/06/23 01:15:19]

We should be consistent and either have all of these URLs defined at the top or
close to where they are used. 

[lshang, 2016/06/23 04:29:42]

Done.
Move them all at the top.

+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            host_content_settings_map->GetContentSetting(
+                host4, host4, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  GURL host5("http://b.b.example.com/");
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetContentSetting(
+                host5, host5, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
 
-  host_content_settings_map->SetContentSettingDefaultScope(
-      host2, GURL(), CONTENT_SETTINGS_TYPE_COOKIES, std::string(),
-      CONTENT_SETTING_BLOCK);
+  host_content_settings_map->ClearSettingsForOneType(
+      CONTENT_SETTINGS_TYPE_COOKIES);
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetDefaultContentSetting(
+                CONTENT_SETTINGS_TYPE_COOKIES, NULL));

[raymes, 2016/06/23 01:15:19]

nullptr

[lshang, 2016/06/23 04:29:42]

Done.

 
-  host_content_settings_map->SetContentSettingDefaultScope(
-      host, GURL(), CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, std::string(),
+  GURL https_host2("https://b.example.com/");
+  GURL https_host3("https://a.b.example.com/");
+  ContentSettingsPattern pattern4 =
+      ContentSettingsPattern::FromString("b.example.com");
+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern4, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_COOKIES, std::string(), CONTENT_SETTING_BLOCK);
+  // Pattern "b.example.com" will affect (http|https)://b.example.com
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            host_content_settings_map->GetContentSetting(
+                host2, host2, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  EXPECT_EQ(CONTENT_SETTING_BLOCK,
+            host_content_settings_map->GetContentSetting(
+                https_host2, https_host2, CONTENT_SETTINGS_TYPE_COOKIES,
+                std::string()));
+  // Pattern "b.example.com" will not affect (http|https)://a.b.example.com
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetContentSetting(
+                host3, host3, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+  EXPECT_EQ(CONTENT_SETTING_ALLOW,
+            host_content_settings_map->GetContentSetting(
+                https_host3, https_host3, CONTENT_SETTINGS_TYPE_COOKIES,
+                std::string()));
+
+  host_content_settings_map->ClearSettingsForOneType(
+      CONTENT_SETTINGS_TYPE_COOKIES);
+
+  // Nested setting of one type doesn't affect other types.

[raymes, 2016/06/23 01:15:19]

I think we could move this (from here down) into a separate test function. See
the comment below for more thoughts.

[lshang, 2016/06/23 04:29:43]

Done.

+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern1, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_POPUPS, std::string(), CONTENT_SETTING_BLOCK);
+
+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern2, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_COOKIES, std::string(), CONTENT_SETTING_BLOCK);
+
+  host_content_settings_map->SetContentSettingCustomScope(
+      pattern3, ContentSettingsPattern::Wildcard(),
+      CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS, std::string(),
       CONTENT_SETTING_BLOCK);
   host_content_settings_map->SetDefaultContentSetting(
       CONTENT_SETTINGS_TYPE_JAVASCRIPT, CONTENT_SETTING_BLOCK);
 
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
+                host3, host3, CONTENT_SETTINGS_TYPE_POPUPS, std::string()));
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_POPUPS, std::string()));
+                host3, host3, CONTENT_SETTINGS_TYPE_COOKIES, std::string()));
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_JAVASCRIPT, std::string()));
+                host3, host3, CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
+                std::string()));
   EXPECT_EQ(CONTENT_SETTING_BLOCK,
             host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_AUTOMATIC_DOWNLOADS,
-                std::string()));
-  EXPECT_EQ(CONTENT_SETTING_ASK,
-            host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_GEOLOCATION, std::string()));
+                host3, host3, CONTENT_SETTINGS_TYPE_JAVASCRIPT, std::string()));

[raymes, 2016/06/23 01:15:19]

It's not completely clear to me why we need to have all those types here.
Couldn't we just set one pattern for one type and then check that geolocation
still returns the correct value? 

Also I don't think it's important to specifically test that nested settings
don't impact other types, so we could just call the test TypeSettingsAreIsolated
or something (if you can think of a better name use that :)

[lshang, 2016/06/23 04:29:43]

I agree.

   EXPECT_EQ(
       CONTENT_SETTING_ASK,
       host_content_settings_map->GetContentSetting(
-          host, host, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string()));
-  EXPECT_EQ(CONTENT_SETTING_ASK,
-            host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_FULLSCREEN, std::string()));
-#if !defined(OS_ANDROID)
-  EXPECT_EQ(CONTENT_SETTING_ASK,
-            host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_MOUSELOCK, std::string()));
-#endif
-  EXPECT_EQ(CONTENT_SETTING_BLOCK,
-            host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_KEYGEN, std::string()));
-  EXPECT_EQ(CONTENT_SETTING_ALLOW,
-            host_content_settings_map->GetContentSetting(
-                host, host, CONTENT_SETTINGS_TYPE_AUTOPLAY, std::string()));
+          host3, host3, CONTENT_SETTINGS_TYPE_GEOLOCATION, std::string()));
 }
 
 TEST_F(HostContentSettingsMapTest, OffTheRecord) {
   TestingProfile profile;
   Profile* otr_profile = profile.GetOffTheRecordProfile();
   HostContentSettingsMap* host_content_settings_map =
       HostContentSettingsMapFactory::GetForProfile(&profile);
   HostContentSettingsMap* otr_map =
       HostContentSettingsMapFactory::GetForProfile(otr_profile);
 
@@ skipping 648 matching lines @@
   base::DictionaryValue test_value;
   test_value.SetString("test", "value");
   host_content_settings_map->SetWebsiteSettingDefaultScope(
       unsupported_url, unsupported_url, CONTENT_SETTINGS_TYPE_APP_BANNER,
       std::string(), base::WrapUnique(test_value.DeepCopy()));
   EXPECT_EQ(nullptr,
             host_content_settings_map->GetWebsiteSetting(
                 unsupported_url, unsupported_url,
                 CONTENT_SETTINGS_TYPE_APP_BANNER, std::string(), nullptr));
 }