Mac: Fix (window modal) sheet placement when bookmark bar is present.

Mac: Fix (window modal) sheet placement when bookmark bar is present.

BUG=25754
TEST=Check placement of (window modal) sheets (e.g., Open File...) with various combinations of the following: NTP/non-NTP, bookmark bar active/not active, infobar present/not present, and also download bar present/not present.

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

[viettrungluu, 2009-10-25 07:41:05]

(This CL hangs sheets below the normal bookmark bar if it's present -- but not
the special NTP bookmark bar -- and also below the infobar if it's present. The
latter is not obviously right, but it's the way things currently work.)

[John Grabowski, 2009-10-26 04:13:37]

LGTM

Also test fullscreen

http://codereview.chromium.org/337019/diff/1001/1002
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/337019/diff/1001/1002#newcode1233
Line 1233: // If the bookmark bar is visible and isn't the NTP "fake" bookmark
bar, which
Please expand on comments in the source so it's quite clear what happens in all
cases.  E.g.
- if bookmark bar shown for a normal window, sheet opens below it.
- if NTP, do nothing here because gthe sheet will be ...

[viettrungluu, 2009-10-26 06:07:55]

Thanks for the review. Sheet placement in full screen mode remains broken
(though no more broken than before); I'll fix that in a separate patch tomorrow.

http://codereview.chromium.org/337019/diff/1001/1002
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/337019/diff/1001/1002#newcode1233
Line 1233: // If the bookmark bar is visible and isn't the NTP "fake" bookmark
bar, which
On 2009/10/26 04:13:38, John Grabowski wrote:
> Please expand on comments in the source so it's quite clear what happens in
all
> cases.  E.g.
> - if bookmark bar shown for a normal window, sheet opens below it.
> - if NTP, do nothing here because gthe sheet will be ...
> 
> 

Done.

[Scott Hess - ex-Googler, 2009-10-26 17:04:07]

LGTM.

[I mean, if jrg is willing to let it slide without a unit test, who am I to
complain?]

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

http://codereview.chromium.org/337019/diff/4001/5001#newcode1244
Line 1244: defaultSheetRect.origin.y -= bookmarkBarFrame.size.height;
What is wrong with bookmarkBarFrame.origin.y, here?  Would seem more robust.

Otherwise, document your assumption that the views are adjacent:
  DCHECK_EQ(NSMaxY(bookmarkBarFrame), NSMinY(toolbarFrame));