Mac: fix/implement app windows (not app mode), popups, drawing; refactor code.

Mac: fix/implement app windows (not app mode), popups, drawing; refactor code.

1. Properly display app windows, including Developer Tools window (no location bar). Also check using --app=http://foobar.com/.
2. Lay out popup windows (in particular, location bar) better. Check using, e.g., <http://www.quirksmode.org/js/popup.html>; make sure it looks good (with a variety of themes).
3. Properly draw border to Omnibox -- so that its border matches the surrounding buttons. Check (looking very closely/zooming) using various themes (Google and artist, light and dark).
4. Re-organize/refactor code in the BrowserWindowController (esp. the layout code). Check that (in a normal window) it still displays the toolbar, bookmark bar (normal and NTP), infobar, and download shelf correctly.

BUG=13148, 20244, 26757, 29103
TEST=See above.

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

[viettrungluu, 2009-12-16 05:56:44]

There are some more things to do, but this CL is probably big enough as it is.

[pink (ping after 24hrs), 2009-12-16 15:25:35]

LGTM, thanks for the cleanup. Much nicer!

http://codereview.chromium.org/495010/diff/3001/3006
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/495010/diff/3001/3006#newcode1258
chrome/browser/cocoa/browser_window_controller.mm:1258: return (browser_->type()
== Browser::TYPE_NORMAL) ? YES : NO;
technically the ? : isn't needed per the style guide, which notes that bool and
BOOL expressions are interchangeable.
http://google-styleguide.googlecode.com/svn/trunk/objcguide.xml#BOOL_Pitfalls

http://codereview.chromium.org/495010/diff/3001/3015
File chrome/browser/cocoa/toolbar_controller.h (right):

http://codereview.chromium.org/495010/diff/3001/3015#newcode114
chrome/browser/cocoa/toolbar_controller.h:114: // the member variable
|locationBar_|. Horrible.
You could rename it to locationBarBridge? Shouldn't be that hard to fix, no?

[viettrungluu, 2009-12-16 20:51:12]

Thanks.

http://codereview.chromium.org/495010/diff/3001/3006
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/495010/diff/3001/3006#newcode1258
chrome/browser/cocoa/browser_window_controller.mm:1258: return (browser_->type()
== Browser::TYPE_NORMAL) ? YES : NO;
On 2009/12/16 15:25:35, pink wrote:
> technically the ? : isn't needed per the style guide, which notes that bool
and
> BOOL expressions are interchangeable.
> http://google-styleguide.googlecode.com/svn/trunk/objcguide.xml#BOOL_Pitfalls

Oh, thanks, good to know. Changed, along with a few others. One day I'll have to
go through and clean them all up.

http://codereview.chromium.org/495010/diff/3001/3015
File chrome/browser/cocoa/toolbar_controller.h (right):

http://codereview.chromium.org/495010/diff/3001/3015#newcode114
chrome/browser/cocoa/toolbar_controller.h:114: // the member variable
|locationBar_|. Horrible.
On 2009/12/16 15:25:35, pink wrote:
> You could rename it to locationBarBridge? Shouldn't be that hard to fix, no?

But it was so much (well, at least slightly) easier to just write a comment
whining about it. Fixed.