Bookmark bar, now with buttons.

Much of this CL is refactoring; it's not as large as it looks.
(Do I always claim that?  It's really true this time.)

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

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

29 files changed, 915 insertions, 238 deletions

diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h
index d0f7d8b..011cab1 100644
--- a/chrome/browser/browser_window.h
+++ b/chrome/browser/browser_window.h
@@ -202,8 +202,10 @@ class BrowserWindow {
   virtual void DestroyBrowser() = 0;
 };
 
+#if defined(OS_WIN)
 class BookmarkBarView;
 class LocationBarView;
+#endif
 
 // A BrowserWindow utility interface used for accessing elements of the browser
 // UI used only by UI test automation.
diff --git a/chrome/browser/cocoa/background_gradient_view.h b/chrome/browser/cocoa/background_gradient_view.h
new file mode 100644
index 0000000..efff342
--- /dev/null
+++ b/chrome/browser/cocoa/background_gradient_view.h
@@ -0,0 +1,16 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_COCOA_BACKGROUND_GRADIENT_VIEW_H_
+#define CHROME_BROWSER_COCOA_BACKGROUND_GRADIENT_VIEW_H_
+
+#import <Cocoa/Cocoa.h>
+
+// A custom view that draws a 'standard' background gradient.
+// Base class for other Chromium views.
+
+@interface BackgroundGradientView : NSView
+@end
+
+#endif  // CHROME_BROWSER_COCOA_BACKGROUND_GRADIENT_VIEW_H_
diff --git a/chrome/browser/cocoa/background_gradient_view.mm b/chrome/browser/cocoa/background_gradient_view.mm
new file mode 100644
index 0000000..b06e6f7
--- /dev/null
+++ b/chrome/browser/cocoa/background_gradient_view.mm
@@ -0,0 +1,29 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/cocoa/background_gradient_view.h"
+
+@implementation BackgroundGradientView
+
+// TODO(jrg): this may be a good spot to add theme changes.
+// "Theme changes" may include both GTMTheme and "Chrome Themes".
+
+- (void)drawRect:(NSRect)rect {
+  BOOL isKey = [[self window] isKeyWindow];
+
+  NSColor* start =
+      [NSColor colorWithCalibratedWhite: isKey ? 0.95 : 0.98 alpha:1.0];
+  NSColor* end = [NSColor colorWithCalibratedWhite:0.90 alpha:1.0];
+  NSGradient *gradient =
+      [[[NSGradient alloc] initWithStartingColor:start endingColor:end]
+          autorelease];
+  [gradient drawInRect:[self bounds] angle:270.0];
+  NSRect borderRect, contentRect;
+  NSDivideRect([self bounds], &borderRect, &contentRect, 1, NSMinYEdge);
+
+  [[NSColor colorWithDeviceWhite:0.0 alpha:0.3] set];
+  NSRectFillUsingOperation(borderRect, NSCompositeSourceOver);
+}
+
+@end
diff --git a/chrome/browser/cocoa/background_gradient_view_unittest.mm b/chrome/browser/cocoa/background_gradient_view_unittest.mm
new file mode 100644
index 0000000..0a7fa7e
--- /dev/null
+++ b/chrome/browser/cocoa/background_gradient_view_unittest.mm
@@ -0,0 +1,39 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/scoped_nsobject.h"
+#import "chrome/browser/cocoa/background_gradient_view.h"
+#import "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class BackgroundGradientViewTest : public testing::Test {
+ public:
+  BackgroundGradientViewTest() {
+    NSRect frame = NSMakeRect(0, 0, 100, 30);
+    view_.reset([[BackgroundGradientView alloc] initWithFrame:frame]);
+    [cocoa_helper_.contentView() addSubview:view_.get()];
+  }
+
+  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
+  scoped_nsobject<BackgroundGradientView> view_;
+};
+
+// Test adding/removing from the view hierarchy, mostly to ensure nothing
+// leaks or crashes.
+TEST_F(BackgroundGradientViewTest, AddRemove) {
+  EXPECT_EQ(cocoa_helper_.contentView(), [view_ superview]);
+  [view_.get() removeFromSuperview];
+  EXPECT_FALSE([view_ superview]);
+}
+
+// Test drawing, mostly to ensure nothing leaks or crashes.
+TEST_F(BackgroundGradientViewTest, Display) {
+  [view_ display];
+}
+
+}  // namespace
diff --git a/chrome/browser/cocoa/bookmark_bar_bridge.h b/chrome/browser/cocoa/bookmark_bar_bridge.h
new file mode 100644
index 0000000..4d5c488
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_bar_bridge.h
@@ -0,0 +1,50 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// C++ bridge class between Chromium and Cocoa to connect the
+// Bookmarks (model) with the Bookmark Bar (view).
+//
+// There is exactly one BookmarkBarBridge per BookmarkBarController /
+// BrowserWindowController / Browser.
+
+#ifndef CHROME_BROWSER_COCOA_BOOKMARK_BAR_BRIDGE_H_
+#define CHROME_BROWSER_COCOA_BOOKMARK_BAR_BRIDGE_H_
+
+#include "chrome/browser/bookmarks/bookmark_model.h"
+
+class Browser;
+@class BookmarkBarController;
+
+class BookmarkBarBridge : public BookmarkModelObserver {
+
+ public:
+  BookmarkBarBridge(BookmarkBarController* controller, BookmarkModel* model);
+  virtual ~BookmarkBarBridge();
+
+  // Overridden from BookmarkModelObserver
+  virtual void Loaded(BookmarkModel* model);
+  virtual void BookmarkModelBeingDeleted(BookmarkModel* model);
+  virtual void BookmarkNodeMoved(BookmarkModel* model,
+                                 BookmarkNode* old_parent,
+                                 int old_index,
+                                 BookmarkNode* new_parent,
+                                 int new_index);
+  virtual void BookmarkNodeAdded(BookmarkModel* model,
+                                 BookmarkNode* parent,
+                                 int index);
+  virtual void BookmarkNodeChanged(BookmarkModel* model,
+                                   BookmarkNode* node);
+  virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
+                                         BookmarkNode* node);
+  virtual void BookmarkNodeChildrenReordered(BookmarkModel* model,
+                                             BookmarkNode* node);
+
+ private:
+  BookmarkBarController* controller_;  // weak; owns me
+  BookmarkModel *model_;  // weak; it is owned by a Profile.
+
+  DISALLOW_COPY_AND_ASSIGN(BookmarkBarBridge);
+};
+
+#endif // CHROME_BROWSER_COCOA_BOOKMARK_BAR_BRIDGE_H_
diff --git a/chrome/browser/cocoa/bookmark_bar_bridge.mm b/chrome/browser/cocoa/bookmark_bar_bridge.mm
new file mode 100644
index 0000000..54b3f5b
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_bar_bridge.mm
@@ -0,0 +1,60 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/cocoa/bookmark_bar_bridge.h"
+#include "chrome/browser/cocoa/bookmark_bar_controller.h"
+
+BookmarkBarBridge::BookmarkBarBridge(BookmarkBarController* controller,
+                                     BookmarkModel* model)
+    : controller_(controller), model_(model) {
+  model_->AddObserver(this);
+
+  // Bookmark loading is async; it may may not have happened yet.
+  // We will be notified when that happens with the AddObserver() call.
+  if (model->IsLoaded())
+    Loaded(model);
+}
+
+BookmarkBarBridge::~BookmarkBarBridge() {
+  model_->RemoveObserver(this);
+}
+
+void BookmarkBarBridge::Loaded(BookmarkModel* model) {
+  [controller_ loaded:model];
+}
+
+void BookmarkBarBridge::BookmarkModelBeingDeleted(BookmarkModel* model) {
+  [controller_ beingDeleted:model];
+}
+
+void BookmarkBarBridge::BookmarkNodeMoved(BookmarkModel* model,
+                                          BookmarkNode* old_parent,
+                                          int old_index,
+                                          BookmarkNode* new_parent,
+                                          int new_index) {
+  [controller_ nodeMoved:model
+               oldParent:old_parent oldIndex:old_index
+               newParent:new_parent newIndex:new_index];
+}
+
+void BookmarkBarBridge::BookmarkNodeAdded(BookmarkModel* model,
+                                          BookmarkNode* parent,
+                                          int index) {
+  [controller_ nodeAdded:model parent:parent index:index];
+}
+
+void BookmarkBarBridge::BookmarkNodeChanged(BookmarkModel* model,
+                                            BookmarkNode* node) {
+  [controller_ nodeChanged:model node:node];
+}
+
+void BookmarkBarBridge::BookmarkNodeFavIconLoaded(BookmarkModel* model,
+                                                  BookmarkNode* node) {
+  [controller_ nodeFavIconLoaded:model node:node];
+}
+
+void BookmarkBarBridge::BookmarkNodeChildrenReordered(BookmarkModel* model,
+                                                      BookmarkNode* node) {
+  [controller_ nodeChildrenReordered:model node:node];
+}
diff --git a/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm b/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm
new file mode 100644
index 0000000..c5ac97b
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm
@@ -0,0 +1,129 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/cocoa/bookmark_bar_bridge.h"
+#include "chrome/browser/cocoa/bookmark_bar_controller.h"
+#include "chrome/browser/cocoa/browser_test_helper.h"
+#include "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// TODO(jrg): add OCMock to Chromium to save some typing.
+
+namespace {
+
+// Information needed to open a URL, as passed to the
+// BookmarkBarController's delegate.
+typedef std::pair<GURL,WindowOpenDisposition> OpenInfo;
+
+}  // The namespace must end here -- I need to use OpenInfo in
+   // FakeBookmarkBarController but can't place
+   // FakeBookmarkBarController itself in the namespace ("error:
+   // Objective-C declarations may only appear in global scope")
+
+// Oddly, we are our own delegate.
+@interface FakeBookmarkBarController :
+  BookmarkBarController<BookmarkURLOpener> {
+ @public
+  scoped_nsobject<NSMutableArray> callbacks_;
+
+  std::vector<OpenInfo> opens_;
+}
+@end
+
+@implementation FakeBookmarkBarController
+
+- (id)initWithProfile:(Profile*)profile
+          contentArea:(NSView*)content {
+  if ((self = [super initWithProfile:profile
+                         contentView:content
+                            delegate:self])) {
+    callbacks_.reset([[NSMutableArray alloc] init]);
+  }
+  return self;
+}
+
+- (void)loaded:(BookmarkModel*)model {
+  [callbacks_ addObject:[NSNumber numberWithInt:0]];
+}
+
+- (void)beingDeleted:(BookmarkModel*)model {
+  [callbacks_ addObject:[NSNumber numberWithInt:1]];
+}
+
+- (void)nodeMoved:(BookmarkModel*)model
+        oldParent:(BookmarkNode*)oldParent oldIndex:(int)oldIndex
+        newParent:(BookmarkNode*)newParent newIndex:(int)newIndex {
+  [callbacks_ addObject:[NSNumber numberWithInt:2]];
+}
+
+- (void)nodeAdded:(BookmarkModel*)model
+           parent:(BookmarkNode*)oldParent index:(int)index {
+  [callbacks_ addObject:[NSNumber numberWithInt:3]];
+}
+
+- (void)nodeChanged:(BookmarkModel*)model
+               node:(BookmarkNode*)node {
+  [callbacks_ addObject:[NSNumber numberWithInt:4]];
+}
+
+- (void)nodeFavIconLoaded:(BookmarkModel*)model
+                     node:(BookmarkNode*)node {
+  [callbacks_ addObject:[NSNumber numberWithInt:5]];
+}
+
+- (void)nodeChildrenReordered:(BookmarkModel*)model
+                         node:(BookmarkNode*)node {
+  [callbacks_ addObject:[NSNumber numberWithInt:6]];
+}
+
+// Save the request.
+- (void)openBookmarkURL:(const GURL&)url
+            disposition:(WindowOpenDisposition)disposition {
+  opens_.push_back(OpenInfo(url, disposition));
+}
+
+@end
+
+
+class BookmarkBarBridgeTest : public testing::Test {
+ public:
+  BookmarkBarBridgeTest() {
+    NSRect content_frame = NSMakeRect(0, 0, 100, 100);
+    view_.reset([[NSView alloc] initWithFrame:content_frame]);
+  }
+
+  CocoaTestHelper cocoa_helper_;
+  BrowserTestHelper browser_test_helper_;
+  scoped_nsobject<NSView> view_;
+};
+
+// Call all the callbacks; make sure they are all redirected to the objc object.
+TEST_F(BookmarkBarBridgeTest, TestRedirect) {
+  Profile *profile = browser_test_helper_.profile();
+  BookmarkModel *model = profile->GetBookmarkModel();
+
+  scoped_nsobject<FakeBookmarkBarController>
+    controller([[FakeBookmarkBarController alloc] initWithProfile:profile
+                                                      contentArea:view_]);
+  EXPECT_TRUE(controller.get());
+  scoped_ptr<BookmarkBarBridge> bridge(new BookmarkBarBridge(controller.get(),
+                                                             model));
+  EXPECT_TRUE(bridge.get());
+
+  bridge->Loaded(NULL);
+  bridge->BookmarkModelBeingDeleted(NULL);
+  bridge->BookmarkNodeMoved(NULL, NULL, 0, NULL, 0);
+  bridge->BookmarkNodeAdded(NULL, NULL, 0);
+  bridge->BookmarkNodeChanged(NULL, NULL);
+  bridge->BookmarkNodeFavIconLoaded(NULL, NULL);
+  bridge->BookmarkNodeChildrenReordered(NULL, NULL);
+
+  // 7 calls above plus an initial Loaded() in init routine makes 8
+  EXPECT_TRUE([controller.get()->callbacks_ count] == 8);
+
+  for (int x = 1; x < 8; x++) {
+    NSNumber *num = [NSNumber numberWithInt:x-1];
+    EXPECT_TRUE([[controller.get()->callbacks_ objectAtIndex:x] isEqual:num]);
+  }
+}
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index eea3f0c..ca9dc0f 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -8,15 +8,24 @@
 #import <Cocoa/Cocoa.h>
 
 #include "base/scoped_nsobject.h"
