Use GURLs instead of patterns for SetContentSetting() in WebsitePreferenceBridge

chrome/browser/android/preferences/website_preference_bridge.cc

Index: chrome/browser/android/preferences/website_preference_bridge.cc
diff --git a/chrome/browser/android/preferences/website_preference_bridge.cc b/chrome/browser/android/preferences/website_preference_bridge.cc
index 1243f3aa8e9752529347e1afe596116f927988da..d7fb84f7812080235a167b96798c50e7778a7d13 100644
--- a/chrome/browser/android/preferences/website_preference_bridge.cc
+++ b/chrome/browser/android/preferences/website_preference_bridge.cc
@@ -131,14 +131,14 @@ ContentSetting GetSettingForOrigin(JNIEnv* env,
 void SetSettingForOrigin(JNIEnv* env,
                          ContentSettingsType content_type,
                          jstring origin,
-                         ContentSettingsPattern secondary_pattern,
+                         jstring embedder,
                          ContentSetting setting,
                          jboolean is_incognito) {
-  GURL url(ConvertJavaStringToUTF8(env, origin));
+  GURL origin_url(ConvertJavaStringToUTF8(env, origin));
+  GURL embedder_url(ConvertJavaStringToUTF8(env, embedder));
   GetHostContentSettingsMap(is_incognito)
-      ->SetContentSetting(ContentSettingsPattern::FromURLNoWildcard(url),
-                          secondary_pattern, content_type, std::string(),
-                          setting);
+      ->SetContentSettingDefaultScope(origin_url, embedder_url, content_type,
+                                      std::string(), setting);
   WebSiteSettingsUmaUtil::LogPermissionChange(content_type, setting);
 }
 
@@ -168,10 +168,8 @@ static void SetFullscreenSettingForOrigin(JNIEnv* env,
                                           const JavaParamRef<jstring>& embedder,
                                           jint value,
                                           jboolean is_incognito) {
-  GURL embedder_url(ConvertJavaStringToUTF8(env, embedder));
-  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_FULLSCREEN, origin,
-                      ContentSettingsPattern::FromURLNoWildcard(embedder_url),
-                      (ContentSetting) value, is_incognito);
+  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_FULLSCREEN, origin, embedder,
+                      (ContentSetting)value, is_incognito);
 }
 
 static void GetGeolocationOrigins(JNIEnv* env,
@@ -200,10 +198,8 @@ static void SetGeolocationSettingForOrigin(
     const JavaParamRef<jstring>& embedder,
     jint value,
     jboolean is_incognito) {
-  GURL embedder_url(ConvertJavaStringToUTF8(env, embedder));
-  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_GEOLOCATION, origin,
-                      ContentSettingsPattern::FromURLNoWildcard(embedder_url),
-                      (ContentSetting) value, is_incognito);
+  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_GEOLOCATION, origin, embedder,
+                      (ContentSetting)value, is_incognito);
 }
 
 static void GetKeygenOrigins(JNIEnv* env,
@@ -229,10 +225,8 @@ static void SetKeygenSettingForOrigin(JNIEnv* env,
                                       const JavaParamRef<jstring>& embedder,
                                       jint value,
                                       jboolean is_incognito) {
-  GURL embedder_url(ConvertJavaStringToUTF8(env, embedder));
-  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_KEYGEN, origin,
-                      ContentSettingsPattern::FromURLNoWildcard(embedder_url),
-                      (ContentSetting) value, is_incognito);
+  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_KEYGEN, origin, NULL,

[svaldez, 2016/03/22 16:05:24]

Should this be NULL, possibly add a comment?

[Bernhard Bauer, 2016/03/22 16:11:58]

And in any case it should be nullptr, not NULL.

[lshang, 2016/03/23 01:46:12]

Done. I changed it to nullptr, and it indicates that wildcard is used for
embedder. 

If it looks weird to you, I can change it back to use embedder. For keygen,
microphone and camera, implicitly SetContentSettingDefaultScope() will ignore
the embedder and directly use wildcard for secondary_pattern in either ways. I
think nullptr makes this point more clear to developers, but I'm happy to accept
either way you like. :-)

[lshang, 2016/03/23 01:46:12]

Done.

[Bernhard Bauer, 2016/03/23 14:12:15]

The biggest issue I have with nullptr specifically is that the argument will be
converted to a std::string, and passing in null to ConvertJavaStringToUTF8()
isn't actually allowed, it's just that the code silently accepts it (see
https://code.google.com/p/chromium/codesearch#chromium/src/base/android/jni_s...).
So, we should probably guard that call by checking for null in
SetSettingForOrigin().

Also, can we get rid of the |embedder| parameter here if it's going to be
ignored anyway?

+                      (ContentSetting)value, is_incognito);

[Bernhard Bauer, 2016/03/22 16:11:58]

Ooh, also we should use static_cast<ContentSetting>(). Sorry for missing that!

[lshang, 2016/03/23 01:46:12]

Done.

 }
 
 static jboolean GetKeygenBlocked(JNIEnv* env,
@@ -266,10 +260,8 @@ static void SetMidiSettingForOrigin(JNIEnv* env,
                                     const JavaParamRef<jstring>& embedder,
                                     jint value,
                                     jboolean is_incognito) {
-  GURL embedder_url(ConvertJavaStringToUTF8(env, embedder));
-  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MIDI_SYSEX, origin,
-                      ContentSettingsPattern::FromURLNoWildcard(embedder_url),
-                      (ContentSetting) value, is_incognito);
+  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MIDI_SYSEX, origin, embedder,
+                      (ContentSetting)value, is_incognito);
 }
 
 static void GetProtectedMediaIdentifierOrigins(
@@ -300,11 +292,8 @@ static void SetProtectedMediaIdentifierSettingForOrigin(
     const JavaParamRef<jstring>& embedder,
     jint value,
     jboolean is_incognito) {
-  GURL embedder_url(ConvertJavaStringToUTF8(env, embedder));
   SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER,
-                      origin,
-                      ContentSettingsPattern::FromURLNoWildcard(embedder_url),
-                      (ContentSetting) value, is_incognito);
+                      origin, embedder, (ContentSetting)value, is_incognito);
 }
 
 static void GetNotificationOrigins(JNIEnv* env,
@@ -397,9 +386,8 @@ static void SetMicrophoneSettingForOrigin(JNIEnv* env,
                                           const JavaParamRef<jstring>& embedder,
                                           jint value,
                                           jboolean is_incognito) {
-  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC, origin,
-                      ContentSettingsPattern::Wildcard(),
-                      (ContentSetting) value, is_incognito);
+  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC, origin, NULL,
+                      (ContentSetting)value, is_incognito);
 }
 
 static void SetCameraSettingForOrigin(JNIEnv* env,
@@ -409,8 +397,7 @@ static void SetCameraSettingForOrigin(JNIEnv* env,
                                       jint value,
                                       jboolean is_incognito) {
   SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA, origin,
-                      ContentSettingsPattern::Wildcard(),
-                      (ContentSetting) value, is_incognito);
+                      NULL, (ContentSetting)value, is_incognito);
 }
 
 static scoped_refptr<content_settings::CookieSettings> GetCookieSettings() {