Make sure the dragged window is smaller than work area in the target display

Make sure the dragged window is smaller than work area in the target display.

- makes sure that minimum amount of window is visible when dropped.

- When moving a window to another due to display disconnect, try to keep relative position in the work are, rather than absolute position to the root window.

BUG=321702
TEST=covered by test
RootWindowController::MoveWindows_Basic
DragWindowResizerTest::WindowDragWithMultiDisplays

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

R=mukai@chromium.org, varkha@chromium.org

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

[varkha, 2013-12-12 16:20:45]

Looks good, other than I need to convince myself that vertical arrangement of
displays is also handled well.

https://codereview.chromium.org/93873013/diff/150001/ash/root_window_controll...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/root_window_controll...
ash/root_window_controller.cc:105: void AdjustBoundsRelativeToSize(const
gfx::Size& src_size,
nit: Maybe ShiftOriginRelativeToParentSize or MoveOriginRelativeToParentSize to
reflect that the bounds_in_out.size() is not actually changing and that the
sizes are of old and new parents?

https://codereview.chromium.org/93873013/diff/150001/ash/root_window_controll...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/root_window_controll...
ash/root_window_controller_unittest.cc:155: TEST_F(RootWindowControllerTest,
MoveWindows_Basic) {
nit: I think you could quote this test in the CL TEST tag since it has changed.

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
File ash/wm/drag_window_resizer.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
ash/wm/drag_window_resizer.cc:138: bounds.set_height(size.height());
Is this enough for a vertical arrangement where the secondary screen is above or
below the primary?

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
ash/wm/drag_window_resizer.cc:149: last_mouse_location_in_screen.x() -
dst_bounds.width());
Same here - does it consider vertical screen arrangement?

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
File ash/wm/drag_window_resizer_unittest.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
ash/wm/drag_window_resizer_unittest.cc:159: TEST_F(DragWindowResizerTest,
WindowDragWithMultiDisplays) {
nit: This test too could be quoted in the CL TEST tag.

[oshima, 2013-12-12 19:07:14]

https://codereview.chromium.org/93873013/diff/150001/ash/root_window_controll...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/root_window_controll...
ash/root_window_controller.cc:105: void AdjustBoundsRelativeToSize(const
gfx::Size& src_size,
On 2013/12/12 16:20:46, varkha wrote:
> nit: Maybe ShiftOriginRelativeToParentSize or MoveOriginRelativeToParentSize
to
> reflect that the bounds_in_out.size() is not actually changing and that the
> sizes are of old and new parents?

Changed to MoveOriginRelativeToSize, as there is no "parent" in this function.

https://codereview.chromium.org/93873013/diff/150001/ash/root_window_controll...
File ash/root_window_controller_unittest.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/root_window_controll...
ash/root_window_controller_unittest.cc:155: TEST_F(RootWindowControllerTest,
MoveWindows_Basic) {
On 2013/12/12 16:20:46, varkha wrote:
> nit: I think you could quote this test in the CL TEST tag since it has
changed.

Done.

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
File ash/wm/drag_window_resizer.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
ash/wm/drag_window_resizer.cc:138: bounds.set_height(size.height());
On 2013/12/12 16:20:46, varkha wrote:
> Is this enough for a vertical arrangement where the secondary screen is above
or
> below the primary?

I don't think the screen layout does matter. I had to adjust a horizontal
position as you can drag using the left/right edge. Can we drag a window using
bottom edge?

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
File ash/wm/drag_window_resizer_unittest.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
ash/wm/drag_window_resizer_unittest.cc:159: TEST_F(DragWindowResizerTest,
WindowDragWithMultiDisplays) {
On 2013/12/12 16:20:46, varkha wrote:
> nit: This test too could be quoted in the CL TEST tag.

Done.

[varkha, 2013-12-12 22:10:47]

LGTM

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
File ash/wm/drag_window_resizer.cc (right):

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
ash/wm/drag_window_resizer.cc:138: bounds.set_height(size.height());
I understand - we are probably never shrinking the window vertically enough to
get the title bar to be not under the cursor.

https://codereview.chromium.org/93873013/diff/150001/ash/wm/drag_window_resiz...
ash/wm/drag_window_resizer.cc:149: last_mouse_location_in_screen.x() -
dst_bounds.width());
sg. Thanks for the explanation.

[commit-bot: I haz the power, 2013-12-12 22:13:56]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a lowly provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[oshima, 2013-12-12 22:31:14]

mukai-san, would you mind taking a look?

[Jun Mukai, 2013-12-12 22:58:35]

lgtm

[commit-bot: I haz the power, 2013-12-12 23:08:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/93873013/190001

[commit-bot: I haz the power, 2013-12-13 01:02:10]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-12-13 01:05:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/93873013/190001

[commit-bot: I haz the power, 2013-12-13 08:21:30]

Change committed as 240542

[commit-bot: I haz the power, 2013-12-16 19:32:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/93873013/210001

[commit-bot: I haz the power, 2013-12-16 21:05:58]

Retried try job too often on linux_chromeos for step(s) ash_unittests,
browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-12-16 22:43:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/93873013/230001

[commit-bot: I haz the power, 2013-12-17 01:11:58]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-12-17 01:29:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/93873013/230001

[oshima, 2013-12-17 02:22:28]

Committed patchset #5 manually as r241114 (presubmit successful).