Mac: correct favicons in bookmarks bar (for pages w/o one and for subfolders).

chrome/browser/cocoa/bookmark_bar_controller.mm

 // 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 "app/l10n_util_mac.h"
 #include "app/resource_bundle.h"
 #include "base/mac_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/bookmarks/bookmark_editor.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
@@ skipping 10 matching lines @@
 #import "chrome/browser/cocoa/bookmark_menu_cocoa_controller.h"
 #import "chrome/browser/cocoa/event_utils.h"
 #import "chrome/browser/cocoa/menu_button.h"
 #import "chrome/browser/cocoa/toolbar_controller.h"
 #import "chrome/browser/cocoa/view_resizer.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
+#include "grit/app_resources.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "skia/ext/skia_utils_mac.h"
 #import "third_party/mozilla/include/NSPasteboard+Utils.h"
 
 // Specialization of NSButton that responds to middle-clicks. By default,
 // NSButton ignores them.
 @interface BookmarkButton : NSButton
 @end
 
 @implementation BookmarkButton
 - (void)otherMouseUp:(NSEvent*) event {
   [self performClick:self];
 }
 @end
 
 @interface BookmarkBarController(Private)
 - (void)applyContentAreaOffset:(BOOL)apply immediately:(BOOL)immediately;
 - (void)showBookmarkBar:(BOOL)enable immediately:(BOOL)immediately;
 - (void)addNode:(const BookmarkNode*)child toMenu:(NSMenu*)menu;
 - (void)addFolderNode:(const BookmarkNode*)node toMenu:(NSMenu*)menu;
 - (void)tagEmptyMenu:(NSMenu*)menu;
 - (void)clearMenuTagMap;
 - (int)preferredHeight;
 - (void)addNonBookmarkButtonsToView;
 - (void)addButtonsToView;
 - (void)resizeButtons;
 - (void)centerNoItemsLabel;
+- (NSImage*)getFavIconForNode:(const BookmarkNode*)node;

[pink (ping after 24hrs), 2009/11/03 19:42:44]

the normal obj-c method naming conventions avoid the form "getFoo".
favIconForNode: would work just fine and would be consistent with the rest of
our naming.

 @end
 
 @implementation BookmarkBarController
 
 - (id)initWithBrowser:(Browser*)browser
          initialWidth:(float)initialWidth
      compressDelegate:(id<ToolbarCompressable>)compressDelegate
        resizeDelegate:(id<ViewResizer>)resizeDelegate
           urlDelegate:(id<BookmarkURLOpener>)urlDelegate {
   if ((self = [super initWithNibName:@"BookmarkBar"
                               bundle:mac_util::MainAppBundle()])) {
     browser_ = browser;
     initialWidth_ = initialWidth;
     bookmarkModel_ = browser_->profile()->GetBookmarkModel();
     buttons_.reset([[NSMutableArray alloc] init]);
     compressDelegate_ = compressDelegate;
     resizeDelegate_ = resizeDelegate;
     urlDelegate_ = urlDelegate;
     tabObserver_.reset(
         new TabStripModelObserverBridge(browser_->tabstrip_model(), self));
 
     ResourceBundle& rb = ResourceBundle::GetSharedInstance();
     folderImage_.reset([rb.GetNSImageNamed(IDR_BOOKMARK_BAR_FOLDER) retain]);
+    defaultImage_.reset([rb.GetNSImageNamed(IDR_DEFAULT_FAVICON) retain]);
   }
   return self;
 }
 
 - (void)dealloc {
   // Remove our view from its superview so it doesn't attempt to reference
   // it when the controller is gone.
   //TODO(dmaclach): Remove -- http://crbug.com/25845
   [[self view] removeFromSuperview];
 
@@ skipping 258 matching lines @@
 }
 
 // Recursively add the given bookmark node and all its children to
 // menu, one menu item per node.
 - (void)addNode:(const BookmarkNode*)child toMenu:(NSMenu*)menu {
   NSString* title = [BookmarkMenuCocoaController menuTitleForNode:child];
   NSMenuItem* item = [[[NSMenuItem alloc] initWithTitle:title
                                                  action:nil
                                           keyEquivalent:@""] autorelease];
   [menu addItem:item];
+  [item setImage:[self getFavIconForNode:child]];
   if (child->is_folder()) {
     NSMenu* submenu = [[[NSMenu alloc] initWithTitle:title] autorelease];
     [menu setSubmenu:submenu forItem:item];
     if (child->GetChildCount()) {
       [self addFolderNode:child toMenu:submenu];  // potentially recursive
     } else {
       [self tagEmptyMenu:submenu];
     }
   } else {
     [item setTarget:self];
     [item setAction:@selector(openBookmarkMenuItem:)];
     [item setTag:[self menuTagFromNodeId:child->id()]];
     // Add a tooltip
     std::string url_string = child->GetURL().possibly_invalid_spec();
     NSString* tooltip = [NSString stringWithFormat:@"%@\n%s",
                                   base::SysWideToNSString(child->GetTitle()),
                                   url_string.c_str()];
     [item setToolTip:tooltip];
-    const SkBitmap& favicon = bookmarkModel_->GetFavIcon(child);
-    if (!favicon.isNull()) {
-      NSImage* image = gfx::SkBitmapToNSImage(favicon);
-      if (image) {
-        [item setImage:image];
-      }
-    }
   }
 }
 
 // Empty menus are odd; if empty, add something to look at.
 // Matches windows behavior.
 - (void)tagEmptyMenu:(NSMenu*)menu {
   NSString* empty_menu_title = l10n_util::GetNSString(IDS_MENU_EMPTY_SUBMENU);
   [menu addItem:[[[NSMenuItem alloc] initWithTitle:empty_menu_title
                                             action:NULL
                                      keyEquivalent:@""] autorelease]];
@@ skipping 193 matching lines @@
 
 // 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 {
   NSString* title = base::SysWideToNSString(node->GetTitle());
   NSButtonCell *cell = [[[BookmarkButtonCell alloc] initTextCell:nil]
                          autorelease];
   DCHECK(cell);
   [cell setRepresentedObject:[NSValue valueWithPointer:node]];
 
-  NSImage* image = NULL;
-  if (node->is_folder()) {
-    image = folderImage_;
-  } else {
-    const SkBitmap& favicon = bookmarkModel_->GetFavIcon(node);
-    if (!favicon.isNull()) {
-      image = gfx::SkBitmapToNSImage(favicon);
-    }
-  }
-  if (image) {
-    [cell setImage:image];
-    [cell setImagePosition:NSImageLeft];
-  }
+  [cell setImage:[self getFavIconForNode:node]];
+  [cell setImagePosition:NSImageLeft];
   [cell setTitle:title];
   [cell setMenu:buttonContextMenu_];
   return cell;
 }
 
 // Return an appropriate width for the given bookmark button cell.
 // The "+2" is needed because, sometimes, Cocoa is off by a tad.
 // Example: for a bookmark named "Moma" or "SFGate", it is one pixel
 // too small.  For "FBL" it is 2 pixels too small.
 // For a bookmark named "SFGateFooWoo", it is just fine.
