Handle Ctrl+F1 and Ctrl+F2 correctly even when no browser window is available.

- Remove accelerators from ash/focus_cycler.cc and chrome/browser/ui/views/accelerator_table.cc. Instead, handle it in ash/accelerator/accelerator_controller.cc.
- Do not try to focus a browser window in ash/focus_cycler.cc when it's not available. Unit tests added.

BUG=129307
TEST=ran aura_shell_unittests + manually checked


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

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

11 files changed, 147 insertions, 41 deletions

diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index 3c9fb09..6233651 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -111,6 +111,22 @@ bool HandleShowTaskManager() {
   return true;
 }
 
+bool HandleRotatePaneFocus(Shell::Direction direction) {
+  if (!Shell::GetInstance()->delegate()->RotatePaneFocus(direction)) {
+    // No browser window is available. Focus the launcher.
+    Shell* shell = Shell::GetInstance();
+    switch (direction) {
+      case Shell::FORWARD:
+        shell->focus_cycler()->RotateFocus(internal::FocusCycler::FORWARD);
+        break;
+      case Shell::BACKWARD:
+        shell->focus_cycler()->RotateFocus(internal::FocusCycler::BACKWARD);
+        break;
+    }
+  }
+  return true;
+}
+
 // Rotates the default window container.
 bool HandleRotateWindows() {
   aura::Window* target =
@@ -373,6 +389,10 @@ bool AcceleratorController::PerformAction(int action,
       if (shell->launcher())
         return shell->focus_cycler()->FocusWidget(shell->launcher()->widget());
       break;
+    case FOCUS_NEXT_PANE:
+      return HandleRotatePaneFocus(Shell::FORWARD);
+    case FOCUS_PREVIOUS_PANE:
+      return HandleRotatePaneFocus(Shell::BACKWARD);
     case FOCUS_SYSTEM_TRAY:
       if (shell->system_tray())
         return shell->focus_cycler()->FocusWidget(
diff --git a/ash/accelerators/accelerator_table.cc b/ash/accelerators/accelerator_table.cc
index 3a6b0c4..9d31ea2 100644
--- a/ash/accelerators/accelerator_table.cc
+++ b/ash/accelerators/accelerator_table.cc
@@ -92,6 +92,8 @@ const AcceleratorData kAcceleratorData[] = {
   { true, ui::VKEY_OEM_PLUS, ui::EF_ALT_DOWN, WINDOW_MAXIMIZE_RESTORE },
   { true, ui::VKEY_OEM_PLUS, ui::EF_SHIFT_DOWN | ui::EF_ALT_DOWN,
     WINDOW_POSITION_CENTER },
+  { true, ui::VKEY_F2, ui::EF_CONTROL_DOWN, FOCUS_NEXT_PANE },
+  { true, ui::VKEY_F1, ui::EF_CONTROL_DOWN, FOCUS_PREVIOUS_PANE },
 
   { true, ui::VKEY_F3,
     ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN | ui::EF_ALT_DOWN,
diff --git a/ash/accelerators/accelerator_table.h b/ash/accelerators/accelerator_table.h
index 526ea91..ada8f2f 100644
--- a/ash/accelerators/accelerator_table.h
+++ b/ash/accelerators/accelerator_table.h
@@ -22,6 +22,8 @@ enum AcceleratorAction {
   CYCLE_FORWARD_MRU,
   EXIT,
   FOCUS_LAUNCHER,
+  FOCUS_NEXT_PANE,
+  FOCUS_PREVIOUS_PANE,
   FOCUS_SYSTEM_TRAY,
   NEW_INCOGNITO_WINDOW,
   NEW_TAB,
diff --git a/ash/focus_cycler.cc b/ash/focus_cycler.cc
index f0854c0..d439644 100644
--- a/ash/focus_cycler.cc
+++ b/ash/focus_cycler.cc
@@ -7,14 +7,22 @@
 #include "ash/shell.h"
 #include "ash/system/tray/system_tray.h"
 #include "ash/wm/window_cycle_controller.h"
-#include "ui/views/widget/widget.h"
-#include "ui/views/focus/focus_search.h"
-#include "ui/aura/window.h"
 #include "ui/aura/client/activation_client.h"
+#include "ui/aura/window.h"
 #include "ui/views/accessible_pane_view.h"
+#include "ui/views/focus/focus_search.h"
+#include "ui/views/widget/widget.h"
 
 namespace ash {
 
+namespace {
+
+bool HasFocusableWindow() {
+  return !WindowCycleController::BuildWindowList().empty();
+}
+
+}  // namespace
+
 namespace internal {
 
 FocusCycler::FocusCycler() : widget_activating_(NULL) {
@@ -25,21 +33,13 @@ FocusCycler::~FocusCycler() {
 
 void FocusCycler::AddWidget(views::Widget* widget) {
   widgets_.push_back(widget);
-
-  widget->GetFocusManager()->RegisterAccelerator(
-      ui::Accelerator(ui::VKEY_F2, ui::EF_CONTROL_DOWN),
-      ui::AcceleratorManager::kNormalPriority,
-      this);
-  widget->GetFocusManager()->RegisterAccelerator(
-      ui::Accelerator(ui::VKEY_F1, ui::EF_CONTROL_DOWN),
-      ui::AcceleratorManager::kNormalPriority,
-      this);
 }
 
 void FocusCycler::RotateFocus(Direction direction) {
+  const bool has_window = HasFocusableWindow();
   int index = 0;
   int count = static_cast<int>(widgets_.size());
-  int browser_index = count;
+  int browser_index = has_window ? count : -1;
 
   for (; index < count; ++index) {
     if (widgets_[index]->IsActive())
@@ -48,7 +48,8 @@ void FocusCycler::RotateFocus(Direction direction) {
 
   int start_index = index;
 
-  count = count + 1;
+  if (has_window)
+    ++count;
 
   for (;;) {
     if (direction == FORWARD)
@@ -88,24 +89,6 @@ bool FocusCycler::FocusWidget(views::Widget* widget) {
   return false;
 }
 
-bool FocusCycler::AcceleratorPressed(const ui::Accelerator& accelerator) {
-  switch (accelerator.key_code()) {
-    case ui::VKEY_F1:
-      RotateFocus(BACKWARD);
-      return true;
-    case ui::VKEY_F2:
-      RotateFocus(FORWARD);
-      return true;
-    default:
-      NOTREACHED();
-      return false;
-  }
-}
-
-bool FocusCycler::CanHandleAccelerators() const {
-  return true;
-}
-
 }  // namespace internal
 
 }  // namespace ash
diff --git a/ash/focus_cycler.h b/ash/focus_cycler.h
index 36ddc68..b30c370 100644
--- a/ash/focus_cycler.h
+++ b/ash/focus_cycler.h
@@ -9,8 +9,8 @@
 #include <vector>
 
 #include "ash/ash_export.h"
+#include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "ui/base/accelerators/accelerator.h"
 
 namespace views {
 class Widget;
@@ -22,7 +22,7 @@ namespace internal {
 
 // This class handles moving focus between a set of widgets and the main browser
 // window.
-class ASH_EXPORT FocusCycler : public ui::AcceleratorTarget {
+class ASH_EXPORT FocusCycler {
  public:
   enum Direction {
     FORWARD,
@@ -30,14 +30,14 @@ class ASH_EXPORT FocusCycler : public ui::AcceleratorTarget {
   };
 
   FocusCycler();
-  virtual ~FocusCycler();
+  ~FocusCycler();
 
   // Returns the widget the FocusCycler is attempting to activate or NULL if
   // FocusCycler is not activating any widgets.
   const views::Widget* widget_activating() const { return widget_activating_; }
 
-  // Add a widget to the focus cycle and set up accelerators.  The widget needs
-  // to have an AccessiblePaneView as the content view.
+  // Add a widget to the focus cycle. The widget needs to have an
+  // AccessiblePaneView as the content view.
   void AddWidget(views::Widget* widget);
 
   // Move focus to the next widget.
@@ -46,10 +46,6 @@ class ASH_EXPORT FocusCycler : public ui::AcceleratorTarget {
   // Moves focus the specified widget. Returns true if the widget was activated.
   bool FocusWidget(views::Widget* widget);
 
-  // ui::AcceleratorTarget overrides
-  virtual bool AcceleratorPressed(const ui::Accelerator& accelerator) OVERRIDE;
-  virtual bool CanHandleAccelerators() const OVERRIDE;
-
  private:
   std::vector<views::Widget*> widgets_;
 
diff --git a/ash/focus_cycler_unittest.cc b/ash/focus_cycler_unittest.cc
index c21a1f9..d0377a2 100644
--- a/ash/focus_cycler_unittest.cc
+++ b/ash/focus_cycler_unittest.cc
@@ -136,6 +136,95 @@ TEST_F(FocusCyclerTest, CycleFocusBackward) {
   EXPECT_TRUE(wm::IsActiveWindow(window0.get()));
 }
 
+TEST_F(FocusCyclerTest, CycleFocusForwardBackward) {
+  scoped_ptr<FocusCycler> focus_cycler(new FocusCycler());
+
+  // Add the Status area
+  scoped_ptr<SystemTray> tray(CreateSystemTray());
+  ASSERT_TRUE(tray->GetWidget());
+  focus_cycler->AddWidget(tray->GetWidget());
+  GetStatusAreaWidgetDelegate(tray->GetWidget())->SetFocusCyclerForTesting(
+      focus_cycler.get());
+
+  // Add the launcher
+  Launcher* launcher = Shell::GetInstance()->launcher();
+  ASSERT_TRUE(launcher);
+  views::Widget* launcher_widget = launcher->widget();
+  ASSERT_TRUE(launcher_widget);
+  launcher->SetFocusCycler(focus_cycler.get());
+
+  // Create a single test window.
+  Window* default_container = Shell::GetContainer(
+      Shell::GetPrimaryRootWindow(),
+      internal::kShellWindowId_DefaultContainer);
+  scoped_ptr<Window> window0(CreateTestWindowWithId(0, default_container));
+  wm::ActivateWindow(window0.get());
+  EXPECT_TRUE(wm::IsActiveWindow(window0.get()));
+
+  // Cycle focus to the launcher
+  focus_cycler->RotateFocus(FocusCycler::BACKWARD);
+  EXPECT_TRUE(launcher_widget->IsActive());
+
+  // Cycle focus to the status area
+  focus_cycler->RotateFocus(FocusCycler::BACKWARD);
+  EXPECT_TRUE(tray->GetWidget()->IsActive());
+
+  // Cycle focus to the browser
+  focus_cycler->RotateFocus(FocusCycler::BACKWARD);
+  EXPECT_TRUE(wm::IsActiveWindow(window0.get()));
+
+  // Cycle focus to the status area
+  focus_cycler->RotateFocus(FocusCycler::FORWARD);
+  EXPECT_TRUE(tray->GetWidget()->IsActive());
+
+  // Cycle focus to the launcher
+  focus_cycler->RotateFocus(FocusCycler::FORWARD);
+  EXPECT_TRUE(launcher_widget->IsActive());
+
+  // Cycle focus to the browser
+  focus_cycler->RotateFocus(FocusCycler::FORWARD);
+  EXPECT_TRUE(wm::IsActiveWindow(window0.get()));
+}
+
+TEST_F(FocusCyclerTest, CycleFocusNoBrowser) {
+  scoped_ptr<FocusCycler> focus_cycler(new FocusCycler());
+
+  // Add the Status area
+  scoped_ptr<SystemTray> tray(CreateSystemTray());
+  ASSERT_TRUE(tray->GetWidget());
+  focus_cycler->AddWidget(tray->GetWidget());
+  GetStatusAreaWidgetDelegate(tray->GetWidget())->SetFocusCyclerForTesting(
+      focus_cycler.get());
+
+  // Add the launcher and focus it
+  Launcher* launcher = Shell::GetInstance()->launcher();
+  ASSERT_TRUE(launcher);
+  views::Widget* launcher_widget = launcher->widget();
+  ASSERT_TRUE(launcher_widget);
+  launcher->SetFocusCycler(focus_cycler.get());
+  focus_cycler->FocusWidget(launcher_widget);
+
+  // Cycle focus to the status area
+  focus_cycler->RotateFocus(FocusCycler::FORWARD);
+  EXPECT_TRUE(tray->GetWidget()->IsActive());
+
+  // Cycle focus to the launcher
+  focus_cycler->RotateFocus(FocusCycler::FORWARD);
+  EXPECT_TRUE(launcher_widget->IsActive());
+
+  // Cycle focus to the status area
+  focus_cycler->RotateFocus(FocusCycler::FORWARD);
+  EXPECT_TRUE(tray->GetWidget()->IsActive());
+
+  // Cycle focus to the launcher
+  focus_cycler->RotateFocus(FocusCycler::BACKWARD);
+  EXPECT_TRUE(launcher_widget->IsActive());
+
+  // Cycle focus to the status area
+  focus_cycler->RotateFocus(FocusCycler::BACKWARD);
+  EXPECT_TRUE(tray->GetWidget()->IsActive());
+}
+
 class FocusCyclerLauncherTest : public AshTestBase {
  public:
   FocusCyclerLauncherTest() : AshTestBase() {}
diff --git a/ash/shell/shell_delegate_impl.cc b/ash/shell/shell_delegate_impl.cc
index c2b591d..661aa51 100644
--- a/ash/shell/shell_delegate_impl.cc
+++ b/ash/shell/shell_delegate_impl.cc
@@ -78,6 +78,10 @@ void ShellDelegateImpl::OpenMobileSetup(const std::string& service_path) {
 void ShellDelegateImpl::RestoreTab() {
 }
 
+bool ShellDelegateImpl::RotatePaneFocus(Shell::Direction direction) {
+  return true;
+}
+
 void ShellDelegateImpl::ShowKeyboardOverlay() {
 }
 
diff --git a/ash/shell/shell_delegate_impl.h b/ash/shell/shell_delegate_impl.h
index c3152cf..cb7d821 100644
--- a/ash/shell/shell_delegate_impl.h
+++ b/ash/shell/shell_delegate_impl.h
@@ -34,6 +34,7 @@ class ShellDelegateImpl : public ash::ShellDelegate {
   virtual void OpenCrosh() OVERRIDE;
   virtual void OpenMobileSetup(const std::string& service_path) OVERRIDE;
   virtual void RestoreTab() OVERRIDE;
+  virtual bool RotatePaneFocus(Shell::Direction direction) OVERRIDE;
   virtual void ShowKeyboardOverlay() OVERRIDE;
   virtual void ShowTaskManager() OVERRIDE;
   virtual content::BrowserContext* GetCurrentBrowserContext() OVERRIDE;
diff --git a/ash/shell_delegate.h b/ash/shell_delegate.h
index 7a43f0d..db7d634 100644
--- a/ash/shell_delegate.h
+++ b/ash/shell_delegate.h
@@ -80,6 +80,10 @@ class ASH_EXPORT ShellDelegate {
   // Invoked when the user uses Shift+Ctrl+T to restore the closed tab.
   virtual void RestoreTab() = 0;
 
+  // Moves keyboard focus to the next pane. Returns false if no browser window
+  // is created.
+  virtual bool RotatePaneFocus(Shell::Direction direction) = 0;
+
   // Shows the keyboard shortcut overlay.
   virtual void ShowKeyboardOverlay() = 0;
 
diff --git a/ash/test/test_shell_delegate.cc b/ash/test/test_shell_delegate.cc
index 356943a..57601c1 100644
--- a/ash/test/test_shell_delegate.cc
+++ b/ash/test/test_shell_delegate.cc
@@ -63,6 +63,10 @@ void TestShellDelegate::OpenMobileSetup(const std::string& service_path) {
 void TestShellDelegate::RestoreTab() {
 }
 
+bool TestShellDelegate::RotatePaneFocus(Shell::Direction direction) {
+  return true;
+}
+
 void TestShellDelegate::ShowKeyboardOverlay() {
 }
 
diff --git a/ash/test/test_shell_delegate.h b/ash/test/test_shell_delegate.h
index 15e5bca..a69642d 100644
--- a/ash/test/test_shell_delegate.h
+++ b/ash/test/test_shell_delegate.h
@@ -30,6 +30,7 @@ class TestShellDelegate : public ShellDelegate {
   virtual void OpenCrosh() OVERRIDE;
   virtual void OpenMobileSetup(const std::string& service_path) OVERRIDE;
   virtual void RestoreTab() OVERRIDE;
+  virtual bool RotatePaneFocus(Shell::Direction direction) OVERRIDE;
   virtual void ShowKeyboardOverlay() OVERRIDE;
   virtual void ShowTaskManager() OVERRIDE;
   virtual content::BrowserContext* GetCurrentBrowserContext() OVERRIDE;