Revert 243986 "Fix AdjustBoundsToEnsureWindowVisibility to work ..."

> 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
> 
> Review URL: https://codereview.chromium.org/106303005

TBR=oshima@chromium.org

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

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

8 files changed, 28 insertions, 105 deletions

diff --git a/ash/display/screen_position_controller.cc b/ash/display/screen_position_controller.cc
index 9f17f68..5b45b7c 100644
--- a/ash/display/screen_position_controller.cc
+++ b/ash/display/screen_position_controller.cc
@@ -11,7 +11,6 @@
 #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"
@@ -194,17 +193,6 @@ void ScreenPositionController::SetBounds(aura::Window* window,
       if (active && focused != active)
         tracker.Add(active);
 
-      window->parent()->RemoveChild(window);
-
-      // 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);
@@ -218,13 +206,9 @@ 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 5d16a24..0a648a0 100644
--- a/ash/wm/dock/docked_window_layout_manager.cc
+++ b/ash/wm/dock/docked_window_layout_manager.cc
@@ -593,9 +593,6 @@ 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 835b204..2a859cf 100644
--- a/ash/wm/drag_window_resizer.cc
+++ b/ash/wm/drag_window_resizer.cc
@@ -13,7 +13,6 @@
 #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"
@@ -150,9 +149,6 @@ 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 2b92bed..fb33a98 100644
--- a/ash/wm/drag_window_resizer_unittest.cc
+++ b/ash/wm/drag_window_resizer_unittest.cc
@@ -244,9 +244,12 @@ TEST_F(DragWindowResizerTest, WindowDragWithMultiDisplays) {
     gfx::Rect intersect(window_->GetRootWindow()->GetBoundsInScreen());
     intersect.Intersect(window_bounds_in_screen);
 
-    EXPECT_LE(10, intersect.width());
-    EXPECT_LE(10, intersect.height());
-    EXPECT_TRUE(window_bounds_in_screen.Contains(gfx::Point(800, 10)));
+    // 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)));
   }
 
   // Dropping a window that is larger than the destination work area
@@ -513,11 +516,6 @@ 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 01accdf..72f5f90 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(10, 0, 50, 100));
+      new ResizableWidgetDelegate, root_window, gfx::Rect(0, 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 cc131d1..2985f66 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->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) {
+  if (bounds->x() + min_width > visible_area.right()) {
     bounds->set_x(visible_area.right() - min_width);
+  } else if (bounds->right() - min_width < 0) {
+    bounds->set_x(min_width - bounds->width());
   }
-  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) {
+  if (bounds->y() + min_height > visible_area.bottom()) {
     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() < visible_area.y())
-    bounds->set_y(visible_area.y());
+  if (bounds->y() < 0)
+    bounds->set_y(0);
 }
 
 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 66618df..8073a38 100644
--- a/ash/wm/window_util_unittest.cc
+++ b/ash/wm/window_util_unittest.cc
@@ -10,16 +10,6 @@
 
 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) {
@@ -39,54 +29,4 @@ 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 2e893ae..672cded 100644
--- a/ash/wm/workspace/workspace_layout_manager.cc
+++ b/ash/wm/workspace/workspace_layout_manager.cc
@@ -254,11 +254,8 @@ 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() ||
-      window_state->is_dragged() ||
-      SetMaximizedOrFullscreenBounds(window_state)) {
+  if (window_state->window()->bounds().IsEmpty())
     return;
-  }
 
   Window* window = window_state->window();
   gfx::Rect bounds = window->bounds();
@@ -269,6 +266,17 @@ 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(