[Mac] Implement WindowSizer::GetDefaultPopupOrigin()...

[Mac] Implement WindowSizer::GetDefaultPopupOrigin()

BUG=http://crbug.com/30239
TEST=Popups should not appear under the Dock when the Dock is on the left.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=35548

[viettrungluu, 2009-12-23 18:15:20]

http://codereview.chromium.org/510001/diff/1/2
File chrome/browser/window_sizer_mac.mm (right):

http://codereview.chromium.org/510001/diff/1/2#newcode145
chrome/browser/window_sizer_mac.mm:145: flipped.set_y(NSHeight(main_area) -
corner.y);
Why don't you just put the |NSHeight(...) - corner.y| in the above statement?

[viettrungluu, 2009-12-23 18:23:45]

http://codereview.chromium.org/510001/diff/1/2
File chrome/browser/window_sizer_mac.mm (right):

http://codereview.chromium.org/510001/diff/1/2#newcode125
chrome/browser/window_sizer_mac.mm:125: NSPoint corner =
NSMakePoint(NSMinX(work_area), NSMaxY(work_area));
So if no browser window is open, we'll place the popup at the upper-left corner?
That seems suboptimal, though I'm not entirely sure what the correct behaviour
is.

[viettrungluu, 2010-01-05 05:35:52]

Ping. (Is this CL still alive?)

[rohitrao (ping after 24h), 2010-01-05 16:17:29]

Still alive.  I had the code changes on one computer and the CL description on
another, so I had to get them both online at the same time.

We talked in IRC about not offsetting the popup vertically, but it looks to me
like Safari does that, so I've kept it in.

http://codereview.chromium.org/510001/diff/1/2
File chrome/browser/window_sizer_mac.mm (right):

http://codereview.chromium.org/510001/diff/1/2#newcode125
chrome/browser/window_sizer_mac.mm:125: NSPoint corner =
NSMakePoint(NSMinX(work_area), NSMaxY(work_area));
On 2009/12/23 18:23:46, viettrungluu wrote:
> So if no browser window is open, we'll place the popup at the upper-left
corner?
> That seems suboptimal, though I'm not entirely sure what the correct behaviour
> is.

How can a popup open if there is no current window?  Doesn't that mean there's
no web at all to open the popup in the first place?

If I'm mistaken and this is in fact possible, let's try it in Safari and see
what happens :)

http://codereview.chromium.org/510001/diff/1/2#newcode145
chrome/browser/window_sizer_mac.mm:145: flipped.set_y(NSHeight(main_area) -
corner.y);
On 2009/12/23 18:15:20, viettrungluu wrote:
> Why don't you just put the |NSHeight(...) - corner.y| in the above statement?

Condensed.

[viettrungluu, 2010-01-05 16:48:47]

Clearly, you just need to carry all your computers with you, all the time. :-)

I stand corrected on the vertical offset thing. (I tried Safari, but obviously I
didn't try very hard.)

http://codereview.chromium.org/510001/diff/1/2
File chrome/browser/window_sizer_mac.mm (right):

http://codereview.chromium.org/510001/diff/1/2#newcode125
chrome/browser/window_sizer_mac.mm:125: NSPoint corner =
NSMakePoint(NSMinX(work_area), NSMaxY(work_area));
On 2010/01/05 16:17:29, rohitrao wrote:
> On 2009/12/23 18:23:46, viettrungluu wrote:
> > So if no browser window is open, we'll place the popup at the upper-left
> corner?
> > That seems suboptimal, though I'm not entirely sure what the correct
behaviour
> > is.
> 
> How can a popup open if there is no current window?  Doesn't that mean there's
> no web at all to open the popup in the first place?
> 
> If I'm mistaken and this is in fact possible, let's try it in Safari and see
> what happens :)

What happens if a window pops up (via Javascript) from a minimized window? (Can
that happen? I really have no idea....) I guess there'd still be a browser
window for it to get.

