Revert 135791 - Let Chrome app handle Ash accelerators first if the app is launched as a window.

Currently, Ash accelerators are handled at a very early stage, right after a native key event is received by aura::RootWindowHost. This CL change the way of handling Ash accelerators as follows to make it more App friendly:

1. If no window is focused, handle an Ash accelerator in ash/accelerators/accelerator_filter.cc in the same way as before.
2. Otherwise, do not handle it in ash/accelerators/accelerator_filter.cc but let a custom views::FocusManager handle it (see ash/shell.cc). There are 3 types of scenarios here depending on the type of the focused window:
2-a. If the focused window is a browser, and the browser is not for an app, let the custom focus manager pre-handle Ash accelerators before passing it to the browser (see PreHandleKeyboardEvent() in chrome/browser/ui/views/frame/browser_view.cc).
2-b. If the focused window is a browser, and the browser is for an app, let the app handle Ash accelerators first (see chrome/browser/ui/views/frame/browser_view.cc). If the accelerator is not consumed by the app, let the custom focus manager handle it.
2-c. If the focused window is not a browser, let the window handle Ash accelerators first. If the accelerator is not consumed by the window, then let the custom focus manager handle it. This means a WebView (without chrome/browser/ layer) can handle Ash accelerators first whenever needed.

Other changes:

chrome/browser/ui/views/frame/browser_view.cc:
Support ET_KEY_RELEASED accelerators in BrowserView::PreHandleKeyboardEvent().

ui/views/focus/focus_manager.cc:
Support ET_KEY_RELEASED accelerators. Also fix code for handing VKEY_MENU so that the Shift+Alt+ET_KEY_RELEASED accelerator for Ash could be handled correctly.

BUG=123856
TEST=ran aura_shell_unittests
TEST=manual; launch Chromoting app as a window, connect to a Chromoting server, focus Chrome on the remote machine, press Ctrl-n, confirm a new window is opened on the remote machine.

Review URL: https://chromiumcodereview.appspot.com/10134036

TBR=yusukes@google.com
Review URL: https://chromiumcodereview.appspot.com/10384074

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

7 files changed, 16 insertions, 122 deletions

diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index b6ce065..d0b63b5 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -287,11 +287,6 @@ bool AcceleratorController::Process(const ui::Accelerator& accelerator) {
   return accelerator_manager_->Process(accelerator);
 }
 
