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

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

 // Copyright (c) 2011 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.
 
 #import "chrome/browser/ui/cocoa/toolbar/back_forward_menu_controller.h"
 
 #include "base/logging.h"
 #include "base/scoped_ptr.h"
 #include "base/sys_string_conversions.h"
 #import "chrome/browser/ui/cocoa/event_utils.h"
 #import "chrome/browser/ui/cocoa/menu_button.h"
 #include "chrome/browser/ui/toolbar/back_forward_menu_model.h"
 #include "skia/ext/skia_utils_mac.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 using base::SysUTF16ToNSString;
 using gfx::SkBitmapToNSImage;
 
 @implementation BackForwardMenuController
 
 // Accessors and mutators:
 
 @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
             modelType:(BackForwardMenuType)type
                button:(MenuButton*)button {
   if ((self = [super init])) {
     type_ = type;
     button_ = button;
     model_.reset(new BackForwardMenuModel(browser, type_));
     DCHECK(model_.get());
     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--)
     [menu removeItemAtIndex:(i - 1)];
 
   // 0-th item must be blank. (This is because we use a pulldown list, for which
   // Cocoa uses the 0-th item as "title" in the button.)
   [menu insertItemWithTitle:@""
                      action:nil
               keyEquivalent:@""
@@ skipping 32 matching lines @@
 
 - (void)executeMenuItem:(id)sender {
   DCHECK([sender isKindOfClass:[NSMenuItem class]]);
   int menuID = [sender tag];
   model_->ActivatedAtWithDisposition(
       menuID,
       event_utils::WindowOpenDispositionFromNSEvent([NSApp currentEvent]));
 }
 
 @end  // @implementation BackForwardMenuController