Fix AdjustBoundsToEnsureWindowVisibility to work with non primary display bounds

 This was asssuming that the visibile area has 0,0 origin.

Move the code to ensure minimum visibility when added, from
WorkspaceLayoutManager to DragWindowResizer.

BUG=none
TEST=WindowUtilTest.AdjustBoundsToEnsureWindowVisibility.
 This also fixes the test that was failing.
TBR=oshima@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243986

Review URL: https://codereview.chromium.org/106303005

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

8 files changed, 103 insertions, 28 deletions

diff --git a/ash/display/screen_position_controller.cc b/ash/display/screen_position_controller.cc
index 5b45b7c..0e11a29 100644
--- a/ash/display/screen_position_controller.cc
+++ b/ash/display/screen_position_controller.cc
@@ -11,6 +11,7 @@
 #include "ash/wm/coordinate_conversion.h"
 #include "ash/wm/system_modal_container_layout_manager.h"
 #include "ash/wm/window_properties.h"
+#include "ash/wm/window_state.h"
 #include "ui/aura/client/activation_client.h"
 #include "ui/aura/client/capture_client.h"
 #include "ui/aura/client/focus_client.h"
@@ -193,6 +194,15 @@ void ScreenPositionController::SetBounds(aura::Window* window,
       if (active && focused != active)
         tracker.Add(active);
 
+      // Set new bounds now so that the container's layout manager
+      // can adjust the bounds if necessary.
+      gfx::Point origin = bounds.origin();
+      const gfx::Point display_origin = display.bounds().origin();
+      origin.Offset(-display_origin.x(), -display_origin.y());
+      gfx::Rect new_bounds = gfx::Rect(origin, bounds.size());
+
+      window->SetBounds(new_bounds);
+
       dst_container->AddChild(window);
 
       MoveAllTransientChildrenToNewRoot(display, window);
@@ -206,9 +216,13 @@ void ScreenPositionController::SetBounds(aura::Window* window,
       } else if (tracker.Contains(active)) {
         activation_client->ActivateWindow(active);
       }
+      // TODO(oshima): We should not have to update the bounds again
+      // below in theory, but we currently do need as there is a code
+      // that assumes that the bounds will never be overridden by the
+      // layout mananger. We should have more explicit control how
+      // constraints are applied by the layout manager.
     }
   }
-
   gfx::Point origin(bounds.origin());
   const gfx::Point display_origin = Shell::GetScreen()->GetDisplayNearestWindow(
       window).bounds().origin();
diff --git a/ash/wm/dock/docked_window_layout_manager.cc b/ash/wm/dock/docked_window_layout_manager.cc
index 0a648a0..5d16a24 100644
--- a/ash/wm/dock/docked_window_layout_manager.cc
+++ b/ash/wm/dock/docked_window_layout_manager.cc
@@ -593,6 +593,9 @@ void DockedWindowLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
   if (child == dragged_window_)
     return;
   // If this is the first window getting docked - update alignment.
+  // TODO(oshima|varkha): A window can be added without proper bounds when
+  // window is moved to another display via API or due to display configuration
+  // change, so the the alignment may not be valid.
   if (alignment_ == DOCKED_ALIGNMENT_NONE) {
     alignment_ = GetAlignmentOfWindow(child);
     DCHECK(alignment_ != DOCKED_ALIGNMENT_NONE);
diff --git a/ash/wm/drag_window_resizer.cc b/ash/wm/drag_window_resizer.cc
index 2a859cf..835b204 100644
--- a/ash/wm/drag_window_resizer.cc
+++ b/ash/wm/drag_window_resizer.cc
@@ -13,6 +13,7 @@
 #include "ash/wm/coordinate_conversion.h"
 #include "ash/wm/drag_window_controller.h"
 #include "ash/wm/window_state.h"
+#include "ash/wm/window_util.h"
 #include "base/memory/weak_ptr.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/env.h"
@@ -149,6 +150,9 @@ void DragWindowResizer::CompleteDrag() {
         dst_bounds.set_x(
             last_mouse_location_in_screen.x() - dst_bounds.width());
     }
+    ash::wm::AdjustBoundsToEnsureMinimumWindowVisibility(
+        dst_display.bounds(), &dst_bounds);
+
     GetTarget()->SetBoundsInScreen(dst_bounds, dst_display);
   }
 }
