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

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

5 files changed, 109 insertions, 49 deletions

diff --git a/ui/views/focus/focus_manager.cc b/ui/views/focus/focus_manager.cc
index 1467f25..1799831 100644
--- a/ui/views/focus/focus_manager.cc
+++ b/ui/views/focus/focus_manager.cc
@@ -11,6 +11,7 @@
 #include "build/build_config.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/keycodes/keyboard_codes.h"
+#include "ui/views/focus/focus_manager_delegate.h"
 #include "ui/views/focus/focus_search.h"
 #include "ui/views/focus/view_storage.h"
 #include "ui/views/focus/widget_focus_manager.h"
@@ -20,8 +21,9 @@
 
 namespace views {
 
-FocusManager::FocusManager(Widget* widget)
+FocusManager::FocusManager(Widget* widget, FocusManagerDelegate* delegate)
     : widget_(widget),
+      delegate_(delegate),
       focused_view_(NULL),
       accelerator_manager_(new ui::AcceleratorManager),
       focus_change_reason_(kReasonDirectFocusChange),
@@ -54,22 +56,23 @@ bool FocusManager::OnKeyEvent(const KeyEvent& event) {
   if (event.type() == ui::ET_KEY_PRESSED) {
     // VKEY_MENU is triggered by key release event.
     // FocusManager::OnKeyEvent() returns false when the key has been consumed.
-    if (key_code == ui::VKEY_MENU) {
+    if ((key_code == ui::VKEY_MENU) &&
+        (event.flags() & ~ui::EF_ALT_DOWN) == 0) {
       should_handle_menu_key_release_ = true;
       return false;
     }
-    // Pass through to the reset of OnKeyEvent.
+    // Pass through to the rest of OnKeyEvent.
   } else if (key_code == ui::VKEY_MENU && should_handle_menu_key_release_ &&
              (event.flags() & ~ui::EF_ALT_DOWN) == 0) {
     // Trigger VKEY_MENU when only this key is pressed and released, and both
     // press and release events are not handled by others.
     ui::Accelerator accelerator(ui::VKEY_MENU, false, false, false);
     return ProcessAccelerator(accelerator);
-  } else {
+  } else if (event.type() != ui::ET_KEY_RELEASED) {
     return false;
   }
 #else
-  if (event.type() != ui::ET_KEY_PRESSED)
+  if (event.type() != ui::ET_KEY_PRESSED && event.type() != ui::ET_KEY_RELEASED)
     return false;
 #endif
 
@@ -77,53 +80,57 @@ bool FocusManager::OnKeyEvent(const KeyEvent& event) {
                               event.IsShiftDown(),
                               event.IsControlDown(),
                               event.IsAltDown());
+  accelerator.set_type(event.type());
 
+  if (event.type() == ui::ET_KEY_PRESSED) {
 #if defined(OS_WIN)
-  // If the focused view wants to process the key event as is, let it be.
-  // This is not used for linux/aura.
-  if (focused_view_ && focused_view_->SkipDefaultKeyEventProcessing(event) &&
-      !accelerator_manager_->HasPriorityHandler(accelerator))
-    return true;
+    // If the focused view wants to process the key event as is, let it be.
+    // This is not used for linux/aura.
+    if (focused_view_ && focused_view_->SkipDefaultKeyEventProcessing(event) &&
+        !accelerator_manager_->HasPriorityHandler(accelerator))
+      return true;
 #endif
 
-  // Intercept Tab related messages for focus traversal.
-  // Note that we don't do focus traversal if the root window is not part of the
-  // active window hierarchy as this would mean we have no focused view and
-  // would focus the first focusable view.
+    // Intercept Tab related messages for focus traversal.
+    // Note that we don't do focus traversal if the root window is not part of
+    // the active window hierarchy as this would mean we have no focused view
+    // and would focus the first focusable view.
 #if defined(OS_WIN) && !defined(USE_AURA)
-  HWND top_window = widget_->GetNativeView();
-  HWND active_window = ::GetActiveWindow();
-  if ((active_window == top_window || ::IsChild(active_window, top_window)) &&
-       IsTabTraversalKeyEvent(event)) {
-    AdvanceFocus(event.IsShiftDown());
-    return false;
-  }
+    HWND top_window = widget_->GetNativeView();
+    HWND active_window = ::GetActiveWindow();
+    if ((active_window == top_window || ::IsChild(active_window, top_window)) &&
+        IsTabTraversalKeyEvent(event)) {
+      AdvanceFocus(event.IsShiftDown());
+      return false;
+    }
 #else
-  if (IsTabTraversalKeyEvent(event)) {
-    AdvanceFocus(event.IsShiftDown());
-    return false;
-  }
+    if (IsTabTraversalKeyEvent(event)) {
+      AdvanceFocus(event.IsShiftDown());
+      return false;
+    }
 #endif
 
-  // Intercept arrow key messages to switch between grouped views.
-  if (focused_view_ && focused_view_->GetGroup() != -1 &&
-      (key_code == ui::VKEY_UP || key_code == ui::VKEY_DOWN ||
-       key_code == ui::VKEY_LEFT || key_code == ui::VKEY_RIGHT)) {
-    bool next = (key_code == ui::VKEY_RIGHT || key_code == ui::VKEY_DOWN);
-    View::Views views;
-    focused_view_->parent()->GetViewsInGroup(focused_view_->GetGroup(), &views);
-    View::Views::const_iterator i(
-        std::find(views.begin(), views.end(), focused_view_));
-    DCHECK(i != views.end());
-    int index = static_cast<int>(i - views.begin());
-    index += next ? 1 : -1;
-    if (index < 0) {
-      index = static_cast<int>(views.size()) - 1;
-    } else if (index >= static_cast<int>(views.size())) {
-      index = 0;
+    // Intercept arrow key messages to switch between grouped views.
+    if (focused_view_ && focused_view_->GetGroup() != -1 &&
+        (key_code == ui::VKEY_UP || key_code == ui::VKEY_DOWN ||
+         key_code == ui::VKEY_LEFT || key_code == ui::VKEY_RIGHT)) {
+      bool next = (key_code == ui::VKEY_RIGHT || key_code == ui::VKEY_DOWN);
+      View::Views views;
+      focused_view_->parent()->GetViewsInGroup(focused_view_->GetGroup(),
+                                               &views);
+      View::Views::const_iterator i(
+          std::find(views.begin(), views.end(), focused_view_));
+      DCHECK(i != views.end());
+      int index = static_cast<int>(i - views.begin());
+      index += next ? 1 : -1;
+      if (index < 0) {
+        index = static_cast<int>(views.size()) - 1;
+      } else if (index >= static_cast<int>(views.size())) {
+        index = 0;
+      }
+      SetFocusedViewWithReason(views[index], kReasonFocusTraversal);
+      return false;
     }
-    SetFocusedViewWithReason(views[index], kReasonFocusTraversal);
-    return false;
   }
 
   // Process keyboard accelerators.
@@ -420,7 +427,11 @@ void FocusManager::UnregisterAccelerators(ui::AcceleratorTarget* target) {
 }
 
 bool FocusManager::ProcessAccelerator(const ui::Accelerator& accelerator) {
-  return accelerator_manager_->Process(accelerator);
+  if (accelerator_manager_->Process(accelerator))
+    return true;
+  if (delegate_.get())
+    return delegate_->ProcessAccelerator(accelerator);
+  return false;
 }
 
 void FocusManager::MaybeResetMenuKeyState(const KeyEvent& key) {
@@ -436,7 +447,11 @@ void FocusManager::MaybeResetMenuKeyState(const KeyEvent& key) {
 
 ui::AcceleratorTarget* FocusManager::GetCurrentTargetForAccelerator(
     const ui::Accelerator& accelerator) const {
-  return accelerator_manager_->GetCurrentTarget(accelerator);
+  ui::AcceleratorTarget* target =
+      accelerator_manager_->GetCurrentTarget(accelerator);
+  if (!target && delegate_.get())
+    target = delegate_->GetCurrentTargetForAccelerator(accelerator);
+  return target;
 }
 
 bool FocusManager::HasPriorityHandler(
diff --git a/ui/views/focus/focus_manager.h b/ui/views/focus/focus_manager.h
index d8c60a6..cf6cb86 100644
--- a/ui/views/focus/focus_manager.h
+++ b/ui/views/focus/focus_manager.h
@@ -80,6 +80,7 @@ class AcceleratorManager;
 
 namespace views {
 
+class FocusManagerDelegate;
 class FocusSearch;
 class RootView;
 class View;
@@ -136,7 +137,7 @@ class VIEWS_EXPORT FocusManager {
     kReasonDirectFocusChange
   };
 
-  explicit FocusManager(Widget* widget);
+  FocusManager(Widget* widget, FocusManagerDelegate* delegate);
   virtual ~FocusManager();
 
   // Processes the passed key event for accelerators and tab traversal.
@@ -270,6 +271,10 @@ class VIEWS_EXPORT FocusManager {
   // The top-level Widget this FocusManager is associated with.
   Widget* widget_;
 
+  // The object which handles an accelerator when |accelerator_manager_| doesn't
+  // handle it.
+  scoped_ptr<FocusManagerDelegate> delegate_;
+
   // The view that currently is focused.
   View* focused_view_;
 
diff --git a/ui/views/focus/focus_manager_delegate.h b/ui/views/focus/focus_manager_delegate.h
new file mode 100644
index 0000000..1763ed2
--- /dev/null
+++ b/ui/views/focus/focus_manager_delegate.h
@@ -0,0 +1,40 @@
+// 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 UI_VIEWS_FOCUS_FOCUS_MANAGER_DELEGATE_H_
+#define UI_VIEWS_FOCUS_FOCUS_MANAGER_DELEGATE_H_
+#pragma once
+
+#include "ui/views/views_export.h"
+
+namespace ui {
+class Accelerator;
+class AcceleratorTarget;
+}
+
+namespace views {
+
+// Delegate interface for views::FocusManager.
+class VIEWS_EXPORT FocusManagerDelegate {
+ public:
+  virtual ~FocusManagerDelegate() {}
+
+  // Activate the target associated with the specified accelerator.
+  // First, AcceleratorPressed handler of the most recently registered target
+  // is called, and if that handler processes the event (i.e. returns true),
+  // this method immediately returns. If not, we do the same thing on the next
+  // target, and so on.
+  // Returns true if an accelerator was activated.
+  virtual bool ProcessAccelerator(const ui::Accelerator& accelerator) = 0;
+
+  // Returns the AcceleratorTarget that should be activated for the specified
+  // keyboard accelerator, or NULL if no view is registered for that keyboard
+  // accelerator.
+  virtual ui::AcceleratorTarget* GetCurrentTargetForAccelerator(
+      const ui::Accelerator& accelerator) const = 0;
+};
+
+}  // namespace views
+
+#endif  // UI_VIEWS_FOCUS_FOCUS_MANAGER_DELEGATE_H_
diff --git a/ui/views/focus/focus_manager_factory.cc b/ui/views/focus/focus_manager_factory.cc
index e3ec805..b8e1a7a 100644
--- a/ui/views/focus/focus_manager_factory.cc
+++ b/ui/views/focus/focus_manager_factory.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -18,7 +18,7 @@ class DefaultFocusManagerFactory : public views::FocusManagerFactory {
 
  protected:
   virtual FocusManager* CreateFocusManager(views::Widget* widget) OVERRIDE {
-    return new FocusManager(widget);
+    return new FocusManager(widget, NULL /* delegate */);
   }
 
  private:
diff --git a/ui/views/focus/focus_manager_unittest.cc b/ui/views/focus/focus_manager_unittest.cc
index 01a33ab..06b8448 100644
--- a/ui/views/focus/focus_manager_unittest.cc
+++ b/ui/views/focus/focus_manager_unittest.cc
@@ -502,7 +502,7 @@ class FocusManagerDtorTest : public FocusManagerTest {
   class FocusManagerDtorTracked : public FocusManager {
    public:
     FocusManagerDtorTracked(Widget* widget, DtorTrackVector* dtor_tracker)
-      : FocusManager(widget),
+      : FocusManager(widget, NULL /* delegate */),
         dtor_tracker_(dtor_tracker) {
     }