Change behaviour of the Alt-] and Alt-[ keys so that it cycles through SnapLeft/SnapRight to DockLe…

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 83a2a9728ee76bd27c16d5d03a585668dc79ac1f..ae83002e00ec4cb878f392694c19329eeb878999 100644
--- a/ash/accelerators/accelerator_controller.cc
+++ b/ash/accelerators/accelerator_controller.cc
@@ -44,6 +44,7 @@
 #include "ash/system/web_notification/web_notification_tray.h"
 #include "ash/touch/touch_hud_debug.h"
 #include "ash/volume_control_delegate.h"
+#include "ash/wm/dock/docked_window_layout_manager.h"
 #include "ash/wm/maximize_mode/maximize_mode_controller.h"
 #include "ash/wm/mru_window_tracker.h"
 #include "ash/wm/overview/window_selector_controller.h"
@@ -502,25 +503,119 @@ bool HandleToggleRootWindowFullScreen() {
   return true;
 }
 
-bool HandleWindowSnap(int action) {
+DockedWindowLayoutManager* GetDockedWindowLayoutManager()
+{
+  aura::Window* active_window = ash::wm::GetActiveWindow();
+  if (active_window) {
+    aura::Window* dock_container = Shell::GetContainer(
+      active_window->GetRootWindow(), kShellWindowId_DockedContainer);
+    DockedWindowLayoutManager* dock_layout =
+      static_cast<DockedWindowLayoutManager*>(
+        dock_container->layout_manager());
+    return dock_layout;
+  }
+  return 0;

[varkha, 2014/09/24 03:16:22]

s/0/NULL

[dtapuska, 2014/09/25 23:27:08]

Done.

+}
+
+class ScopedPreferredAlignmentResetter {
+ public:
+  ScopedPreferredAlignmentResetter(DockedAlignment dockAlignment)
+  : docked_window_layout_manager_(GetDockedWindowLayoutManager()) {
+    docked_window_layout_manager_->SetPreferredAlignment(dockAlignment);
+  }
+  ~ScopedPreferredAlignmentResetter() {
+    docked_window_layout_manager_->SetPreferredAlignment(DOCKED_ALIGNMENT_NONE);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ScopedPreferredAlignmentResetter);
+  DockedWindowLayoutManager* docked_window_layout_manager_;
+};
+
+void RestoreWindow(bool left) {

[varkha, 2014/09/24 03:16:23]

Do we need to know the side here? If it is only for UMA we can ignore it I
think.
Also using an enum instead of bool would be more self descriptive at a call
site.

[dtapuska, 2014/09/25 23:27:08]

Done.

+  wm::WindowState* window_state = wm::GetActiveWindowState();
+  if (!window_state)
+    return;
+
+  base::RecordAction(UserMetricsAction(left ? "Accel_Window_Restore_Left" :
+                                              "Accel_Window_Restore_Right"));
+  const wm::WMEvent event(wm::WM_EVENT_NORMAL);
+  window_state->OnWMEvent(&event);
+}
+
+void DockWindow(DockedAlignment dockAlignment) {
+  wm::WindowState* window_state = wm::GetActiveWindowState();
+  if (!window_state)
+    return;
+
+  ScopedPreferredAlignmentResetter alignmentResetter(dockAlignment);
+  base::RecordAction(UserMetricsAction(dockAlignment == DOCKED_ALIGNMENT_LEFT ?
+      "Accel_Window_Dock_Left" : "Accel_Window_Dock_Right"));
+  const wm::WMEvent event(wm::WM_EVENT_DOCK);
+  window_state->OnWMEvent(&event);
+}
+
+void SnapWindow(bool left) {

[varkha, 2014/09/24 03:16:23]

I think using WM_EVENT_SNAP_xxx instead of bool would be more self descriptive
at a call site.

[dtapuska, 2014/09/25 23:27:09]

Done.

+  wm::WindowState* window_state = wm::GetActiveWindowState();
+  if (!window_state)
+      return;
+  base::RecordAction(UserMetricsAction(left ? "Accel_Window_Snap_Left" :
+                                              "Accel_Window_Snap_Right"));
+  const wm::WMEvent event(left ? wm::WM_EVENT_SNAP_LEFT :
+                                 wm::WM_EVENT_SNAP_RIGHT);
+  window_state->OnWMEvent(&event);
+}
+
+bool CanDock(DockedAlignment desired_alignment) {
+  aura::Window* active_window = ash::wm::GetActiveWindow();
+  DockedWindowLayoutManager* dock_layout = GetDockedWindowLayoutManager();
+  if (dock_layout) {

[varkha, 2014/09/24 03:16:23]

nit: no need for parentheses.

[dtapuska, 2014/09/25 23:27:08]

Done.

+    return dock_layout->CanDockWindow(active_window, desired_alignment);
+  }
+  return false;
+}
+
+DockedAlignment CurrentDockAlignment() {
+  DockedWindowLayoutManager* dock_layout = GetDockedWindowLayoutManager();
+  if (dock_layout) {

[varkha, 2014/09/24 03:16:22]

nit: no need for parentheses.

[dtapuska, 2014/09/25 23:27:08]

Done.

+    return dock_layout->CalculateAlignment();
+  }
+  return DOCKED_ALIGNMENT_NONE;
+}
+
+bool HandleWindowSnapOrDock(int action) {
   wm::WindowState* window_state = wm::GetActiveWindowState();
   // Disable window snapping shortcut key for full screen window due to
   // http://crbug.com/135487.
   if (!window_state ||
-      window_state->window()->type() != ui::wm::WINDOW_TYPE_NORMAL ||
-      window_state->IsFullscreen() ||
-      !window_state->CanSnap()) {
+      (window_state->window()->type() != ui::wm::WINDOW_TYPE_NORMAL &&
+      window_state->window()->type() != ui::wm::WINDOW_TYPE_PANEL) ||
+      window_state->IsFullscreen()) {
     return false;
   }
 
-  if (action == WINDOW_SNAP_LEFT) {
-    base::RecordAction(UserMetricsAction("Accel_Window_Snap_Left"));
-  } else {
-    base::RecordAction(UserMetricsAction("Accel_Window_Snap_Right"));
+  wm::WindowStateType desiredSnapState = action == WINDOW_SNAP_OR_DOCK_LEFT ?
+      wm::WINDOW_STATE_TYPE_LEFT_SNAPPED : wm::WINDOW_STATE_TYPE_RIGHT_SNAPPED;
+  DockedAlignment desiredDockAlignment = action == WINDOW_SNAP_OR_DOCK_LEFT ?
+          DOCKED_ALIGNMENT_LEFT : DOCKED_ALIGNMENT_RIGHT;
+  DockedAlignment currentDockAlignment = CurrentDockAlignment();
+
+  if (!window_state->IsDocked() ||

[varkha, 2014/09/24 03:16:23]

Do we still need to check CanSnap() somewhere?

[dtapuska, 2014/09/25 23:27:08]

Done.

+      (currentDockAlignment != DOCKED_ALIGNMENT_NONE &&
+       currentDockAlignment != desiredDockAlignment)) {

[varkha, 2014/09/24 03:16:23]

The logic could be just a bit simpler if the condition is revered and restore
action is taken first with an early return. The next two conditions then do not
need additional scope.

[dtapuska, 2014/09/25 23:27:08]

I don't agree. The 3rd patch now has some additional logic to bounce the window.
I think it might be complicated reversing all this logic.

+    if (window_state->GetStateType() != desiredSnapState &&
+        window_state->window()->type() != ui::wm::WINDOW_TYPE_PANEL) {
+      SnapWindow(desiredSnapState == wm::WINDOW_STATE_TYPE_LEFT_SNAPPED);
+      return true;
+    }
+
+    if (CanDock(desiredDockAlignment)) {
+      DockWindow(desiredDockAlignment);
+      return true;
+    }
   }
-  const wm::WMEvent event(action == WINDOW_SNAP_LEFT ?
-                          wm::WM_EVENT_SNAP_LEFT : wm::WM_EVENT_SNAP_RIGHT);
-  window_state->OnWMEvent(&event);
+
+  RestoreWindow(action == WINDOW_SNAP_OR_DOCK_LEFT);
   return true;
 }
 
@@ -1036,9 +1131,9 @@ bool AcceleratorController::PerformAction(int action,
       return HandleLaunchAppN(7);
     case LAUNCH_LAST_APP:
       return HandleLaunchLastApp();
-    case WINDOW_SNAP_LEFT:
-    case WINDOW_SNAP_RIGHT:
-      return HandleWindowSnap(action);
+    case WINDOW_SNAP_OR_DOCK_LEFT:
+    case WINDOW_SNAP_OR_DOCK_RIGHT:
+      return HandleWindowSnapOrDock(action);
     case WINDOW_MINIMIZE:
       return HandleWindowMinimize();
     case TOGGLE_FULLSCREEN: