Windows docking should get triggered by pressing against the screen edge

ash/wm/dock/docked_window_resizer.cc

Index: ash/wm/dock/docked_window_resizer.cc
diff --git a/ash/wm/dock/docked_window_resizer.cc b/ash/wm/dock/docked_window_resizer.cc
index 3e1ef29b0ad76f1ee1e6e8cad4eff34d0a61d55f..d94fabb0d3a415bbae8e3f3a4399ef25be960cd3 100644
--- a/ash/wm/dock/docked_window_resizer.cc
+++ b/ash/wm/dock/docked_window_resizer.cc
@@ -5,6 +5,7 @@
 #include "ash/wm/dock/docked_window_resizer.h"
 
 #include "ash/ash_switches.h"
+#include "ash/display/display_controller.h"
 #include "ash/launcher/launcher.h"
 #include "ash/root_window_controller.h"
 #include "ash/screen_ash.h"
@@ -17,7 +18,6 @@
 #include "ash/wm/property_util.h"
 #include "ash/wm/window_properties.h"
 #include "ash/wm/workspace/magnetism_matcher.h"
-#include "ash/wm/workspace/phantom_window_controller.h"
 #include "ash/wm/workspace/workspace_window_resizer.h"
 #include "base/command_line.h"
 #include "ui/aura/client/aura_constants.h"
@@ -37,9 +37,13 @@ namespace {
 
 DockedWindowLayoutManager* GetDockedLayoutManagerAtPoint(
     const gfx::Point& point) {
+  gfx::Display display = ScreenAsh::FindDisplayContainingPoint(point);
+  if (!display.is_valid())
+    return NULL;
+  aura::RootWindow* root = Shell::GetInstance()->display_controller()->
+      GetRootWindowForDisplayId(display.id());
   aura::Window* dock_container = Shell::GetContainer(
-      wm::GetRootWindowAt(point),
-      kShellWindowId_DockedContainer);
+      root, kShellWindowId_DockedContainer);
   return static_cast<DockedWindowLayoutManager*>(
       dock_container->layout_manager());
 }
@@ -73,6 +77,7 @@ void DockedWindowResizer::Drag(const gfx::Point& location, int event_flags) {
   }
   gfx::Point offset;
   gfx::Rect bounds(CalculateBoundsForDrag(details_, location));
+  LOG(INFO) << "Drag to " << bounds.ToString();
   bool set_tracked_by_workspace = MaybeSnapToEdge(bounds, &offset);
 
   // Temporarily clear kWindowTrackedByWorkspaceKey for windows that are snapped
