Apply theme color to bookmark button text.

BUG=http://crbug.com/17608

TEST=Add some bookmarks (you must use both folders and marks).  Apply
a theme and make sure theme color applies bookmark button text for all
buttons (including "Other bookmarks").  Create new window and make
sure bookmark buttons still fine in new window.  Quit and restart
Chrome.  Make sure theme color still applied to bookmark button text
in the same way as before quitting (e.g. same color and still applies
to all).  Change theme 1 more time and verify color.

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

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

6 files changed, 129 insertions, 13 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index aadec09..1f7bd8c 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -232,6 +232,7 @@ willAnimateFromState:(bookmarks::VisualState)oldState
 - (NSButton*)offTheSideButton;
 - (NSButton*)otherBookmarksButton;
 - (BookmarkNode*)nodeFromMenuItem:(id)sender;
+- (void)updateTheme:(GTMTheme*)theme;
 @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 d1c9861..7b8bf6a 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -182,6 +182,13 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
     folderImage_.reset([rb.GetNSImageNamed(IDR_BOOKMARK_BAR_FOLDER) retain]);
     defaultImage_.reset([rb.GetNSImageNamed(IDR_DEFAULT_FAVICON) retain]);
 
+  // Register for theme changes.
+  NSNotificationCenter* defaultCenter = [NSNotificationCenter defaultCenter];
+  [defaultCenter addObserver:self
+                    selector:@selector(themeDidChangeNotification:)
+                        name:kGTMThemeDidChangeNotification
+                      object:nil];
+
     // This call triggers an awakeFromNib, which builds the bar, which
     // might uses folderImage_.  So make sure it happens after
     // folderImage_ is loaded.
@@ -204,6 +211,30 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   [super dealloc];
 }
 
