ash: Clean up window cycling accelerator code

ash/accelerators/accelerator_controller.cc

Index: ash/accelerators/accelerator_controller.cc
diff --git a/ash/accelerators/accelerator_controller.cc b/ash/accelerators/accelerator_controller.cc
index a9a5f9a9ff228382fde3f35848e2d207664995f6..0ef31d9633fc97915118144c33eedc279d970569 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -87,27 +87,52 @@ bool DebugShortcutsEnabled() {
 #endif
 }
 
-bool HandleCycleWindowMRU(WindowCycleController::Direction direction,
-                          bool is_alt_down) {
-  Shell::GetInstance()->
-      window_cycle_controller()->HandleCycleWindow(direction, is_alt_down);
-  // Always report we handled the key, even if the window didn't change.
-  return true;
-}
+void HandleCycleBackwardMRU(const ui::Accelerator& accelerator) {

[James Cook, 2013/09/04 23:16:46]

These are the implementations from the old switch statement with the old
HandleCycle*() functions inlined.

+  Shell* shell = Shell::GetInstance();
 
-bool HandleCycleWindowOverviewMRU(WindowSelector::Direction direction) {
-  Shell::GetInstance()->
-      window_selector_controller()->HandleCycleWindow(direction);
-  return true;
+  if (accelerator.key_code() == ui::VKEY_TAB)
+    shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_PREVWINDOW_TAB);
+
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kAshEnableOverviewMode)) {
+    shell->window_selector_controller()->HandleCycleWindow(
+        WindowSelector::BACKWARD);
+    return;
+  }
+  shell->window_cycle_controller()->HandleCycleWindow(
+      WindowCycleController::BACKWARD, accelerator.IsAltDown());
 }
 
-void HandleCycleWindowLinear(CycleDirection direction) {
-  Shell::GetInstance()->
-      window_cycle_controller()->HandleLinearCycleWindow();

[James Cook, 2013/09/04 23:16:46]

Note that this ignores |direction|. Heh. :-)

+void HandleCycleForwardMRU(const ui::Accelerator& accelerator) {
+  Shell* shell = Shell::GetInstance();
+
+  if (accelerator.key_code() == ui::VKEY_TAB)
+    shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_NEXTWINDOW_TAB);
+
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kAshEnableOverviewMode)) {
+    shell->window_selector_controller()->HandleCycleWindow(
+        WindowSelector::FORWARD);
+    return;
+  }
+  shell->window_cycle_controller()->HandleCycleWindow(
+      WindowCycleController::FORWARD, accelerator.IsAltDown());
 }
 
-void ToggleOverviewMode() {
-  Shell::GetInstance()->window_selector_controller()->ToggleOverview();
+void HandleCycleLinear(const ui::Accelerator& accelerator) {
+  Shell* shell = Shell::GetInstance();
+
+  // TODO(jamescook): When overview becomes the default the AcceleratorAction
+  // should be renamed from CYCLE_LINEAR to TOGGLE_OVERVIEW.
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kAshEnableOverviewMode)) {
+    shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_OVERVIEW_F5);
+    shell->window_selector_controller()->ToggleOverview();
+    return;
+  }
+  if (accelerator.key_code() == ui::VKEY_MEDIA_LAUNCH_APP1)
+    shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_NEXTWINDOW_F5);
+  shell->window_cycle_controller()->HandleLinearCycleWindow();
 }
 
 bool HandleAccessibleFocusCycle(bool reverse) {
@@ -517,50 +542,23 @@ bool AcceleratorController::PerformAction(int action,
   // You *MUST* return true when some action is performed. Otherwise, this
   // function might be called *twice*, via BrowserView::PreHandleKeyboardEvent
   // and BrowserView::HandleKeyboardEvent, for a single accelerator press.
+  //
+  // If your accelerator invokes more than one line of code, please either
+  // implement it in your module's controller code (like TOGGLE_MIRROR_MODE
+  // below) or pull it into a HandleFoo() function above.
   switch (action) {
     case ACCESSIBLE_FOCUS_NEXT:
       return HandleAccessibleFocusCycle(false);
     case ACCESSIBLE_FOCUS_PREVIOUS:
       return HandleAccessibleFocusCycle(true);
     case CYCLE_BACKWARD_MRU:
-      if (key_code == ui::VKEY_TAB)
-        shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_PREVWINDOW_TAB);
-      if (CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kAshEnableOverviewMode)) {
-        return HandleCycleWindowOverviewMRU(WindowSelector::BACKWARD);
-      }
-      return HandleCycleWindowMRU(WindowCycleController::BACKWARD,
-                                  accelerator.IsAltDown());
+      HandleCycleBackwardMRU(accelerator);
+      return true;
     case CYCLE_FORWARD_MRU:
-      if (key_code == ui::VKEY_TAB)
-        shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_NEXTWINDOW_TAB);
-      if (CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kAshEnableOverviewMode)) {
-        return HandleCycleWindowOverviewMRU(WindowSelector::FORWARD);
-      }
-      return HandleCycleWindowMRU(WindowCycleController::FORWARD,
-                                  accelerator.IsAltDown());
-    case CYCLE_BACKWARD_LINEAR:
-      if (CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kAshEnableOverviewMode)) {
-        shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_OVERVIEW_F5);
-        ToggleOverviewMode();
-        return true;
-      }
-      if (key_code == ui::VKEY_MEDIA_LAUNCH_APP1)
-        shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_PREVWINDOW_F5);
-      HandleCycleWindowLinear(CYCLE_BACKWARD);
+      HandleCycleForwardMRU(accelerator);
       return true;
-    case CYCLE_FORWARD_LINEAR:
-      if (CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kAshEnableOverviewMode)) {
-        shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_OVERVIEW_F5);
-        ToggleOverviewMode();
-        return true;
-      }
-      if (key_code == ui::VKEY_MEDIA_LAUNCH_APP1)
-        shell->delegate()->RecordUserMetricsAction(UMA_ACCEL_NEXTWINDOW_F5);
-      HandleCycleWindowLinear(CYCLE_FORWARD);
+    case CYCLE_LINEAR:
+      HandleCycleLinear(accelerator);
       return true;
 #if defined(OS_CHROMEOS)
     case ADD_REMOVE_DISPLAY:
@@ -718,19 +716,15 @@ bool AcceleratorController::PerformAction(int action,
     case VOLUME_MUTE:
       return shell->system_tray_delegate()->GetVolumeControlDelegate()->
           HandleVolumeMute(accelerator);
-      break;
     case VOLUME_DOWN:
       return shell->system_tray_delegate()->GetVolumeControlDelegate()->
           HandleVolumeDown(accelerator);
-      break;
     case VOLUME_UP:
       return shell->system_tray_delegate()->GetVolumeControlDelegate()->
           HandleVolumeUp(accelerator);
-      break;
     case FOCUS_LAUNCHER:
       return shell->focus_cycler()->FocusWidget(
           Launcher::ForPrimaryDisplay()->shelf_widget());
-      break;
     case FOCUS_NEXT_PANE:
       return HandleRotatePaneFocus(Shell::FORWARD);
     case FOCUS_PREVIOUS_PANE: