Ignoring alignment when it pushes a window out of the screen

BUG=121580, 122841
TEST=None


Review URL: http://codereview.chromium.org/9969164

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@132316 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 88 insertions, 58 deletions

diff --git a/ash/wm/workspace/workspace_layout_manager.cc b/ash/wm/workspace/workspace_layout_manager.cc
index 991db32..ca28cd4 100644
--- a/ash/wm/workspace/workspace_layout_manager.cc
+++ b/ash/wm/workspace/workspace_layout_manager.cc
@@ -44,10 +44,6 @@ void WorkspaceLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
 
   if (child->IsVisible()) {
     workspace_manager_->AddWindow(child);
-  } else if (wm::IsWindowNormal(child)) {
-    // Align non-maximized/fullscreen windows to a grid.
-    SetChildBoundsDirect(
-        child, workspace_manager_->AlignBoundsToGrid(child->GetTargetBounds()));
   }
 }
 
diff --git a/ash/wm/workspace/workspace_manager.cc b/ash/wm/workspace/workspace_manager.cc
index 05ab750..e710137 100644
--- a/ash/wm/workspace/workspace_manager.cc
+++ b/ash/wm/workspace/workspace_manager.cc
@@ -11,7 +11,6 @@
 #include "ash/wm/property_util.h"
 #include "ash/wm/shelf_layout_manager.h"
 #include "ash/wm/window_animations.h"
-#include "ash/wm/window_resizer.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm/workspace/managed_workspace.h"
 #include "ash/wm/workspace/maximized_workspace.h"
@@ -43,15 +42,6 @@ void BuildWindowList(const std::vector<aura::Window*>& windows,
   }
 }
 
