Don't allow new windows to be created outside the monitor's work area....

Don't allow new windows to be created outside the monitor's work area.
BUG=281
TEST=Drag a tab down to the bottom of the screen. Check that the new window is created with its border accessible.

[Jamie, 2011-01-20 19:04:41]

Could you please review this patch?

[sky, 2011-01-21 18:55:19]

http://codereview.chromium.org/6380007/diff/4001/chrome/browser/ui/views/tabs...
File chrome/browser/ui/views/tabs/dragged_tab_controller.cc (right):

http://codereview.chromium.org/6380007/diff/4001/chrome/browser/ui/views/tabs...
chrome/browser/ui/views/tabs/dragged_tab_controller.cc:543:
std::set<gfx::NativeView>());
this should be indented by 4.

Using DockInfo to get the monitor bounds is a bit of a hack. Instead add a
method to views/screen, say GetMonitorWorkArea, that you call and DockInfo
calls.

[Jamie, 2011-01-26 19:52:00]

On 2011/01/21 18:55:19, sky wrote:
>
http://codereview.chromium.org/6380007/diff/4001/chrome/browser/ui/views/tabs...
> File chrome/browser/ui/views/tabs/dragged_tab_controller.cc (right):
> 
>
http://codereview.chromium.org/6380007/diff/4001/chrome/browser/ui/views/tabs...
> chrome/browser/ui/views/tabs/dragged_tab_controller.cc:543:
> std::set<gfx::NativeView>());
> this should be indented by 4.
> 
> Using DockInfo to get the monitor bounds is a bit of a hack. Instead add a
> method to views/screen, say GetMonitorWorkArea, that you call and DockInfo
> calls.

On 2011/01/21 18:55:19, sky wrote:
>
http://codereview.chromium.org/6380007/diff/4001/chrome/browser/ui/views/tabs...
> File chrome/browser/ui/views/tabs/dragged_tab_controller.cc (right):
> 
>
http://codereview.chromium.org/6380007/diff/4001/chrome/browser/ui/views/tabs...
> chrome/browser/ui/views/tabs/dragged_tab_controller.cc:543:
> std::set<gfx::NativeView>());
> this should be indented by 4.
> 
> Using DockInfo to get the monitor bounds is a bit of a hack. Instead add a
> method to views/screen, say GetMonitorWorkArea, that you call and DockInfo
> calls.

I've implemented the fix you suggested. There are a couple of caveats:

1. GetMonitorWorkAreaNearestPoint isn't implemented for Linux (it just returns
GetMonitorAreaNearestPoint). I've added a TODO to this effect. I've never seen
the problem on Linux, and it seems that that file has several TODOs already so I
don't think the cost:benefit ratio justifies a fix. Let me know if you disagree.

2. Due to the similarity of GetMonitorWorkAreaNearestPoint and
GetMonitorAreaNearestPoint, they are both implemented by a common helper
function. This means that the semantics of GetMonitorAreaNearestPoint have been
tightened in the case where GetMonitorInfo fails, because the DockToWindowFinder
ctor needs to be able to detect this case.

[sky, 2011-01-27 00:22:07]

http://codereview.chromium.org/6380007/diff/16001/chrome/browser/ui/views/tab...
File chrome/browser/ui/views/tabs/dragged_tab_controller.cc (right):

http://codereview.chromium.org/6380007/diff/16001/chrome/browser/ui/views/tab...
chrome/browser/ui/views/tabs/dragged_tab_controller.cc:544: cursor_point);
You should only do all this if work_area is not empty.

http://codereview.chromium.org/6380007/diff/16001/views/screen_win.cc
File views/screen_win.cc (right):

http://codereview.chromium.org/6380007/diff/16001/views/screen_win.cc#newcode37
views/screen_win.cc:37: bool work_area) {
align this with ( on previous line.

[Jamie, 2011-01-27 02:36:21]

Ready for review.

[sky, 2011-01-27 16:07:33]

LGTM

[Jamie, 2011-01-27 16:53:40]

Could you please commit this for me?

[sky, 2011-01-28 17:40:39]

Landed at 72970.

  -Scott

On Thu, Jan 27, 2011 at 8:53 AM,  <jamiewalch@chromium.org> wrote:
> Could you please commit this for me?
>
> http://codereview.chromium.org/6380007/
>