Always prompt for permission on fullscreen and mouse lock on file:// URLs

chrome/browser/ui/website_settings/permission_menu_model.cc

Index: chrome/browser/ui/website_settings/permission_menu_model.cc
diff --git a/chrome/browser/ui/website_settings/permission_menu_model.cc b/chrome/browser/ui/website_settings/permission_menu_model.cc
index 7c6b450fc8bfb288405ddd78d6781be5c7c7b0d9..aec4bb63562618170ecb7972e477b52551c6e8b7 100644
--- a/chrome/browser/ui/website_settings/permission_menu_model.cc
+++ b/chrome/browser/ui/website_settings/permission_menu_model.cc
@@ -41,9 +41,19 @@ PermissionMenuModel::PermissionMenuModel(
   }
   AddCheckItem(CONTENT_SETTING_DEFAULT, label);
 
-  // Media only support CONTENTE_SETTTING_ALLOW for https.
-  if (permission_.type != CONTENT_SETTINGS_TYPE_MEDIASTREAM ||
-      url.SchemeIsSecure()) {
+  // CONTENT_SETTING_ALLOW and CONTENT_SETTING_BLOCK are not allowed for
+  // fullscreen or mouse lock on file:// URLs, because there wouldn't be
+  // a reasonable origin with which to associate the preference.
+  // TODO(estark): Revisit this when crbug.com/455882 is fixed.
+  bool is_exclusive_access_on_file =
+      (permission_.type == CONTENT_SETTINGS_TYPE_FULLSCREEN ||
+       permission_.type == CONTENT_SETTINGS_TYPE_MOUSELOCK) &&
+      url.SchemeIsFile();

[meacer, 2015/02/11 00:45:33]

Looks like you could do an early return from here?

[estark, 2015/02/11 21:21:58]

Agree logically, but I'm hesitant to introduce a return point in the middle of
the function, out of fear that someone will come along and add completely
unrelated code at the bottom of the function without realizing that their code
won't run in this special case. What do you think?

(I'm not usually as argumentative as I come across in this CL, I swear!)

[meacer, 2015/02/11 21:39:51]

I'd say that that would in fact be good, they should have added a test case for
it :) But it's okay if you want to leave it as it is as well.
But arguing is fun!

+
+  // Media only support CONTENT_SETTTING_ALLOW for https.
+  if ((permission_.type != CONTENT_SETTINGS_TYPE_MEDIASTREAM ||
+       url.SchemeIsSecure()) &&
+      !is_exclusive_access_on_file) {
     label = l10n_util::GetStringUTF16(
         IDS_WEBSITE_SETTINGS_MENU_ITEM_ALLOW);
     AddCheckItem(CONTENT_SETTING_ALLOW, label);
@@ -55,7 +65,8 @@ PermissionMenuModel::PermissionMenuModel(
     AddCheckItem(CONTENT_SETTING_DETECT_IMPORTANT_CONTENT, label);
   }
 
-  if (permission_.type != CONTENT_SETTINGS_TYPE_FULLSCREEN) {
+  if (permission_.type != CONTENT_SETTINGS_TYPE_FULLSCREEN &&
+      !is_exclusive_access_on_file) {
     label = l10n_util::GetStringUTF16(
         IDS_WEBSITE_SETTINGS_MENU_ITEM_BLOCK);
     AddCheckItem(CONTENT_SETTING_BLOCK, label);