cros: Don't let v2 apps block the brightness and volume system keys

Previously system hardware keys were passed to both v1 and v2 apps. Poorly behaved apps could then consume those keys and prevent the user from changing the volume.

This patch makes the system hardware keys be handled by default in the ash layer. However, because some legacy v1 apps still expect those key events we allow v1 apps to continue to receive them via a window state setting.

This paves the way for a v2 app permission to request access to these keys, see issue 166928.

BUG=281761
TEST=ash_unittests, also launch app "Keyboard Handler Sample"[1] and volume keys should still control volume. [1] https://chrome.google.com/webstore/detail/keyboard-handler-sample/pcbbaldjljokfnnkphllabnpolciapjc

Review URL: https://codereview.chromium.org/25372006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226645 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 104 insertions, 7 deletions

diff --git a/ash/accelerators/accelerator_filter.cc b/ash/accelerators/accelerator_filter.cc
index ea0a88e..57ac695 100644
--- a/ash/accelerators/accelerator_filter.cc
+++ b/ash/accelerators/accelerator_filter.cc
@@ -11,6 +11,7 @@
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/accelerators/accelerator_manager.h"
 #include "ui/events/event.h"
+#include "ui/views/corewm/window_util.h"
 
 namespace ash {
 namespace {
@@ -19,6 +20,37 @@ const int kModifierFlagMask = (ui::EF_SHIFT_DOWN |
                                ui::EF_CONTROL_DOWN |
                                ui::EF_ALT_DOWN);
 
+// Returns true if |key_code| is a key usually handled directly by the shell.
+bool IsSystemKey(ui::KeyboardCode key_code) {
+#if defined(OS_CHROMEOS)
+  switch (key_code) {
+    case ui::VKEY_MEDIA_LAUNCH_APP2:  // Fullscreen button.
+    case ui::VKEY_MEDIA_LAUNCH_APP1:  // Overview button.
+    case ui::VKEY_BRIGHTNESS_DOWN:
+    case ui::VKEY_BRIGHTNESS_UP:
+    case ui::VKEY_KBD_BRIGHTNESS_DOWN:
+    case ui::VKEY_KBD_BRIGHTNESS_UP:
+    case ui::VKEY_VOLUME_MUTE:
+    case ui::VKEY_VOLUME_DOWN:
+    case ui::VKEY_VOLUME_UP:
+      return true;
+    default:
+      return false;
+  }
+#endif  // defined(OS_CHROMEOS)
+  return false;
+}
+
+// Returns true if the window should be allowed a chance to handle system keys.
+// Uses the top level window so if the target is a web contents window the
+// containing parent window will be checked for the property.
+bool CanConsumeSystemKeys(aura::Window* target) {
+  if (!target)  // Can be NULL in tests.
+    return false;
+  aura::Window* top_level = views::corewm::GetToplevelWindow(target);
+  return top_level && wm::GetWindowState(top_level)->can_consume_system_keys();
+}
+
 // Returns true if the |accelerator| should be processed now, inside Ash's env
 // event filter.
 bool ShouldProcessAcceleratorsNow(const ui::Accelerator& accelerator,
@@ -80,13 +112,24 @@ void AcceleratorFilter::OnKeyEvent(ui::KeyEvent* event) {
 
   // Fill out context object so AcceleratorController will know what
   // was the previous accelerator or if the current accelerator is repeated.
-  Shell::GetInstance()->accelerator_controller()->context()->
-      UpdateContext(accelerator);
+  AcceleratorController* accelerator_controller =
+      Shell::GetInstance()->accelerator_controller();
+  accelerator_controller->context()->UpdateContext(accelerator);
 
   aura::Window* target = static_cast<aura::Window*>(event->target());
+  // Handle special hardware keys like brightness and volume. However, some
+  // windows can override this behavior (e.g. Chrome v1 apps by default and
+  // Chrome v2 apps with permission) by setting a window property.
+  if (IsSystemKey(event->key_code()) && !CanConsumeSystemKeys(target)) {
+    accelerator_controller->Process(accelerator);
+    // These keys are always consumed regardless of whether they trigger an
+    // accelerator to prevent windows from seeing unexpected key up events.
+    event->StopPropagation();
+    return;
+  }
   if (!ShouldProcessAcceleratorsNow(accelerator, target))
     return;
-  if (Shell::GetInstance()->accelerator_controller()->Process(accelerator))
+  if (accelerator_controller->Process(accelerator))
     event->StopPropagation();
 }
 
diff --git a/ash/accelerators/accelerator_filter_unittest.cc b/ash/accelerators/accelerator_filter_unittest.cc
index 2c03486..4dff94b 100644
--- a/ash/accelerators/accelerator_filter_unittest.cc
+++ b/ash/accelerators/accelerator_filter_unittest.cc
@@ -9,6 +9,7 @@
 #include "ash/shell_window_ids.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/test_screenshot_delegate.h"
+#include "ash/wm/window_state.h"
 #include "ash/wm/window_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -16,6 +17,7 @@
 #include "ui/aura/test/event_generator.h"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window.h"
+#include "ui/events/event.h"
 #include "ui/gfx/rect.h"
 
 namespace ash {
@@ -88,5 +90,42 @@ TEST_F(AcceleratorFilterTest, TestCapsLockMask) {
   EXPECT_EQ(2, delegate->handle_take_screenshot_count());
 }
 
+#if defined(OS_CHROMEOS)
+// Tests if special hardware keys like brightness and volume are consumed as
+// expected by the shell.
+TEST_F(AcceleratorFilterTest, CanConsumeSystemKeys) {
+  internal::AcceleratorFilter filter;
+
+  // Normal keys are not consumed.
+  ui::KeyEvent press_a(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE, false);
+  filter.OnKeyEvent(&press_a);
+  EXPECT_FALSE(press_a.stopped_propagation());
+
+  // System keys are directly consumed.
+  ui::KeyEvent press_mute(
+      ui::ET_KEY_PRESSED, ui::VKEY_VOLUME_MUTE, ui::EF_NONE, false);
+  filter.OnKeyEvent(&press_mute);
+  EXPECT_TRUE(press_mute.stopped_propagation());
+
+  // Setting a window property on the target allows system keys to pass through.
+  scoped_ptr<aura::Window> window(CreateTestWindowInShellWithId(1));
+  wm::GetWindowState(window.get())->set_can_consume_system_keys(true);
+  ui::KeyEvent press_volume_up(
+      ui::ET_KEY_PRESSED, ui::VKEY_VOLUME_UP, ui::EF_NONE, false);
+  ui::Event::DispatcherApi dispatch_helper(&press_volume_up);
+  dispatch_helper.set_target(window.get());
+  filter.OnKeyEvent(&press_volume_up);
+  EXPECT_FALSE(press_volume_up.stopped_propagation());
+
+  // System keys pass through to a child window if the parent (top level)
+  // window has the property set.
+  scoped_ptr<aura::Window> child(CreateTestWindowInShellWithId(2));
+  window->AddChild(child.get());
+  dispatch_helper.set_target(child.get());
+  filter.OnKeyEvent(&press_volume_up);
+  EXPECT_FALSE(press_volume_up.stopped_propagation());
+}
+#endif  // defined(OS_CHROMEOS)
+
 }  // namespace test
 }  // namespace ash
diff --git a/ash/accelerators/accelerator_table.h b/ash/accelerators/accelerator_table.h
index b4fd857..38b156c 100644
--- a/ash/accelerators/accelerator_table.h
+++ b/ash/accelerators/accelerator_table.h
@@ -19,10 +19,11 @@ namespace ash {
 // * For example, Alt-Tab window cycling.
 // * See kReservedActions below.
 //
-// Ash (OS) non-reserved:
-// * Packaged apps can cancel but web pages cannot.
-// * For example, volume up and down.
-// * See kActionsAllowedInAppMode below.
+// Chrome OS system keys:
+// * For legacy reasons, v1 apps can process and cancel. Otherwise handled
+//   directly by Ash.
+// * Brightness, volume control, etc.
+// * See IsSystemKey() in ash/accelerators/accelerator_filter.cc.
 //
 // Browser reserved:
 // * Packaged apps can cancel but web pages cannot.
@@ -38,6 +39,10 @@ namespace ash {
 // In particular, there is not an accelerator processing pass for Ash after
 // the browser gets the accelerator.  See crbug.com/285308 for details.
 //
+// There are also various restrictions on accelerators allowed at the login
+// screen, when running in "forced app mode" (like a kiosk), etc. See the
+// various kActionsAllowed* below.
+//
 // Please put if/def sections at the end of the bare section and keep the list
 // within each section in alphabetical order.
 enum AcceleratorAction {
diff --git a/ash/wm/window_state.cc b/ash/wm/window_state.cc
index 775a3f7..e72b46c 100644
--- a/ash/wm/window_state.cc
+++ b/ash/wm/window_state.cc
@@ -34,6 +34,7 @@ WindowState::WindowState(aura::Window* window)
       panel_attached_(true),
       continue_drag_after_reparent_(false),
       ignored_by_shelf_(false),
+      can_consume_system_keys_(false),
       always_restores_to_restore_bounds_(false),
       window_show_type_(ToWindowShowType(GetShowState())) {
   window_->AddObserver(this);
diff --git a/ash/wm/window_state.h b/ash/wm/window_state.h
index 2cdccc6..625a51c 100644
--- a/ash/wm/window_state.h
+++ b/ash/wm/window_state.h
@@ -178,6 +178,14 @@ class ASH_EXPORT WindowState : public aura::WindowObserver {
     ignored_by_shelf_ = ignored_by_shelf;
   }
 
+  // True if the window should be offered a chance to consume special system
+  // keys such as brightness, volume, etc. that are usually handled by the
+  // shell.
+  bool can_consume_system_keys() const { return can_consume_system_keys_; }
+  void set_can_consume_system_keys(bool can_consume_system_keys) {
+    can_consume_system_keys_ = can_consume_system_keys;
+  }
+
   // aura::WindowObserver overrides:
   virtual void OnWindowPropertyChanged(aura::Window* window,
                                        const void* key,
@@ -198,6 +206,7 @@ class ASH_EXPORT WindowState : public aura::WindowObserver {
   bool panel_attached_;
   bool continue_drag_after_reparent_;
   bool ignored_by_shelf_;
+  bool can_consume_system_keys_;
 
   bool always_restores_to_restore_bounds_;