diff --git a/ash/wm/drag_window_resizer_unittest.cc b/ash/wm/drag_window_resizer_unittest.cc
index fb33a98..2b92bed 100644
--- a/ash/wm/drag_window_resizer_unittest.cc
+++ b/ash/wm/drag_window_resizer_unittest.cc
@@ -244,12 +244,9 @@ TEST_F(DragWindowResizerTest, WindowDragWithMultiDisplays) {
     gfx::Rect intersect(window_->GetRootWindow()->GetBoundsInScreen());
     intersect.Intersect(window_bounds_in_screen);
 
-    // TODO(oshima): Following condition fails without docked window resizer.
-    // Proper fix caused the other failures, so I'm disabling these
-    // for m33 (Which is harmless).
-    // EXPECT_LE(10, intersect.width());
-    // EXPECT_LE(10, intersect.height());
-    // EXPECT_TRUE(window_bounds_in_screen.Contains(gfx::Point(800, 10)));
+    EXPECT_LE(10, intersect.width());
+    EXPECT_LE(10, intersect.height());
+    EXPECT_TRUE(window_bounds_in_screen.Contains(gfx::Point(800, 10)));
   }
 
   // Dropping a window that is larger than the destination work area
@@ -516,6 +513,11 @@ TEST_F(DragWindowResizerTest, CursorDeviceScaleFactor) {
   // Move window from the root window with 2.0 device scale factor to the root
   // window with 1.0 device scale factor.
   {
+    // Make sure the window is on the default container first.
+    aura::Window* default_container =
+        GetRootWindowController(root_windows[1])->GetContainer(
+            internal::kShellWindowId_DefaultContainer);
+    default_container->AddChild(window_.get());
     window_->SetBoundsInScreen(
         gfx::Rect(600, 0, 50, 60),
         Shell::GetScreen()->GetDisplayNearestWindow(root_windows[1]));
diff --git a/ash/wm/system_gesture_event_filter_unittest.cc b/ash/wm/system_gesture_event_filter_unittest.cc
index 72f5f90..01accdf 100644
--- a/ash/wm/system_gesture_event_filter_unittest.cc
+++ b/ash/wm/system_gesture_event_filter_unittest.cc
@@ -411,7 +411,7 @@ TEST_P(SystemGestureEventFilterTest, TwoFingerDragTwoWindows) {
   aura::Window* root_window = Shell::GetPrimaryRootWindow();
   ui::GestureConfiguration::set_max_separation_for_gesture_touches_in_pixels(0);
   views::Widget* first = views::Widget::CreateWindowWithContextAndBounds(
-      new ResizableWidgetDelegate, root_window, gfx::Rect(0, 0, 50, 100));
+      new ResizableWidgetDelegate, root_window, gfx::Rect(10, 0, 50, 100));
   first->Show();
   views::Widget* second = views::Widget::CreateWindowWithContextAndBounds(
       new ResizableWidgetDelegate, root_window, gfx::Rect(100, 0, 100, 100));
diff --git a/ash/wm/window_util.cc b/ash/wm/window_util.cc
index 2985f66..cc131d1 100644
--- a/ash/wm/window_util.cc
+++ b/ash/wm/window_util.cc
@@ -94,18 +94,18 @@ void AdjustBoundsToEnsureWindowVisibility(const gfx::Rect& visible_area,
   min_width = std::min(min_width, visible_area.width());
   min_height = std::min(min_height, visible_area.height());
 
-  if (bounds->x() + min_width > visible_area.right()) {
+  if (bounds->right() < visible_area.x() + min_width) {
+    bounds->set_x(visible_area.x() + min_width - bounds->width());
+  } else if (bounds->x() > visible_area.right() - min_width) {
     bounds->set_x(visible_area.right() - min_width);
-  } else if (bounds->right() - min_width < 0) {
-    bounds->set_x(min_width - bounds->width());
   }
-  if (bounds->y() + min_height > visible_area.bottom()) {
+  if (bounds->bottom() < visible_area.y() + min_height) {
+    bounds->set_y(visible_area.y() + min_height - bounds->height());
+  } else if (bounds->y() > visible_area.bottom() - min_height) {
     bounds->set_y(visible_area.bottom() - min_height);
-  } else if (bounds->bottom() - min_height < 0) {
-    bounds->set_y(min_height - bounds->height());
   }
-  if (bounds->y() < 0)
-    bounds->set_y(0);
+  if (bounds->y() < visible_area.y())
+    bounds->set_y(visible_area.y());
 }
 
 bool MoveWindowToEventRoot(aura::Window* window, const ui::Event& event) {
diff --git a/ash/wm/window_util_unittest.cc b/ash/wm/window_util_unittest.cc
index 8073a38..66618df 100644
--- a/ash/wm/window_util_unittest.cc
+++ b/ash/wm/window_util_unittest.cc
@@ -10,6 +10,16 @@
 
 namespace ash {
 
+namespace {
+
+std::string GetAdjustedBounds(const gfx::Rect& visible,
+                              gfx::Rect to_be_adjusted) {
+  wm::AdjustBoundsToEnsureMinimumWindowVisibility(visible, &to_be_adjusted);
+  return to_be_adjusted.ToString();
+}
+
+}
+
 typedef test::AshTestBase WindowUtilTest;
 
 TEST_F(WindowUtilTest, CenterWindow) {
@@ -29,4 +39,54 @@ TEST_F(WindowUtilTest, CenterWindow) {
   EXPECT_EQ("750,126 100x100", window->GetBoundsInScreen().ToString());
 }
 
+TEST_F(WindowUtilTest, AdjustBoundsToEnsureMinimumVisibility) {
+  const gfx::Rect visible_bounds(0, 0, 100, 100);
+
+  EXPECT_EQ("0,0 90x90",
+            GetAdjustedBounds(visible_bounds, gfx::Rect(0, 0, 90, 90)));
+  EXPECT_EQ("0,0 100x100",
+            GetAdjustedBounds(visible_bounds, gfx::Rect(0, 0, 150, 150)));
+  EXPECT_EQ("-50,0 100x100",
+            GetAdjustedBounds(visible_bounds, gfx::Rect(-50, -50, 150, 150)));
+  EXPECT_EQ("-90,10 100x100",
+            GetAdjustedBounds(visible_bounds, gfx::Rect(-100, 10, 150, 150)));
+  EXPECT_EQ("90,90 100x100",
+            GetAdjustedBounds(visible_bounds, gfx::Rect(100, 100, 150, 150)));
+
+  const gfx::Rect visible_bounds_right(200, 50, 100, 100);
+
+  EXPECT_EQ(
+      "210,60 90x90",
+      GetAdjustedBounds(visible_bounds_right, gfx::Rect(210, 60, 90, 90)));
+  EXPECT_EQ(
+      "210,60 100x100",
+      GetAdjustedBounds(visible_bounds_right, gfx::Rect(210, 60, 150, 150)));
+  EXPECT_EQ(
+      "110,50 100x100",
+      GetAdjustedBounds(visible_bounds_right, gfx::Rect(0, 0, 150, 150)));
+  EXPECT_EQ(
+      "290,50 100x100",
+      GetAdjustedBounds(visible_bounds_right, gfx::Rect(300, 20, 150, 150)));
+  EXPECT_EQ(
+      "110,140 100x100",
+      GetAdjustedBounds(visible_bounds_right, gfx::Rect(-100, 150, 150, 150)));
+
+  const gfx::Rect visible_bounds_left(-200, -50, 100, 100);
+  EXPECT_EQ(
+      "-190,-40 90x90",
+      GetAdjustedBounds(visible_bounds_left, gfx::Rect(-190, -40, 90, 90)));
+  EXPECT_EQ(
+      "-190,-40 100x100",
+      GetAdjustedBounds(visible_bounds_left, gfx::Rect(-190, -40, 150, 150)));
+  EXPECT_EQ(
+      "-250,-40 100x100",
+      GetAdjustedBounds(visible_bounds_left, gfx::Rect(-250, -40, 150, 150)));
+  EXPECT_EQ(
+      "-290,-50 100x100",
+      GetAdjustedBounds(visible_bounds_left, gfx::Rect(-400, -60, 150, 150)));
+  EXPECT_EQ(
+      "-110,0 100x100",
+      GetAdjustedBounds(visible_bounds_left, gfx::Rect(0, 0, 150, 150)));
+}
+
 }  // namespace ash
diff --git a/ash/wm/workspace/workspace_layout_manager.cc b/ash/wm/workspace/workspace_layout_manager.cc
index 672cded..2e893ae 100644
--- a/ash/wm/workspace/workspace_layout_manager.cc
+++ b/ash/wm/workspace/workspace_layout_manager.cc
@@ -254,8 +254,11 @@ void WorkspaceLayoutManager::AdjustWindowBoundsWhenAdded(
   // When a window is dragged and dropped onto a different
   // root window, the bounds will be updated after they are added
   // to the root window.
-  if (window_state->window()->bounds().IsEmpty())
+  if (window_state->window()->bounds().IsEmpty() ||
+      window_state->is_dragged() ||
+      SetMaximizedOrFullscreenBounds(window_state)) {
     return;
+  }
 
   Window* window = window_state->window();
   gfx::Rect bounds = window->bounds();
@@ -266,17 +269,6 @@ void WorkspaceLayoutManager::AdjustWindowBoundsWhenAdded(
   // moved.
   gfx::Rect display_area = ScreenAsh::GetDisplayBoundsInParent(window);
 
-  if (window_state->is_dragged()) {
-    ash::wm::AdjustBoundsToEnsureMinimumWindowVisibility(
-        display_area, &bounds);
-    if (window->bounds() != bounds)
-      window->SetBounds(bounds);
-    return;
-  }
-
-  if (SetMaximizedOrFullscreenBounds(window_state))
-    return;
-
   int min_width = bounds.width() * kMinimumPercentOnScreenArea;
   int min_height = bounds.height() * kMinimumPercentOnScreenArea;
   ash::wm::AdjustBoundsToEnsureWindowVisibility(