+#include "chrome/browser/cocoa/bookmark_bar_bridge.h"
+#include "webkit/glue/window_open_disposition.h"
 
 @class BookmarkBarStateController;
 class BookmarkModel;
+class BookmarkNode;
+@class BookmarkBarView;
 class Profile;
-@class ToolbarView;
+
+// The interface for an object which can open URLs for a bookmark.
+@protocol BookmarkURLOpener
+- (void)openBookmarkURL:(const GURL&)url
+            disposition:(WindowOpenDisposition)disposition;
+@end
+
 
 // A controller for the bookmark bar in the browser window. Handles showing
 // and hiding based on the preference in the given profile.
-
 @interface BookmarkBarController : NSObject {
  @private
   BookmarkModel* bookmarkModel_;  // weak; part of the profile owned by the
@@ -25,17 +34,32 @@ class Profile;
   // Currently these two are always the same, but they mean slightly
   // different things.  contentAreaHasOffset_ is an implementation
   // detail of bookmark bar visibility.
-  BOOL contentAreaHasOffset_;
+  BOOL contentViewHasOffset_;
   BOOL barIsVisible_;
 
-  // TODO(jrg): write a BookmarkView
-  IBOutlet ToolbarView* /* BookmarkView* */ bookmarkView_;
-  IBOutlet NSView* contentArea_;
+  // 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_;
+
+  // The tab content area for the window (where the web goes)
+  IBOutlet NSView* contentView_;
+
+  // Bridge from Chrome-style C++ notifications (e.g. derived from
+  // BookmarkModelObserver)
+  scoped_ptr<BookmarkBarBridge> bridge_;
+
+  // Delegate which can open URLs for us.
+  id<BookmarkURLOpener> delegate_;  // weak
 }
 