I'm not hugely concerned about this, since even if possible it's unlikely (watch
someone prove me wrong).

http://codereview.chromium.org/510001/diff/5001/5003
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/510001/diff/5001/5003#newcode258
chrome/browser/cocoa/browser_window_controller.mm:258: // none were given by the
window.open() command.
Sigh. Is there really no better way to do this?

http://codereview.chromium.org/510001/diff/5001/5002
File chrome/browser/window_sizer_mac.mm (right):

http://codereview.chromium.org/510001/diff/5001/5002#newcode7
chrome/browser/window_sizer_mac.mm:7: #include "base/logging.h"
While I'm of the opinion that base/logging.h should always be included (because
otherwise you have to add it when putting in some debugging LOG()s or
DCHECK()s), others presumably differ, and you don't appear to use anything which
needs this. (I do this all the time too.)

[rohitrao (ping after 24h), 2010-01-05 19:08:10]

http://codereview.chromium.org/510001/diff/1/2
File chrome/browser/window_sizer_mac.mm (right):

http://codereview.chromium.org/510001/diff/1/2#newcode125
chrome/browser/window_sizer_mac.mm:125: NSPoint corner =
NSMakePoint(NSMinX(work_area), NSMaxY(work_area));
On 2010/01/05 16:48:47, viettrungluu wrote:
> What happens if a window pops up (via Javascript) from a minimized window?
(Can
> that happen? I really have no idea....) I guess there'd still be a browser
> window for it to get.

I think in that case there will be a "last active browser" to offset from.  I'm
not overly concerned with this edge case either.

http://codereview.chromium.org/510001/diff/5001/5003
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/510001/diff/5001/5003#newcode258
chrome/browser/cocoa/browser_window_controller.mm:258: // none were given by the
window.open() command.
On 2010/01/05 16:48:48, viettrungluu wrote:
> Sigh. Is there really no better way to do this?

This is what the other platforms do, so I'm following it for now.

http://codereview.chromium.org/510001/diff/5001/5002
File chrome/browser/window_sizer_mac.mm (right):

http://codereview.chromium.org/510001/diff/5001/5002#newcode7
chrome/browser/window_sizer_mac.mm:7: #include "base/logging.h"
On 2010/01/05 16:48:48, viettrungluu wrote:
> While I'm of the opinion that base/logging.h should always be included
(because
> otherwise you have to add it when putting in some debugging LOG()s or
> DCHECK()s), others presumably differ, and you don't appear to use anything
which
> needs this. (I do this all the time too.)

I'm sure I had some LOGs in here while debugging, which is why I added this in
the first place.  It's gone now, for now.

[viettrungluu, 2010-01-05 19:10:16]

LG.

http://codereview.chromium.org/510001/diff/5001/5003
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/510001/diff/5001/5003#newcode258
chrome/browser/cocoa/browser_window_controller.mm:258: // none were given by the
window.open() command.
On 2010/01/05 19:08:10, rohitrao wrote:
> On 2010/01/05 16:48:48, viettrungluu wrote:
> > Sigh. Is there really no better way to do this?
> 
> This is what the other platforms do, so I'm following it for now.

Okay, good enough for me.

http://codereview.chromium.org/510001/diff/5001/5002
File chrome/browser/window_sizer_mac.mm (right):

http://codereview.chromium.org/510001/diff/5001/5002#newcode7
chrome/browser/window_sizer_mac.mm:7: #include "base/logging.h"
On 2010/01/05 19:08:10, rohitrao wrote:
> On 2010/01/05 16:48:48, viettrungluu wrote:
> > While I'm of the opinion that base/logging.h should always be included
> (because
> > otherwise you have to add it when putting in some debugging LOG()s or
> > DCHECK()s), others presumably differ, and you don't appear to use anything
> which
> > needs this. (I do this all the time too.)
> 
> I'm sure I had some LOGs in here while debugging, which is why I added this in
> the first place.  It's gone now, for now.

That's what I figured. :-)