Native menu implementation for bug 5679. Followup to http://codereview.chromium.org/2928005/

chrome/browser/ui/cocoa/toolbar/back_forward_menu_controller.mm

Index: chrome/browser/ui/cocoa/toolbar/back_forward_menu_controller.mm
diff --git a/chrome/browser/ui/cocoa/toolbar/back_forward_menu_controller.mm b/chrome/browser/ui/cocoa/toolbar/back_forward_menu_controller.mm
index 63bab2aed8e6b2cad53786f64ceac485fce0285e..ca4361c628e450b5b070bf18d3c52ce31e8d073e 100644
--- a/chrome/browser/ui/cocoa/toolbar/back_forward_menu_controller.mm
+++ b/chrome/browser/ui/cocoa/toolbar/back_forward_menu_controller.mm
@@ -22,6 +22,23 @@ using gfx::SkBitmapToNSImage;
 
 @synthesize type = type_;
 
+namespace BackForwardMenuControllerInternal {
+
+class MenuDelegate : public ui::MenuModelDelegate {
+ public:
+  explicit MenuDelegate(BackForwardMenuController* owner)
+      : owner_(owner) {}
+  // Called by ui::MenuModelDelegate when an icon changes.
+  virtual void OnIconChanged(int modelIndex) {

[Avi (use Gerrit), 2011/03/25 00:19:35]

OVERRIDE

+    [owner_ onIconChanged:modelIndex];
+  }
+
+ private:
+  BackForwardMenuController* owner_;
+};
+
+} // BackForwardMenuControllerInternal namespace
+
 // Own methods:
 
 - (id)initWithBrowser:(Browser*)browser
@@ -35,19 +52,35 @@ using gfx::SkBitmapToNSImage;
     backForwardMenu_.reset([[NSMenu alloc] initWithTitle:@""]);
     DCHECK(backForwardMenu_.get());
     [backForwardMenu_ setDelegate:self];
-
+    contextMenuDelegate_.reset(
+        new BackForwardMenuControllerInternal::MenuDelegate(self));
+    model_->SetMenuModelDelegate(contextMenuDelegate_.get());
     [button_ setAttachedMenu:backForwardMenu_];
     [button_ setOpenMenuOnClick:NO];
   }
   return self;
 }
 
+// Called by MenuDelegate when an icon changes.
+- (void)onIconChanged:(int) modelIndex {

[Avi (use Gerrit), 2011/03/25 00:19:35]

No space after ).

+  // modelIndex + 1 since 0-th item is the "title"
+  NSMenuItem* menuItem =
+    [backForwardMenu_ itemAtIndex:(NSInteger)modelIndex + 1];

[Avi (use Gerrit), 2011/03/25 00:19:35]

Why the cast? Drop the "(NSInteger)".

+  SkBitmap icon;
+  // Icon (if it has one).

[Avi (use Gerrit), 2011/03/25 00:19:35]

Comment is obvious; drop.

+  if (model_->GetIconAt(modelIndex, &icon)) {
+    [menuItem setImage:SkBitmapToNSImage(icon)];
+  }
+}
+
 // Methods as delegate:
 
 // Called by backForwardMenu_ just before tracking begins.
 //TODO(viettrungluu): should we do anything for chapter stops (see model)?
 - (void)menuNeedsUpdate:(NSMenu*)menu {
   DCHECK(menu == backForwardMenu_);
+  // Retrieving favicons from history requires nestable tasks.
+  MessageLoop::ScopedNestableTaskAllower allow(MessageLoop::current());
 
   // Remove old menu items (backwards order is as good as any).
   for (NSInteger i = [menu numberOfItems]; i > 0; i--)