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

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

Makes the overflow menu on cocoa update properly.

BUG=133094
TEST=Enable script badges; have extensions with page actions; resize the browser action area to put some of the in the overflow menu; observe them changing state upon tab switches.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=147950

[Yoyo Zhou, 2012-07-20 22:22:08]

On 2012/07/20 22:20:01, Yoyo Zhou wrote:

I meant to write:
Is there a convention for where to put overridden methods (such as
-menuNeedsUpdate:) in ObjC code? Above, below, but probably not in the middle of
the other methods?

[Robert Sesek, 2012-07-23 16:21:08]

On 2012/07/20 22:22:08, Yoyo Zhou wrote:
> On 2012/07/20 22:20:01, Yoyo Zhou wrote:
> 
> I meant to write:
> Is there a convention for where to put overridden methods (such as
> -menuNeedsUpdate:) in ObjC code? Above, below, but probably not in the middle
of
> the other methods?

The convention is to not forward declare. Declaring that the class implements
the delegate protocol is enough.

Also, are there any tests for this code?

[Yoyo Zhou, 2012-07-23 17:22:43]

On 2012/07/23 16:21:08, rsesek wrote:
> On 2012/07/20 22:22:08, Yoyo Zhou wrote:
> > On 2012/07/20 22:20:01, Yoyo Zhou wrote:
> > 
> > I meant to write:
> > Is there a convention for where to put overridden methods (such as
> > -menuNeedsUpdate:) in ObjC code? Above, below, but probably not in the
middle
> of
> > the other methods?
> 
> The convention is to not forward declare. Declaring that the class implements
> the delegate protocol is enough.

Yes, I meant in the .mm file, is there someplace in particular to put it?

> Also, are there any tests for this code?

I think there haven't been any. I'd need to think about how to; possibly a
platform-specific browsertest.

[Robert Sesek, 2012-07-23 17:24:54]

On 2012/07/23 17:22:43, Yoyo Zhou wrote:
> On 2012/07/23 16:21:08, rsesek wrote:
> > On 2012/07/20 22:22:08, Yoyo Zhou wrote:
> > > On 2012/07/20 22:20:01, Yoyo Zhou wrote:
> > > 
> > > I meant to write:
> > > Is there a convention for where to put overridden methods (such as
> > > -menuNeedsUpdate:) in ObjC code? Above, below, but probably not in the
> middle
> > of
> > > the other methods?
> > 
> > The convention is to not forward declare. Declaring that the class
implements
> > the delegate protocol is enough.
> 
> Yes, I meant in the .mm file, is there someplace in particular to put it?

Usually after the publicly declared methods but before any privately declared
ones. This reflects it being a private implementation for a publicly declared
protocol.

> > Also, are there any tests for this code?
> 
> I think there haven't been any. I'd need to think about how to; possibly a
> platform-specific browsertest.

We can usually unittest controllers. I was just wondering if there's tests to
update. It doesn't look like there are, which isn't ideal, but OK for now.

[Robert Sesek, 2012-07-23 17:39:39]

http://chromiumcodereview.appspot.com/10797054/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm (right):

http://chromiumcodereview.appspot.com/10797054/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm:169: -
(void)menuNeedsUpdate:(NSMenu*)menu;
Can remove

http://chromiumcodereview.appspot.com/10797054/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm:794: [menu
removeAllItems];
nit: blank line after.

http://chromiumcodereview.appspot.com/10797054/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm:795: // See
menu_button.h for documentation on why this is needed.
nit: indention

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

I moved -menuNeedsUpdate: to where you suggested.

http://chromiumcodereview.appspot.com/10797054/diff/1/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm (right):

http://chromiumcodereview.appspot.com/10797054/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm:169: -
(void)menuNeedsUpdate:(NSMenu*)menu;
On 2012/07/23 17:39:40, rsesek wrote:
> Can remove

Removed.

http://chromiumcodereview.appspot.com/10797054/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm:794: [menu
removeAllItems];
On 2012/07/23 17:39:40, rsesek wrote:
> nit: blank line after.

Done.

http://chromiumcodereview.appspot.com/10797054/diff/1/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm:795: // See
menu_button.h for documentation on why this is needed.
On 2012/07/23 17:39:40, rsesek wrote:
> nit: indention

Done.

[Robert Sesek, 2012-07-23 18:47:50]

lgtm