- Fix janklist issue #1: "there is a pixel line below the main

  toolbar. The main toolbar should blend in with the bookmark bar when
  it's open"
- Fix janklist issue #2: "It's way too tall - the distance from the
  bottom of the bar to a bookmark button bottom edge should be the
  same as the distance from the omnibox to the bookmark button top
  edge (this will probably mean that the bar has to overlap the
  toolbar)."
- Fix janklist issue #4 (first part): "the bookmark bar bookmark buttons
  have a frame around them ... "
- Fix janklist issue #9: "the show/hide animation is very janky... I see
  a dark gray area behind".  Even with animators the grey is gone, but
  animators are disabled for now due to races.
- Fix unlisted jank related to 9: don't use animator when opening bar on
  launch.
- Also chipped away on unit tests.

TEST=Launch with bookmark bar both open and closed.  Make sure OK on launch.
In each case open and close a few times fast.
Repeat with multiple windows open.
Sanity check jank descriptions listed above are fixed.

BUG=crbug.com/14139, crbug.com/8381, crbug.com/14724

Review URL: http://codereview.chromium.org/149308

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@20244 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 192 insertions, 108 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm b/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm
index 2853cca..687c64a 100644
--- a/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm
@@ -26,17 +26,16 @@ typedef std::pair<GURL,WindowOpenDisposition> OpenInfo;
   BookmarkBarController<BookmarkURLOpener> {
  @public
   scoped_nsobject<NSMutableArray> callbacks_;
-
   std::vector<OpenInfo> opens_;
 }
 @end
 
 @implementation FakeBookmarkBarController
 
-- (id)initWithProfile:(Profile*)profile
-          contentArea:(NSView*)content {
+- (id)initWithProfile:(Profile*)profile view:(NSView*)view {
   if ((self = [super initWithProfile:profile
-                         contentView:content
+                                view:(BookmarkBarView*)view
+                      webContentView:nil
                             delegate:self])) {
     callbacks_.reset([[NSMutableArray alloc] init]);
   }
@@ -103,9 +102,12 @@ TEST_F(BookmarkBarBridgeTest, TestRedirect) {
   Profile *profile = browser_test_helper_.profile();
   BookmarkModel *model = profile->GetBookmarkModel();
 
+  scoped_nsobject<NSView> view([[NSView alloc]
+                                 initWithFrame:NSMakeRect(0,0,10,10)]);
+  [view.get() setHidden:YES];
   scoped_nsobject<FakeBookmarkBarController>
     controller([[FakeBookmarkBarController alloc] initWithProfile:profile
-                                                      contentArea:view_]);
+                                                             view:view.get()]);
   EXPECT_TRUE(controller.get());
   scoped_ptr<BookmarkBarBridge> bridge(new BookmarkBarBridge(controller.get(),
                                                              model));
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index d7a93d4..1993411 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -45,12 +45,10 @@ class PrefService;
   BOOL barIsEnabled_;
 
   // The view of the bookmark bar itself.
-  // Not made into a scoped_nsobject since I may move it into a nib.
-  // (See TODO in initWithProfile: in bookmark_bar_controller.mm).
-  IBOutlet BookmarkBarView* bookmarkBarView_;
+  // Owned by the toolbar view, its parent view.
+  BookmarkBarView* bookmarkBarView_;  // weak
 
-  // The tab content area for the window (where the web goes)
-  IBOutlet NSView* contentView_;
+  NSView* webContentView_;  // weak; where the web goes
 
   // Bridge from Chrome-style C++ notifications (e.g. derived from
   // BookmarkModelObserver)
@@ -61,16 +59,14 @@ class PrefService;
 }
 
 // Initializes the controller with the given browser profile and
-// content view.  We use |content| as a parent view for the bookmark
+// content view.  We use |webContentView| as the view for the bookmark
 // bar view and for geometry management.  |delegate| is used for
-// opening URLs.
+// opening URLs.  |view| is expected to be hidden.
 - (id)initWithProfile:(Profile*)profile
-          contentView:(NSView*)content
+                 view:(BookmarkBarView*)view
+       webContentView:(NSView*)webContentView
              delegate:(id<BookmarkURLOpener>)delegate;
 