@@ skipping 245 matching lines @@
 // this looks bad I may need to change widthForBookmarkButtonCell to
 // add space for an image even if not there on the assumption that
 // favicons will eventually load.
 - (void)nodeFavIconLoaded:(BookmarkModel*)model
                      node:(const BookmarkNode*)node {
   for (NSButton* button in buttons_.get()) {
     NSButtonCell* cell = [button cell];
     void* pointer = [[cell representedObject] pointerValue];
     const BookmarkNode* cellnode = static_cast<const BookmarkNode*>(pointer);
     if (cellnode == node) {
-      NSImage* image = gfx::SkBitmapToNSImage(bookmarkModel_->GetFavIcon(node));
-      if (image) {
-        [cell setImage:image];
-        [cell setImagePosition:NSImageLeft];
-        // Adding an image means we might need more room for the
-        // bookmark.  Test for it by growing the button (if needed)
-        // and shifting everything else over.
-        [self checkForBookmarkButtonGrowth:button];
-      }
-      return;
+      [cell setImage:[self getFavIconForNode:node]];
+      [cell setImagePosition:NSImageLeft];
+      // Adding an image means we might need more room for the
+      // bookmark.  Test for it by growing the button (if needed)
+      // and shifting everything else over.
+      [self checkForBookmarkButtonGrowth:button];
     }
   }
 }
 
 // TODO(jrg): for now this is brute force.
 - (void)nodeChildrenReordered:(BookmarkModel*)model
                          node:(const BookmarkNode*)node {
   [self loaded:model];
 }
 
 - (void)setUrlDelegate:(id<BookmarkURLOpener>)urlDelegate {
   urlDelegate_ = urlDelegate;
 }
 
 - (NSArray*)buttons {
   return buttons_.get();
 }
 
 - (NSButton*)offTheSideButton {
   return offTheSideButton_;
 }
 
 - (NSButton*)otherBookmarksButton {
   return otherBookmarksButton_.get();
 }
 
+- (NSImage*)getFavIconForNode:(const BookmarkNode*)node {
+  if (node->is_folder())
+    return folderImage_;
+
+  const SkBitmap& favIcon = bookmarkModel_->GetFavIcon(node);
+  if (!favIcon.isNull())
+    return gfx::SkBitmapToNSImage(favIcon);
+
+  return defaultImage_;
+}
+
 @end