-// Initializes the controller with the given browser profile and content view.
+// Initializes the controller with the given browser profile and
+// content view.  We use |content| as a parent view for the bookmark
+// bar view and for geometry management.  |delegate| is used for
+// opening URLs.
 - (id)initWithProfile:(Profile*)profile
-          contentArea:(NSView*)content;
+          contentView:(NSView*)content
+             delegate:(id<BookmarkURLOpener>)delegate;
 
 // Resizes the bookmark bar based on the state of the content area.
 - (void)resizeBookmarkBar;
@@ -48,6 +72,25 @@ class Profile;
 
 @end
 
+// Redirects from BookmarkBarBridge, the C++ object which glues us to
+// the rest of Chromium.  Internal to BookmarkBarController.
+@interface BookmarkBarController(BridgeRedirect)
+- (void)loaded:(BookmarkModel*)model;
+- (void)beingDeleted:(BookmarkModel*)model;
+- (void)nodeMoved:(BookmarkModel*)model
+        oldParent:(BookmarkNode*)oldParent oldIndex:(int)oldIndex
+        newParent:(BookmarkNode*)newParent newIndex:(int)newIndex;
+- (void)nodeAdded:(BookmarkModel*)model
+           parent:(BookmarkNode*)oldParent index:(int)index;
+- (void)nodeChanged:(BookmarkModel*)model
+               node:(BookmarkNode*)node;
+- (void)nodeFavIconLoaded:(BookmarkModel*)model
+                     node:(BookmarkNode*)node;
+- (void)nodeChildrenReordered:(BookmarkModel*)model
+                         node:(BookmarkNode*)node;
+@end
+
+
 // These APIs should only be used by unit tests, in place of "friend" classes.
 @interface BookmarkBarController(TestingAPI)
 // Access to the bookmark bar's view represented by this controller.
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index f7ffbda..0b3d8e7 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -2,9 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/sys_string_conversions.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/browser.h"
+#include "chrome/browser/browser_list.h"
+#import "chrome/browser/cocoa/bookmark_bar_bridge.h"
 #import "chrome/browser/cocoa/bookmark_bar_controller.h"
-
-#import "chrome/browser/cocoa/toolbar_view.h"
+#import "chrome/browser/cocoa/bookmark_bar_view.h"
+#import "chrome/browser/cocoa/bookmark_button_cell.h"
 #include "chrome/browser/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
@@ -15,27 +20,40 @@
 - (void)showBookmarkBar:(BOOL)enable;
 @end
 
+namespace {
+const int kBookmarkBarHeight = 30;
+// Magic numbers from Cole
+const CGFloat kDefaultBookmarkWidth = 150.0;
+const CGFloat kBookmarkVerticalPadding = 2.0;
+const CGFloat kBookmarkHorizontalPadding = 8.0;
+};
+
 @implementation BookmarkBarController
 
 - (id)initWithProfile:(Profile*)profile
-          contentArea:(NSView*)content {
+          contentView:(NSView*)content
+             delegate:(id<BookmarkURLOpener>)delegate {
   if ((self = [super init])) {
     bookmarkModel_ = profile->GetBookmarkModel();
-    contentArea_ = content;
+    contentView_ = content;
+    delegate_ = delegate;
     // 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);
-    bookmarkView_ = [[ToolbarView alloc] initWithFrame:frame];
+    bookmarkBarView_ = [[BookmarkBarView alloc] initWithFrame:frame];
     [self positionBar];
     if (profile->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar))
       [self showBookmarkBar:YES];
-    [[[contentArea_ window] contentView] addSubview:bookmarkView_];
+    [[[contentView_ window] contentView] addSubview:bookmarkBarView_];
   }
+  // Don't pass ourself along until our init is done.
+  // Thus, this call is (almost) last.
+  bridge_.reset(new BookmarkBarBridge(self, bookmarkModel_));
   return self;
 }
 
 - (void)dealloc {
-  [bookmarkView_ release];
+  [bookmarkBarView_ release];
   [super dealloc];
 }
 
@@ -44,43 +62,40 @@
 // all, you need to call |-showBookmarkBar:| to do that.
 - (void)positionBar {
   // Hide or show bar based on initial visibility and set the resize flags.
-  [bookmarkView_ setAutoresizingMask:NSViewWidthSizable | NSViewMinYMargin];
-  [bookmarkView_ setHidden:!barIsVisible_];
+  [bookmarkBarView_ setAutoresizingMask:NSViewWidthSizable | NSViewMinYMargin];
+  [bookmarkBarView_ setHidden:!barIsVisible_];
 
   // 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 = [contentArea_ frame];
+  NSRect contentFrame = [contentView_ frame];
   NSRect barFrame = NSMakeRect(0, NSMaxY(contentFrame),
                                contentFrame.size.width, 0);
-  [bookmarkView_ setFrame:barFrame];
+  [bookmarkBarView_ setFrame:barFrame];
 }
 
-// Called when the contentArea's frame changes.  Enlarge the view to
-// stay with the top of the contentArea.
+// Called when the contentView's frame changes.  Enlarge the view to
+// stay with the top of the contentView.
 - (void)resizeBookmarkBar {
-  NSRect barFrame = [bookmarkView_ frame];
+  NSRect barFrame = [bookmarkBarView_ frame];
   const int maxY = NSMaxY(barFrame);
-  barFrame.origin.y = NSMaxY([contentArea_ frame]);
+  barFrame.origin.y = NSMaxY([contentView_ frame]);
   barFrame.size.height = maxY - barFrame.origin.y;
-  [bookmarkView_ setFrame:barFrame];
+  [bookmarkBarView_ setFrame:barFrame];
 }
 
 // Show or hide the bar based on the value of |enable|. Handles animating the
 // resize of the content view.
 - (void)showBookmarkBar:(BOOL)enable {
-  contentAreaHasOffset_ = enable;
-  [bookmarkView_ setHidden:enable ? NO : YES];
+  contentViewHasOffset_ = enable;
+  [bookmarkBarView_ setHidden:enable ? NO : YES];
   [self applyContentAreaOffset:enable];
 
+  barIsVisible_ = enable;
   if (enable) {
-    // TODO(jrg): display something useful in the bookmark bar
-    // TODO(jrg): use a BookmarksView, not a ToolbarView
-    // TODO(jrg): don't draw a border around it
-    // TODO(jrg): ...
+    [self loaded:bookmarkModel_];
   }
-  barIsVisible_ = enable;
 }
 
 // Apply a contents box offset to make (or remove) room for the