-// Resizes the bookmark bar based on the state of the content area.
-- (void)resizeBookmarkBar;
-
 // Returns whether or not the bookmark bar is visible.
 - (BOOL)isBookmarkBarVisible;
 
@@ -103,10 +99,14 @@ class PrefService;
 @end
 
 
-// These APIs should only be used by unit tests, in place of "friend" classes.
+// These APIs should only be used by unit tests (or used internally).
 @interface BookmarkBarController(TestingAPI)
 // Access to the bookmark bar's view represented by this controller.
 - (NSView*)view;
+// Set the delegate for a unit test.
+- (void)setDelegate:(id<BookmarkURLOpener>)delegate;
+// Action for our bookmark buttons.
+- (void)openBookmark:(id)sender;
 @end
 
 #endif  // CHROME_BROWSER_COCOA_BOOKMARK_BAR_CONTROLLER_H_
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index d2805ec..09c988f 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -17,12 +17,18 @@
 
 @interface BookmarkBarController(Private)
 - (void)applyContentAreaOffset:(BOOL)apply immediately:(BOOL)immediately;
-- (void)positionBar;
 - (void)showBookmarkBar:(BOOL)enable immediately:(BOOL)immediately;
 @end
 
 namespace {
+
+// Our height, when opened.
 const int kBookmarkBarHeight = 30;
+// How much to adjust our parent view.
+const int kBookmarkBarSuperviewHeightAdjustment = 25;
+// How much to adjust the web frame.
+const int kBookmarkBarWebframeHeightAdjustment = 25;
+
 // Magic numbers from Cole
 const CGFloat kDefaultBookmarkWidth = 150.0;
 const CGFloat kBookmarkVerticalPadding = 2.0;
@@ -32,23 +38,28 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
 @implementation BookmarkBarController
 
 - (id)initWithProfile:(Profile*)profile
-          contentView:(NSView*)content
+                 view:(BookmarkBarView*)view
+       webContentView:(NSView*)webContentView
              delegate:(id<BookmarkURLOpener>)delegate {
   if ((self = [super init])) {
     bookmarkModel_ = profile->GetBookmarkModel();
-    contentView_ = content;
+    bookmarkBarView_ = view;
+    // We default to NOT open, which means height=0.
+    DCHECK([view isHidden]);  // OK to change
+    NSRect frame = [view frame];
+    frame.size.height = 0;
+    [view setFrame:frame];
+
+    // Make sure the nodes stay bottom-aligned.
+    [bookmarkBarView_ setAutoresizingMask:(NSViewWidthSizable |
+                                           NSViewMinYMargin)];
+    webContentView_ = webContentView;
     delegate_ = delegate;
     // Be sure to enable the bar before trying to show it...
     barIsEnabled_ = YES;
-    // Create and initialize the view's position. Show it if appropriate.
-    // TODO(jrg): put it in a nib if necessary.
-    NSRect frame = NSMakeRect(0, 0, 0, 30);
-    bookmarkBarView_ = [[BookmarkBarView alloc] initWithFrame:frame];
-    [self positionBar];
     preferences_ = profile->GetPrefs();
     if (preferences_->GetBoolean(prefs::kShowBookmarkBar))
-      [self showBookmarkBar:YES immediately:NO];
-    [[[contentView_ window] contentView] addSubview:bookmarkBarView_];
+      [self showBookmarkBar:YES immediately:YES];
   }
   // Don't pass ourself along until our init is done.
   // Thus, this call is (almost) last.
@@ -56,39 +67,6 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
   return self;
 }
 
-- (void)dealloc {
-  [bookmarkBarView_ release];
-  [super dealloc];
-}
-
-// Initializes the bookmark bar at the top edge of |contentArea_| and the
-// view's visibility to match the pref. This doesn't move the content view at
-// all, you need to call |-showBookmarkBar:| to do that.
-- (void)positionBar {
-  // Hide or show bar based on initial visibility and set the resize flags.
-  [bookmarkBarView_ setAutoresizingMask:NSViewWidthSizable | NSViewMinYMargin];
-  [bookmarkBarView_ setHidden:!barShouldBeShown_];
-
-  // Set the bar's height to zero and position it at the top of the
-  // content area, within the window's content view (as opposed to the
-  // tab strip, which is a sibling). We'll enlarge it and slide the
-  // content area down when we need to show this strip.
-  NSRect contentFrame = [contentView_ frame];
-  NSRect barFrame = NSMakeRect(0, NSMaxY(contentFrame),
-                               contentFrame.size.width, 0);
-  [bookmarkBarView_ setFrame:barFrame];
-}
-
-// Called when the contentView's frame changes.  Enlarge the view to
-// stay with the top of the contentView.
-- (void)resizeBookmarkBar {
-  NSRect barFrame = [bookmarkBarView_ frame];
-  const int maxY = NSMaxY(barFrame);
-  barFrame.origin.y = NSMaxY([contentView_ frame]);
-  barFrame.size.height = maxY - barFrame.origin.y;
-  [bookmarkBarView_ setFrame:barFrame];
-}
-
 // Show or hide the bar based on the value of |show|. Handles
 // animating the resize of the content view.  if |immediately| is YES,
 // make changes immediately instead of using an animator.  If the bar
@@ -96,14 +74,14 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
 // bar will show it if relevant using other mechanisms (the pref) to
 // determine desired state.
 - (void)showBookmarkBar:(BOOL)show immediately:(BOOL)immediately {
-  if (barIsEnabled_) {
+  if (barIsEnabled_ && (barShouldBeShown_ != show)) {
     contentViewHasOffset_ = show;
     [bookmarkBarView_ setHidden:show ? NO : YES];
-    [self applyContentAreaOffset:show immediately:immediately];
     barShouldBeShown_ = show;
     if (show) {
       [self loaded:bookmarkModel_];
     }
+    [self applyContentAreaOffset:show immediately:immediately];
   }
 }
 
@@ -111,6 +89,13 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
 // bookmark bar.  If apply==YES, always make room (the contentView_ is
 // "full size").  If apply==NO we are trying to undo an offset.  If no
 // offset there is nothing to undo.
+//
+// TODO(jrg): it is awkward we change the sizes of views for our
+// parent and siblings; ideally they change their own sizes.
+//
+// TODO(jrg): unlike windows, we process events while an animator is
+// running.  Thus, if you resize the window while the bookmark bar is
+// animating, you'll mess things up.  Fix.
 - (void)applyContentAreaOffset:(BOOL)apply immediately:(BOOL)immediately {
   if (bookmarkBarView_ == nil) {
     // We're too early, but awakeFromNib will call me again.
@@ -121,21 +106,40 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
     return;
   }
 
-  NSRect frame = [contentView_ frame];
-  if (apply)
-    frame.size.height -= kBookmarkBarHeight;
-  else
+  // None of these locals are members of the Hall of Justice.
+  NSView* superview = [bookmarkBarView_ superview];
+  NSRect superframe = [superview frame];
+  NSRect frame = [bookmarkBarView_ frame];
+  NSRect webframe = [webContentView_ frame];
+  if (apply) {
+    superframe.size.height += kBookmarkBarSuperviewHeightAdjustment;
+    superframe.origin.y -= kBookmarkBarSuperviewHeightAdjustment;
     frame.size.height += kBookmarkBarHeight;
+    webframe.size.height -= kBookmarkBarWebframeHeightAdjustment;
+  } else {
+    superframe.size.height -= kBookmarkBarSuperviewHeightAdjustment;
+    superframe.origin.y += kBookmarkBarSuperviewHeightAdjustment;
+    frame.size.height -= kBookmarkBarHeight;
+    webframe.size.height += kBookmarkBarWebframeHeightAdjustment;
+  }
 
-  if (immediately) {
-    [contentView_ setFrame:frame];
-    [contentView_ setNeedsDisplay:YES];
+  // TODO(jrg): Animators can be a little fussy.  Setting these three
+  // off can sometimes causes races where the finish isn't as
+  // expected.  Fix, or clean out the animators as an option.
+  // Odd racing is FAR worse than a lack of an animator.
+  if (1 /* immediately */) {
+    [superview setFrame:superframe];
+    [webContentView_ setFrame:webframe];
+    [bookmarkBarView_ setFrame:frame];
   } else {
-    [[contentView_ animator] setFrame:frame];
+    [[superview animator] setFrame:superframe];
+    [[webContentView_ animator] setFrame:webframe];
+    [[bookmarkBarView_ animator] setFrame:frame];
   }
 
   [bookmarkBarView_ setNeedsDisplay:YES];
-  [contentView_ setNeedsDisplay:YES];
+  [superview setNeedsDisplay:YES];
+  [webContentView_ setNeedsDisplay:YES];
 }
 
 - (BOOL)isBookmarkBarVisible {
@@ -173,12 +177,13 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
 // has a different disposition.
 - (void)openBookmark:(id)sender {
   const BookmarkNode* node = static_cast<const BookmarkNode*>(
-      [[[sender cell]representedObject]pointerValue]);
+      [[[sender cell] representedObject] pointerValue]);
   DCHECK(node);
   [delegate_ openBookmarkURL:node->GetURL() disposition:CURRENT_TAB];
 }
 
 // Return an autoreleased NSCell suitable for a bookmark button.
+// TODO(jrg): move much of the cell config into the BookmarkButtonCell class.
 - (NSCell *)cellForBookmarkNode:(const BookmarkNode*)node frame:(NSRect)frame {
   NSString* title = base::SysWideToNSString(node->GetTitle());
   NSButtonCell *cell = [[[BookmarkButtonCell alloc] initTextCell:nil]
@@ -206,7 +211,6 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
   [cell setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]];
   [cell setWraps:NO];
   [cell setLineBreakMode:NSLineBreakByTruncatingMiddle];
-  [cell setBordered:NO];
   return cell;
 }
 
@@ -229,7 +233,6 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
 // Add all items from the given model to our bookmark bar.
 // TODO(jrg): lots of things!
 //  - bookmark folders (e.g. menu from the button)
-//  - favicos
 //  - button and menu on the right for when bookmarks don't all fit on the
 //    screen
 //  - ...
@@ -341,4 +344,8 @@ const CGFloat kBookmarkHorizontalPadding = 8.0;
   return bookmarkBarView_;
 }
 
+- (void)setDelegate:(id<BookmarkURLOpener>)delegate {
+  delegate_ = delegate;
+}
+
 @end
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index c75262f..671e9f1 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -10,6 +10,21 @@
 #import "chrome/browser/cocoa/cocoa_test_helper.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+// Pretend BookmarkURLOpener delegate to keep track of requests
+@interface BookmarkURLOpenerPong : NSObject<BookmarkURLOpener> {
+ @public
+  GURL url_;
+}
+@end
+
+@implementation BookmarkURLOpenerPong
+- (void)openBookmarkURL:(const GURL&)url
+            disposition:(WindowOpenDisposition)disposition {
+  url_ = url;
+}
+@end
+
+
 namespace {
 
 static const int kContentAreaHeight = 500;
@@ -18,10 +33,15 @@ class BookmarkBarControllerTest : public testing::Test {
  public:
   BookmarkBarControllerTest() {
     NSRect content_frame = NSMakeRect(0, 0, 800, kContentAreaHeight);
+    NSRect bar_frame = NSMakeRect(0, 0, 800, 0);
     content_area_.reset([[NSView alloc] initWithFrame:content_frame]);
+    bar_view_.reset([[NSView alloc] initWithFrame:bar_frame]);
+    [bar_view_ setHidden:YES];
+    BookmarkBarView *bbv = (BookmarkBarView*)bar_view_.get();
     bar_.reset(
         [[BookmarkBarController alloc] initWithProfile:helper_.profile()
-                                           contentView:content_area_.get()
+                                                  view:bbv
+                                        webContentView:content_area_.get()
                                               delegate:nil]);
     NSView* parent = cocoa_helper_.contentView();
     [parent addSubview:content_area_.get()];
@@ -30,6 +50,7 @@ class BookmarkBarControllerTest : public testing::Test {
 
   CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
   scoped_nsobject<NSView> content_area_;
+  scoped_nsobject<NSView> bar_view_;
   BrowserTestHelper helper_;
   scoped_nsobject<BookmarkBarController> bar_;
 };
@@ -38,6 +59,7 @@ TEST_F(BookmarkBarControllerTest, ShowHide) {
   // Assume hidden by default in a new profile.
   EXPECT_FALSE([bar_ isBookmarkBarVisible]);
   EXPECT_TRUE([[bar_ view] isHidden]);
+  EXPECT_EQ([bar_view_ frame].size.height, 0);
 
   // Show and hide it by toggling.
   [bar_ toggleBookmarkBar];
@@ -45,18 +67,34 @@ TEST_F(BookmarkBarControllerTest, ShowHide) {
   EXPECT_FALSE([[bar_ view] isHidden]);
   NSRect content_frame = [content_area_ frame];
   EXPECT_NE(content_frame.size.height, kContentAreaHeight);
+  EXPECT_GT([bar_view_ frame].size.height, 0);
   [bar_ toggleBookmarkBar];
   EXPECT_FALSE([bar_ isBookmarkBarVisible]);
   EXPECT_TRUE([[bar_ view] isHidden]);
   content_frame = [content_area_ frame];
   EXPECT_EQ(content_frame.size.height, kContentAreaHeight);
+  EXPECT_EQ([bar_view_ frame].size.height, 0);
 }
 
-// TODO(jrg): replace getTabContents
+// Confirm openBookmark: forwards the request to the controller's delegate
 TEST_F(BookmarkBarControllerTest, OpenBookmark) {
+  GURL gurl("http://walla.walla.ding.dong.com");
+  scoped_ptr<BookmarkNode> node(new BookmarkNode(gurl));
+  scoped_nsobject<BookmarkURLOpenerPong> pong([[BookmarkURLOpenerPong alloc]
+                                                init]);
+  [bar_ setDelegate:pong.get()];
+
+  scoped_nsobject<NSButtonCell> cell([[NSButtonCell alloc] init]);
+  scoped_nsobject<NSButton> button([[NSButton alloc] init]);
+  [button setCell:cell.get()];
+  [cell setRepresentedObject:[NSValue valueWithPointer:node.get()]];
+  [bar_ openBookmark:button];
+
+  EXPECT_EQ(pong.get()->url_, node->GetURL());
 }
 
-// TODO(jrg): Make sure showing the bookmark bar calls loaded: (to process bookmarks)
+// TODO(jrg): Make sure showing the bookmark bar calls loaded: (to
+// process bookmarks)
 TEST_F(BookmarkBarControllerTest, ShowAndLoad) {
 }
 
diff --git a/chrome/browser/cocoa/bookmark_button_cell.mm b/chrome/browser/cocoa/bookmark_button_cell.mm
index fc30621..8f78010 100644
--- a/chrome/browser/cocoa/bookmark_button_cell.mm
+++ b/chrome/browser/cocoa/bookmark_button_cell.mm
@@ -6,6 +6,13 @@
 
 @implementation BookmarkButtonCell
 
+- (id)initTextCell:(NSString *)string {
+  if ((self = [super initTextCell:string])) {
+    [self setBordered:NO];
+  }
+  return self;
+}
+
 - (NSSize)cellSizeForBounds:(NSRect)aRect {
   NSSize size = [super cellSizeForBounds:aRect];
   size.width += 2;
diff --git a/chrome/browser/cocoa/browser_window_controller.h b/chrome/browser/cocoa/browser_window_controller.h
index 54ed849..bcd3c44 100644
--- a/chrome/browser/cocoa/browser_window_controller.h
+++ b/chrome/browser/cocoa/browser_window_controller.h
@@ -5,10 +5,10 @@
 #ifndef CHROME_BROWSER_COCOA_BROWSER_WINDOW_CONTROLLER_H_
 #define CHROME_BROWSER_COCOA_BROWSER_WINDOW_CONTROLLER_H_
 
-// A class acting as the Objective-C controller for the Browser object. Handles
-// interactions between Cocoa and the cross-platform code. Each window has a
-// single set of toolbars (main toolbar, bookmark bar, etc) and, by virtue of
-// being a TabWindowController, a tab strip along the top.
+// A class acting as the Objective-C controller for the Browser
+// object. Handles interactions between Cocoa and the cross-platform
+// code. Each window has a single toolbar and, by virtue of being a
+// TabWindowController, a tab strip along the top.
 
 #import <Cocoa/Cocoa.h>
 
@@ -48,7 +48,6 @@ class TabStripModelObserverBridge;
   scoped_ptr<TabStripModelObserverBridge> tabObserver_;
   scoped_ptr<BrowserWindowCocoa> windowShim_;
   scoped_nsobject<ToolbarController> toolbarController_;
-  scoped_nsobject<BookmarkBarController> bookmarkBarController_;
   scoped_nsobject<TabStripController> tabStripController_;
   scoped_nsobject<FindBarCocoaController> findBarCocoaController_;
   scoped_ptr<StatusBubble> statusBubble_;
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index 041d6e1..3ab343b 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -158,19 +158,11 @@ willPositionSheet:(NSWindow *)sheet
     toolbarController_.reset([[ToolbarController alloc]
                                initWithModel:browser->toolbar_model()
                                     commands:browser->command_updater()
-                                     profile:browser->profile()]);
+                                     profile:browser->profile()
+                              webContentView:[self tabContentArea]
+                            bookmarkDelegate:self]);
     [self positionToolbar];
-
-    // After we've adjusted the toolbar, create a controller for the bookmark
-    // bar. It will show/hide itself based on the global preference and handle
-    // positioning itself (if visible) above the content area, which is why
-    // we need to do it after we've placed the toolbar.
-    bookmarkBarController_.reset([[BookmarkBarController alloc]
-                                initWithProfile:browser_->profile()
-                                    contentView:[self tabContentArea]
-                                       delegate:self]);
-
-   [self fixWindowGradient];
+    [self fixWindowGradient];
 
     // Create the bridge for the status bubble.
     statusBubble_.reset(new StatusBubbleMac([self window]));
@@ -582,12 +574,12 @@ willPositionSheet:(NSWindow *)sheet
 }
 
 - (BOOL)isBookmarkBarVisible {
-  return [bookmarkBarController_ isBookmarkBarVisible];
+  return [[toolbarController_ bookmarkBarController] isBookmarkBarVisible];
 
 }
 
 - (void)toggleBookmarkBar {
-  [bookmarkBarController_ toggleBookmarkBar];
+  [[toolbarController_ bookmarkBarController] toggleBookmarkBar];
 }
 
 - (BOOL)isDownloadShelfVisible {
@@ -619,7 +611,7 @@ willPositionSheet:(NSWindow *)sheet
   if (fullscreen) {
     // Disable showing of the bookmark bar.  This does not toggle the
     // preference.
-    [bookmarkBarController_ setBookmarkBarEnabled:NO];
+    [[toolbarController_ bookmarkBarController] setBookmarkBarEnabled:NO];
     // Make room for more content area.
     [self removeToolbar];
     // Hide the menubar, and allow it to un-hide when moving the mouse
@@ -629,7 +621,7 @@ willPositionSheet:(NSWindow *)sheet
   } else {
     SetSystemUIMode(kUIModeNormal, 0);
     [self positionToolbar];
-    [bookmarkBarController_ setBookmarkBarEnabled:YES];
+    [[toolbarController_ bookmarkBarController] setBookmarkBarEnabled:YES];
   }
 }
 
@@ -821,8 +813,6 @@ willPositionSheet:(NSWindow *)sheet
 - (void)tabContentAreaFrameChanged:(id)sender {
   // TODO(rohitrao): This is triggered by window resizes also.  Make
   // sure we aren't doing anything wasteful in those cases.
-  [bookmarkBarController_ resizeBookmarkBar];
-
   [downloadShelfController_ resizeDownloadShelf];
 
   [findBarCocoaController_ positionFindBarView:[self tabContentArea]];
@@ -878,7 +868,6 @@ willPositionSheet:(NSWindow *)sheet
   NSArray* views = [super viewsToMoveToOverlay];
   NSArray* browserViews =
       [NSArray arrayWithObjects:[toolbarController_ view],
-                                [bookmarkBarController_ view],
                                 nil];
   return [views arrayByAddingObjectsFromArray:browserViews];
 }
diff --git a/chrome/browser/cocoa/browser_window_controller_unittest.mm b/chrome/browser/cocoa/browser_window_controller_unittest.mm
index 25a5e43..c953dc2 100644
--- a/chrome/browser/cocoa/browser_window_controller_unittest.mm
+++ b/chrome/browser/cocoa/browser_window_controller_unittest.mm
@@ -95,4 +95,10 @@ TEST_F(BrowserWindowControllerTest, TestNormal) {
   controller_.release();
 }
 
+TEST_F(BrowserWindowControllerTest, BookmarkBarControllerIndirection) {
+  EXPECT_FALSE([controller_ isBookmarkBarVisible]);
+  [controller_ toggleBookmarkBar];
+  EXPECT_TRUE([controller_ isBookmarkBarVisible]);
+}
+
 /* TODO(???): test other methods of BrowserWindowController */
diff --git a/chrome/browser/cocoa/toolbar_controller.h b/chrome/browser/cocoa/toolbar_controller.h
index 236f777..b6ec8d4 100644
--- a/chrome/browser/cocoa/toolbar_controller.h
+++ b/chrome/browser/cocoa/toolbar_controller.h
@@ -10,8 +10,10 @@
 #include "base/scoped_ptr.h"
 #include "base/scoped_nsobject.h"
 #import "chrome/browser/cocoa/command_observer_bridge.h"
+#import "chrome/browser/cocoa/bookmark_bar_controller.h"
 #include "chrome/common/pref_member.h"
 
+@class BookmarkBarView;
 class CommandUpdater;
 class LocationBar;
 class LocationBarViewMac;
@@ -32,8 +34,10 @@ class ToolbarView;
 - (void)performCut:(NSPasteboard*)pb;
 @end
 
-// A controller for the toolbar in the browser window. Manages updating the
-// state for location bar and back/fwd/reload/go buttons.
+// A controller for the toolbar in the browser window. Manages
+// updating the state for location bar and back/fwd/reload/go buttons.
+// Manages the bookmark bar and it's position in the window relative to
+// the web content view.
 
 @interface ToolbarController : NSViewController<CommandObserverProtocol> {
  @private
@@ -43,6 +47,9 @@ class ToolbarView;
   scoped_ptr<CommandObserverBridge> commandObserver_;
   scoped_ptr<LocationBarViewMac> locationBarView_;
   scoped_nsobject<LocationBarFieldEditor> locationBarFieldEditor_;  // strong
+  scoped_nsobject<BookmarkBarController> bookmarkBarController_;
+  id<BookmarkURLOpener> bookmarkBarDelegate_;  // weak
+  NSView* webContentView_;  // weak; where the web goes
 
   // Used for monitoring the optional toolbar button prefs.
   scoped_ptr<ToolbarControllerInternal::PrefObserverBridge> prefObserver_;
@@ -64,13 +71,16 @@ class ToolbarView;
   IBOutlet NSButton* pageButton_;
   IBOutlet NSButton* wrenchButton_;
   IBOutlet NSTextField* locationBar_;
+  IBOutlet BookmarkBarView* bookmarkBarView_;
 }
 
 // Initialize the toolbar and register for command updates. The profile is
 // needed for initializing the location bar.
 - (id)initWithModel:(ToolbarModel*)model
            commands:(CommandUpdater*)commands
-            profile:(Profile*)profile;
+            profile:(Profile*)profile
+     webContentView:(NSView*)webContentView
+   bookmarkDelegate:(id<BookmarkURLOpener>)delegate;
 
 // Get the C++ bridge object representing the location bar for this tab.
 - (LocationBar*)locationBar;
@@ -98,6 +108,9 @@ class ToolbarView;
 // state.
 - (void)setIsLoading:(BOOL)isLoading;
 
+// Return the bookmark bar controller.
+- (BookmarkBarController*)bookmarkBarController;
+
 // Actions for the optional menu buttons for the page and wrench menus. These
 // will show a menu while the mouse is down.
 - (IBAction)showPageMenu:(id)sender;
diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm
index 27466c4..eba4d89 100644
--- a/chrome/browser/cocoa/toolbar_controller.mm
+++ b/chrome/browser/cocoa/toolbar_controller.mm
@@ -74,13 +74,17 @@ class PrefObserverBridge : public NotificationObserver {
 
 - (id)initWithModel:(ToolbarModel*)model
            commands:(CommandUpdater*)commands
-            profile:(Profile*)profile {
+            profile:(Profile*)profile
+     webContentView:(NSView*)webContentView
+   bookmarkDelegate:(id<BookmarkURLOpener>)delegate {
   DCHECK(model && commands && profile);
   if ((self = [super initWithNibName:@"Toolbar"
                               bundle:mac_util::MainAppBundle()])) {
     toolbarModel_ = model;
     commands_ = commands;
     profile_ = profile;
+    bookmarkBarDelegate_ = delegate;
+    webContentView_ = webContentView;
 
     // Register for notifications about state changes for the toolbar buttons
     commandObserver_.reset(new CommandObserverBridge(self, commands));
@@ -111,6 +115,13 @@ class PrefObserverBridge : public NotificationObserver {
                               prefObserver_.get());
   [self showOptionalHomeButton];
   [self showOptionalPageWrenchButtons];
+
+  // Create a sub-controller for the bookmark bar.
+  bookmarkBarController_.reset([[BookmarkBarController alloc]
+                                   initWithProfile:profile_
+                                              view:bookmarkBarView_
+                                    webContentView:webContentView_
+                                          delegate:bookmarkBarDelegate_]);
 }
 
 - (LocationBar*)locationBar {
@@ -178,6 +189,10 @@ class PrefObserverBridge : public NotificationObserver {
   [goButton_ setTag:tag];
 }
 
+- (BookmarkBarController*)bookmarkBarController {
+  return bookmarkBarController_.get();
+}
+
 - (id)customFieldEditorForObject:(id)obj {
   if (obj == locationBar_) {
     // Lazilly construct Field editor, Cocoa UI code always runs on the
@@ -199,7 +214,7 @@ class PrefObserverBridge : public NotificationObserver {
 - (NSArray*)toolbarViews {
   return [NSArray arrayWithObjects:backButton_, forwardButton_, reloadButton_,
             homeButton_, starButton_, goButton_, pageButton_, wrenchButton_,
-            locationBar_, nil];
+            locationBar_, bookmarkBarView_, nil];
 }
 
 // Moves |rect| to the right by |delta|, keeping the right side fixed by
diff --git a/chrome/browser/cocoa/toolbar_controller_unittest.mm b/chrome/browser/cocoa/toolbar_controller_unittest.mm
index 6562cce..93e1cfe 100644
--- a/chrome/browser/cocoa/toolbar_controller_unittest.mm
+++ b/chrome/browser/cocoa/toolbar_controller_unittest.mm
@@ -22,7 +22,7 @@ class ToolbarControllerTest : public testing::Test {
   // |-toolbarViews| method.
   enum {
     kBackIndex, kForwardIndex, kReloadIndex, kHomeIndex, kStarIndex, kGoIndex,
-    kPageIndex, kWrenchIndex, kLocationIndex,
+    kPageIndex, kWrenchIndex, kLocationIndex, kBookmarkIndex,
   };
 
   ToolbarControllerTest() {
@@ -35,7 +35,9 @@ class ToolbarControllerTest : public testing::Test {
     bar_.reset(
         [[ToolbarController alloc] initWithModel:browser->toolbar_model()
                                         commands:browser->command_updater()
-                                         profile:helper_.profile()]);
+                                         profile:helper_.profile()
+                                  webContentView:nil
+                                bookmarkDelegate:nil]);
     EXPECT_TRUE([bar_ view]);
     NSView* parent = [cocoa_helper_.window() contentView];
     [parent addSubview:[bar_ view]];
@@ -67,6 +69,12 @@ TEST_F(ToolbarControllerTest, InitialState) {
   CompareState(updater, [bar_ toolbarViews]);
 }
 
+// Make sure awakeFromNib created a bookmarkBarController
+TEST_F(ToolbarControllerTest, AwakeFromNibCreatesBMBController) {
+  EXPECT_TRUE([bar_ bookmarkBarController]);
+}
+
+
 // Make some changes to the enabled state of a few of the buttons and ensure
 // that we're still in sync.
 TEST_F(ToolbarControllerTest, UpdateEnabledState) {