-gfx::Rect AlignRectToGrid(const gfx::Rect& rect, int grid_size) {
-  if (grid_size <= 1)
-    return rect;
-  return gfx::Rect(ash::WindowResizer::AlignToGrid(rect.x(), grid_size),
-                   ash::WindowResizer::AlignToGrid(rect.y(), grid_size),
-                   ash::WindowResizer::AlignToGrid(rect.width(), grid_size),
-                   ash::WindowResizer::AlignToGrid(rect.height(), grid_size));
-}
-
 }
 
 namespace ash {
@@ -105,9 +95,6 @@ void WorkspaceManager::AddWindow(aura::Window* window) {
     return;
   }
 
-  if (wm::IsWindowNormal(window) && grid_size_ > 1)
-    SetWindowBounds(window, AlignBoundsToGrid(window->GetTargetBounds()));
-
   Workspace* workspace = NULL;
   Workspace::Type type_for_window = Workspace::TypeForWindow(window);
   switch (type_for_window) {
@@ -143,12 +130,6 @@ void WorkspaceManager::SetActiveWorkspaceByWindow(aura::Window* window) {
     workspace->Activate();
 }
 
-gfx::Rect WorkspaceManager::AlignBoundsToGrid(const gfx::Rect& bounds) {
-  if (grid_size_ <= 1)
-    return bounds;
-  return AlignRectToGrid(bounds, grid_size_);
-}
-
 void WorkspaceManager::UpdateShelfVisibility() {
   shelf_->UpdateVisibilityState();
 }
diff --git a/ash/wm/workspace/workspace_manager.h b/ash/wm/workspace/workspace_manager.h
index 6da00e7..4f80f36 100644
--- a/ash/wm/workspace/workspace_manager.h
+++ b/ash/wm/workspace/workspace_manager.h
@@ -85,9 +85,6 @@ class ASH_EXPORT WorkspaceManager {
   void set_grid_size(int size) { grid_size_ = size; }
   int grid_size() const { return grid_size_; }
 
-  // Returns a bounds aligned to the grid. Returns |bounds| if grid_size is 0.
-  gfx::Rect AlignBoundsToGrid(const gfx::Rect& bounds);
-
   void set_shelf(ShelfLayoutManager* shelf) { shelf_ = shelf; }
 
   // Updates the visibility and whether any windows overlap the shelf.
diff --git a/ash/wm/workspace/workspace_manager_unittest.cc b/ash/wm/workspace/workspace_manager_unittest.cc
index 6d112c1..a46c35d 100644
--- a/ash/wm/workspace/workspace_manager_unittest.cc
+++ b/ash/wm/workspace/workspace_manager_unittest.cc
@@ -306,7 +306,11 @@ TEST_F(WorkspaceManagerTest, SnapToGrid) {
   scoped_ptr<Window> w1(CreateTestWindowUnparented());
   w1->SetBounds(gfx::Rect(1, 6, 25, 30));
   w1->SetParent(GetViewport());
-  EXPECT_EQ(gfx::Rect(0, 8, 24, 32), w1->bounds());
+  // We are not aligning this anymore this way. When the window gets shown
+  // the window is expected to be handled differently, but this cannot be
+  // tested with this test. So the result of this test should be that the
+  // bounds are exactly as passed in.
+  EXPECT_EQ(gfx::Rect(1, 6, 25, 30), w1->bounds());
 }
 
 // Assertions around a fullscreen window.
diff --git a/chrome/browser/ui/views/ash/window_positioner.cc b/chrome/browser/ui/views/ash/window_positioner.cc
index 4b19f6d..2780060 100644
--- a/chrome/browser/ui/views/ash/window_positioner.cc
+++ b/chrome/browser/ui/views/ash/window_positioner.cc
@@ -6,6 +6,7 @@
 
 #include "ash/shell.h"
 #include "ash/wm/window_cycle_controller.h"
+#include "ash/wm/window_resizer.h"
 #include "ash/wm/window_util.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_delegate.h"
@@ -55,10 +56,10 @@ gfx::Rect WindowPositioner::GetPopupPosition(const gfx::Rect& old_pos) {
       work_area.width()) ||
       (old_pos.height() + popup_position_offset_from_screen_corner_y >=
        work_area.height()))
-    return old_pos;
-  const gfx::Rect result = SmartPopupPosition(old_pos, work_area);
+    return AlignPopupPosition(old_pos, work_area, grid);
+  const gfx::Rect result = SmartPopupPosition(old_pos, work_area, grid);
   if (!result.IsEmpty())
-    return result;
+    return AlignPopupPosition(result, work_area, grid);
   return NormalPopupPosition(old_pos, work_area);
 }
 
@@ -97,7 +98,8 @@ gfx::Rect WindowPositioner::NormalPopupPosition(
 
 gfx::Rect WindowPositioner::SmartPopupPosition(
     const gfx::Rect& old_pos,
-    const gfx::Rect& work_area) {
+    const gfx::Rect& work_area,
+    int grid) {
   const std::vector<aura::Window*> windows =
       ash::WindowCycleController::BuildWindowList();
 
@@ -150,7 +152,11 @@ gfx::Rect WindowPositioner::SmartPopupPosition(
         for (i = 0; i < regions.size(); i++) {
           if (regions[i]->Intersects(gfx::Rect(x + work_area.x(),
                                                y + work_area.y(), w, h))) {
-            y = regions[i]->y() + regions[i]->height() - work_area.y();
+            y = regions[i]->bottom() - work_area.y();
+            if (grid > 1) {
+              // Align to the (next) grid step.
+              y = ash::WindowResizer::AlignToGridRoundUp(y, grid);
+            }
             break;
           }
         }
@@ -161,3 +167,24 @@ gfx::Rect WindowPositioner::SmartPopupPosition(
   }
   return gfx::Rect(0, 0, 0, 0);
 }
+
+gfx::Rect WindowPositioner::AlignPopupPosition(
+    const gfx::Rect& pos,
+    const gfx::Rect& work_area,
+    int grid) {
+  if (grid <= 1)
+    return pos;
+
+  int x = pos.x() - (pos.x() - work_area.x()) % grid;
+  int y = pos.y() - (pos.y() - work_area.y()) % grid;
+  int w = pos.width();
+  int h = pos.height();
+
+  // If the alignment was pushing the window out of the screen, we ignore the
+  // alignment for that call.
+  if (abs(pos.right() - work_area.right()) < grid)
+    x = work_area.right() - w;
+  if (abs(pos.bottom() - work_area.bottom()) < grid)
+    y = work_area.bottom() - h;
+  return gfx::Rect(x, y, w, h);
+}
diff --git a/chrome/browser/ui/views/ash/window_positioner.h b/chrome/browser/ui/views/ash/window_positioner.h
index 599ac53..fd84450 100644
--- a/chrome/browser/ui/views/ash/window_positioner.h
+++ b/chrome/browser/ui/views/ash/window_positioner.h
@@ -31,11 +31,17 @@ class WindowPositioner {
   // Find a smart way to position the popup window. If there is no space this
   // function will return an empty rectangle.
   gfx::Rect SmartPopupPosition(const gfx::Rect& old_pos,
-                               const gfx::Rect& work_space);
+                               const gfx::Rect& work_area,
+                               int grid);
 
   // Find the next available cascading popup position (on the given screen).
   gfx::Rect NormalPopupPosition(const gfx::Rect& old_pos,
-                                const gfx::Rect& work_space);
+                                const gfx::Rect& work_area);
+
+  // Align the location to the grid / snap to the right / bottom corner.
+  gfx::Rect AlignPopupPosition(const gfx::Rect &pos,
+                               const gfx::Rect &work_area,
+                               int grid);
 
   // Constants to identify the type of resize.
   static const int kBoundsChange_None;
diff --git a/chrome/browser/ui/views/ash/window_positioner_unittest.cc b/chrome/browser/ui/views/ash/window_positioner_unittest.cc
index 943f4bd..3e1b18c 100644
--- a/chrome/browser/ui/views/ash/window_positioner_unittest.cc
+++ b/chrome/browser/ui/views/ash/window_positioner_unittest.cc
@@ -8,6 +8,7 @@
 #include "ash/shell_window_ids.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/test_shell_delegate.h"
+#include "ash/wm/window_resizer.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "chrome/browser/ui/browser.h"
@@ -112,7 +113,10 @@ WindowPositionerTest::WindowPositionerTest()
   profile_.reset(new TestingProfile());
 }
 
-WindowPositionerTest::~WindowPositionerTest() {}
+WindowPositionerTest::~WindowPositionerTest() {
+  profile_.reset(NULL);
+  ui_thread_.reset(NULL);
+}
 
 void WindowPositionerTest::SetUp() {
   AshTestBase::SetUp();
@@ -251,7 +255,7 @@ TEST_F(WindowPositionerTest, cascading) {
 
 TEST_F(WindowPositionerTest, filling) {
   const gfx::Rect work_area = gfx::Screen::GetPrimaryMonitorWorkArea();
-
+  int grid = ash::Shell::GetInstance()->GetGridSize();
   gfx::Rect popup_position(0, 0, 256, 128);
   // Leave space on the left and the right and see if we fill top to bottom.
   window()->SetBounds(gfx::Rect(work_area.x() + popup_position.width(),
@@ -270,20 +274,23 @@ TEST_F(WindowPositionerTest, filling) {
   popup()->Show();
   gfx::Rect mid_left = window_positioner()->GetPopupPosition(popup_position);
   EXPECT_EQ(gfx::Rect(work_area.x(),
-                      work_area.y() + top_left.height(),
-                      popup_position.width(), popup_position.height()),
+                      ash::WindowResizer::AlignToGridRoundDown(
+                          work_area.y() + top_left.height(), grid),
+                          popup_position.width(), popup_position.height()),
                       mid_left);
 
   // Block now everything so that we can only put the popup on the bottom
   // of the left side.
+  // Note: We need to keep one "grid spacing free" if the window does not
+  // fit into the grid (which is true for 200 height).`
   popup()->SetBounds(gfx::Rect(work_area.x(), work_area.y(),
                                popup_position.width(),
-                               work_area.height() - popup_position.height()));
+                               work_area.height() - popup_position.height() -
+                                   grid + 1));
   gfx::Rect bottom_left = window_positioner()->GetPopupPosition(
                               popup_position);
   EXPECT_EQ(gfx::Rect(work_area.x(),
-                      work_area.y() + work_area.height() -
-                          popup_position.height(),
+                      work_area.bottom() - popup_position.height(),
                       popup_position.width(), popup_position.height()),
                       bottom_left);
 
@@ -294,8 +301,8 @@ TEST_F(WindowPositionerTest, filling) {
                                1));
   gfx::Rect top_right = window_positioner()->GetPopupPosition(
                             popup_position);
-  EXPECT_EQ(gfx::Rect(work_area.x() + work_area.width() -
-                          popup_position.width(),
+  EXPECT_EQ(gfx::Rect(ash::WindowResizer::AlignToGridRoundDown(
+                         work_area.right() - popup_position.width(), grid),
                       work_area.y(),
                       popup_position.width(), popup_position.height()),
                       top_right);
diff --git a/chrome/browser/ui/window_sizer_ash.cc b/chrome/browser/ui/window_sizer_ash.cc
index 561316f..45c4b7f 100644
--- a/chrome/browser/ui/window_sizer_ash.cc
+++ b/chrome/browser/ui/window_sizer_ash.cc
@@ -107,10 +107,14 @@ void WindowSizer::GetDefaultWindowBounds(gfx::Rect* default_bounds) const {
   // There should also be a 'desktop' border around the window at the top.
   // Since the workspace excludes the tray area we only need one border size.
   int default_height = work_area.height() - kDesktopBorderSize;
+  // We align the size to the grid size to avoid any surprise when the
+  // monitor height isn't divide-able by our alignment factor.
+  default_width -= default_width % kDesktopBorderSize;
+  default_height -= default_height % kDesktopBorderSize;
   int offset_x = kDesktopBorderSize;
   int maximum_window_width = 1280;
   if (default_width > maximum_window_width) {
-    // The window should get centered on the screen as well.
+    // The window should get centered on the screen and not follow the grid.
     offset_x = (work_area.width() - maximum_window_width) / 2;
     // Never make a window wider then 1280.
     default_width = maximum_window_width;
diff --git a/chrome/browser/ui/window_sizer_ash_unittest.cc b/chrome/browser/ui/window_sizer_ash_unittest.cc
index 3420f5c..a82e750 100644
--- a/chrome/browser/ui/window_sizer_ash_unittest.cc
+++ b/chrome/browser/ui/window_sizer_ash_unittest.cc
@@ -10,6 +10,7 @@
 #include "ash/shell_window_ids.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/test_shell_delegate.h"
+#include "ash/wm/window_resizer.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "chrome/browser/ui/browser.h"
@@ -83,8 +84,8 @@ TestBrowserWindowAura::~TestBrowserWindowAura() {}
 // Test that the window is sized appropriately for the first run experience
 // where the default window bounds calculation is invoked.
 TEST_F(WindowSizerTest, DefaultSizeCase) {
-  EXPECT_EQ(WindowSizer::kDesktopBorderSize,
-            ash::Shell::GetInstance()->GetGridSize());
+  int grid = ash::Shell::GetInstance()->GetGridSize();
+  EXPECT_EQ(WindowSizer::kDesktopBorderSize, grid);
   { // 4:3 monitor case, 1024x768, no taskbar
     gfx::Rect window_bounds;
     GetWindowBounds(tentwentyfour, tentwentyfour, gfx::Rect(), gfx::Rect(),
@@ -103,9 +104,10 @@ TEST_F(WindowSizerTest, DefaultSizeCase) {
     EXPECT_EQ(gfx::Rect(WindowSizer::kDesktopBorderSize,
                         WindowSizer::kDesktopBorderSize,
                         1024 - WindowSizer::kDesktopBorderSize * 2,
-                        (taskbar_bottom_work_area.height() -
-                            WindowSizer::kDesktopBorderSize)),
-              window_bounds);
+                        ash::WindowResizer::AlignToGridRoundDown(
+                            taskbar_bottom_work_area.height() -
+                            WindowSizer::kDesktopBorderSize, grid)),
+                        window_bounds);
   }
 
   { // 4:3 monitor case, 1024x768, taskbar on right
@@ -114,8 +116,9 @@ TEST_F(WindowSizerTest, DefaultSizeCase) {
                     gfx::Rect(), gfx::Rect(), DEFAULT, &window_bounds, NULL);
     EXPECT_EQ(gfx::Rect(WindowSizer::kDesktopBorderSize,
                         WindowSizer::kDesktopBorderSize,
-                        taskbar_right_work_area.width() -
-                          WindowSizer::kDesktopBorderSize*2,
+                        ash::WindowResizer::AlignToGridRoundDown(
+                            taskbar_right_work_area.width() -
+                            WindowSizer::kDesktopBorderSize * 2, grid),
                         768 - WindowSizer::kDesktopBorderSize),
               window_bounds);
   }
@@ -127,10 +130,12 @@ TEST_F(WindowSizerTest, DefaultSizeCase) {
     EXPECT_EQ(gfx::Rect(taskbar_left_work_area.x() +
                           WindowSizer::kDesktopBorderSize,
                         WindowSizer::kDesktopBorderSize,
-                        taskbar_left_work_area.width() -
-                          WindowSizer::kDesktopBorderSize * 2,
-                        taskbar_left_work_area.height() -
-                          WindowSizer::kDesktopBorderSize),
+                        ash::WindowResizer::AlignToGridRoundDown(
+                            taskbar_left_work_area.width() -
+                        WindowSizer::kDesktopBorderSize * 2, grid),
+                            ash::WindowResizer::AlignToGridRoundDown(
+                            taskbar_left_work_area.height() -
+                            WindowSizer::kDesktopBorderSize, grid)),
               window_bounds);
   }
 
@@ -142,8 +147,9 @@ TEST_F(WindowSizerTest, DefaultSizeCase) {
                         taskbar_top_work_area.y() +
                           WindowSizer::kDesktopBorderSize,
                         1024 - WindowSizer::kDesktopBorderSize * 2,
-                        taskbar_top_work_area.height() -
-                          WindowSizer::kDesktopBorderSize),
+                        ash::WindowResizer::AlignToGridRoundDown(
+                            taskbar_top_work_area.height() -
+                            WindowSizer::kDesktopBorderSize, grid)),
               window_bounds);
   }
 
@@ -174,7 +180,9 @@ TEST_F(WindowSizerTest, DefaultSizeCase) {
                     gfx::Rect(), DEFAULT, &window_bounds, NULL);
     EXPECT_EQ(gfx::Rect((1680 - 1280) / 2, WindowSizer::kDesktopBorderSize,
                         1280,
-                        1050 - WindowSizer::kDesktopBorderSize),
+                        ash::WindowResizer::AlignToGridRoundDown(
+                            1050 - WindowSizer::kDesktopBorderSize,
+                            grid)),
               window_bounds);
   }