-bool AcceleratorController::IsRegistered(
-    const ui::Accelerator& accelerator) const {
-  return accelerator_manager_->GetCurrentTarget(accelerator) != NULL;
-}
-
 void AcceleratorController::SetBrightnessControlDelegate(
     scoped_ptr<BrightnessControlDelegate> brightness_control_delegate) {
   brightness_control_delegate_.swap(brightness_control_delegate);
diff --git a/ash/accelerators/accelerator_controller.h b/ash/accelerators/accelerator_controller.h
index 1c8193d..45ca052 100644
--- a/ash/accelerators/accelerator_controller.h
+++ b/ash/accelerators/accelerator_controller.h
@@ -56,9 +56,6 @@ class ASH_EXPORT AcceleratorController : public ui::AcceleratorTarget {
   // Returns true if an accelerator was activated.
   bool Process(const ui::Accelerator& accelerator);
 
-  // Returns true if the |accelerator| is registered.
-  bool IsRegistered(const ui::Accelerator& accelerator) const;
-
   // Overridden from ui::AcceleratorTarget:
   virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE;
   virtual bool CanHandleAccelerators() const OVERRIDE;
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
index 377d4ee..c85a4d6 100644
--- a/ash/accelerators/accelerator_controller_unittest.cc
+++ b/ash/accelerators/accelerator_controller_unittest.cc
@@ -363,17 +363,6 @@ TEST_F(AcceleratorControllerTest, Process) {
   EXPECT_FALSE(GetController()->Process(accelerator_b));
 }
 
-TEST_F(AcceleratorControllerTest, IsRegistered) {
-  const ui::Accelerator accelerator_a(ui::VKEY_A, false, false, false);
-  const ui::Accelerator accelerator_shift_a(ui::VKEY_A, true, false, false);
-  TestTarget target;
-  GetController()->Register(accelerator_a, &target);
-  EXPECT_TRUE(GetController()->IsRegistered(accelerator_a));
-  EXPECT_FALSE(GetController()->IsRegistered(accelerator_shift_a));
-  GetController()->UnregisterAll(&target);
-  EXPECT_FALSE(GetController()->IsRegistered(accelerator_a));
-}
-
 #if defined(OS_WIN) || defined(USE_X11)
 TEST_F(AcceleratorControllerTest, ProcessOnce) {
   ui::Accelerator accelerator_a(ui::VKEY_A, false, false, false);
diff --git a/ash/accelerators/accelerator_filter.cc b/ash/accelerators/accelerator_filter.cc
index bcecdfe2..dcc95ba 100644
--- a/ash/accelerators/accelerator_filter.cc
+++ b/ash/accelerators/accelerator_filter.cc
@@ -12,22 +12,9 @@
 #include "ui/base/accelerators/accelerator_manager.h"
 
 namespace {
-
 const int kModifierFlagMask = (ui::EF_SHIFT_DOWN |
                                ui::EF_CONTROL_DOWN |
                                ui::EF_ALT_DOWN);
-
-// Returns true if an Ash accelerator should be processed now.
-bool ShouldProcessAcceleratorsNow(aura::Window* target) {
-  if (!target)
-    return true;
-  if (target == ash::Shell::GetInstance()->GetRootWindow())
-    return true;
-  // Unless |target| is the root window, return false to let the custom focus
-  // manager (see ash/shell.cc) handle Ash accelerators.
-  return false;
-}
-
 }  // namespace
 
 namespace ash {
@@ -52,8 +39,6 @@ bool AcceleratorFilter::PreHandleKeyEvent(aura::Window* target,
     return false;
   if (event->is_char())
     return false;
-  if (!ShouldProcessAcceleratorsNow(target))
-    return false;
 
   ui::Accelerator accelerator(event->key_code(),
                               event->flags() & kModifierFlagMask);
diff --git a/ash/accelerators/accelerator_filter_unittest.cc b/ash/accelerators/accelerator_filter_unittest.cc
index d2f7a84..cb08bde 100644
--- a/ash/accelerators/accelerator_filter_unittest.cc
+++ b/ash/accelerators/accelerator_filter_unittest.cc
@@ -75,7 +75,7 @@ TEST_F(AcceleratorFilterTest, TestFilterWithoutFocus) {
   EXPECT_EQ(1, delegate->handle_take_screenshot_count());
 }
 
-// Tests if AcceleratorFilter works as expected with a focused window.
+// Tests if AcceleratorFilter works with a focused window.
 TEST_F(AcceleratorFilterTest, TestFilterWithFocus) {
   aura::Window* default_container = Shell::GetInstance()->GetContainer(
       internal::kShellWindowId_DefaultContainer);
@@ -92,13 +92,11 @@ TEST_F(AcceleratorFilterTest, TestFilterWithFocus) {
       scoped_ptr<ScreenshotDelegate>(delegate).Pass());
   EXPECT_EQ(0, delegate->handle_take_screenshot_count());
 
-  // AcceleratorFilter should ignore the key events since the root window is
-  // not focused.
   aura::test::EventGenerator generator(Shell::GetRootWindow());
   generator.PressKey(ui::VKEY_PRINT, 0);
-  EXPECT_EQ(0, delegate->handle_take_screenshot_count());
+  EXPECT_EQ(1, delegate->handle_take_screenshot_count());
   generator.ReleaseKey(ui::VKEY_PRINT, 0);
-  EXPECT_EQ(0, delegate->handle_take_screenshot_count());
+  EXPECT_EQ(1, delegate->handle_take_screenshot_count());
 
   // Reset window before |test_delegate| gets deleted.
   window.reset();
@@ -106,6 +104,16 @@ TEST_F(AcceleratorFilterTest, TestFilterWithFocus) {
 
 // Tests if AcceleratorFilter ignores the flag for Caps Lock.
 TEST_F(AcceleratorFilterTest, TestCapsLockMask) {
+  aura::Window* default_container = Shell::GetInstance()->GetContainer(
+      internal::kShellWindowId_DefaultContainer);
+  aura::test::TestWindowDelegate test_delegate;
+  scoped_ptr<aura::Window> window(aura::test::CreateTestWindowWithDelegate(
+      &test_delegate,
+      -1,
+      gfx::Rect(),
+      default_container));
+  wm::ActivateWindow(window.get());
+
   DummyScreenshotDelegate* delegate = new DummyScreenshotDelegate;
   GetController()->SetScreenshotDelegate(
       scoped_ptr<ScreenshotDelegate>(delegate).Pass());
@@ -123,6 +131,9 @@ TEST_F(AcceleratorFilterTest, TestCapsLockMask) {
   EXPECT_EQ(2, delegate->handle_take_screenshot_count());
   generator.ReleaseKey(ui::VKEY_PRINT, ui::EF_CAPS_LOCK_DOWN);
   EXPECT_EQ(2, delegate->handle_take_screenshot_count());
+
+  // Reset window before |test_delegate| gets deleted.
+  window.reset();
 }
 
 }  // namespace test
diff --git a/ash/accelerators/focus_manager_factory.cc b/ash/accelerators/focus_manager_factory.cc
deleted file mode 100644
index 76dcd9f..0000000
--- a/ash/accelerators/focus_manager_factory.cc
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright (c) 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 "ash/accelerators/focus_manager_factory.h"
-
-#include "ash/accelerators/accelerator_controller.h"
-#include "ash/shell.h"
-#include "ui/views/focus/focus_manager.h"
-
-namespace ash {
-
-AshFocusManagerFactory::AshFocusManagerFactory() {}
-AshFocusManagerFactory::~AshFocusManagerFactory() {}
-
-views::FocusManager* AshFocusManagerFactory::CreateFocusManager(
-    views::Widget* widget) {
-  return new views::FocusManager(widget, new Delegate);
-}
-
-bool AshFocusManagerFactory::Delegate::ProcessAccelerator(
-    const ui::Accelerator& accelerator) {
-  AcceleratorController* controller =
-      Shell::GetInstance()->accelerator_controller();
-  if (controller)
-    return controller->Process(accelerator);
-  return false;
-}
-
-ui::AcceleratorTarget*
-AshFocusManagerFactory::Delegate::GetCurrentTargetForAccelerator(
-    const ui::Accelerator& accelerator) const {
-  AcceleratorController* controller =
-      Shell::GetInstance()->accelerator_controller();
-  if (controller && controller->IsRegistered(accelerator))
-    return controller;
-  return NULL;
-}
-
-}  // namespace ash
diff --git a/ash/accelerators/focus_manager_factory.h b/ash/accelerators/focus_manager_factory.h
deleted file mode 100644
index 8469ff9..0000000
--- a/ash/accelerators/focus_manager_factory.h
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright (c) 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.
-
-#ifndef ASH_ACCELERATORS_FOCUS_MANAGER_FACTORY_H_
-#define ASH_ACCELERATORS_FOCUS_MANAGER_FACTORY_H_
-#pragma once
-
-#include "base/basictypes.h"
-#include "base/compiler_specific.h"
-#include "ui/views/focus/focus_manager_delegate.h"
-#include "ui/views/focus/focus_manager_factory.h"
-
-namespace ash {
-
-// A factory class for creating a custom views::FocusManager object which
-// supports Ash shortcuts.
-class AshFocusManagerFactory : public views::FocusManagerFactory {
- public:
-  AshFocusManagerFactory();
-  virtual ~AshFocusManagerFactory();
-
- protected:
-  // views::FocusManagerFactory overrides:
-  virtual views::FocusManager* CreateFocusManager(
-      views::Widget* widget) OVERRIDE;
-
- private:
-  class Delegate : public views::FocusManagerDelegate {
-   public:
-    // views::FocusManagerDelegate overrides:
-    virtual bool ProcessAccelerator(
-        const ui::Accelerator& accelerator) OVERRIDE;
-    virtual ui::AcceleratorTarget* GetCurrentTargetForAccelerator(
-        const ui::Accelerator& accelerator) const OVERRIDE;
-  };
-
-  DISALLOW_COPY_AND_ASSIGN(AshFocusManagerFactory);
-};
-
-}  // namespace ash
-
-#endif  // ASH_ACCELERATORS_FOCUS_MANAGER_FACTORY_H_