From 1897e4200966b76a87ecdfa79c1901e663b07bfb Mon Sep 17 00:00:00 2001 From: Mounir Lamouri Date: Thu, 19 Nov 2015 03:03:07 +0000 Subject: [PATCH] Merge "Make resetting notification settings actually work on Android." The current code expect that DEFAULT is -1 instead of 0. Thus, the permission is never actually reset and the NOTREACHED is hit on Debug builds. This CL tries to do the ContentSetting -> jint / jint -> ContentSetting conversions as close to the JNI layer as possible to prevent further confusions. BUG=554838 TBR=newt@chromium.org Review URL: https://codereview.chromium.org/1439933002 Cr-Commit-Position: refs/heads/master@{#359763} (cherry picked from commit c717aeea0502e1217b3a799a953c8b85597e1694) Review URL: https://codereview.chromium.org/1460483004 . Cr-Commit-Position: refs/branch-heads/2564@{#53} Cr-Branched-From: 1283eca15bd9f772387f75241576cde7bdec7f54-refs/heads/master@{#359700} --- .../preferences/website/CameraInfo.java | 4 +- .../preferences/website/CookieInfo.java | 4 +- .../preferences/website/FullscreenInfo.java | 4 +- .../preferences/website/GeolocationInfo.java | 4 +- .../preferences/website/MicrophoneInfo.java | 4 +- .../browser/preferences/website/MidiInfo.java | 5 +- .../preferences/website/PermissionInfo.java | 5 +- .../website/ProtectedMediaIdentifierInfo.java | 4 +- .../website/PushNotificationInfo.java | 4 +- .../preferences/website_preference_bridge.cc | 48 ++++++++----------- 10 files changed, 38 insertions(+), 48 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/CameraInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/CameraInfo.java index d054ebd37b587..8b225f18c14d5 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/CameraInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/CameraInfo.java @@ -18,8 +18,8 @@ protected int getNativePreferenceValue(String origin, String embedder, boolean i } protected void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito) { + String origin, String embedder, ContentSetting value, boolean isIncognito) { WebsitePreferenceBridge.nativeSetCameraSettingForOrigin( - origin, embedder, value, isIncognito); + origin, embedder, value.toInt(), isIncognito); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/CookieInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/CookieInfo.java index 8a74e365c0b73..b499213125f64 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/CookieInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/CookieInfo.java @@ -18,8 +18,8 @@ protected int getNativePreferenceValue(String origin, String embedder, boolean i } protected void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito) { + String origin, String embedder, ContentSetting value, boolean isIncognito) { WebsitePreferenceBridge.nativeSetCookieSettingForOrigin( - origin, embedder, value, isIncognito); + origin, embedder, value.toInt(), isIncognito); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/FullscreenInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/FullscreenInfo.java index d943499f923f0..6bebf80a575be 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/FullscreenInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/FullscreenInfo.java @@ -20,8 +20,8 @@ protected int getNativePreferenceValue(String origin, String embedder, boolean i @Override protected void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito) { + String origin, String embedder, ContentSetting value, boolean isIncognito) { WebsitePreferenceBridge.nativeSetFullscreenSettingForOrigin( - origin, embedder, value, isIncognito); + origin, embedder, value.toInt(), isIncognito); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/GeolocationInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/GeolocationInfo.java index a3651260aaac6..af8755b02ef9c 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/GeolocationInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/GeolocationInfo.java @@ -20,8 +20,8 @@ protected int getNativePreferenceValue(String origin, String embedder, boolean i @Override protected void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito) { + String origin, String embedder, ContentSetting value, boolean isIncognito) { WebsitePreferenceBridge.nativeSetGeolocationSettingForOrigin( - origin, embedder, value, isIncognito); + origin, embedder, value.toInt(), isIncognito); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/MicrophoneInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/MicrophoneInfo.java index ba9d4e8c9b5e9..b3fa0564495a1 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/MicrophoneInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/MicrophoneInfo.java @@ -18,8 +18,8 @@ protected int getNativePreferenceValue(String origin, String embedder, boolean i } protected void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito) { + String origin, String embedder, ContentSetting value, boolean isIncognito) { WebsitePreferenceBridge.nativeSetMicrophoneSettingForOrigin( - origin, embedder, value, isIncognito); + origin, embedder, value.toInt(), isIncognito); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/MidiInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/MidiInfo.java index be90e8ff22ff9..1a4eb144be0a6 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/MidiInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/MidiInfo.java @@ -17,7 +17,8 @@ protected int getNativePreferenceValue(String origin, String embedder, boolean i } protected void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito) { - WebsitePreferenceBridge.nativeSetMidiSettingForOrigin(origin, embedder, value, isIncognito); + String origin, String embedder, ContentSetting value, boolean isIncognito) { + WebsitePreferenceBridge.nativeSetMidiSettingForOrigin( + origin, embedder, value.toInt(), isIncognito); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/PermissionInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/PermissionInfo.java index aeff056facf23..b9f0f001a8cd5 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/PermissionInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/PermissionInfo.java @@ -48,13 +48,12 @@ public ContentSetting getContentSetting() { * Sets the native ContentSetting value for this origin. */ public void setContentSetting(ContentSetting value) { - setNativePreferenceValue( - mOrigin, getEmbedderSafe(), value.toInt(), mIsIncognito); + setNativePreferenceValue(mOrigin, getEmbedderSafe(), value, mIsIncognito); } protected abstract int getNativePreferenceValue( String origin, String embedder, boolean isIncognito); protected abstract void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito); + String origin, String embedder, ContentSetting value, boolean isIncognito); } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ProtectedMediaIdentifierInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ProtectedMediaIdentifierInfo.java index 55d7eddc74081..cea301c9df7b4 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ProtectedMediaIdentifierInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/ProtectedMediaIdentifierInfo.java @@ -21,8 +21,8 @@ protected int getNativePreferenceValue(String origin, String embedder, boolean i @Override protected void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito) { + String origin, String embedder, ContentSetting value, boolean isIncognito) { WebsitePreferenceBridge.nativeSetProtectedMediaIdentifierSettingForOrigin( - origin, embedder, value, isIncognito); + origin, embedder, value.toInt(), isIncognito); } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/PushNotificationInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/PushNotificationInfo.java index d80b9912a7581..2c301a595eb05 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/PushNotificationInfo.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/PushNotificationInfo.java @@ -20,8 +20,8 @@ protected int getNativePreferenceValue(String origin, String embedder, boolean i @Override protected void setNativePreferenceValue( - String origin, String embedder, int value, boolean isIncognito) { + String origin, String embedder, ContentSetting value, boolean isIncognito) { WebsitePreferenceBridge.nativeSetPushNotificationSettingForOrigin( - origin, embedder, value, isIncognito); + origin, embedder, value.toInt(), isIncognito); } } diff --git a/chrome/browser/android/preferences/website_preference_bridge.cc b/chrome/browser/android/preferences/website_preference_bridge.cc index d47f4c9b99cc7..e5d3fc4a85760 100644 --- a/chrome/browser/android/preferences/website_preference_bridge.cc +++ b/chrome/browser/android/preferences/website_preference_bridge.cc @@ -139,11 +139,11 @@ static void GetOrigins(JNIEnv* env, } } -static jint GetSettingForOrigin(JNIEnv* env, - ContentSettingsType content_type, - jstring origin, - jstring embedder, - jboolean is_incognito) { +static ContentSetting GetSettingForOrigin(JNIEnv* env, + ContentSettingsType content_type, + jstring origin, + jstring embedder, + jboolean is_incognito) { GURL url(ConvertJavaStringToUTF8(env, origin)); GURL embedder_url(ConvertJavaStringToUTF8(env, embedder)); ContentSetting setting = @@ -156,19 +156,9 @@ static void SetSettingForOrigin(JNIEnv* env, ContentSettingsType content_type, jstring origin, ContentSettingsPattern secondary_pattern, - jint value, + ContentSetting setting, jboolean is_incognito) { GURL url(ConvertJavaStringToUTF8(env, origin)); - ContentSetting setting = CONTENT_SETTING_DEFAULT; - switch (value) { - case -1: break; - case 0: setting = CONTENT_SETTING_DEFAULT; break; - case 1: setting = CONTENT_SETTING_ALLOW; break; - case 2: setting = CONTENT_SETTING_BLOCK; break; - default: - // Note: CONTENT_SETTINGS_ASK is not and should not be supported. - NOTREACHED(); - } GetHostContentSettingsMap(is_incognito) ->SetContentSetting(ContentSettingsPattern::FromURLNoWildcard(url), secondary_pattern, content_type, std::string(), @@ -201,7 +191,7 @@ static void SetFullscreenSettingForOrigin(JNIEnv* env, GURL embedder_url(ConvertJavaStringToUTF8(env, embedder)); SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_FULLSCREEN, origin, ContentSettingsPattern::FromURLNoWildcard(embedder_url), - value, is_incognito); + (ContentSetting) value, is_incognito); } static void GetGeolocationOrigins(JNIEnv* env, @@ -231,7 +221,7 @@ static void SetGeolocationSettingForOrigin( GURL embedder_url(ConvertJavaStringToUTF8(env, embedder)); SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_GEOLOCATION, origin, ContentSettingsPattern::FromURLNoWildcard(embedder_url), - value, is_incognito); + (ContentSetting) value, is_incognito); } static void GetMidiOrigins(JNIEnv* env, @@ -258,7 +248,7 @@ static void SetMidiSettingForOrigin(JNIEnv* env, GURL embedder_url(ConvertJavaStringToUTF8(env, embedder)); SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MIDI_SYSEX, origin, ContentSettingsPattern::FromURLNoWildcard(embedder_url), - value, is_incognito); + (ContentSetting) value, is_incognito); } static void GetProtectedMediaIdentifierOrigins( @@ -291,7 +281,7 @@ static void SetProtectedMediaIdentifierSettingForOrigin( SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER, origin, ContentSettingsPattern::FromURLNoWildcard(embedder_url), - value, is_incognito); + (ContentSetting) value, is_incognito); } static void GetPushNotificationOrigins(JNIEnv* env, @@ -322,19 +312,17 @@ static void SetPushNotificationSettingForOrigin( // permission types. See https://crbug.com/416894. Profile* profile = GetActiveUserProfile(is_incognito); GURL url = GURL(ConvertJavaStringToUTF8(env, origin)); - ContentSetting setting = CONTENT_SETTING_DEFAULT; - switch (value) { - case -1: + ContentSetting setting = (ContentSetting) value; + switch (setting) { + case CONTENT_SETTING_DEFAULT: DesktopNotificationProfileUtil::ClearSetting( profile, ContentSettingsPattern::FromURLNoWildcard(url)); break; - case 1: + case CONTENT_SETTING_ALLOW: DesktopNotificationProfileUtil::GrantPermission(profile, url); - setting = CONTENT_SETTING_ALLOW; break; - case 2: + case CONTENT_SETTING_BLOCK: DesktopNotificationProfileUtil::DenyPermission(profile, url); - setting = CONTENT_SETTING_BLOCK; break; default: NOTREACHED(); @@ -382,7 +370,8 @@ static void SetMicrophoneSettingForOrigin(JNIEnv* env, jint value, jboolean is_incognito) { SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC, origin, - ContentSettingsPattern::Wildcard(), value, is_incognito); + ContentSettingsPattern::Wildcard(), + (ContentSetting) value, is_incognito); } static void SetCameraSettingForOrigin(JNIEnv* env, @@ -392,7 +381,8 @@ static void SetCameraSettingForOrigin(JNIEnv* env, jint value, jboolean is_incognito) { SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA, origin, - ContentSettingsPattern::Wildcard(), value, is_incognito); + ContentSettingsPattern::Wildcard(), + (ContentSetting) value, is_incognito); } static scoped_refptr GetCookieSettings() {