Disable browser key reservation in fullscreen mode

chrome/browser/ui/browser_command_controller.cc

Index: chrome/browser/ui/browser_command_controller.cc
diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc
index 0d947e9a4f24cf96a056d565a89e227d60a8f26f..956dbf398529f5f426e87e04a2aee3388cbdc5d4 100644
--- a/chrome/browser/ui/browser_command_controller.cc
+++ b/chrome/browser/ui/browser_command_controller.cc
@@ -88,18 +88,6 @@ using content::NavigationEntry;
 using content::NavigationController;
 using content::WebContents;
 
-namespace {
-
-enum WindowState {
-  // Not in fullscreen mode.
-  WINDOW_STATE_NOT_FULLSCREEN,
-
-  // Fullscreen mode, occupying the whole screen.
-  WINDOW_STATE_FULLSCREEN,
-};
-
-}  // namespace
-
 namespace chrome {
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -199,8 +187,25 @@ bool BrowserCommandController::IsReservedCommandOrKey(
   }
 #endif
 
-  if (window()->IsFullscreen() && command_id == IDC_FULLSCREEN)
-    return true;
+  const bool is_tab_or_window_command =
+      command_id == IDC_CLOSE_TAB ||
+      command_id == IDC_CLOSE_WINDOW ||
+      command_id == IDC_NEW_INCOGNITO_WINDOW ||
+      command_id == IDC_NEW_TAB ||
+      command_id == IDC_NEW_WINDOW ||
+      command_id == IDC_RESTORE_TAB ||
+      command_id == IDC_SELECT_NEXT_TAB ||
+      command_id == IDC_SELECT_PREVIOUS_TAB;
+  if (window()->IsFullscreen()) {

[msw, 2017/02/10 02:03:39]

Can we replace this block with:
  if (window()->IsFullscreen())
    return command_id == IDC_FULLSCREEN || command_id == IDC_EXIT;
That seems to be a more concise implementation of the intended behavior, and
matches what's described in the comment. (if so, you can revert the
|is_tab_or_window_command| change)

[Hzj_jie, 2017/02/10 19:18:08]

It looks like your solution changes the behavior on Linux. The following
ui::TextEditKeyBindingDelegateAuraLinux will be ignored when pressing ESC in
fullscreen.

[msw, 2017/02/10 23:56:14]

Can you elaborate? It looks like the Linux block only ever returns false, and
afaict, MatchEvent() doesn't have any side-effects; so the only conditions under
which fullscreen would return true below line 190 is if the command is
IDC_FULLSCREEN or IDC_EXIT. So, my suggested code should keep the intended
behavior, right? Please let me know if I'm missing something.
  if (window()->IsFullscreen())
    return command_id == IDC_FULLSCREEN || command_id == IDC_EXIT;

+    // In fullscreen, all commands except for IDC_FULLSCREEN and IDC_EXIT should
+    // be delivered to the web page. See https://goo.gl/4tJ32G.
+    if (command_id == IDC_FULLSCREEN) {

[msw, 2017/02/10 02:03:39]

nit: curlies not needed

[Hzj_jie, 2017/02/10 19:18:08]

Done.

+      return true;
+    }
+    if (is_tab_or_window_command) {

[msw, 2017/02/10 02:03:39]

nit: curlies not needed

[Hzj_jie, 2017/02/10 19:18:08]

Done.

+      return false;
+    }
+  }
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
   // If this key was registered by the user as a content editing hotkey, then
