[Mac] Refactor bookmark bar controller

chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm

Index: chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
index 4a0ef76af8fd63a1c480b931aae42591b363dd4b..bd82f03ffcdf6430c706f6654cb5246b46ea11db 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.mm
@@ -37,6 +37,19 @@ const int kDefaultFontSize = 12;
 
 };  // namespace
 
+@interface BookmarkButtonCell (Private)
+// Returns YES if the cell is the offTheSide button cell.
+- (BOOL)isOffTheSideButtonCell;
+- (void)configureBookmarkButtonCell;
+- (void)applyTextColor;
+// Returns the title the button cell displays. Note that a button cell can
+// have a title string assigned but it won't be visible if its image position
+// is NSImageOnly.
+- (NSString*)visibleTitle;
+// Returns the dictionary of attributes to associate with the button title.
+- (NSDictionary*)titleTextAttributes;
+@end
+
 // TODO(lgrey): Bake setting the chevron image into this
 // class instead of setting it externally.
 @interface OffTheSideButtonCell : BookmarkButtonCell
@@ -65,19 +78,6 @@ const int kDefaultFontSize = 12;
 
 @end
 
-@interface BookmarkButtonCell(Private)
-// Returns YES if the cell is the offTheSide button cell.
-- (BOOL)isOffTheSideButtonCell;
-- (void)configureBookmarkButtonCell;
-- (void)applyTextColor;
-// Returns the title the button cell displays. Note that a button cell can
-// have a title string assigned but it won't be visible if its image position
-// is NSImageOnly.
-- (NSString*)visibleTitle;
-// Returns the dictionary of attributes to associate with the button title.
-- (NSDictionary*)titleTextAttributes;
-@end
-
 
 @implementation BookmarkButtonCell
 
@@ -112,6 +112,22 @@ const int kDefaultFontSize = 12;
   return [[[OffTheSideButtonCell alloc] init] autorelease];
 }
 
++ (CGFloat)cellWidthForNode:(const bookmarks::BookmarkNode*)node
+                      image:(NSImage*)image {
+  NSString* title =
+      [self cleanTitle:base::SysUTF16ToNSString(node->GetTitle())];
+  CGFloat width = kIconLeftPadding + [image size].width;
+  if ([title length] > 0) {
+    CGSize titleSize = [title sizeWithAttributes:@{
+      NSParagraphStyleAttributeName : [self paragraphStyleForBookmarkBarCell]
+    }];
+    width += kIconTextSpacer + std::ceil(titleSize.width) + kTextRightPadding;
+  } else {
+    width += kIconLeftPadding;
+  }
+  return width;
+}
+
 - (id)initForNode:(const BookmarkNode*)node
              text:(NSString*)text
             image:(NSImage*)image
@@ -120,7 +136,7 @@ const int kDefaultFontSize = 12;
     menuController_ = menuController;
     [self configureBookmarkButtonCell];
     [self setTextColor:[NSColor blackColor]];
-    [self setBookmarkNode:node];
+    [self setBookmarkNode:node image:image];
     // When opening a bookmark folder, the default behavior is that the
     // favicon is greyed when menu item is hovered with the mouse cursor.
     // When using NSNoCellMask, the favicon won't be greyed when menu item
@@ -130,15 +146,6 @@ const int kDefaultFontSize = 12;
     // It makes the behavior of the bookmark folder consistent with hovering
     // on the bookmark bar.
     [self setHighlightsBy:NSNoCellMask];
-
-    if (node) {
-      NSString* title = base::SysUTF16ToNSString(node->GetTitle());
-      [self setBookmarkCellText:title image:image];
-    } else {
-      [self setEmpty:YES];
-      [self setBookmarkCellText:l10n_util::GetNSString(IDS_MENU_EMPTY_SUBMENU)
-                          image:nil];
-    }
   }
 
   return self;
@@ -185,7 +192,7 @@ const int kDefaultFontSize = 12;
   [self setShowsBorderOnlyWhileMouseInside:YES];
   [self setControlSize:NSSmallControlSize];
   [self setAlignment:NSLeftTextAlignment];
