Use NSMenuDelegate for browser action overflow menu to update it upon opening.

chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm

 // Copyright (c) 2012 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 "browser_actions_controller.h"
 
 #include <cmath>
 #include <string>
 
 #include "base/sys_string_conversions.h"
@@ skipping 103 matching lines @@
 - (void)containerDragStart:(NSNotification*)notification;
 
 // Sends a notification for the toolbar to reposition surrounding UI elements.
 - (void)containerDragging:(NSNotification*)notification;
 
 // Determines which buttons need to be hidden based on the new size, hides them
 // and updates the chevron overflow menu. Also fires a notification to let the
 // toolbar know that the drag has finished.
 - (void)containerDragFinished:(NSNotification*)notification;
 
-// Updates the image associated with the button should it be within the chevron
-// menu.
-- (void)actionButtonUpdated:(NSNotification*)notification;
-
 // Adjusts the position of the surrounding action buttons depending on where the
 // button is within the container.
 - (void)actionButtonDragging:(NSNotification*)notification;
 
 // Updates the position of the Browser Actions within the container. This fires
 // when _any_ Browser Action button is done dragging to keep all open windows in
 // sync visually.
 - (void)actionButtonDragFinished:(NSNotification*)notification;
 
 // Moves the given button both visually and within the toolbar model to the
@@ skipping 22 matching lines @@
 - (void)updateChevronPositionInFrame:(NSRect)frame;
 
 // Shows or hides the chevron, animating as specified by |animate|.
 - (void)setChevronHidden:(BOOL)hidden
                  inFrame:(NSRect)frame
                  animate:(BOOL)animate;
 
 // Handles when a menu item within the chevron overflow menu is selected.
 - (void)chevronItemSelected:(id)menuItem;
 
+// NSMenuDelegate override:
 // Clears and then populates the overflow menu based on the contents of
 // |hiddenButtons_|.
-- (void)updateOverflowMenu;
+- (void)menuNeedsUpdate:(NSMenu*)menu;

[Robert Sesek, 2012/07/23 17:39:40]

Can remove

[Yoyo Zhou, 2012/07/23 18:47:24]

Removed.

 
 // Updates the container's grippy cursor based on the number of hidden buttons.
 - (void)updateGrippyCursors;
 
 // Returns the ID of the currently selected tab or -1 if none exists.
 - (int)currentTabId;
 @end
 
 // A helper class to proxy extension notifications to the view controller's
 // appropriate methods.
@@ skipping 248 matching lines @@
 
   NSUInteger i = 0;
   for (ExtensionList::iterator iter = toolbarModel_->begin();
        iter != toolbarModel_->end(); ++iter) {
     if (![self shouldDisplayBrowserAction:*iter])
       continue;
 
     [self createActionButtonForExtension:*iter withIndex:i++];
   }
 
-  [[NSNotificationCenter defaultCenter]
-      addObserver:self
-         selector:@selector(actionButtonUpdated:)
-             name:kBrowserActionButtonUpdatedNotification
-           object:nil];
-
   CGFloat width = [self savedWidth];
   [containerView_ resizeToWidth:width animate:NO];
 }
 
 - (void)createActionButtonForExtension:(const Extension*)extension
                              withIndex:(NSUInteger)index {
   if (!extension->browser_action())
     return;
 
   if (![self shouldDisplayBrowserAction:extension])
@@ skipping 47 matching lines @@
   BrowserActionButton* button = [buttons_ objectForKey:buttonKey];
   // This could be the case in incognito, where only a subset of extensions are
   // shown.
   if (!button)
     return;
 
   [button removeFromSuperview];
   // It may or may not be hidden, but it won't matter to NSMutableArray either
   // way.
   [hiddenButtons_ removeObject:button];
-  [self updateOverflowMenu];
 
   [buttons_ removeObjectForKey:buttonKey];
   if ([self buttonCount] == 0) {
     // No more buttons? Hide the container.
     [containerView_ setHidden:YES];
   } else {
     [self positionActionButtonsAndAnimate:NO];
   }
   [containerView_ setMaxWidth:
       [self containerWidthWithButtonCount:[self buttonCount]]];
@@ skipping 104 matching lines @@
         NSWidth(NSIntersectionRect([containerView_ bounds], buttonFrame));
     // Pad the threshold by 5 pixels in order to have the buttons hide more
     // easily.
     if (([containerView_ grippyPinned] && intersectionWidth > 0) ||
         (intersectionWidth <= (NSWidth(buttonFrame) / 2) + 5.0)) {
       [button setAlphaValue:0.0];
       [button removeFromSuperview];
       [hiddenButtons_ addObject:button];
     }
   }
-  [self updateOverflowMenu];
   [self updateGrippyCursors];
 
   if (!profile_->IsOffTheRecord())
     toolbarModel_->SetVisibleIconCount([self visibleButtonCount]);
 
   [[NSNotificationCenter defaultCenter]
       postNotificationName:kBrowserActionGrippyDragFinishedNotification
                     object:self];
 }
 
