[Mac] Keep download buttons alive while processing events.

[Mac] Keep download buttons alive while processing events.

Other places which spin a nested loop pin the containing object in
place in case something fires a window close.  There are crashes which
indicate that this is happening here.

BUG=92390
TEST=monitor crash server.


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

[Scott Hess - ex-Googler, 2011-08-23 22:49:19]

You look like the last one to modify these files ...

[Scott Hess - ex-Googler, 2011-08-23 22:50:28]

On 2011/08/23 22:49:19, shess wrote:
> You look like the last one to modify these files ...

Also, if you think it's a really good idea, let me know and I'll adapt it for
M-14.  The crash rate for M-15 is so low that I'll never notice if this does
anything.

[Robert Sesek, 2011-08-23 22:56:48]

http://codereview.chromium.org/7715019/diff/1/chrome/browser/ui/cocoa/draggab...
File chrome/browser/ui/cocoa/draggable_button.mm (right):

http://codereview.chromium.org/7715019/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button.mm:42: scoped_nsobject<DraggableButton>
keepAlive([self retain]);
Would it be better to do this in the impl?

[Scott Hess - ex-Googler, 2011-08-23 23:04:37]

http://codereview.chromium.org/7715019/diff/1/chrome/browser/ui/cocoa/draggab...
File chrome/browser/ui/cocoa/draggable_button.mm (right):

http://codereview.chromium.org/7715019/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button.mm:42: scoped_nsobject<DraggableButton>
keepAlive([self retain]);
On 2011/08/23 22:56:48, rsesek wrote:
> Would it be better to do this in the impl?

How?

[Robert Sesek, 2011-08-23 23:10:13]

http://codereview.chromium.org/7715019/diff/1/chrome/browser/ui/cocoa/draggab...
File chrome/browser/ui/cocoa/draggable_button.mm (right):

http://codereview.chromium.org/7715019/diff/1/chrome/browser/ui/cocoa/draggab...
chrome/browser/ui/cocoa/draggable_button.mm:42: scoped_nsobject<DraggableButton>
keepAlive([self retain]);
On 2011/08/23 23:04:37, shess wrote:
> On 2011/08/23 22:56:48, rsesek wrote:
> > Would it be better to do this in the impl?
> 
> How?

Retain button_ in the Impl in the loop?

[Scott Hess - ex-Googler, 2011-08-23 23:13:03]

On 2011/08/23 23:10:13, rsesek wrote:
>
http://codereview.chromium.org/7715019/diff/1/chrome/browser/ui/cocoa/draggab...
> File chrome/browser/ui/cocoa/draggable_button.mm (right):
> 
>
http://codereview.chromium.org/7715019/diff/1/chrome/browser/ui/cocoa/draggab...
> chrome/browser/ui/cocoa/draggable_button.mm:42:
scoped_nsobject<DraggableButton>
> keepAlive([self retain]);
> On 2011/08/23 23:04:37, shess wrote:
> > On 2011/08/23 22:56:48, rsesek wrote:
> > > Would it be better to do this in the impl?
> > 
> > How?
> 
> Retain button_ in the Impl in the loop?

It would be released before this code calls [super mouseDown:].  Unless I did
[[button_ retain] autorelease], in which case it would be released at some
indeterminate future point, which I don't like much at all.

[Robert Sesek, 2011-08-24 18:16:10]

On 2011/08/23 23:13:03, shess wrote:
> It would be released before this code calls [super mouseDown:].  Unless I did
> [[button_ retain] autorelease], in which case it would be released at some
> indeterminate future point, which I don't like much at all.

It depends on whose loop it's getting destroyed. This code has been refactored
since I've last looked at this crash, and it's not clear if it's happening
within the nested drag loop of the Impl or if it's in the super.

[Scott Hess - ex-Googler, 2011-08-24 18:22:10]

On 2011/08/24 18:16:10, rsesek wrote:
> On 2011/08/23 23:13:03, shess wrote:
> > It would be released before this code calls [super mouseDown:].  Unless I
did
> > [[button_ retain] autorelease], in which case it would be released at some
> > indeterminate future point, which I don't like much at all.
> 
> It depends on whose loop it's getting destroyed. This code has been refactored
> since I've last looked at this crash, and it's not clear if it's happening
> within the nested drag loop of the Impl or if it's in the super.

I'm not sure where you're going, here!  The problem is calling [super
mouseDown:] after self has been deallocated.  Since the call is in the main
class -mouseDown: implementation, we have to arrange for the button to continue
to exist at that point.  Since the impl method is completed at that point, it
would have to use autorelease to bounce ownership out to the event loop which
originally forwarded the event.

[Robert Sesek, 2011-08-24 18:34:18]

On 2011/08/24 18:22:10, shess wrote:
> On 2011/08/24 18:16:10, rsesek wrote:
> > On 2011/08/23 23:13:03, shess wrote:
> > > It would be released before this code calls [super mouseDown:].  Unless I
> did
> > > [[button_ retain] autorelease], in which case it would be released at some
> > > indeterminate future point, which I don't like much at all.
> > 
> > It depends on whose loop it's getting destroyed. This code has been
refactored
> > since I've last looked at this crash, and it's not clear if it's happening
> > within the nested drag loop of the Impl or if it's in the super.
> 
> I'm not sure where you're going, here!  The problem is calling [super
> mouseDown:] after self has been deallocated.  Since the call is in the main
> class -mouseDown: implementation, we have to arrange for the button to
continue
> to exist at that point.  Since the impl method is completed at that point, it
> would have to use autorelease to bounce ownership out to the event loop which
> originally forwarded the event.

Ah I see what you're talking about now. I thought the maybeStart loop was more
conditional than it is.

LGTM

[commit-bot: I haz the power, 2011-08-25 02:13:29]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is linux, revision is 98154, job name
was 7715019-1 (retry) (retry) (retry) (retry).

[commit-bot: I haz the power, 2011-08-25 20:28:31]

Change committed as 98286