Initial cut at scaffolding for mac sidetabs. No visible changes to browser wi...

Initial cut at scaffolding for mac sidetabs. No visible changes to browser without --enable-vertical-tabs, and even then, don't expect it to work.
BUG=44773
TEST=infobars, bookmark bar, tab strip, etc should all behave correctly when showing, hiding, and widnow resizing. This cl should have no visible impact without the above flag on the command line.

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

[rohitrao (ping after 24h), 2010-06-02 13:45:55]

The layout code looks fine to me, although we'll have to address fullscreen at
some point.  I think that rather than subclassing the existing controllers, we
may want to introduce protocols or new base classes.

http://codereview.chromium.org/2475002/diff/1/5
File chrome/browser/cocoa/browser_window_controller.h (right):

http://codereview.chromium.org/2475002/diff/1/5#newcode253
chrome/browser/cocoa/browser_window_controller.h:253: -
(void)tabStripDisplayModeChanged;
How about calling this toggleTabStripDisplayMode?  The current name makes it
sound like an observer method called after the toggle rather than a method that
actually does all the work.

http://codereview.chromium.org/2475002/diff/1/6
File chrome/browser/cocoa/browser_window_controller.mm (right):

http://codereview.chromium.org/2475002/diff/1/6#newcode242
chrome/browser/cocoa/browser_window_controller.mm:242: // XXXPINK: load
SideTabController when written and add it to the
Is this a TODO or a DO NOT SUBMIT?

http://codereview.chromium.org/2475002/diff/1/6#newcode1629
chrome/browser/cocoa/browser_window_controller.mm:1629: //XXXPINK re-initialize
tab strip
TODO?

This method is only half written, right?  When finished, it'll need to remove
the old tabstrip and toolbar, create both new controllers, and install the two
new views.  I think.

http://codereview.chromium.org/2475002/diff/1/8
File chrome/browser/cocoa/browser_window_controller_private.mm (right):

http://codereview.chromium.org/2475002/diff/1/8#newcode175
chrome/browser/cocoa/browser_window_controller_private.mm:175: // tabs enabled,
the toolbar takes up the entire top width.
Looking at the mocks you sent me, I'd argue that the toolbar is the same width
as the web, and the sidebar stretches all the way to the top of the screen, but
this is easy enough to fix later.

http://codereview.chromium.org/2475002/diff/1/8#newcode183
chrome/browser/cocoa/browser_window_controller_private.mm:183: // XXXPINK
position the side bar at |minX|.
TODO?

http://codereview.chromium.org/2475002/diff/1/8#newcode196
chrome/browser/cocoa/browser_window_controller_private.mm:196: // The floating
bar backing view doesn't actually add any height.
This is fine for now, but we'll probably end up re-thinking how fullscreen mode
and side tabs interact.  The floating bar tabstrip doesn't really work.

http://codereview.chromium.org/2475002/diff/1/8#newcode420
chrome/browser/cocoa/browser_window_controller_private.mm:420: bool
contentShifted = NSMaxY(tabContentFrame) != NSMaxY(newFrame);
Do you know the difference between "==" and "NSEqualRects"?

http://codereview.chromium.org/2475002/diff/1/9
File chrome/browser/cocoa/side_tabs_toolbar_controller.h (right):

http://codereview.chromium.org/2475002/diff/1/9#newcode15
chrome/browser/cocoa/side_tabs_toolbar_controller.h:15: @interface
SideTabsToolbarController : ToolbarController {
I don't know how much of the ToolbarController code is relevant to the side tabs
 toolbar.  We may want to move to either a common protocol or a simpler base
class shared by the two.

[rohitrao (ping after 24h), 2010-06-02 13:56:40]

LGTM

We can worry about the protocols and base classes later.  Please clean up the
XXXPINK and maybe change the one method name before submitting.

[pink (ping after 24hrs), 2010-06-02 14:22:15]

All fixed. Fleshed out comments where things were obviously half-finished.

On Wed, Jun 2, 2010 at 9:56 AM,  <rohitrao@chromium.org> wrote:
> LGTM
>
> We can worry about the protocols and base classes later.  Please clean up
> the
> XXXPINK and maybe change the one method name before submitting.
>
> http://codereview.chromium.org/2475002/show
>



-- 
Mike Pinkerton
Mac Weenie
pinkerton@google.com