Add fullscreen permission controls to settings

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 93 matching lines @@
             env, list, jorigin.obj(), jembedder.obj());
         break;
       case CONTENT_SETTINGS_TYPE_PROTECTED_MEDIA_IDENTIFIER:
         Java_WebsitePreferenceBridge_insertProtectedMediaIdentifierInfoIntoList(
             env, list, jorigin.obj(), jembedder.obj());
         break;
       case CONTENT_SETTINGS_TYPE_NOTIFICATIONS:
         Java_WebsitePreferenceBridge_insertPushNotificationIntoList(
             env, list, jorigin.obj(), jembedder.obj());
         break;
+      case CONTENT_SETTINGS_TYPE_FULLSCREEN:
+        Java_WebsitePreferenceBridge_insertFullscreenInfoIntoList(
+            env, list, jorigin.obj(), jembedder.obj());
       default:
         DCHECK(false);
         break;
     }
   }
 }
 
 static jint GetSettingForOrigin(JNIEnv* env,
                                 ContentSettingsType content_type,
                                 jstring origin,
@@ skipping 13 matching lines @@
                                 jstring origin,
                                 ContentSettingsPattern secondary_pattern,
                                 jint value) {
   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;
+    case 3: setting = CONTENT_SETTING_ASK; break;

[Finnur, 2015/03/13 10:42:14]

Hmmm, this does not seem right to me.

Category defaults can be set to ASK, but for individual origins the UI should
only support ALLOW and BLOCK. Why is this needed?

[qinmin, 2015/03/17 23:49:30]

BLOCK is not allowed for fullscreen. Check
HostContentSettingsMap::IsSettingAllowedForType(). So for fullscreen, the UI
only supports toggling between ALLOW and ASK.

On 2015/03/13 10:42:14, Finnur wrote:

[Finnur, 2015/03/19 10:35:26]

I suspect we're conflating two things here.

The function you refer to (IsSettingAllowedForType) is used by
SetDefaultContentSetting when setting a default value for a category. In that
specific case, many different combinations are valid, including ASK/ALLOW. And
that's fine... for default values.

This function, however, (SetSettingForOrigin) is not about setting default
values for a whole category, but overrides (of the default value) for individual
origins. It is therefore not governed by IsSettingAllowedForType. If the user is
configuring this value, then the only values that make sense are Allow and Deny.
There's no point for the user in setting it to "Ask me later" when they can make
a decision for this specific origin now: Allow or Block.

If we need to add Ask, then we should at least consult Rebecca Rolfe, who
oversaw the UI design of this to see if that's OK. But lets take a step back...
I don't see any code to support Ask in SingleWebsitePreferences.java, which
makes me think you are showing Allow/Block in the UI for an origin today and it
is simply working. What happens if you comment out this line?

If it JustWorks(tm) then I'd like a comment over the NOTREACHED on line 152, as
in:

  default:
    // Note: CONTENT_SETTINGS_ASK is not and should not be supported.
    NOTREACHED();

But even if it doesn't work, I think a better alternative would be to look into
what it takes to support BLOCK in the FullScreen API.

[qinmin, 2015/03/19 18:15:00]

I see. I commented out the line and everything still works fine. For single
origin settings on android, we just let user to reset the settings to the
current default value. So there is no need to support ASK in
SingleWebsitePreferences as user can only set the setting to ALLOW or clear it
to DEFAULT later. 
Added the comments to note that CONTENT_SETTING_ASK is not supported.


On 2015/03/19 10:35:26, Finnur wrote:

     default:
       NOTREACHED();
   }
   GetHostContentSettingsMap()->SetContentSetting(
       ContentSettingsPattern::FromURLNoWildcard(url),
       secondary_pattern,
       content_type,
       std::string(),
       setting);
   WebSiteSettingsUmaUtil::LogPermissionChange(content_type, setting);
@@ skipping 501 matching lines @@
 }
 
 static void ClearCookieData(JNIEnv* env, jclass clazz, jstring jorigin) {
   Profile* profile = ProfileManager::GetActiveUserProfile();
   GURL url(ConvertJavaStringToUTF8(env, jorigin));
   scoped_refptr<SiteDataDeleteHelper> site_data_deleter(
       new SiteDataDeleteHelper(profile, url));
   site_data_deleter->Run();
 }
 
+static void GetFullscreenOrigins(JNIEnv* env,
+                                  jclass clazz,
+                                  jobject list,
+                                  jboolean managedOnly) {
+  GetOrigins(env, CONTENT_SETTINGS_TYPE_FULLSCREEN, list, managedOnly);
+}
+
+static jint GetFullscreenSettingForOrigin(JNIEnv* env, jclass clazz,
+    jstring origin, jstring embedder) {
+  return GetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_FULLSCREEN,
+                             origin, embedder);
+}
+
+static void SetFullscreenSettingForOrigin(JNIEnv* env, jclass clazz,
+    jstring origin, jstring embedder, jint value) {
+  GURL embedder_url(ConvertJavaStringToUTF8(env, embedder));
+  SetSettingForOrigin(env, CONTENT_SETTINGS_TYPE_FULLSCREEN,
+      origin, ContentSettingsPattern::FromURLNoWildcard(embedder_url), value);
+}

[Finnur, 2015/03/13 10:42:14]

There's a host of similar functions on line 170 and down. Can you move this next
to those?

[qinmin, 2015/03/17 23:49:30]

Done.

+
 // Register native methods
 bool RegisterWebsitePreferenceBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }