Make sure that 30%of restored window is always visible.

ash/wm/workspace/workspace_layout_manager.cc

 // Copyright (c) 2012 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/workspace/workspace_layout_manager.h"
 
 #include "ash/root_window_controller.h"
 #include "ash/screen_ash.h"
 #include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shell.h"
@@ skipping 14 matching lines @@
 #include "ui/views/corewm/window_util.h"
 
 using aura::Window;
 
 namespace ash {
 
 namespace internal {
 
 namespace {
 
-// This specifies how much percent (2/3=66%) of a window must be visible when
-// the window is added to the workspace.
-const float kMinimumPercentOnScreenArea = 0.66f;
+// This specifies how much percent 1/3 of a window rect (width / height)
+// must be visible when the window is added to the workspace.

[Mr4D (OOO till 08-26), 2013/08/06 20:22:51]

you might want to say 30% - seems better then 1/3.

[oshima, 2013/08/06 23:13:20]

Done.

+const float kMinimumPercentOnScreenArea = 0.33f;
 
 bool IsMaximizedState(ui::WindowShowState state) {
   return state == ui::SHOW_STATE_MAXIMIZED ||
       state == ui::SHOW_STATE_FULLSCREEN;
 }
 
 }  // namespace
 
 WorkspaceLayoutManager::WorkspaceLayoutManager(aura::Window* window)
     : BaseLayoutManager(window->GetRootWindow()),
       shelf_(NULL),
       window_(window),
       work_area_(ScreenAsh::GetDisplayWorkAreaBoundsInParent(
           window->parent())) {
 }
 
 WorkspaceLayoutManager::~WorkspaceLayoutManager() {
 }
 
 void WorkspaceLayoutManager::SetShelf(internal::ShelfLayoutManager* shelf) {
   shelf_ = shelf;
 }
 
 void WorkspaceLayoutManager::OnWindowAddedToLayout(Window* child) {
   // Adjust window bounds in case that the new child is out of the workspace.

[Mr4D (OOO till 08-26), 2013/08/06 20:22:51]

You should at least change the comment to explain what this really does (as
discussed) since this will likely haunt us later and it is not clear (looking at
the function names) what it really does.

[oshima, 2013/08/06 23:13:20]

Done.

-  AdjustWindowSizeForScreenChange(child, ADJUST_WINDOW_WINDOW_ADDED);
+  if (!child->bounds().IsEmpty() &&
+      !wm::HasUserChangedWindowPositionOrSize(child))
+    AdjustWindowBounds(child, ADJUST_WINDOW_WINDOW_ADDED);
   BaseLayoutManager::OnWindowAddedToLayout(child);
   UpdateDesktopVisibility();
   RearrangeVisibleWindowOnShow(child);
 }
 
 void WorkspaceLayoutManager::OnWillRemoveWindowFromLayout(Window* child) {
   BaseLayoutManager::OnWillRemoveWindowFromLayout(child);
   if (child->TargetVisibility())
     RearrangeVisibleWindowOnHideOrRemove(child);
 }
@@ skipping 30 matching lines @@
         std::min(work_area_.height(), child_bounds.height()));
     SetChildBoundsDirect(child, child_bounds);
   }
   UpdateDesktopVisibility();
 }
 
 void WorkspaceLayoutManager::OnDisplayWorkAreaInsetsChanged() {
   const gfx::Rect work_area(ScreenAsh::GetDisplayWorkAreaBoundsInParent(
       window_->parent()));
   if (work_area != work_area_)
-    AdjustWindowSizesForScreenChange(ADJUST_WINDOW_DISPLAY_INSETS_CHANGED);
+    AdjustWindowsBounds(ADJUST_WINDOW_WORK_AREA_INSETS_CHANGED);
 }
 
 void WorkspaceLayoutManager::OnWindowPropertyChanged(Window* window,
                                                      const void* key,
                                                      intptr_t old) {
   if (key == aura::client::kShowStateKey) {
     ui::WindowShowState old_state = static_cast<ui::WindowShowState>(old);
     ui::WindowShowState new_state =
         window->GetProperty(aura::client::kShowStateKey);
     if (old_state != ui::SHOW_STATE_MINIMIZED &&
@@ skipping 38 matching lines @@
   }
 }
 
 void WorkspaceLayoutManager::ShowStateChanged(
     Window* window,
     ui::WindowShowState last_show_state) {
   BaseLayoutManager::ShowStateChanged(window, last_show_state);
   UpdateDesktopVisibility();
 }
 
-void WorkspaceLayoutManager::AdjustWindowSizesForScreenChange(
+void WorkspaceLayoutManager::AdjustWindowsBounds(
     AdjustWindowReason reason) {
   work_area_ = ScreenAsh::GetDisplayWorkAreaBoundsInParent(window_->parent());
-  BaseLayoutManager::AdjustWindowSizesForScreenChange(reason);
+  BaseLayoutManager::AdjustWindowsBounds(reason);
 }
 
-void WorkspaceLayoutManager::AdjustWindowSizeForScreenChange(
+void WorkspaceLayoutManager::AdjustWindowBounds(
     Window* window,
     AdjustWindowReason reason) {
   if (!GetTrackedByWorkspace(window))
     return;
 
   // Use cross fade transition for the maximized window if the adjustment
   // happens due to the shelf's visibility change. Otherwise the background
   // can be seen slightly between the bottom edge of resized-window and
   // the animating shelf.
   // TODO(mukai): this cause slight blur at the window frame because of the
   // cross fade. I think this is better, but should reconsider if someone
   // raises voice for this.
   if (wm::IsWindowMaximized(window) &&
-      reason == ADJUST_WINDOW_DISPLAY_INSETS_CHANGED) {
+      reason == ADJUST_WINDOW_WORK_AREA_INSETS_CHANGED) {
     CrossFadeToBounds(window, ScreenAsh::GetMaximizedWindowBoundsInParent(
         window->parent()->parent()));
     return;
   }
 
   if (SetMaximizedOrFullscreenBounds(window))
     return;
 
   gfx::Rect bounds = window->bounds();
-  if (reason == ADJUST_WINDOW_SCREEN_SIZE_CHANGED) {
-    // The work area may be smaller than the full screen.  Put as much of the
-    // window as possible within the display area.
-    bounds.AdjustToFit(work_area_);
-  } else if (reason == ADJUST_WINDOW_DISPLAY_INSETS_CHANGED) {
-    ash::wm::AdjustBoundsToEnsureMinimumWindowVisibility(work_area_, &bounds);
-  } else if (reason == ADJUST_WINDOW_WINDOW_ADDED) {
-    int min_width = bounds.width() * kMinimumPercentOnScreenArea;
-    int min_height = bounds.height() * kMinimumPercentOnScreenArea;
-    ash::wm::AdjustBoundsToEnsureWindowVisibility(
-        work_area_, min_width, min_height, &bounds);
+  switch (reason) {
+    case ADJUST_WINDOW_DISPLAY_SIZE_CHANGED:
+      // The work area may be smaller than the full screen.  Put as much of the
+      // window as possible within the display area.
+      bounds.AdjustToFit(work_area_);
+      break;
+    case ADJUST_WINDOW_WORK_AREA_INSETS_CHANGED:
+      ash::wm::AdjustBoundsToEnsureMinimumWindowVisibility(work_area_, &bounds);
+      break;
+    case ADJUST_WINDOW_WINDOW_ADDED: {

[Mr4D (OOO till 08-26), 2013/08/06 20:22:51]

I am weary about this case that it might adjust windows which get added to the
desktop for a different reason (DnD, ..) => and suddenly the users windows get
repositioned in a way they should not be. With the old name this made more
sense, but with the new function name this seems to ask for trouble.

Side note: There is some code in the window positioner which does not allow the
user to drag the window further out then a few pixels. 

[oshima, 2013/08/06 23:13:20]

These conditions are excluded by if statement in OnWindowAddedToLayout, and
test fails if you remove these if.

This is also better fallback if there is any other case we're missing because
it's better to force a window inside work area, rather than leaving outside.

I agree that the current situation isn't ideal. I will revisit/refactor/cleanup
positioning code for m31 to address your concern.

+      int min_width = bounds.width() * kMinimumPercentOnScreenArea;
+      int min_height = bounds.height() * kMinimumPercentOnScreenArea;
+      ash::wm::AdjustBoundsToEnsureWindowVisibility(
+          work_area_, min_width, min_height, &bounds);
+      break;
+    }
   }
   if (window->bounds() != bounds)
     window->SetBounds(bounds);
 }
 
 void WorkspaceLayoutManager::UpdateDesktopVisibility() {
   if (shelf_)
     shelf_->UpdateVisibilityState();
   FramePainter::UpdateSoloWindowHeader(window_->GetRootWindow());
 }
 
 void WorkspaceLayoutManager::UpdateBoundsFromShowState(Window* window) {
   // See comment in SetMaximizedOrFullscreenBounds() as to why we use parent in
   // these calculation.
   switch (window->GetProperty(aura::client::kShowStateKey)) {
     case ui::SHOW_STATE_DEFAULT:
     case ui::SHOW_STATE_NORMAL: {
       const gfx::Rect* restore = GetRestoreBoundsInScreen(window);
+      // Make sure that the part of the window is always visible
+      // when restored.
+      gfx::Rect bounds_in_parent;
       if (restore) {
-        gfx::Rect bounds_in_parent =
+        bounds_in_parent =
             ScreenAsh::ConvertRectFromScreen(window->parent()->parent(),
                                              *restore);
+
+        ash::wm::AdjustBoundsToEnsureMinimumWindowVisibility(
+            work_area_, &bounds_in_parent);
+      } else {
+        // Minimized windows have no restore bounds.
+        // Use the current bounds instead.
+        bounds_in_parent = window->bounds();
+        ash::wm::AdjustBoundsToEnsureMinimumWindowVisibility(
+            work_area_, &bounds_in_parent);
+        // Don't start animation if the bounds didn't change.
+        if (bounds_in_parent == window->bounds())
+          bounds_in_parent.SetRect(0, 0, 0, 0);
+      }
+      if (!bounds_in_parent.IsEmpty()) {
         CrossFadeToBounds(
             window,
             BaseLayoutManager::BoundsWithScreenEdgeVisible(
                 window->parent()->parent(),
                 bounds_in_parent));
       }
       ClearRestoreBounds(window);
       break;
     }
 
@@ skipping 29 matching lines @@
     SetChildBoundsDirect(
         window,
         ScreenAsh::GetDisplayBoundsInParent(window->parent()->parent()));
     return true;
   }
   return false;
 }
 
 }  // namespace internal
 }  // namespace ash