-  [self setFont:[NSFont systemFontOfSize:kDefaultFontSize]];
+  [self setFont:[[self class] fontForBookmarkBarCell]];
   [self setBordered:NO];
   [self setBezeled:NO];
   [self setWraps:NO];
@@ -220,12 +227,8 @@ const int kDefaultFontSize = 12;
 
 - (void)setBookmarkCellText:(NSString*)title
                       image:(NSImage*)image {
-  title = [title stringByReplacingOccurrencesOfString:@"\n"
-                                           withString:@" "];
-  title = [title stringByReplacingOccurrencesOfString:@"\r"
-                                           withString:@" "];
-
-  if ([title length]) {
+  title = [[self class] cleanTitle:title];
+  if ([title length] && ![self isOffTheSideButtonCell]) {
     [self setImagePosition:NSImageLeft];
     [self setTitle:title];
   } else if ([self isFolderButtonCell]) {
@@ -245,7 +248,19 @@ const int kDefaultFontSize = 12;
 }
 
 - (void)setBookmarkNode:(const BookmarkNode*)node {
+  [self setBookmarkNode:node image:nil];
+}
+
+- (void)setBookmarkNode:(const BookmarkNode*)node image:(NSImage*)image {
   [self setRepresentedObject:[NSValue valueWithPointer:node]];
+  if (node) {
+    NSString* title = base::SysUTF16ToNSString(node->GetTitle());
+    [self setBookmarkCellText:title image:image];
+  } else {
+    [self setEmpty:YES];
+    [self setBookmarkCellText:l10n_util::GetNSString(IDS_MENU_EMPTY_SUBMENU)
+                        image:nil];
+  }
 }
 
 - (const BookmarkNode*)bookmarkNode {
@@ -315,10 +330,7 @@ const int kDefaultFontSize = 12;
 }
 
 - (NSDictionary*)titleTextAttributes {
-  base::scoped_nsobject<NSMutableParagraphStyle> style(
-      [NSMutableParagraphStyle new]);
-  [style setAlignment:NSNaturalTextAlignment];
-  [style setLineBreakMode:NSLineBreakByTruncatingTail];
+  NSParagraphStyle* style = [[self class] paragraphStyleForBookmarkBarCell];
   NSColor* textColor = textColor_.get();
   if (!textColor) {
     textColor = [NSColor blackColor];
@@ -332,10 +344,10 @@ const int kDefaultFontSize = 12;
   }
 
   return @{
-     NSFontAttributeName : theFont,
-     NSForegroundColorAttributeName : textColor,
-     NSParagraphStyleAttributeName : style.get(),
-     NSKernAttributeName : [NSNumber numberWithFloat:0.2]
+    NSFontAttributeName : theFont,
+    NSForegroundColorAttributeName : textColor,
+    NSParagraphStyleAttributeName : style,
+    NSKernAttributeName : [NSNumber numberWithFloat:0.2]
   };
 }
 
@@ -441,5 +453,23 @@ const int kDefaultFontSize = 12;
   return 0.0;
 }
 
++ (NSFont*)fontForBookmarkBarCell {

[Elly Fong-Jones, 2017/03/23 15:40:22]

I like this pattern of breaking sorta-constants out into descriptively named
helpers.

+  return [NSFont systemFontOfSize:kDefaultFontSize];
+}
+
++ (NSParagraphStyle*)paragraphStyleForBookmarkBarCell {
+  NSMutableParagraphStyle* style = [[NSMutableParagraphStyle alloc] init];
+  [style setAlignment:NSNaturalTextAlignment];
+  [style setLineBreakMode:NSLineBreakByTruncatingTail];
+  return [style autorelease];
+}
+
+// Returns |title| with newlines and line feeds replaced with
+// spaces.
++ (NSString*)cleanTitle:(NSString*)title {
+  title = [title stringByReplacingOccurrencesOfString:@"\n" withString:@" "];
+  title = [title stringByReplacingOccurrencesOfString:@"\r" withString:@" "];
+  return title;
+}
 
 @end