-- (void)actionButtonUpdated:(NSNotification*)notification {
-  BrowserActionButton* button = [notification object];
-  if (![hiddenButtons_ containsObject:button])
-    return;
-
-  // +1 item because of the title placeholder. See |updateOverflowMenu|.
-  NSUInteger menuIndex = [hiddenButtons_ indexOfObject:button] + 1;
-  NSMenuItem* item = [[chevronMenuButton_ attachedMenu] itemAtIndex:menuIndex];
-  DCHECK(button == [item representedObject]);
-  [item setImage:[button compositedImage]];
-}
-
 - (void)actionButtonDragging:(NSNotification*)notification {
   if (![self chevronIsHidden])
     [self setChevronHidden:YES inFrame:[containerView_ frame] animate:YES];
 
   // Determine what index the dragged button should lie in, alter the model and
   // reposition the buttons.
   CGFloat dragThreshold = std::floor(kBrowserActionWidth / 2);
   BrowserActionButton* draggedButton = [notification object];
   NSRect draggedButtonFrame = [draggedButton frame];
 
@@ skipping 32 matching lines @@
     // Make sure the button is within the visible container.
     if ([button superview] != containerView_) {
       [containerView_ addSubview:button];
       [button setAlphaValue:1.0];
       [hiddenButtons_ removeObjectIdenticalTo:button];
     }
   } else if (![hiddenButtons_ containsObject:button]) {
     [hiddenButtons_ addObject:button];
     [button removeFromSuperview];
     [button setAlphaValue:0.0];
-    [self updateOverflowMenu];
   }
 }
 
 - (void)browserActionClicked:(BrowserActionButton*)button {
   const Extension* extension = [button extension];
   GURL popupUrl;
   switch (toolbarModel_->ExecuteBrowserAction(extension, browser_, &popupUrl)) {
     case ExtensionToolbarModel::ACTION_NONE:
       break;
     case ExtensionToolbarModel::ACTION_SHOW_POPUP: {
@@ skipping 42 matching lines @@
     [chevronMenuButton_ setBordered:NO];
     [chevronMenuButton_ setShowsBorderOnlyWhileMouseInside:YES];
 
     [[chevronMenuButton_ cell] setImageID:IDR_BROWSER_ACTIONS_OVERFLOW
                            forButtonState:image_button_cell::kDefaultState];
     [[chevronMenuButton_ cell] setImageID:IDR_BROWSER_ACTIONS_OVERFLOW_H
                            forButtonState:image_button_cell::kHoverState];
     [[chevronMenuButton_ cell] setImageID:IDR_BROWSER_ACTIONS_OVERFLOW_P
                            forButtonState:image_button_cell::kPressedState];
 
+    overflowMenu_.reset([[NSMenu alloc] initWithTitle:@""]);
+    [overflowMenu_ setAutoenablesItems:NO];
+    [overflowMenu_ setDelegate:self];
+    [chevronMenuButton_ setAttachedMenu:overflowMenu_];
+
     [containerView_ addSubview:chevronMenuButton_];
   }
 
-  if (!hidden)
-    [self updateOverflowMenu];
-
   [self updateChevronPositionInFrame:frame];
 
   // Stop any running animation.
   [chevronAnimation_ stopAnimation];
 
   if (!animate) {
     [chevronMenuButton_ setHidden:hidden];
     return;
   }
 
@@ skipping 12 matching lines @@
   }
   [chevronAnimation_ setViewAnimations:
       [NSArray arrayWithObject:animationDictionary]];
   [chevronAnimation_ startAnimation];
 }
 
 - (void)chevronItemSelected:(id)menuItem {
   [self browserActionClicked:[menuItem representedObject]];
 }
 
-// TODO(yoz): This only gets called when the set of actions in the overflow
-// menu changes (not for things that would update page actions).
-// It should instead be called each time the menu is opened.
-- (void)updateOverflowMenu {
-  overflowMenu_.reset([[NSMenu alloc] initWithTitle:@""]);
-  // See menu_button.h for documentation on why this is needed.
-  [overflowMenu_ addItemWithTitle:@"" action:nil keyEquivalent:@""];
-  [overflowMenu_ setAutoenablesItems:NO];
+- (void)menuNeedsUpdate:(NSMenu*)menu {
+  [menu removeAllItems];

[Robert Sesek, 2012/07/23 17:39:40]

nit: blank line after.

[Yoyo Zhou, 2012/07/23 18:47:24]

Done.

+    // See menu_button.h for documentation on why this is needed.

[Robert Sesek, 2012/07/23 17:39:40]

nit: indention

[Yoyo Zhou, 2012/07/23 18:47:24]

Done.

+  [menu addItemWithTitle:@"" action:nil keyEquivalent:@""];
 
   for (BrowserActionButton* button in hiddenButtons_.get()) {
     NSString* name = base::SysUTF8ToNSString([button extension]->name());
     NSMenuItem* item =
-        [overflowMenu_ addItemWithTitle:name
-                                 action:@selector(chevronItemSelected:)
-                          keyEquivalent:@""];
+        [menu addItemWithTitle:name
+                        action:@selector(chevronItemSelected:)
+                 keyEquivalent:@""];
     [item setRepresentedObject:button];
     [item setImage:[button compositedImage]];
     [item setTarget:self];
     [item setEnabled:[button isEnabled]];
   }
-  [chevronMenuButton_ setAttachedMenu:overflowMenu_];
 }
 
 - (void)updateGrippyCursors {
   [containerView_ setCanDragLeft:[hiddenButtons_ count] > 0];
   [containerView_ setCanDragRight:[self visibleButtonCount] > 0];
   [[containerView_ window] invalidateCursorRectsForView:containerView_];
 }
 
 - (int)currentTabId {
   TabContents* selected_tab = chrome::GetActiveTabContents(browser_);
@@ skipping 10 matching lines @@
   if (profile_->IsOffTheRecord())
     index = toolbarModel_->IncognitoIndexToOriginal(index);
   if (index < toolbarModel_->size()) {
     const Extension* extension = toolbarModel_->GetExtensionByIndex(index);
     return [buttons_ objectForKey:base::SysUTF8ToNSString(extension->id())];
   }
   return nil;
 }
 
 @end