Fix AdjustBoundsToEnsureWindowVisibility to work with non primary dis…

…play 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

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();

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);

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);
   }
 }

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]));

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));

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) {

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

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(