Fix a browser crasher with negative index set on select popups.

webkit/glue/webmenurunner_mac.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 "webkit/glue/webmenurunner_mac.h"
 
 #include "base/sys_string_conversions.h"
 
 namespace {
 
@@ skipping 74 matching lines @@
                                    keyEquivalent:@""];
   [menuItem setEnabled:(item.enabled && item.type != WebMenuItem::GROUP)];
   [menuItem setTarget:self];
   if (attrs) {
     scoped_nsobject<NSAttributedString> attrTitle(
         [[NSAttributedString alloc] initWithString:title
                                         attributes:attrs]);
     [menuItem setAttributedTitle:attrTitle];
   }
   if (gNewNSMenuAPI)
     [menuItem setTag:[menu_ numberOfItems] - 1];

[Scott Hess - ex-Googler, 2010/12/01 01:29:51]

You can safely make this unconditional.

[Jay Civelli, 2010/12/01 02:07:13]

Done.

 }
 
 // Reflects the result of the user's interaction with the popup menu. If NO, the
 // menu was dismissed without the user choosing an item, which can happen if the
 // user clicked outside the menu region or hit the escape key. If YES, the user
 // selected an item from the menu.
 - (BOOL)menuItemWasChosen {
   return menuItemWasChosen_;
 }
 
 - (void)menuItemSelected:(id)sender {
   menuItemWasChosen_ = YES;
   if (gNewNSMenuAPI)
     index_ = [sender tag];
 }
 
 - (void)runMenuInView:(NSView*)view
            withBounds:(NSRect)bounds
          initialIndex:(int)index {
   if (gNewNSMenuAPI) {
-    NSMenuItem* selectedItem = [menu_ itemAtIndex:index];
-    [selectedItem setState:NSOnState];
+    int actualIndex;
+    if (index < 0 || index > [menu_ numberOfItems]) {
+      actualIndex = 0;
+    } else {
+      actualIndex = index;
+    }
+    NSMenuItem* selectedItem = [menu_ itemAtIndex:actualIndex];
+    if (actualIndex == index)
+      [selectedItem setState:NSOnState];

[Scott Hess - ex-Googler, 2010/12/01 01:29:51]

If I understand this correctly, what you want is the actual item, if possible,
else the item at 0?  This seems sort of convoluted for that.  How about:

NSMenuItem* selectedItem;
if (index < 0 || index > [menu_ numberOfItems]) {
  selectedItem = [menu_ itemAtIndex:0];
} else {
  selectedItem = [menu_ itemAtIndex:index];
  [selectedItem setState:NSOnState];
}

Hmm.  Or even:

NSMenuItem* selectedItem = [menu_ itemWithTag:index];
if (selectedItem) {
  [selectedItem setState:NSOnState];
} else {
  selectedItem = [menu_ itemWithTag:0];  // In case menu is empty.
}

[Jay Civelli, 2010/12/01 02:07:13]

Good idea. Done.

     NSPoint anchor = NSMakePoint(NSMinX(bounds) + kPopupXOffset,
                                  NSMaxY(bounds));
     [menu_ popUpMenuPositioningItem:selectedItem
                          atLocation:anchor
                              inView:view];
   } else {
     // Set up the button cell, converting to NSView coordinates. The menu is

[Jay Civelli, 2010/12/01 01:08:52]

I am not sure how to test that case? Should I just hack gNewNSMenuAPI to be
false?

[Scott Hess - ex-Googler, 2010/12/01 01:29:51]

OMG.

I guess that's about your only option.  Or run on a 10.5 system.  Or just use
the older code everywhere (which is what our UI code does), though that's too
big a change to consider for 9.0 I think.

If you take my -itemWithTag: idea, consider -selectItemWithTag: below, to be
consistent.

[Jay Civelli, 2010/12/01 02:07:13]

OK, I just hacked the value.
The API for NSPopUpButtonCell is slightly different, so I cannot have
consistency with the other case. Interestingly, -1 is OK for selectItemAtIndex,
so we only have to take care of out of bounds indeces.

     // positioned such that the currently selected menu item appears over the
     // popup button, which is the expected Mac popup menu behavior.
     NSPopUpButtonCell* button = [[NSPopUpButtonCell alloc] initTextCell:@""
                                                               pullsDown:NO];
     [button autorelease];
     [button setMenu:menu_];
     [button selectItemAtIndex:index];
     [button setFont:[NSFont menuFontOfSize:fontSize_]];
 
     // Create a dummy view to associate the popup with, since the OS will use
@@ skipping 68 matching lines @@
                            characters:@""
           charactersIgnoringModifiers:escape_str
                             isARepeat:NO
                               keyCode:0x1B];
   }
 
   return event;
 }
 
 }  // namespace webkit_glue