[Mac] Retain a reference within a scoped_nsobject of a Page Action's context menu.

[Mac] Retain a reference within a scoped_nsobject of a Page Action's context menu.

The problem was that the menu was being dealloc'd upon selection of the 'Uninstall' option, which asynchonously loads the extension's icon. Upon deallocation, the ImageTracker was also being destructed and the loading cancelled, leaving OnImageLoad to never be called.

BUG=40233
TEST=see bug for repro steps. should be able to uninstall page actions through their context menus.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=56532

[Miranda Callahan, 2010-08-18 02:50:25]

LGTM.

[Andrew Bonventre (Bons), 2010-08-18 02:51:00]

Shess, since you wrote the original PageActionDecoration for this, if you could
do a once-over that would be great.

[Scott Hess, 2010-08-18 03:45:41]

LGTM, except fix the extra ref (or tell me where I missed a trick).

On 2010/08/18 02:51:00, Andrew Bonventre (Bons) wrote:
> Shess, since you wrote the original PageActionDecoration for this, if you
could
> do a once-over that would be great.

Fortunately, I think it was broken before my change.  Unfortunate that a tiny
bit of digging while I was in there would have let me trivially fix it for M6.

http://codereview.chromium.org/3148018/diff/1/3
File chrome/browser/cocoa/location_bar/page_action_decoration.mm (right):

http://codereview.chromium.org/3148018/diff/1/3#newcode225
chrome/browser/cocoa/location_bar/page_action_decoration.mm:225:
extensionAction:page_action_] retain]);
I think this leaves you with one too many references (the one from alloc/init,
plus one from retain).

As I mentioned on IM, this feels a little heavyweight for the effect desired,
but I see how it matches the existing browser-action code, so it makes sense.

[Andrew Bonventre (Bons), 2010-08-18 15:47:38]

On 2010/08/18 03:45:41, shess wrote:
> LGTM, except fix the extra ref (or tell me where I missed a trick).
> 
> On 2010/08/18 02:51:00, Andrew Bonventre (Bons) wrote:
> > Shess, since you wrote the original PageActionDecoration for this, if you
> could
> > do a once-over that would be great.
> 
> Fortunately, I think it was broken before my change.  Unfortunate that a tiny
> bit of digging while I was in there would have let me trivially fix it for M6.
> 
> http://codereview.chromium.org/3148018/diff/1/3
> File chrome/browser/cocoa/location_bar/page_action_decoration.mm (right):
> 
> http://codereview.chromium.org/3148018/diff/1/3#newcode225
> chrome/browser/cocoa/location_bar/page_action_decoration.mm:225:
> extensionAction:page_action_] retain]);
> I think this leaves you with one too many references (the one from alloc/init,
> plus one from retain).
Fixed.

> 
> As I mentioned on IM, this feels a little heavyweight for the effect desired,
> but I see how it matches the existing browser-action code, so it makes sense.