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

Bookmarks for pages without a favicon should display a generic "blank page" icon (this also helps avoid a blank button if the bookmark also doesn't have a title). Also, folders within folders also need to display the folder icon.

BUG=23558
TEST=See above; be sure to test things on the bar and in folders, and make sure that pages with favicons are still displayed correctly.

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

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

2 files changed, 26 insertions, 30 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index dbf8e7c..eed8c55 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -73,6 +73,9 @@ const CGFloat kNoBookmarksNTPVerticalOffset = 27.0;
   // The folder image so we can use one copy for all buttons
   scoped_nsobject<NSImage> folderImage_;
 
+  // The default image, so we can use one copy for all buttons.
+  scoped_nsobject<NSImage> defaultImage_;
+
   // If the bar is disabled, we hide it and ignore show/hide commands.
   // Set when using fullscreen mode.
   BOOL barIsEnabled_;
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index 7100bbb..4118ba4 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -28,6 +28,7 @@
 #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"
@@ -56,6 +57,7 @@
 - (void)addButtonsToView;
 - (void)resizeButtons;
 - (void)centerNoItemsLabel;
+- (NSImage*)getFavIconForNode:(const BookmarkNode*)node;
 @end
 
 @implementation BookmarkBarController
@@ -79,6 +81,7 @@
 
     ResourceBundle& rb = ResourceBundle::GetSharedInstance();
     folderImage_.reset([rb.GetNSImageNamed(IDR_BOOKMARK_BAR_FOLDER) retain]);
+    defaultImage_.reset([rb.GetNSImageNamed(IDR_DEFAULT_FAVICON) retain]);
   }
   return self;
 }
@@ -357,6 +360,7 @@
                                                  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];
@@ -375,13 +379,6 @@
                                   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];
-      }
-    }
   }
 }
 
@@ -595,19 +592,8 @@
   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;
@@ -873,16 +859,12 @@
     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];
     }
   }
 }
@@ -909,4 +891,15 @@
   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