Undocks window first before side-snapping bounds

ash/wm/window_state.cc

 // Copyright 2013 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.
 
 #include "ash/wm/window_state.h"
 
 #include "ash/root_window_controller.h"
 #include "ash/screen_ash.h"
 #include "ash/shell_window_ids.h"
 #include "ash/wm/window_properties.h"
@@ skipping 70 matching lines @@
 
 bool WindowState::IsActive() const {
   return IsActiveWindow(window_);
 }
 
 bool WindowState::IsDocked() const {
   return window_->parent() &&
       window_->parent()->id() == internal::kShellWindowId_DockedContainer;
 }
 
+bool WindowState::IsSnapped() const {
+  return window_show_type_ == SHOW_TYPE_LEFT_SNAPPED ||
+      window_show_type_ == SHOW_TYPE_RIGHT_SNAPPED;
+}
+
 bool WindowState::CanMaximize() const {
   return window_->GetProperty(aura::client::kCanMaximizeKey);
 }
 
 bool WindowState::CanMinimize() const {
   internal::RootWindowController* controller =
       internal::RootWindowController::ForWindow(window_);
   if (!controller)
     return false;
   aura::Window* lockscreen = controller->GetContainer(
@@ skipping 28 matching lines @@
 
 void WindowState::Maximize() {
   window_->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MAXIMIZED);
 }
 
 void WindowState::SnapLeft(const gfx::Rect& bounds) {
   SnapWindow(SHOW_TYPE_LEFT_SNAPPED, bounds);
 }
 
 void WindowState::SnapRight(const gfx::Rect& bounds) {
-  SnapWindow(SHOW_TYPE_LEFT_SNAPPED, bounds);
+  SnapWindow(SHOW_TYPE_RIGHT_SNAPPED, bounds);
 }
 
 void WindowState::Minimize() {
   window_->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MINIMIZED);
 }
 
 void WindowState::Unminimize() {
   window_->SetProperty(
       aura::client::kShowStateKey,
       window_->GetProperty(aura::client::kRestoreShowStateKey));
   window_->ClearProperty(aura::client::kRestoreShowStateKey);
 }
 
 void WindowState::Activate() {
   ActivateWindow(window_);
 }
 
 void WindowState::Deactivate() {
   DeactivateWindow(window_);
 }
 
 void WindowState::Restore() {
+  // When a window |IsSnapped| it is auto-positioned by layout manager to stay
+  // snapped at a screen edge. Stop this auto-positioning when Restore() is
+  // called such as at the end of a drag or when centering a window.
+  if (IsSnapped())
+    window_show_type_ = SHOW_TYPE_NORMAL;

[pkotwicz, 2013/11/28 06:15:10]

Nit: Let's set |window_show_type_| to SHOW_TYPE_NORMAL always.
How about: "Set |window_show_type_| to SHOW_TYPE_NORMAL now so that an observer
observing kShowStateKey gets the correct value when querying IsSnapped()."

[varkha, 2013/11/28 15:39:40]

Done.

   window_->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
 }
 
 void WindowState::ToggleMaximized() {
   if (IsMaximized())
     Restore();
   else if (CanMaximize())
     Maximize();
 }
 
@@ skipping 86 matching lines @@
         OnWindowShowTypeChanged(this, ToWindowShowType(old_state)));
   }
 }
 
 void WindowState::OnWindowDestroying(aura::Window* window) {
   window_->RemoveObserver(this);
 }
 
 void WindowState::SnapWindow(WindowShowType left_or_right,
                              const gfx::Rect& bounds) {
-  if (IsMaximizedOrFullscreen()) {
-    // Before we can set the bounds we need to restore the window.
-    // Restoring the window will set the window to its restored bounds.
-    // To avoid an unnecessary bounds changes (which may have side effects)
-    // we set the restore bounds to the bounds we want, restore the window,
-    // then reset the restore bounds. This way no unnecessary bounds
-    // changes occurs and the original restore bounds is remembered.
-    gfx::Rect restore_bounds_in_screen =
-        GetRestoreBoundsInScreen();
-    SetRestoreBoundsInParent(bounds);
+  // Original restore bounds (if they exist) or the current bounds are
+  // preserved in order to allow restoring a window that has been snapped to its
+  // original restored bounds regardless of the WindowShowType it had when it
+  // got snapped.

[pkotwicz, 2013/11/28 06:15:10]

How about: "Compute the bounds that the window will restore to. If the window
does not already have restore bounds, the window will be restored to its current
bounds when unsnapped."

[varkha, 2013/11/28 15:39:40]

Done (slightly modified).

+  gfx::Rect restore_bounds_in_screen(HasRestoreBounds() ?
+      GetRestoreBoundsInScreen() : window_->GetBoundsInScreen());
+  SetRestoreBoundsInParent(bounds);

[pkotwicz, 2013/11/28 06:15:10]

How about: "Set the window's restore bounds so that WorkspaceLayoutManager knows
which width to use for the snapped window."

[varkha, 2013/11/28 15:39:40]

Done.

+
+  bool was_maximized = IsMaximizedOrFullscreen();
+  // Before we can set the bounds we need to restore the window.
+  // Restoring the window will set the window to its restored bounds set above.
+  // Restore will cause OnWindowPropertyChanged() so it needs to be done
+  // before notifying that the WindowShowType has changed to |left_or_right|.
+  if (was_maximized)
     Restore();
-    SetRestoreBoundsInScreen(restore_bounds_in_screen);
-  } else {
-    window_->SetBounds(bounds);
-  }
   DCHECK(left_or_right == SHOW_TYPE_LEFT_SNAPPED ||
          left_or_right == SHOW_TYPE_RIGHT_SNAPPED);
+  WindowShowType old_type = window_show_type_;
   window_show_type_ = left_or_right;
+  FOR_EACH_OBSERVER(
+      WindowStateObserver, observer_list_,
+      OnWindowShowTypeChanged(this, old_type));
+  // TODO(varkha): Ideally the bounds should be changed in a LayoutManager upon
+  // observing the WindowShowType change.

[pkotwicz, 2013/11/28 06:15:10]

How about: "If the window is a child of kShellWindowId_DockedContainer, the
window's bounds are not updated as a result of OnWindowShowTypeChanged(). Set
them here. Skip setting the bounds otherwise to avoid stopping the slide
animation which was started as a result of OnWindowShowTypeChanged()."

[varkha, 2013/11/28 15:39:40]

Done.

+  // When window is not docked such as when it is snapped using shortcuts
+  // setting bounds here would have stopped slide animation that was started
+  // in WorkspaceLayoutManager.
+  if (IsDocked())
+    window_->SetBounds(bounds);
+  SetRestoreBoundsInScreen(restore_bounds_in_screen);
 }
 
 WindowState* GetActiveWindowState() {
   aura::Window* active = GetActiveWindow();
   return active ? GetWindowState(active) : NULL;
 }
 
 WindowState* GetWindowState(aura::Window* window) {
   if (!window)
     return NULL;
   WindowState* settings = window->GetProperty(internal::kWindowStateKey);
   if(!settings) {
     settings = new WindowState(window);
     window->SetProperty(internal::kWindowStateKey, settings);
   }
   return settings;
 }
 
 const WindowState* GetWindowState(const aura::Window* window) {
   return GetWindowState(const_cast<aura::Window*>(window));
 }
 
 }  // namespace wm
 }  // namespace ash