Fix AdjustBoundsToEnsureWindowVisibility to work with non primary display bounds

ash/display/screen_position_controller.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/display/screen_position_controller.h"
 
 #include "ash/display/display_controller.h"
 #include "ash/root_window_controller.h"
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
 #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"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window_tracker.h"
 #include "ui/compositor/dip_util.h"
 #include "ui/gfx/display.h"
 #include "ui/gfx/screen.h"
 
 namespace ash {
@@ skipping 160 matching lines @@
       aura::client::ActivationClient* activation_client =
           aura::client::GetActivationClient(window->GetRootWindow());
       aura::Window* active = activation_client->GetActiveWindow();
 
       aura::WindowTracker tracker;
       if (focused)
         tracker.Add(focused);
       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);

[varkha, 2014/01/02 22:42:42]

I tried this in patch #1 of my CL
(https://codereview.chromium.org/57963003/#ps30001) and I think I had to protect
the popup windows from this early set bounds. Not sure if this was breaking some
tests.

[oshima, 2014/01/08 02:16:11]

What was broken? Drag code overrides the bounds after reparent.

+
       dst_container->AddChild(window);
 
       MoveAllTransientChildrenToNewRoot(display, window);
 
       // Restore focused/active window.
       if (tracker.Contains(focused)) {
         aura::client::GetFocusClient(window)->FocusWindow(focused);
         // TODO(beng): replace with GetRootWindow().
         ash::Shell::GetInstance()->set_target_root_window(
             focused->GetRootWindow());
       } 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();
   origin.Offset(-display_origin.x(), -display_origin.y());
   window->SetBounds(gfx::Rect(origin, bounds.size()));
 }
 
 }  // internal
 }  // ash