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 118 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));
@@ skipping 87 matching lines @@
 
 void WindowState::SetTrackedByWorkspace(bool tracked_by_workspace) {
   if (tracked_by_workspace_ == tracked_by_workspace)
     return;
   bool old = tracked_by_workspace_;
   tracked_by_workspace_ = tracked_by_workspace;
   FOR_EACH_OBSERVER(WindowStateObserver, observer_list_,
                     OnTrackedByWorkspaceChanged(this, old));
 }
 
+void WindowState::SetWindowShowTypeUnsnapped() {
+  if (window_show_type_ != SHOW_TYPE_LEFT_SNAPPED &&
+      window_show_type_ != SHOW_TYPE_RIGHT_SNAPPED) {
+    return;
+  }
+  window_show_type_ = SHOW_TYPE_NORMAL;
+}
+
+void WindowState::SetBoundsChangedByUser(bool bounds_changed_by_user) {
+  bounds_changed_by_user_ = bounds_changed_by_user;
+  if (bounds_changed_by_user_)
+    SetWindowShowTypeUnsnapped();

[pkotwicz, 2013/11/21 17:26:14]

oshima@ can you comment as to whether this method is optimal. This method is an
improvement because previously if a user dragged a window which was snapped left
or right the window's state would not change.

+}
+
 void WindowState::OnWindowPropertyChanged(aura::Window* window,
                                           const void* key,
                                           intptr_t old) {
   DCHECK_EQ(window, window_);
   if (key == aura::client::kShowStateKey) {
     window_show_type_ = ToWindowShowType(GetShowState());
     ui::WindowShowState old_state = static_cast<ui::WindowShowState>(old);
     // TODO(oshima): Notify only when the state has changed.
     // Doing so break a few tests now.
     FOR_EACH_OBSERVER(
         WindowStateObserver, observer_list_,
         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) {
+  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;

[pkotwicz, 2013/11/21 17:26:14]

Can you set the restore bounds to the desired bounds before notifying the
observers? Then the observer can use the restore bounds as the "desired bounds".
Ideally we can avoid calling WindowState::Restore() at all.

[varkha, 2013/11/21 22:47:27]

This was actually a bug. Maximized window would not remember that it was snapped
since Restore would cause OnWindowPropertyChanged and override the
window_show_type_ that was set to one of the snapped states. I have corrected
that as well as saving the restore_bounds properly in the other case (when a
window is not maximized). Otherwise we get strange restored to snapped bounds
behavior when using accelerators.

+  FOR_EACH_OBSERVER(
+      WindowStateObserver, observer_list_,
+      OnWindowShowTypeChanged(this, old_type));
   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);
     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);
-  window_show_type_ = left_or_right;
 }
 
 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