Use GURLs instead of patterns for SetContentSetting() in WebsitePreferenceBridge

chrome/browser/android/preferences/website_preference_bridge.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 "chrome/browser/android/preferences/website_preference_bridge.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
@@ skipping 113 matching lines @@
   GURL embedder_url(ConvertJavaStringToUTF8(env, embedder));
   ContentSetting setting =
       GetHostContentSettingsMap(is_incognito)
           ->GetContentSetting(url, embedder_url, content_type, std::string());
   return setting;
 }
 
 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);
 }
 
 }  // anonymous namespace
 
 static void GetFullscreenOrigins(JNIEnv* env,
                                  const JavaParamRef<jclass>& clazz,
                                  const JavaParamRef<jobject>& list,
                                  jboolean managedOnly) {
   GetOrigins(env, CONTENT_SETTINGS_TYPE_FULLSCREEN,
              &Java_WebsitePreferenceBridge_insertFullscreenInfoIntoList, list,
              managedOnly);
 }
 
 static jint GetFullscreenSettingForOrigin(JNIEnv* env,
                                           const JavaParamRef<jclass>& clazz,
                                           const JavaParamRef<jstring>& origin,
                                           const JavaParamRef<jstring>& embedder,
                                           jboolean is_incognito) {
   return GetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_FULLSCREEN, origin,
                              embedder, is_incognito);
 }
 
 static void SetFullscreenSettingForOrigin(JNIEnv* env,
                                           const JavaParamRef<jclass>& clazz,
                                           const JavaParamRef<jstring>& origin,
                                           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,
                                   const JavaParamRef<jclass>& clazz,
                                   const JavaParamRef<jobject>& list,
                                   jboolean managedOnly) {
   GetOrigins(env, CONTENT_SETTINGS_TYPE_GEOLOCATION,
              &Java_WebsitePreferenceBridge_insertGeolocationInfoIntoList, list,
              managedOnly);
 }
 
 static jint GetGeolocationSettingForOrigin(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz,
     const JavaParamRef<jstring>& origin,
     const JavaParamRef<jstring>& embedder,
     jboolean is_incognito) {
   return GetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_GEOLOCATION, origin,
                              embedder, is_incognito);
 }
 
 static void SetGeolocationSettingForOrigin(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz,
     const JavaParamRef<jstring>& origin,
     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,
                              const JavaParamRef<jclass>& clazz,
                              const JavaParamRef<jobject>& list) {
   GetOrigins(env, CONTENT_SETTINGS_TYPE_KEYGEN,
              &Java_WebsitePreferenceBridge_insertKeygenInfoIntoList, list,
              false);
 }
 
 static jint GetKeygenSettingForOrigin(JNIEnv* env,
                                       const JavaParamRef<jclass>& clazz,
                                       const JavaParamRef<jstring>& origin,
                                       const JavaParamRef<jstring>& embedder,
                                       jboolean is_incognito) {
   return GetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_KEYGEN, origin,
                              embedder, is_incognito);
 }
 
 static void SetKeygenSettingForOrigin(JNIEnv* env,
                                       const JavaParamRef<jclass>& clazz,
                                       const JavaParamRef<jstring>& origin,
                                       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,
                              const JavaParamRef<jclass>& clazz,
                              const JavaParamRef<jobject>& java_web_contents) {
   content::WebContents* web_contents =
       content::WebContents::FromJavaWebContents(java_web_contents);
   return TabSpecificContentSettings::FromWebContents(
       web_contents)->IsContentBlocked(CONTENT_SETTINGS_TYPE_KEYGEN);
 }
@@ skipping 13 matching lines @@
   return GetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MIDI_SYSEX, origin,
                              embedder, is_incognito);
 }
 
 static void SetMidiSettingForOrigin(JNIEnv* env,
                                     const JavaParamRef<jclass>& clazz,
                                     const JavaParamRef<jstring>& origin,
                                     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(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz,
     const JavaParamRef<jobject>& list) {
   GetOrigins(
       env, CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER,
       &Java_WebsitePreferenceBridge_insertProtectedMediaIdentifierInfoIntoList,
       list, false);
@@ skipping 10 matching lines @@
                              origin, embedder, is_incognito);
 }
 
 static void SetProtectedMediaIdentifierSettingForOrigin(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz,
     const JavaParamRef<jstring>& origin,
     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,
                                    const JavaParamRef<jclass>& clazz,
                                    const JavaParamRef<jobject>& list) {
   GetOrigins(env, CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
              &Java_WebsitePreferenceBridge_insertNotificationIntoList, list,
              false);
 }
 
@@ skipping 72 matching lines @@
   return GetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
                              origin, embedder, is_incognito);
 }
 
 static void SetMicrophoneSettingForOrigin(JNIEnv* env,
                                           const JavaParamRef<jclass>& clazz,
                                           const JavaParamRef<jstring>& origin,
                                           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,
                                       const JavaParamRef<jclass>& clazz,
                                       const JavaParamRef<jstring>& origin,
                                       const JavaParamRef<jstring>& embedder,
                                       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() {
   // A single cookie setting applies to both incognito and non-incognito.
   Profile* profile = ProfileManager::GetActiveUserProfile();
   return CookieSettingsFactory::GetForProfile(profile);
 }
 
 static void GetCookieOrigins(JNIEnv* env,
                              const JavaParamRef<jclass>& clazz,
@@ skipping 340 matching lines @@
   GURL url(ConvertJavaStringToUTF8(env, jorigin));
   scoped_refptr<SiteDataDeleteHelper> site_data_deleter(
       new SiteDataDeleteHelper(profile, url));
   site_data_deleter->Run();
 }
 
 // Register native methods
 bool RegisterWebsitePreferenceBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }