Use GURLs instead of patterns in SetContentSetting() in geolocation and push_messaging

chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc

Index: chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc
diff --git a/chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc b/chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc
index 6a8e1ac400e01c4377b54ef4250c5c2f7bfad243..30972d2d2d8d8be9344e02cdc7f704787329b3d3 100644
--- a/chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc
+++ b/chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc
@@ -65,16 +65,22 @@ class PushMessagingPermissionContextTest
     // PermissionContextBase::UpdateContentSetting, since the tests below use
     // this method to overwrite patterns set as a result of
     // PushMessagingPermissionContext::NotifyPermissionSet.
-    ContentSettingsPattern pattern_a =
-        ContentSettingsPattern::FromURLNoWildcard(GURL(kOriginA));
-    ContentSettingsPattern insecure_pattern =
-        ContentSettingsPattern::FromURLNoWildcard(GURL(kInsecureOrigin));
+    GURL url_a = GURL(kOriginA);
+    GURL insecure_url = GURL(kInsecureOrigin);
     HostContentSettingsMap* host_content_settings_map =
         HostContentSettingsMapFactory::GetForProfile(profile);
-    host_content_settings_map->SetContentSetting(pattern_a, pattern_a, setting,
-                                                 std::string(), value);
-    host_content_settings_map->SetContentSetting(
-        insecure_pattern, insecure_pattern, setting, std::string(), value);
+
+    if (setting == CONTENT_SETTINGS_TYPE_NOTIFICATIONS) {

[Michael van Ouwerkerk, 2016/02/17 11:19:26]

Why is this branching needed, just to pass a blank GURL() for the notifications
case? What would fail if |url_a| or |insecure_url| was passed?

[lshang, 2016/02/18 00:23:53]

It will fail in DCHECK in:
https://codereview.chromium.org/1686343002/diff/180001/components/content_set...
line134
since the scoping type of notification is REQUESTING_ORIGIN_ONLY_SCOPE  so its
top_level_url needs to be empty().
Or we can use SetContentSettingCustomScope() here which can pass in specific
patterns. It doesn't check scoping types so that we can get rid of the branch.
But unless adding a custom-scoped setting(e.g. in tests and special cases), we
recommend developers to use SetContentSettingDefaultScope() instead.

[Michael van Ouwerkerk, 2016/02/18 09:42:44]

I see, thank you!

+      host_content_settings_map->SetContentSettingDefaultScope(
+          url_a, GURL(), setting, std::string(), value);
+      host_content_settings_map->SetContentSettingDefaultScope(
+          insecure_url, GURL(), setting, std::string(), value);
+    } else if (setting == CONTENT_SETTINGS_TYPE_PUSH_MESSAGING) {
+      host_content_settings_map->SetContentSettingDefaultScope(
+          url_a, url_a, setting, std::string(), value);
+      host_content_settings_map->SetContentSettingDefaultScope(
+          insecure_url, insecure_url, setting, std::string(), value);
+    }
   }
 };