+// Adapt appearance of buttons to the current theme. Called after
+// theme changes, or when our view is added to the view hierarchy.
+// Oddly, the view pings us instead of us pinging our view.  This is
+// because our trigger is an [NSView viewWillMoveToWindow:], which the
+// controller doesn't normally know about.  Otherwise we don't have
+// access to the theme before we know what window we will be on.
+- (void)updateTheme:(GTMTheme*)theme {
+  if (!theme)
+    return;
+  NSColor* color = [theme textColorForStyle:GTMThemeStyleBookmarksBarButton
+                                      state:GTMThemeStateActiveWindow];
+  for (BookmarkButton* button in buttons_.get()) {
+    BookmarkButtonCell* cell = [button cell];
+    [cell setTextColor:color];
+  }
+  [[otherBookmarksButton_ cell] setTextColor:color];
+}
+
+// Called after the current theme has changed.
+- (void)themeDidChangeNotification:(NSNotification*)aNotification {
+  GTMTheme* theme = [aNotification object];
+  [self updateTheme:theme];
+}
+
 - (void)awakeFromNib {
   // We default to NOT open, which means height=0.
   DCHECK([[self view] isHidden]);  // Hidden so it's OK to change.
@@ -866,7 +897,8 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 - (NSCell*)cellForBookmarkNode:(const BookmarkNode*)node {
   NSString* title = base::SysWideToNSString(node->GetTitle());
   BookmarkButtonCell* cell =
-      [[[BookmarkButtonCell alloc] initTextCell:nil] autorelease];
+    [[[BookmarkButtonCell alloc] initTextCell:nil]
+      autorelease];
   DCHECK(cell);
   [cell setRepresentedObject:[NSValue valueWithPointer:node]];
 
@@ -1088,6 +1120,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
   [self clearBookmarkBar];
   [self addNodesToButtonList:node];
   [self createOtherBookmarksButton];
+  [self updateTheme:[[self view] gtm_theme]];
   [self resizeButtons];
   [self positionOffTheSideButton];
   [self addNonBookmarkButtonsToView];
@@ -1148,7 +1181,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
     void* pointer = [[cell representedObject] pointerValue];
     const BookmarkNode* cellnode = static_cast<const BookmarkNode*>(pointer);
     if (cellnode == node) {
-      [cell setBookmarkCellText:[cell title]
+      [cell setBookmarkCellText:nil
                           image:[self getFavIconForNode:node]];
       // Adding an image means we might need more room for the
       // bookmark.  Test for it by growing the button (if needed)
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index 8c14dee..91c9fb4 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -79,6 +79,27 @@
 
 @end
 
+@interface FakeTheme : GTMTheme {
+  scoped_nsobject<NSColor> color_;
+}
+@end
+
+@implementation FakeTheme
+- (id)initWithColor:(NSColor*)color {
+  if ((self = [super init])) {
+    color_.reset([color retain]);
+  }
+  return self;
+}
+
+- (NSColor*)textColorForStyle:(GTMThemeStyle)style
+                        state:(GTMThemeState)state {
+  return color_.get();
+}
+@end
+
+
+
 namespace {
 
 static const int kContentAreaHeight = 500;
@@ -847,6 +868,31 @@ TEST_F(BookmarkBarControllerTest, TestDragButton) {
   EXPECT_TRUE([[[[bar_ buttons] objectAtIndex:2] title] isEqual:@"a"]);
 }
 
+// Fake a theme with colored text.  Apply it and make sure bookmark
+// buttons have the same colored text.  Repeat more than once.
+TEST_F(BookmarkBarControllerTest, TestThemedButton) {
+  BookmarkModel* model = helper_.profile()->GetBookmarkModel();
+  model->SetURLStarred(GURL("http://www.foo.com"), L"small", true);
+  BookmarkButton* button = [[bar_ buttons] objectAtIndex:0];
+  EXPECT_TRUE(button);
+
+  NSArray* colors = [NSArray arrayWithObjects:[NSColor redColor],
+                                              [NSColor blueColor],
+                                              nil];
+  for (NSColor* color in colors) {
+    scoped_nsobject<FakeTheme> theme([[FakeTheme alloc] initWithColor:color]);
+    [bar_ updateTheme:theme.get()];
+    NSAttributedString* astr = [button attributedTitle];
+    EXPECT_TRUE(astr);
+    EXPECT_TRUE([[astr string] isEqual:@"small"]);
+    // Pick a char in the middle to test (index 3)
+    NSDictionary* attributes = [astr attributesAtIndex:3 effectiveRange:NULL];
+    NSColor* newColor =
+        [attributes objectForKey:NSForegroundColorAttributeName];
+    EXPECT_TRUE([newColor isEqual:color]);
+  }
+}
+
 // TODO(viettrungluu): figure out how to test animations.
 
 }  // namespace
diff --git a/chrome/browser/cocoa/bookmark_bar_view.mm b/chrome/browser/cocoa/bookmark_bar_view.mm
index e6976af..31d167f 100644
--- a/chrome/browser/cocoa/bookmark_bar_view.mm
+++ b/chrome/browser/cocoa/bookmark_bar_view.mm
@@ -40,9 +40,12 @@
   [self registerForDraggedTypes:types];
 }
 
-- (void)viewDidMoveToWindow {
-  if ([self window])
-    [self updateTheme:[self gtm_theme]];
+// We need the theme to color the bookmark buttons properly.  But our
+// controller desn't have access to it until it's placed in the view
+// hierarchy.  This is the spot where we close the loop.
+- (void)viewWillMoveToWindow:(NSWindow*)window {
+  [self updateTheme:[window gtm_theme]];
+  [controller_ updateTheme:[window gtm_theme]];
 }
 
 // Called after the current theme has changed.
diff --git a/chrome/browser/cocoa/bookmark_button_cell.h b/chrome/browser/cocoa/bookmark_button_cell.h
index f8ca25d..89d461b 100644
--- a/chrome/browser/cocoa/bookmark_button_cell.h
+++ b/chrome/browser/cocoa/bookmark_button_cell.h
@@ -13,11 +13,16 @@
 
 @interface BookmarkButtonCell : GradientButtonCell<NSMenuDelegate> {
 }
-// |-setBookmarkCellText:image:| is used to set the text and image
-// of a BookmarkButtonCell, and align the image to the left (NSImageLeft)
-// if there is text in the title, and centered (NSImageCenter) if there is
+// |-setBookmarkCellText:image:| is used to set the text and image of
+// a BookmarkButtonCell, and align the image to the left (NSImageLeft)
+// if there is text in the title, and centered (NSImageCenter) if
+// there is not.  If |title| is nil, do not reset the title.
 - (void)setBookmarkCellText:(NSString*)title
                       image:(NSImage*)image;
+
+// Set the color of text in this cell.
+- (void)setTextColor:(NSColor*)color;
+
 @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
index 147ae04..cd8e5a8 100644
--- a/chrome/browser/cocoa/bookmark_button_cell.mm
+++ b/chrome/browser/cocoa/bookmark_button_cell.mm
@@ -2,13 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/logging.h"
 #import "chrome/browser/cocoa/bookmark_button_cell.h"
 #import "chrome/browser/cocoa/bookmark_menu.h"
 #import "third_party/GTM/AppKit/GTMTheme.h"
 
 @implementation BookmarkButtonCell
 
-- (id)initTextCell:(NSString *)string {
+- (id)initTextCell:(NSString*)string {
   if ((self = [super initTextCell:string])) {
     [self setButtonType:NSMomentaryPushInButton];
     [self setBezelStyle:NSShadowlessSquareBezelStyle];
@@ -41,15 +42,21 @@
                                            withString:@" "];
   title = [title stringByReplacingOccurrencesOfString:@"\r"
                                            withString:@" "];
+  // Center the image if we have a title, or if there already was a
+  // title set.
+  BOOL hasTitle = (([title length] > 0) ||
+                   ([[self title] length] > 0));
   if (image) {
     [self setImage:image];
-    if ([title length] < 1) {
-      [self setImagePosition:NSImageOnly];
-    } else {
+    if (hasTitle) {
       [self setImagePosition:NSImageLeft];
+    } else {
+      [self setImagePosition:NSImageOnly];
     }
   }
-  [self setTitle:title];
+
+  if (title)
+    [self setTitle:title];
 }
 
 // We share the context menu among all bookmark buttons.  To allow us
@@ -61,4 +68,25 @@
   return menu;
 }
 
+// Unfortunately, NSCell doesn't already have something like this.
+// TODO(jrg): consider placing in GTM.
+- (void)setTextColor:(NSColor*)color {
+  scoped_nsobject<NSMutableParagraphStyle> style([NSMutableParagraphStyle new]);
+  [style setAlignment:NSCenterTextAlignment];
+  NSDictionary* dict = [NSDictionary
+                         dictionaryWithObjectsAndKeys:color,
+                         NSForegroundColorAttributeName,
+                         [self font], NSFontAttributeName,
+                         style.get(), NSParagraphStyleAttributeName,
+                         nil];
+  scoped_nsobject<NSAttributedString> ats([[NSAttributedString alloc]
+                                            initWithString:[self title]
+                                                attributes:dict]);
+  NSButton* button = static_cast<NSButton*>([self controlView]);
+  if (button) {
+    DCHECK([button isKindOfClass:[NSButton class]]);
+    [button setAttributedTitle:ats.get()];
+  }
+}
+
 @end