@@ -97,7 +102,7 @@ void DockedWindowResizer::Drag(const gfx::Point& location, int event_flags) {
 
   DockedWindowLayoutManager* new_dock_layout =
       GetDockedLayoutManagerAtPoint(last_location_);
-  if (new_dock_layout != dock_layout_) {
+  if (new_dock_layout && new_dock_layout != dock_layout_) {
     // The window is being dragged to a new display. If the previous
     // container is the current parent of the window it will be informed of
     // the end of drag when the window is reparented, otherwise let the
@@ -115,29 +120,11 @@ void DockedWindowResizer::Drag(const gfx::Point& location, int event_flags) {
     if (new_dock_layout != initial_dock_layout_)
       new_dock_layout->StartDragging(GetTarget());
   }
-
-  // Show snapping animation when a window touches a screen edge or when
-  // it is about to get docked.
-  DockedAlignment new_docked_alignment = GetDraggedWindowAlignment();
-  if (new_docked_alignment != DOCKED_ALIGNMENT_NONE) {
-    if (!is_docked_) {
-      dock_layout_->DockDraggedWindow(GetTarget());
-      is_docked_ = true;
-    }
-    UpdateSnapPhantomWindow();
-  } else {
-    if (is_docked_) {
-      dock_layout_->UndockDraggedWindow();
-      is_docked_ = false;
-    }
-    // Clear phantom window when a window gets undocked.
-    snap_phantom_window_controller_.reset();
-  }
+  // Window could get marked as docked by the SnapSizer, so update the state.
+  is_docked_ = dock_layout_->is_dragged_window_docked();
 }
 
 void DockedWindowResizer::CompleteDrag(int event_flags) {
-  snap_phantom_window_controller_.reset();
-
   // Temporarily clear kWindowTrackedByWorkspaceKey for panels so that they
   // don't get forced into the workspace that may be shrunken because of docked
   // windows.
@@ -153,8 +140,6 @@ void DockedWindowResizer::CompleteDrag(int event_flags) {
 }
 
 void DockedWindowResizer::RevertDrag() {
-  snap_phantom_window_controller_.reset();
-
   // Temporarily clear kWindowTrackedByWorkspaceKey for panels so that they
   // don't get forced into the workspace that may be shrunken because of docked
   // windows.
@@ -163,6 +148,11 @@ void DockedWindowResizer::RevertDrag() {
   if (set_tracked_by_workspace)
     SetTrackedByWorkspace(GetTarget(), false);
   next_window_resizer_->RevertDrag();
+  // Restore docked state to what it was before the drag if necessary.
+  if (was_docked_ && !is_docked_) {
+    dock_layout_->DockDraggedWindow(GetTarget());
+    is_docked_ = was_docked_;
+  }
   FinishedDragging();
   if (set_tracked_by_workspace)
     SetTrackedByWorkspace(GetTarget(), tracked_by_workspace);
@@ -197,51 +187,15 @@ DockedWindowResizer::DockedWindowResizer(WindowResizer* next_window_resizer,
   is_docked_ = was_docked_;
 }
 
-DockedAlignment DockedWindowResizer::GetDraggedWindowAlignment() {
-  aura::Window* window = GetTarget();
-  DockedWindowLayoutManager* layout_manager =
-      GetDockedLayoutManagerAtPoint(last_location_);
-  const DockedAlignment alignment = layout_manager->CalculateAlignment();
-  const gfx::Rect& bounds(window->GetBoundsInScreen());
-
-  // Check if the window is touching the edge - it may need to get docked.
-  if (alignment == DOCKED_ALIGNMENT_NONE)
-    return layout_manager->GetAlignmentOfWindow(window);
-
-  // Both bounds and pointer location are checked because some drags involve
-  // stickiness at the workspace-to-dock boundary and so the |location| may be
-  // outside of the |bounds|.
-  // It is also possible that all the docked windows are minimized or hidden
-  // in which case the dragged window needs to be exactly touching the same
-  // edge that those docked windows were aligned before they got minimized.
-  // TODO(varkha): Consider eliminating sticky behavior on that boundary when
-  // a pointer enters docked area.
-  if ((layout_manager->docked_bounds().Intersects(bounds) &&
-       layout_manager->docked_bounds().Contains(last_location_)) ||
-      alignment == layout_manager->GetAlignmentOfWindow(window)) {
-    // A window is being added to other docked windows (on the same side).
-    return alignment;
-  }
-  return DOCKED_ALIGNMENT_NONE;
-}
-
 bool DockedWindowResizer::MaybeSnapToEdge(const gfx::Rect& bounds,
                                           gfx::Point* offset) {
-  aura::Window* dock_container = Shell::GetContainer(
-      wm::GetRootWindowAt(last_location_),
-      kShellWindowId_DockedContainer);
-  DockedAlignment dock_alignment =
-      GetDockedLayoutManagerAtPoint(last_location_)->CalculateAlignment();
-  gfx::Rect dock_bounds = ScreenAsh::ConvertRectFromScreen(
-      GetTarget()->parent(), dock_container->GetBoundsInScreen());
-  // Windows only snap magnetically when they are close to the edge of the
-  // screen and when the cursor is over other docked windows.
-  // When a window being dragged is the last window that was previously
-  // docked it is still allowed to magnetically snap to either side.
-  bool can_snap = was_docked_ ||
-      (GetDraggedWindowAlignment() != DOCKED_ALIGNMENT_NONE);
-  if (!can_snap)
+  // Windows only snap magnetically when they were previously docked.
+  if (!was_docked_)
     return false;
+  DockedAlignment dock_alignment = dock_layout_->CalculateAlignment();
+  gfx::Rect dock_bounds = ScreenAsh::ConvertRectFromScreen(
+      GetTarget()->parent(),
+      dock_layout_->dock_container()->GetBoundsInScreen());
 
   // Distance in pixels that the cursor must move past an edge for a window
   // to move beyond that edge. Same constant as in WorkspaceWindowResizer
@@ -253,19 +207,17 @@ bool DockedWindowResizer::MaybeSnapToEdge(const gfx::Rect& bounds,
   const int kSnapToDockDistance = MagnetismMatcher::kMagneticDistance;
 
   if (dock_alignment == DOCKED_ALIGNMENT_LEFT ||
-      (dock_alignment == DOCKED_ALIGNMENT_NONE && was_docked_)) {
+      dock_alignment == DOCKED_ALIGNMENT_NONE) {
     const int distance = bounds.x() - dock_bounds.x();
-    if (distance < (was_docked_ ? kSnapToDockDistance : 0) &&
-        distance > -kStickyDistance) {
+    if (distance < kSnapToDockDistance && distance > -kStickyDistance) {
       offset->set_x(-distance);
       return true;
     }
   }
   if (dock_alignment == DOCKED_ALIGNMENT_RIGHT ||
-      (dock_alignment == DOCKED_ALIGNMENT_NONE && was_docked_)) {
+      dock_alignment == DOCKED_ALIGNMENT_NONE) {
     const int distance = dock_bounds.right() - bounds.right();
-    if (distance < (was_docked_ ? kSnapToDockDistance : 0) &&
-        distance > -kStickyDistance) {
+    if (distance < kSnapToDockDistance && distance > -kStickyDistance) {
       offset->set_x(distance);
       return true;
     }
@@ -302,16 +254,16 @@ void DockedWindowResizer::FinishedDragging() {
     return;
 
   aura::Window* window = GetTarget();
-  bool should_dock = was_docked_;
-  const bool attached_panel =
+  bool should_dock = is_docked_;

[flackr, 2013/09/06 02:18:43]

if should_dock == is_docked_, can we just use is_docked_ below?

[varkha, 2013/09/09 15:38:42]

Done.

+  bool attached_panel = false;
+  if (details_.bounds_change & WindowResizer::kBoundsChange_Resizes) {
+    // No longer restore to pre-docked bounds if a window has been resized.
+    if (is_docked_)
+      ClearRestoreBounds(window);
+  } else {
+    attached_panel =

[flackr, 2013/09/06 02:18:43]

Won't moving this check here make it possible to dock attached panels by
resizing them?

[varkha, 2013/09/09 15:38:42]

No. is_docked_ will be false unless the window was docked before. So resizing a
docked window won't undock but will never dock an undocked window.

[flackr, 2013/09/09 19:06:57]

I still think it feels wrong to only have attached_panel set correctly in
certain conditions. Why is this necessary?

[varkha, 2013/09/09 20:37:54]

See more explicit logic for preventing panel attachment on resize. Do you think
this belongs more in panel resizer?

       window->type() == aura::client::WINDOW_TYPE_PANEL &&

[flackr, 2013/09/06 02:18:43]

nit: indent 4 spaces.

[varkha, 2013/09/09 15:38:42]

Done.

       window->GetProperty(kPanelAttachedKey);
-  // If a window was previously docked then keep it docked if it is resized and
-  // still aligned at the screen edge.
-  if ((was_docked_ ||
-       ((details_.bounds_change & WindowResizer::kBoundsChange_Repositions) &&
-        !(details_.bounds_change & WindowResizer::kBoundsChange_Resizes)))) {
-    should_dock = GetDraggedWindowAlignment() != DOCKED_ALIGNMENT_NONE;
   }
 
   // Check if the window needs to be docked or returned to workspace.
@@ -343,17 +295,16 @@ void DockedWindowResizer::FinishedDragging() {
   if (initial_dock_layout_ != dock_layout_)
     initial_dock_layout_->FinishDragging();
   is_docked_ = false;
-}
-
-void DockedWindowResizer::UpdateSnapPhantomWindow() {
-  if (!did_move_or_resize_ || details_.window_component != HTCAPTION)
-    return;
-
-  if (!snap_phantom_window_controller_) {
-    snap_phantom_window_controller_.reset(
-        new PhantomWindowController(GetTarget()));
+  // Retain restore bounds when dragging inside the docked area.
+  if (was_docked_ &&

[flackr, 2013/09/06 02:18:43]

was_docked_ is true iff the window was docked before the drag. Shouldn't we
retain restore bounds if the window was not initially docked?

[varkha, 2013/09/09 15:38:42]

That part is already handled by existing code in
WorkspaceWindowResizer::CompleteDrag that saves the restore bounds when snapping
(and now docking). What I am doing here is restoring the restore bounds after
they have been cleared by DefaultWindowResizer::Drag. I do this only in case
then a docked window is dragged but not resized inside the dock.

[flackr, 2013/09/09 19:06:57]

I think it would make more sense to add an exception or check in
DefaultWindowResizer::Drag (that the window is in the default / workspace
container) rather than undo what it does here.

[varkha, 2013/09/09 20:37:54]

I have noticed that I no longer need this since the docked windows get snapped
again when dragged even within the dock and that restores the restore bounds.

+      window->parent()->id() == kShellWindowId_DockedContainer &&
+      !(details_.bounds_change & WindowResizer::kBoundsChange_Resizes)) {
+    gfx::Rect initial_bounds = ScreenAsh::ConvertRectToScreen(
+        window->parent(), details_.initial_bounds_in_parent);
+    SetRestoreBoundsInScreen(window, details_.restore_bounds.IsEmpty() ?
+                             initial_bounds :
+                             details_.restore_bounds);
   }
-  snap_phantom_window_controller_->Show(dock_layout_->dragged_bounds());
 }
 
 }  // namespace internal