@@ -211,15 +216,7 @@ bool BrowserCommandController::IsReservedCommandOrKey(
     return false;
 #endif
 
-  return command_id == IDC_CLOSE_TAB ||
-         command_id == IDC_CLOSE_WINDOW ||
-         command_id == IDC_NEW_INCOGNITO_WINDOW ||
-         command_id == IDC_NEW_TAB ||
-         command_id == IDC_NEW_WINDOW ||
-         command_id == IDC_RESTORE_TAB ||
-         command_id == IDC_SELECT_NEXT_TAB ||
-         command_id == IDC_SELECT_PREVIOUS_TAB ||
-         command_id == IDC_EXIT;
+  return is_tab_or_window_command || command_id == IDC_EXIT;
 }
 
 void BrowserCommandController::SetBlockCommandExecution(bool block) {
@@ -1020,13 +1017,9 @@ void BrowserCommandController::UpdateCommandsForFileSelectionDialogs() {
 }
 
 void BrowserCommandController::UpdateCommandsForFullscreenMode() {
-  WindowState window_state = WINDOW_STATE_NOT_FULLSCREEN;
-  if (window() && window()->IsFullscreen()) {
-    window_state = WINDOW_STATE_FULLSCREEN;
-  }
-  bool show_main_ui = IsShowingMainUI();
-  bool main_not_fullscreen =
-      show_main_ui && window_state == WINDOW_STATE_NOT_FULLSCREEN;
+  const bool is_fullscreen = window() && window()->IsFullscreen();
+  const bool show_main_ui = IsShowingMainUI();
+  const bool main_not_fullscreen = show_main_ui && !is_fullscreen;
 
   // Navigation commands
   command_updater_.UpdateCommandEnabled(IDC_OPEN_CURRENT_URL, show_main_ui);
@@ -1034,8 +1027,7 @@ void BrowserCommandController::UpdateCommandsForFullscreenMode() {
   // Window management commands
   command_updater_.UpdateCommandEnabled(
       IDC_SHOW_AS_TAB,
-      !browser_->is_type_tabbed() &&
-          window_state == WINDOW_STATE_NOT_FULLSCREEN);
+      !browser_->is_type_tabbed() && !is_fullscreen);
 
   // Focus various bits of UI
   command_updater_.UpdateCommandEnabled(IDC_FOCUS_TOOLBAR, show_main_ui);
@@ -1077,19 +1069,29 @@ void BrowserCommandController::UpdateCommandsForFullscreenMode() {
   if (base::debug::IsProfilingSupported())
     command_updater_.UpdateCommandEnabled(IDC_PROFILING_ENABLED, show_main_ui);
 
-  bool fullscreen_enabled = true;
 #if !defined(OS_MACOSX)
-  if (window_state == WINDOW_STATE_NOT_FULLSCREEN &&
-      !profile()->GetPrefs()->GetBoolean(prefs::kFullscreenAllowed)) {
-    // Disable toggling into fullscreen mode if disallowed by pref.
-    fullscreen_enabled = false;
-  }
+  // Disable toggling into fullscreen mode if disallowed by pref.
+  const bool fullscreen_enabled = is_fullscreen ||
+      profile()->GetPrefs()->GetBoolean(prefs::kFullscreenAllowed);
+#else
+  const bool fullscreen_enabled = true;
 #endif
 
   command_updater_.UpdateCommandEnabled(IDC_FULLSCREEN, fullscreen_enabled);
   command_updater_.UpdateCommandEnabled(IDC_TOGGLE_FULLSCREEN_TOOLBAR,
                                         fullscreen_enabled);
 
+  command_updater_.UpdateCommandEnabled(IDC_CLOSE_TAB, !is_fullscreen);
+  command_updater_.UpdateCommandEnabled(IDC_CLOSE_WINDOW, !is_fullscreen);
+  command_updater_.UpdateCommandEnabled(IDC_NEW_INCOGNITO_WINDOW,
+                                        !is_fullscreen);
+  command_updater_.UpdateCommandEnabled(IDC_NEW_TAB, !is_fullscreen);
+  command_updater_.UpdateCommandEnabled(IDC_NEW_WINDOW, !is_fullscreen);
+  command_updater_.UpdateCommandEnabled(IDC_RESTORE_TAB, !is_fullscreen);
+  command_updater_.UpdateCommandEnabled(IDC_SELECT_NEXT_TAB, !is_fullscreen);
+  command_updater_.UpdateCommandEnabled(IDC_SELECT_PREVIOUS_TAB,
+                                        !is_fullscreen);
+
   UpdateCommandsForBookmarkBar();
 }