@@ -88,25 +103,24 @@
 // "full size").  If apply==NO we are trying to undo an offset.  If no
 // offset there is nothing to undo.
 - (void)applyContentAreaOffset:(BOOL)apply {
-  if (bookmarkView_ == nil) {
+  if (bookmarkBarView_ == nil) {
     // We're too early, but awakeFromNib will call me again.
     return;
   }
-  if (!contentAreaHasOffset_ && apply) {
+  if (!contentViewHasOffset_ && apply) {
     // There is no offset to unconditionally apply.
     return;
   }
 
-  static const int kBookmarkBarHeight = 30;
-  NSRect frame = [contentArea_ frame];
+  NSRect frame = [contentView_ frame];
   if (apply)
     frame.size.height -= kBookmarkBarHeight;
   else
     frame.size.height += kBookmarkBarHeight;
 
-  [[contentArea_ animator] setFrame:frame];
-  [bookmarkView_ setNeedsDisplay:YES];
-  [contentArea_ setNeedsDisplay:YES];
+  [[contentView_ animator] setFrame:frame];
+  [bookmarkBarView_ setNeedsDisplay:YES];
+  [contentView_ setNeedsDisplay:YES];
 }
 
 - (BOOL)isBookmarkBarVisible {
@@ -120,8 +134,160 @@
   [self showBookmarkBar:!barIsVisible_];
 }
 
+// Delete all items from the bookmark bar.
+- (void)clearBookmarkBar {
+  [bookmarkBarView_ setSubviews:[NSArray array]];
+}
+
+// TODO(jrg): add an openBookmarkInBackground() for ctrl-click which
+// has a different disposition.
+- (void)openBookmark:(id)sender {
+  BookmarkNode* node = static_cast<BookmarkNode*>([[[sender cell]
+                                                     representedObject]
+                                                    pointerValue]);
+  DCHECK(node);
+  [delegate_ openBookmarkURL:node->GetURL() disposition:CURRENT_TAB];
+}
+
+// Return an NSCell suitable for a bookmark button.
+- (NSCell *)cellForBookmarkNode:(BookmarkNode*)node frame:(NSRect)frame {
+  NSString* title = base::SysWideToNSString(node->GetTitle());
+  NSButtonCell *cell = [[[BookmarkButtonCell alloc] initTextCell:nil]
+                         autorelease];
+  DCHECK(cell);
+  [cell setRepresentedObject:[NSValue valueWithPointer:node]];
+  [cell setButtonType:NSMomentaryPushInButton];
+  [cell setBezelStyle:NSShadowlessSquareBezelStyle];
+  [cell setShowsBorderOnlyWhileMouseInside:YES];
+
+  // TODO(jrg): add the real image.  Find or write an SkBitmap-to-NSImage helper.
+  // For now I'm using the nav icon just to have something.
+  [cell setImage:[NSImage imageNamed:@"nav"]];
+  [cell setImagePosition:NSImageLeft];
+
+  [cell setTitle:title];
+  [cell setControlSize:NSSmallControlSize];
+  [cell setAlignment:NSLeftTextAlignment];
+  [cell setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]];
+  [cell setWraps:NO];
+  [cell setLineBreakMode:NSLineBreakByTruncatingMiddle];
+  [cell setBordered:NO];
+  return cell;
+}
+
+// TODO(jrg): accomodation for bookmarks less than minimum width in
+// size (like Windows)?
+- (NSRect)frameForBookmarkAtIndex:(int)index {
+  NSRect bounds = [[self view] bounds];
+  // TODO: be smarter about this; the animator delays the right height
+  if (bounds.size.height == 0)
+    bounds.size.height = kBookmarkBarHeight;
+
+  NSRect frame = NSInsetRect(bounds,
+                             kBookmarkHorizontalPadding,
+                             kBookmarkVerticalPadding);
+  frame.origin.x += (kDefaultBookmarkWidth * index);
+  frame.size.width = kDefaultBookmarkWidth;
+  return frame;
+}
+
+// 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
+//  - ...
+//
+// TODO(jrg): contextual menu (e.g. Open In New Tab) for each button
+// in this function)
+//
+// TODO(jrg): write a "build bar" so there is a nice spot for things
+// like the contextual menu which is invoked when not over a
+// bookmark.  On Safari that menu has a "new folder" option.
+- (void)addNodesToBar:(BookmarkNode*)node {
+  for (int i = 0; i < node->GetChildCount(); i++) {
+    BookmarkNode* child = node->GetChild(i);
+
+    NSRect frame = [self frameForBookmarkAtIndex:i];
+    NSButton* button = [[[NSButton alloc] initWithFrame:frame]
+                         autorelease];
+    DCHECK(button);
+    // [NSButton setCell:] warns to NOT use setCell: other than in the
+    // initializer of a control.  However, we are using a basic
+    // NSButton whose initializer does not take an NSCell as an
+    // object.  To honor the assumed semantics, we do nothing with
+    // NSButton between alloc/init and setCell:.
+    [button setCell:[self cellForBookmarkNode:child frame:frame]];
+    // [button sizeToFit];
+
+    if (child->is_folder()) {
+      // For now just disable the button if it's a folder.
+      // TODO(jrg): recurse.
+      [button setEnabled:NO];
+    } else {
+      // Make the button do something
+      [button setTarget:self];
+      [button setAction:@selector(openBookmark:)];
+      // Add a tooltip.
+      NSString* title = base::SysWideToNSString(child->GetTitle());
+      std::string url_string = child->GetURL().possibly_invalid_spec();
+      NSString* tooltip = [NSString stringWithFormat:@"%@\n%s", title,
+                                    url_string.c_str()];
+      [button setToolTip:tooltip];
+    }
+    // Finally, add it to the bookmark bar.
+    [bookmarkBarView_ addSubview:button];
+  }
+}
+
+// TODO(jrg): for now this is brute force.
+- (void)loaded:(BookmarkModel*)model {
+  // Do nothing if not visible or too early
+  if ((barIsVisible_ == NO) || !model->IsLoaded())
+    return;
+  // Else brute force nuke and build.
+  BookmarkNode* node = model->GetBookmarkBarNode();
+  [self clearBookmarkBar];
+  [self addNodesToBar:node];
+}
+
+- (void)beingDeleted:(BookmarkModel*)model {
+  [self clearBookmarkBar];
+}
+
+// TODO(jrg): for now this is brute force.
+- (void)nodeMoved:(BookmarkModel*)model
+        oldParent:(BookmarkNode*)oldParent oldIndex:(int)oldIndex
+        newParent:(BookmarkNode*)newParent newIndex:(int)newIndex {
+  [self loaded:model];
+}
+
+// TODO(jrg): for now this is brute force.
+- (void)nodeAdded:(BookmarkModel*)model
+           parent:(BookmarkNode*)oldParent index:(int)index {
+  [self loaded:model];
+}
+
+// TODO(jrg): for now this is brute force.
+- (void)nodeChanged:(BookmarkModel*)model
+               node:(BookmarkNode*)node {
+  [self loaded:model];
+}
+
+- (void)nodeFavIconLoaded:(BookmarkModel*)model
+                     node:(BookmarkNode*)node {
+  // TODO(jrg): no icons yet
+}
+
+// TODO(jrg): for now this is brute force.
+- (void)nodeChildrenReordered:(BookmarkModel*)model
+                         node:(BookmarkNode*)node {
+  [self loaded:model];
+}
+
 - (NSView*)view {
-  return bookmarkView_;
+  return bookmarkBarView_;
 }
 
 @end
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index 892a96f..c75262f 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -21,7 +21,8 @@ class BookmarkBarControllerTest : public testing::Test {
     content_area_.reset([[NSView alloc] initWithFrame:content_frame]);
     bar_.reset(
         [[BookmarkBarController alloc] initWithProfile:helper_.profile()
-                                           contentArea:content_area_.get()]);
+                                           contentView:content_area_.get()
+                                              delegate:nil]);
     NSView* parent = cocoa_helper_.contentView();
     [parent addSubview:content_area_.get()];
     [parent addSubview:[bar_ view]];
@@ -51,8 +52,36 @@ TEST_F(BookmarkBarControllerTest, ShowHide) {
   EXPECT_EQ(content_frame.size.height, kContentAreaHeight);
 }
 
+// TODO(jrg): replace getTabContents
+TEST_F(BookmarkBarControllerTest, OpenBookmark) {
+}
+
+// TODO(jrg): Make sure showing the bookmark bar calls loaded: (to process bookmarks)
+TEST_F(BookmarkBarControllerTest, ShowAndLoad) {
+}
+
+// TODO(jrg): Make sure a cleared bar has no subviews
+TEST_F(BookmarkBarControllerTest, Clear) {
+}
+
+// TODO(jrg): Make sure loaded: does something useful
+TEST_F(BookmarkBarControllerTest, Loaded) {
+  // Clear; make sure no views
+  // Call loaded:
+  // Make sure subviews
+}
+
+// TODO(jrg): Test cellForBookmarkNode:
+TEST_F(BookmarkBarControllerTest, Cell) {
+}
+
+// TODO(jrg): Test frameForBookmarkAtIndex
+TEST_F(BookmarkBarControllerTest, FrameAtIndex) {
+}
+
 TEST_F(BookmarkBarControllerTest, Contents) {
-  // TODO(jrg): When there are items on the bar, flesh this out.
+  // TODO(jrg): addNodesToBar has a LOT of TODOs; when flushed out, write
+  // appropriate tests.
 }
 
 // Test drawing, mostly to ensure nothing leaks or crashes.
