Deprecating high-conflict accelerators

chrome/browser/ui/views/frame/browser_view.cc

 // Copyright 2012 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/ui/views/frame/browser_view.h"
 
 #include <algorithm>
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
@@ skipping 23 matching lines @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/browser/sessions/tab_restore_service_factory.h"
 #include "chrome/browser/signin/signin_header_helper.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/translate/chrome_translate_client.h"
+#include "chrome/browser/ui/ash/ash_util.h"
 #include "chrome/browser/ui/bookmarks/bookmark_bar_constants.h"
 #include "chrome/browser/ui/bookmarks/bookmark_bubble_delegate.h"
 #include "chrome/browser/ui/bookmarks/bookmark_bubble_sign_in_delegate.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window_state.h"
@@ skipping 1342 matching lines @@
   DCHECK(focus_manager);
 
   if (focus_manager->shortcut_handling_suspended())
     return false;
 
   ui::Accelerator accelerator =
       ui::GetAcceleratorFromNativeWebKeyboardEvent(event);
 
   // What we have to do here is as follows:
   // - If the |browser_| is for an app, do nothing.
+  // - On CrOS if |accelerator| is deprecated, we allow web contents to consume
+  //   it if needed.
   // - If the |browser_| is not for an app, and the |accelerator| is not
   //   associated with the browser (e.g. an Ash shortcut), process it.
   // - If the |browser_| is not for an app, and the |accelerator| is associated
   //   with the browser, and it is a reserved one (e.g. Ctrl+w), process it.
   // - If the |browser_| is not for an app, and the |accelerator| is associated
   //   with the browser, and it is not a reserved one, do nothing.
 
   if (browser_->is_app()) {
     // Let all keys fall through to a v1 app's web content, even accelerators.
     // We don't have to flip |is_keyboard_shortcut| here. If we do that, the app
     // might not be able to see a subsequent Char event. See OnHandleInputEvent
     // in content/renderer/render_widget.cc for details.
     return false;
   }
 
+#if defined(OS_CHROMEOS)
+  if (chrome::IsAcceleratorDeprecated(accelerator)) {
+    if (event.type == blink::WebInputEvent::RawKeyDown)

[sky, 2015/06/19 21:00:48]

nit: combine these two ifs.

[afakhry, 2015/06/19 22:14:19]

Actually the second if is only for *is_keyboard_shortcut = true; not for return
false;

+      *is_keyboard_shortcut = true;
+    return false;
+  }
+#endif  // defined(OS_CHROMEOS)
+
   chrome::BrowserCommandController* controller = browser_->command_controller();
 
   // Here we need to retrieve the command id (if any) associated to the
   // keyboard event. Instead of looking up the command id in the
   // |accelerator_table_| by ourselves, we block the command execution of
   // the |browser_| object then send the keyboard event to the
   // |focus_manager| as if we are activating an accelerator key.
   // Then we can retrieve the command id from the |browser_| object.
   bool original_block_command_state = controller->block_command_execution();
   controller->SetBlockCommandExecution(true);
@@ skipping 1186 matching lines @@
   return immersive_mode_controller()->IsEnabled();
 }
 
 views::Widget* BrowserView::GetBubbleAssociatedWidget() {
   return GetWidget();
 }
 
 gfx::Rect BrowserView::GetTopContainerBoundsInScreen() {
   return top_container_->GetBoundsInScreen();
 }