diff --git a/chrome/browser/cocoa/bookmark_bar_view.h b/chrome/browser/cocoa/bookmark_bar_view.h
new file mode 100644
index 0000000..a8b343e
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_bar_view.h
@@ -0,0 +1,19 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_COCOA_BOOKMARK_BAR_VIEW_H_
+#define CHROME_BROWSER_COCOA_BOOKMARK_BAR_VIEW_H_
+
+#import <Cocoa/Cocoa.h>
+#import "chrome/browser/cocoa/background_gradient_view.h"
+
+// A view that handles any special rendering of the bookmark bar.  At
+// this time it only draws a gradient like the toolbar.  In the future
+// I expect changes for new features (themes, extensions, etc).
+
+@interface BookmarkBarView : BackgroundGradientView {
+}
+@end
+
+#endif  // CHROME_BROWSER_COCOA_BOOKMARK_BAR_VIEW_H_
diff --git a/chrome/browser/cocoa/bookmark_bar_view.mm b/chrome/browser/cocoa/bookmark_bar_view.mm
new file mode 100644
index 0000000..63171e4
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_bar_view.mm
@@ -0,0 +1,8 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/cocoa/bookmark_bar_view.h"
+
+@implementation BookmarkBarView
+@end
diff --git a/chrome/browser/cocoa/bookmark_bar_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm
new file mode 100644
index 0000000..51b9f12
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_bar_view_unittest.mm
@@ -0,0 +1,6 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// This file is intentionally empty; there is no code in
+// BookmarkBarView to test yet.
diff --git a/chrome/browser/cocoa/bookmark_button_cell.h b/chrome/browser/cocoa/bookmark_button_cell.h
new file mode 100644
index 0000000..f6929cb
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_button_cell.h
@@ -0,0 +1,17 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_COCOA_BOOKMARK_BUTTON_CELL_H_
+#define CHROME_BROWSER_COCOA_BOOKMARK_BUTTON_CELL_H_
+
+#import "chrome/browser/cocoa/gradient_button_cell.h"
+
+// A button cell that handles drawing/highlighting of buttons in the
+// bookmark bar.
+
+@interface BookmarkButtonCell : GradientButtonCell {
+}
+@end
+
+#endif  // CHROME_BROWSER_COCOA_BOOKMARK_BUTTON_CELL_H_
diff --git a/chrome/browser/cocoa/bookmark_button_cell.mm b/chrome/browser/cocoa/bookmark_button_cell.mm
new file mode 100644
index 0000000..fc30621
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_button_cell.mm
@@ -0,0 +1,16 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/cocoa/bookmark_button_cell.h"
+
+@implementation BookmarkButtonCell
+
+- (NSSize)cellSizeForBounds:(NSRect)aRect {
+  NSSize size = [super cellSizeForBounds:aRect];
+  size.width += 2;
+  size.height += 4;
+  return size;
+}
+
+@end
diff --git a/chrome/browser/cocoa/bookmark_button_cell_unittest.mm b/chrome/browser/cocoa/bookmark_button_cell_unittest.mm
new file mode 100644
index 0000000..7b61ebd
--- /dev/null
+++ b/chrome/browser/cocoa/bookmark_button_cell_unittest.mm
@@ -0,0 +1,34 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/scoped_nsobject.h"
+#import "chrome/browser/cocoa/bookmark_button_cell.h"
+#import "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class BookmarkButtonCellTest : public testing::Test {
+ public:
+  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
+  scoped_nsobject<NSButton> view_;
+};
+
+// Make sure it's not totally bogus
+TEST_F(BookmarkButtonCellTest, SizeForBounds) {
+  NSRect frame = NSMakeRect(0, 0, 50, 30);
+  view_.reset([[NSButton alloc] initWithFrame:frame]);
+  scoped_nsobject<BookmarkButtonCell> cell([[BookmarkButtonCell alloc]
+                                               initTextCell:@"Testing"]);
+  [view_ setCell:cell.get()];
+  [cocoa_helper_.contentView() addSubview:view_.get()];
+
+  NSRect r = NSMakeRect(0, 0, 100, 100);
+  NSSize size = [cell.get() cellSizeForBounds:r];
+  EXPECT_TRUE(size.width > 0 && size.height > 0);
+  EXPECT_TRUE(size.width < 200 && size.height < 200);
+}
+
+
+}  // namespace
diff --git a/chrome/browser/cocoa/bookmark_menu_bridge.h b/chrome/browser/cocoa/bookmark_menu_bridge.h
index 7c4565c..2516b4f 100644
--- a/chrome/browser/cocoa/bookmark_menu_bridge.h
+++ b/chrome/browser/cocoa/bookmark_menu_bridge.h
@@ -31,7 +31,7 @@ class BookmarkMenuBridge : public BookmarkModelObserver,
                            public BrowserList::Observer {
  public:
   BookmarkMenuBridge();
-  ~BookmarkMenuBridge();
+  virtual ~BookmarkMenuBridge();
 
   // Overridden from BookmarkModelObserver
   virtual void Loaded(BookmarkModel* model);
diff --git a/chrome/browser/cocoa/browser_window_controller.h b/chrome/browser/cocoa/browser_window_controller.h
index 0ccd73a..e567db7 100644
--- a/chrome/browser/cocoa/browser_window_controller.h
+++ b/chrome/browser/cocoa/browser_window_controller.h
@@ -15,8 +15,8 @@
 #include "base/scoped_nsobject.h"
 #include "base/scoped_ptr.h"
 #import "chrome/browser/cocoa/tab_window_controller.h"
+#import "chrome/browser/cocoa/bookmark_bar_controller.h"
 
-@class BookmarkBarController;
 class Browser;
 class BrowserWindow;
 class BrowserWindowCocoa;
@@ -31,7 +31,7 @@ class TabStripModelObserverBridge;
 @class ToolbarController;
 
 @interface BrowserWindowController :
-    TabWindowController<NSUserInterfaceValidations> {
+  TabWindowController<NSUserInterfaceValidations,BookmarkURLOpener> {
  @private
   // The ordering of these members is important as it determines the order in
   // which they are destroyed. |browser_| needs to be destroyed last as most of
@@ -46,7 +46,7 @@ class TabStripModelObserverBridge;
   scoped_ptr<TabStripModelObserverBridge> tabObserver_;
   scoped_ptr<BrowserWindowCocoa> windowShim_;
   scoped_nsobject<ToolbarController> toolbarController_;
-  scoped_nsobject<BookmarkBarController> bookmarkController_;
+  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 a138905..9fa2ee0 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -136,9 +136,10 @@ willPositionSheet:(NSWindow *)sheet
     // 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.
-    bookmarkController_.reset([[BookmarkBarController alloc]
+    bookmarkBarController_.reset([[BookmarkBarController alloc]
                                 initWithProfile:browser_->profile()
-                                    contentArea:[self tabContentArea]]);
+                                    contentView:[self tabContentArea]
+                                       delegate:self]);
 
    [self fixWindowGradient];
 
@@ -233,11 +234,11 @@ willPositionSheet:(NSWindow *)sheet
                         defaultFrame:(NSRect)frame {
   // If the shift key is down, maximize. Hopefully this should make the
   // "switchers" happy.
-  if ([[[NSApplication sharedApplication] currentEvent] modifierFlags] & 
+  if ([[[NSApplication sharedApplication] currentEvent] modifierFlags] &
           NSShiftKeyMask) {
     return [[window screen] visibleFrame];
   }
-  
+
   const int kMinimumIntrinsicWidth = 700;
   const int kScrollbarWidth = 16;
   const int kSpaceForIcons = 50;
@@ -428,7 +429,7 @@ willPositionSheet:(NSWindow *)sheet
 
 - (NSView *)selectedTabView {
   return [tabStripController_ selectedTabView];
-}  
+}
 
 - (TabStripController *)tabStripController {
   return tabStripController_;
@@ -462,7 +463,7 @@ willPositionSheet:(NSWindow *)sheet
 - (TabWindowController*)detachTabToNewWindow:(TabView*)tabView {
   // Disable screen updates so that this appears as a single visual change.
   NSDisableScreenUpdates();
-  
+
   // Fetch the tab contents for the tab being dragged
   int index = [tabStripController_ indexForTabView:tabView];
   TabContents* contents = browser_->tabstrip_model()->GetTabContentsAt(index);
@@ -505,7 +506,7 @@ willPositionSheet:(NSWindow *)sheet
 
   // And make sure we use the correct frame in the new view.
   [[controller tabStripController] setFrameOfSelectedTab:tabRect];
-  
+
   NSEnableScreenUpdates();
   return controller;
 }
@@ -526,12 +527,12 @@ willPositionSheet:(NSWindow *)sheet
 }
 
 - (BOOL)isBookmarkBarVisible {
-  return [bookmarkController_ isBookmarkBarVisible];
+  return [bookmarkBarController_ isBookmarkBarVisible];
 
 }
 
 - (void)toggleBookmarkBar {
-  [bookmarkController_ toggleBookmarkBar];
+  [bookmarkBarController_ toggleBookmarkBar];
 }
 
 - (void)addFindBar:(FindBarCocoaController*)findBarCocoaController {
@@ -544,6 +545,15 @@ willPositionSheet:(NSWindow *)sheet
   [findBarCocoaController_ positionFindBarView:[self tabContentArea]];
 }
 
+// Called by the bookmark bar to open a URL.
+- (void)openBookmarkURL:(const GURL&)url
+            disposition:(WindowOpenDisposition)disposition {
+  TabContents* tab_contents = browser_->GetSelectedTabContents();
+  DCHECK(tab_contents);
+  tab_contents->OpenURL(url, GURL(), disposition,
+                        PageTransition::AUTO_BOOKMARK);
+}
+
 - (NSInteger)numberOfTabs {
   return browser_->tabstrip_model()->count();
 }
@@ -637,7 +647,7 @@ 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.
-  [bookmarkController_ resizeBookmarkBar];
+  [bookmarkBarController_ resizeBookmarkBar];
 
   if (findBarCocoaController_.get()) {
     [findBarCocoaController_ positionFindBarView:[self tabContentArea]];
@@ -694,7 +704,7 @@ willPositionSheet:(NSWindow *)sheet
   NSArray* views = [super viewsToMoveToOverlay];
   NSArray* browserViews =
       [NSArray arrayWithObjects:[toolbarController_ view],
-                                [bookmarkController_ view],
+                                [bookmarkBarController_ view],
                                 nil];
   return [views arrayByAddingObjectsFromArray:browserViews];
 }
diff --git a/chrome/browser/cocoa/gradient_button_cell.h b/chrome/browser/cocoa/gradient_button_cell.h
new file mode 100644
index 0000000..ae85e6e
--- /dev/null
+++ b/chrome/browser/cocoa/gradient_button_cell.h
@@ -0,0 +1,28 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_COCOA_CHROMIUM_BUTTON_CELL_H_
+#define CHROME_BROWSER_COCOA_CHROMIUM_BUTTON_CELL_H_
+
+#import <Cocoa/Cocoa.h>
+
+// Base class for button cells for toolbar and bookmark bar.
+//
+// This is a button cell that handles drawing/highlighting of buttons.
+// The appearance is determined by setting the cell's tag (not the
+// view's) to one of the constants below (ButtonType).
+
+enum {
+  kLeftButtonType = -1,
+  kLeftButtonWithShadowType = -2,
+  kStandardButtonType = 0,
+  kRightButtonType = 1,
+};
+typedef NSInteger ButtonType;
+
+@interface GradientButtonCell : NSButtonCell {
+}
+@end
+
+#endif  // CHROME_BROWSER_COCOA_CHROMIUM_BUTTON_CELL_H_
diff --git a/chrome/browser/cocoa/gradient_button_cell.mm b/chrome/browser/cocoa/gradient_button_cell.mm
new file mode 100644
index 0000000..91cec0d
--- /dev/null
+++ b/chrome/browser/cocoa/gradient_button_cell.mm
@@ -0,0 +1,78 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/cocoa/gradient_button_cell.h"
+
+@implementation GradientButtonCell
+
+- (NSBackgroundStyle)interiorBackgroundStyle {
+  return [self isHighlighted] ?
+      NSBackgroundStyleLowered : NSBackgroundStyleRaised;
+}
+
+- (void)drawWithFrame:(NSRect)cellFrame inView:(NSView *)controlView {
+  // Constants from Cole.  Will kConstant them once the feedback loop
+  // is complete.
+  NSRect drawFrame = NSInsetRect(cellFrame, 1.5, 1.5);
+  ButtonType type = [[(NSControl*)controlView cell] tag];
+  switch (type) {
+    case kRightButtonType:
+      drawFrame.origin.x -= 20;
+    case kLeftButtonType:
+    case kLeftButtonWithShadowType:
+      drawFrame.size.width += 20;
+    default:
+      break;
+  }
+
+  const float radius = 3.5;
+  BOOL highlighted = [self isHighlighted];
+
+  // TODO(jrg): convert to GTMTheme
+
+  NSBezierPath *path = [NSBezierPath bezierPathWithRoundedRect:drawFrame
+                                                       xRadius:radius
+                                                       yRadius:radius];
+  NSBezierPath *outerPath =
+      [NSBezierPath bezierPathWithRoundedRect:NSInsetRect(drawFrame, -1, -1)
+                                      xRadius:radius + 1
+                                      yRadius:radius + 1];
+  NSGradient *gradient = nil;
+
+  if (highlighted) {
+    NSColor* start = [NSColor colorWithCalibratedHue:0.6
+                                          saturation:1.0
+                                          brightness:0.6
+                                               alpha:1.0];
+    NSColor* end = [NSColor colorWithCalibratedHue:0.6
+                                        saturation:1.0
+                                        brightness:0.8
+                                             alpha:1.0];
+    gradient = [[[NSGradient alloc] initWithStartingColor:start
+                                              endingColor:end] autorelease];
+  } else {
+    NSColor* start = [NSColor colorWithCalibratedWhite:1.0 alpha:1.0];
+    NSColor* end = [NSColor colorWithCalibratedWhite:0.90 alpha:1.0];
+    gradient = [[[NSGradient alloc] initWithStartingColor:start
+                                              endingColor:end] autorelease];
+  }
+
+  [[NSColor colorWithCalibratedWhite:1.0 alpha:0.25] set];
+  [outerPath stroke];
+  [gradient drawInBezierPath:path angle:90.0];
+  [[NSColor colorWithCalibratedWhite:0.0 alpha:0.15] set];
+  [path stroke];
+
+  if (type == kLeftButtonWithShadowType) {
+    NSRect borderRect, contentRect;
+    NSDivideRect(cellFrame, &borderRect, &contentRect, 1.0, NSMaxXEdge);
+    [[NSColor colorWithCalibratedWhite:0.0 alpha:0.15] set];
+    NSRectFillUsingOperation(NSInsetRect(borderRect, 0, 2),
+                             NSCompositeSourceOver);
+  }
+
+  [self drawInteriorWithFrame:NSOffsetRect(cellFrame, 0, 1) inView:controlView];
+}
+
+@end
diff --git a/chrome/browser/cocoa/gradient_button_cell_unittest.mm b/chrome/browser/cocoa/gradient_button_cell_unittest.mm
new file mode 100644
index 0000000..42abf28
--- /dev/null
+++ b/chrome/browser/cocoa/gradient_button_cell_unittest.mm
@@ -0,0 +1,42 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/scoped_nsobject.h"
+#import "chrome/browser/cocoa/gradient_button_cell.h"
+#import "chrome/browser/cocoa/cocoa_test_helper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class GradientButtonCellTest : public testing::Test {
+ public:
+  GradientButtonCellTest() {
+    NSRect frame = NSMakeRect(0, 0, 50, 30);
+    view_.reset([[NSButton alloc] initWithFrame:frame]);
+    scoped_nsobject<GradientButtonCell> cell([[GradientButtonCell alloc]
+                                              initTextCell:@"Testing"]);
+    [view_ setCell:cell.get()];
+    [cocoa_helper_.contentView() addSubview:view_.get()];
+  }
+
+  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
+  scoped_nsobject<NSButton> view_;
+};
+
+// Test adding/removing from the view hierarchy, mostly to ensure nothing
+// leaks or crashes.
+TEST_F(GradientButtonCellTest, AddRemove) {
+  EXPECT_EQ(cocoa_helper_.contentView(), [view_ superview]);
+  [view_.get() removeFromSuperview];
+  EXPECT_FALSE([view_ superview]);
+}
+
+// Test drawing, mostly to ensure nothing leaks or crashes.
+TEST_F(GradientButtonCellTest, Display) {
+  [view_ display];
+}
+
+}  // namespace
diff --git a/chrome/browser/cocoa/toolbar_button_cell.h b/chrome/browser/cocoa/toolbar_button_cell.h
index a0991d2..da12036 100644
--- a/chrome/browser/cocoa/toolbar_button_cell.h
+++ b/chrome/browser/cocoa/toolbar_button_cell.h
@@ -6,20 +6,21 @@
 #define CHROME_BROWSER_COCOA_TOOLBAR_BUTTON_CELL_H_
 
 #import <Cocoa/Cocoa.h>
+#import "chrome/browser/cocoa/gradient_button_cell.h"
 
-// A button cell that handles drawing/highlighting of buttons in the
-// toolbar bar. The appearance is determined by setting the cell's tag (not the
-// view's) to one of the constants below (ButtonType).
+// A button cell for the toolbar.
 
-enum {
-  kLeftButtonType = -1,
-  kLeftButtonWithShadowType = -2,
-  kStandardButtonType = 0,
-  kRightButtonType = 1,
-};
-typedef NSInteger ButtonType;
+// TODO(jrg): Why have a class at all when the base class does it all?
+// I anticipate making changes for extensions.  Themes may also
+// require changes.  I don't yet know if those will be common across
+// the toolbar and bookmark bar or not.  The initial CL which made
+// this empty was the use of the base class for both toolbar and
+// bookmark bar button cells.  It seems wasteful to remove the files
+// then add them back in soon after.
+// TODO(jrg): If no differences come up, remove this file and use
+// the base class explicitly for both the toolbar and bookmark bar.
 
-@interface ToolbarButtonCell : NSButtonCell {
+@interface ToolbarButtonCell : GradientButtonCell {
 }
 @end
 
diff --git a/chrome/browser/cocoa/toolbar_button_cell.mm b/chrome/browser/cocoa/toolbar_button_cell.mm
index 22030e6..b434df9 100644
--- a/chrome/browser/cocoa/toolbar_button_cell.mm
+++ b/chrome/browser/cocoa/toolbar_button_cell.mm
@@ -2,82 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/cocoa/toolbar_button_cell.h"
+#import "chrome/browser/cocoa/toolbar_button_cell.h"
 
 @implementation ToolbarButtonCell
-
-- (NSBackgroundStyle)interiorBackgroundStyle {
-  return [self isHighlighted] ?
-      NSBackgroundStyleLowered : NSBackgroundStyleRaised;
-}
-
-- (void)awakeFromNib {
-  [[self image] setTemplate:YES];
-}
-
-- (void)drawWithFrame:(NSRect)cellFrame inView:(NSView *)controlView {
-  NSRect drawFrame = NSInsetRect(cellFrame, 1.5, 1.5);
-  ButtonType type = [[(NSControl*)controlView cell] tag];
-  switch (type) {
-    case kRightButtonType:
-      drawFrame.origin.x -= 20;
-    case kLeftButtonType:
-    case kLeftButtonWithShadowType:
-      drawFrame.size.width += 20;
-    default:
-      break;
-  }
-
-  float radius = 3.5;
-  BOOL highlighted = [self isHighlighted];
-
-  NSBezierPath *path = [NSBezierPath bezierPathWithRoundedRect:drawFrame
-                                                       xRadius:radius
-                                                       yRadius:radius];
-  NSBezierPath *outerPath =
-      [NSBezierPath bezierPathWithRoundedRect:NSInsetRect(drawFrame, -1, -1)
-                                      xRadius:radius + 1
-                                      yRadius:radius + 1];
-  NSGradient *gradient = nil;
-
-  if (highlighted) {
-    NSColor* start = [NSColor colorWithCalibratedHue:0.6
-                                          saturation:1.0
-                                          brightness:0.6
-                                               alpha:1.0];
-    NSColor* end = [NSColor colorWithCalibratedHue:0.6
-                                        saturation:1.0
-                                        brightness:0.8
-                                             alpha:1.0];
-    gradient = [[[NSGradient alloc] initWithStartingColor:start
-                                              endingColor:end] autorelease];
-  } else {
-    NSColor* start = [NSColor colorWithCalibratedWhite:1.0 alpha:1.0];
-    NSColor* end = [NSColor colorWithCalibratedWhite:0.90 alpha:1.0];
-    gradient = [[[NSGradient alloc] initWithStartingColor:start
-                                              endingColor:end] autorelease];
-  }
-
-  [[NSColor colorWithCalibratedWhite:1.0 alpha:0.25] set];
-  [outerPath stroke];
-  [gradient drawInBezierPath:path angle:90.0];
-  [[NSColor colorWithCalibratedWhite:0.0 alpha:0.15] set];
-  [path stroke];
-
-  if (type == kLeftButtonWithShadowType) {
-    NSRect borderRect, contentRect;
-    NSDivideRect(cellFrame, &borderRect, &contentRect, 1.0, NSMaxXEdge);
-    [[NSColor colorWithCalibratedWhite:0.0 alpha:0.15] set];
-    NSRectFillUsingOperation(NSInsetRect(borderRect, 0, 2),
-                             NSCompositeHighlight);
-  }
-  CGContextRef ctx = static_cast<CGContextRef>
-    ([[NSGraphicsContext currentContext] graphicsPort]);
-
-  CGContextSetAlpha(ctx, 0.8);
-  CGContextBeginTransparencyLayer(ctx, NULL);
-  [self drawInteriorWithFrame:NSOffsetRect(cellFrame, 0, 1) inView:controlView];
-  CGContextEndTransparencyLayer(ctx);
-}
-
 @end
diff --git a/chrome/browser/cocoa/toolbar_button_cell_unittest.mm b/chrome/browser/cocoa/toolbar_button_cell_unittest.mm
index e1cb467..2963827 100644
--- a/chrome/browser/cocoa/toolbar_button_cell_unittest.mm
+++ b/chrome/browser/cocoa/toolbar_button_cell_unittest.mm
@@ -2,41 +2,5 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import <Cocoa/Cocoa.h>
-
-#include "base/scoped_nsobject.h"
-#import "chrome/browser/cocoa/toolbar_button_cell.h"
-#import "chrome/browser/cocoa/cocoa_test_helper.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace {
-
-class ToolbarButtonCellTest : public testing::Test {
- public:
-  ToolbarButtonCellTest() {
-    NSRect frame = NSMakeRect(0, 0, 50, 30);
-    view_.reset([[NSButton alloc] initWithFrame:frame]);
-    scoped_nsobject<ToolbarButtonCell> cell([[ToolbarButtonCell alloc]
-                                              initTextCell:@"Testing"]);
-    [view_ setCell:cell.get()];
-    [cocoa_helper_.contentView() addSubview:view_.get()];
-  }
-
-  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
-  scoped_nsobject<NSButton> view_;
-};
-
-// Test adding/removing from the view hierarchy, mostly to ensure nothing
-// leaks or crashes.
-TEST_F(ToolbarButtonCellTest, AddRemove) {
-  EXPECT_EQ(cocoa_helper_.contentView(), [view_ superview]);
-  [view_.get() removeFromSuperview];
-  EXPECT_FALSE([view_ superview]);
-}
-
-// Test drawing, mostly to ensure nothing leaks or crashes.
-TEST_F(ToolbarButtonCellTest, Display) {
-  [view_ display];
-}
-
-}  // namespace
+// This file is intentionally empty; there is no code in
+// ToolbarButtonCell to test.
diff --git a/chrome/browser/cocoa/toolbar_view.h b/chrome/browser/cocoa/toolbar_view.h
index 761b3db..807a7e8 100644
--- a/chrome/browser/cocoa/toolbar_view.h
+++ b/chrome/browser/cocoa/toolbar_view.h
@@ -6,11 +6,13 @@
 #define CHROME_BROWSER_COCOA_TOOLBAR_VIEW_H_
 
 #import <Cocoa/Cocoa.h>
+#import "chrome/browser/cocoa/background_gradient_view.h"
 
-// A view that handles any special rendering the toolbar, mostly just painting
-// a gradient.
+// A view that handles any special rendering of the toolbar bar.  At
+// this time it only draws a gradient.  Future changes (e.g. themes)
+// may require new functionality here.
 
-@interface ToolbarView : NSView {
+@interface ToolbarView : BackgroundGradientView {
 }
 @end
 
diff --git a/chrome/browser/cocoa/toolbar_view.mm b/chrome/browser/cocoa/toolbar_view.mm
index 87d16e3..7220241 100644
--- a/chrome/browser/cocoa/toolbar_view.mm
+++ b/chrome/browser/cocoa/toolbar_view.mm
@@ -5,23 +5,4 @@
 #include "chrome/browser/cocoa/toolbar_view.h"
 
 @implementation ToolbarView
-
-- (void)drawRect:(NSRect)rect {
-  BOOL isKey = [[self window] isKeyWindow];
-  rect = [self bounds];
-
-  NSColor* start =
-      [NSColor colorWithCalibratedWhite: isKey ? 0.95 : 0.98 alpha:1.0];
-  NSColor* end = [NSColor colorWithCalibratedWhite:0.90 alpha:1.0];
-  NSGradient *gradient =
-      [[[NSGradient alloc] initWithStartingColor:start endingColor:end]
-          autorelease];
-  [gradient drawInRect:[self bounds] angle:270.0];
-  NSRect borderRect, contentRect;
-  NSDivideRect(rect, &borderRect, &contentRect, 1, NSMinYEdge);
-
-  [[NSColor colorWithDeviceWhite:0.0 alpha:0.3] set];
-  NSRectFillUsingOperation(borderRect, NSCompositeSourceOver);
-}
-
 @end
diff --git a/chrome/browser/cocoa/toolbar_view_unittest.mm b/chrome/browser/cocoa/toolbar_view_unittest.mm
index ab04aed..c8e8930 100644
--- a/chrome/browser/cocoa/toolbar_view_unittest.mm
+++ b/chrome/browser/cocoa/toolbar_view_unittest.mm
@@ -2,38 +2,4 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import <Cocoa/Cocoa.h>
-
-#include "base/scoped_nsobject.h"
-#import "chrome/browser/cocoa/toolbar_view.h"
-#import "chrome/browser/cocoa/cocoa_test_helper.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace {
-
-class ToolbarViewTest : public testing::Test {
- public:
-  ToolbarViewTest() {
-    NSRect frame = NSMakeRect(0, 0, 100, 30);
-    view_.reset([[ToolbarView alloc] initWithFrame:frame]);
-    [cocoa_helper_.contentView() addSubview:view_.get()];
-  }
-
-  scoped_nsobject<ToolbarView> view_;
-  CocoaTestHelper cocoa_helper_;  // Inits Cocoa, creates window, etc...
-};
-
-// Test adding/removing from the view hierarchy, mostly to ensure nothing
-// leaks or crashes.
-TEST_F(ToolbarViewTest, AddRemove) {
-  EXPECT_EQ(cocoa_helper_.contentView(), [view_ superview]);
-  [view_.get() removeFromSuperview];
-  EXPECT_FALSE([view_ superview]);
-}
-
-// Test drawing, mostly to ensure nothing leaks or crashes.
-TEST_F(ToolbarViewTest, Display) {
-  [view_ display];
-}
-
-}  // namespace
+// This file is intentionally empty; there is no code in ToolbarView to test.
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index dc498ed..cc7aa96 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -668,10 +668,18 @@
         'browser/chrome_thread.h',
         'browser/cocoa/about_window_controller.h',
         'browser/cocoa/about_window_controller.mm',
+        'browser/cocoa/background_gradient_view.h',
+        'browser/cocoa/background_gradient_view.mm',
         'browser/cocoa/base_view.h',
         'browser/cocoa/base_view.mm',
+        'browser/cocoa/bookmark_bar_bridge.h',
+        'browser/cocoa/bookmark_bar_bridge.mm',
         'browser/cocoa/bookmark_bar_controller.h',
         'browser/cocoa/bookmark_bar_controller.mm',
+        'browser/cocoa/bookmark_bar_view.h',
+        'browser/cocoa/bookmark_bar_view.mm',
+        'browser/cocoa/bookmark_button_cell.h',
+        'browser/cocoa/bookmark_button_cell.mm',
         'browser/cocoa/bookmark_menu_bridge.h',
         'browser/cocoa/bookmark_menu_bridge.mm',
         'browser/cocoa/bookmark_menu_cocoa_controller.h',
@@ -698,6 +706,8 @@
         'browser/cocoa/find_bar_view.mm',
         'browser/cocoa/first_run_dialog.h',
         'browser/cocoa/first_run_dialog.mm',
+        'browser/cocoa/gradient_button_cell.h',
+        'browser/cocoa/gradient_button_cell.mm',
         'browser/cocoa/grow_box_view.h',
         'browser/cocoa/grow_box_view.m',
         'browser/cocoa/location_bar_cell.h',
@@ -3095,7 +3105,11 @@
         # exclude them from non-Mac builds.
         'browser/cocoa/about_window_controller_unittest.mm',
         'browser/cocoa/base_view_unittest.mm',
+        'browser/cocoa/background_gradient_view_unittest.mm',
+        'browser/cocoa/bookmark_bar_bridge_unittest.mm',
         'browser/cocoa/bookmark_bar_controller_unittest.mm',
+        'browser/cocoa/bookmark_bar_view_unittest.mm',
+        'browser/cocoa/bookmark_button_cell_unittest.mm',
         'browser/cocoa/bookmark_menu_bridge_unittest.mm',
         'browser/cocoa/bookmark_menu_cocoa_controller_unittest.mm',
         'browser/cocoa/browser_window_cocoa_unittest.mm',
@@ -3107,6 +3121,7 @@
         'browser/cocoa/find_bar_view_unittest.mm',
         'browser/cocoa/location_bar_cell_unittest.mm',
         'browser/cocoa/location_bar_view_mac_unittest.mm',
+        'browser/cocoa/gradient_button_cell_unittest.mm',
         'browser/cocoa/grow_box_view_unittest.mm',
         'browser/cocoa/preferences_window_controller_unittest.mm',
         'browser/cocoa/sad_tab_view_unittest.mm',
@@ -4393,6 +4408,8 @@
           # In gyp, booleans are 0/1 not True/False.
           'suppress_wildcard': 1,
           'type': 'none',
+	  # If you add new tests here you may need to update the croc configs.
+	  # E.g. build/{linux|mac}/chrome_linux.croc
           'dependencies': [
             '../base/base.gyp:base_unittests',
             '../media/media.gyp:media_unittests',