Fix bookmark bubble crash....

Fix bookmark bubble crash.

BUG=http://crbug.com/20131
TEST=see bug; was very specific.


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

[pink (ping after 24hrs), 2009-08-26 00:06:05]

LGTM.

http://codereview.chromium.org/173428/diff/1/2
File chrome/browser/cocoa/bookmark_bubble_controller.mm (right):

http://codereview.chromium.org/173428/diff/1/2#newcode51
Line 51: // This window needs to live just a little bit longer (to accept a
i'm a little surprised we need to do this as we don't anywhere else.

http://codereview.chromium.org/173428/diff/1/2#newcode54
Line 54: 
omit blank line.

[Avi (use Gerrit), 2009-08-26 00:31:34]

LG

[John Grabowski, 2009-08-26 01:38:23]

http://codereview.chromium.org/173428/diff/1/2
File chrome/browser/cocoa/bookmark_bubble_controller.mm (right):

http://codereview.chromium.org/173428/diff/1/2#newcode51
Line 51: // This window needs to live just a little bit longer (to accept a
On 2009/08/26 00:06:05, pink wrote:
> i'm a little surprised we need to do this as we don't anywhere else.

The answer is because of the way we find out when to go away.
When the bubble loses key it's delegate (me) gets notified.
The delegate (me) tells our owner we are done and can be deallocated.
That deallocates both me and my window.
But oops, that all happens in the middle of my window handling resignKeyWindow.

Alternatively we could autorelease ourselves in our (delegate method)
windowDidResignKey:, and not have an "owner".  That would also solve the
problem.  Would you prefer that model instead?  I dislike self-deletion (we own
ourselves?!?!?) but will switch over if you think it cleaner.

[John Grabowski, 2009-08-26 17:00:49]

On 2009/08/26 01:38:23, John Grabowski wrote:
> http://codereview.chromium.org/173428/diff/1/2
> File chrome/browser/cocoa/bookmark_bubble_controller.mm (right):
> 
> http://codereview.chromium.org/173428/diff/1/2#newcode51
> Line 51: // This window needs to live just a little bit longer (to accept a
> On 2009/08/26 00:06:05, pink wrote:
> > i'm a little surprised we need to do this as we don't anywhere else.
> 
> The answer is because of the way we find out when to go away.
> When the bubble loses key it's delegate (me) gets notified.
> The delegate (me) tells our owner we are done and can be deallocated.
> That deallocates both me and my window.
> But oops, that all happens in the middle of my window handling
resignKeyWindow.
> 
> Alternatively we could autorelease ourselves in our (delegate method)
> windowDidResignKey:, and not have an "owner".  That would also solve the
> problem.  Would you prefer that model instead?  I dislike self-deletion (we
own
> ourselves?!?!?) but will switch over if you think it cleaner.

After discussing with pink I'm moving this logic to windowDidResignKey: (and
adding comments) to make things more clear.

[jon, 2009-08-27 17:55:03]

Merged into 4.0.203.2

On 2009/08/26 17:00:49, John Grabowski wrote:
> On 2009/08/26 01:38:23, John Grabowski wrote:
> > http://codereview.chromium.org/173428/diff/1/2
> > File chrome/browser/cocoa/bookmark_bubble_controller.mm (right):
> > 
> > http://codereview.chromium.org/173428/diff/1/2#newcode51
> > Line 51: // This window needs to live just a little bit longer (to accept a
> > On 2009/08/26 00:06:05, pink wrote:
> > > i'm a little surprised we need to do this as we don't anywhere else.
> > 
> > The answer is because of the way we find out when to go away.
> > When the bubble loses key it's delegate (me) gets notified.
> > The delegate (me) tells our owner we are done and can be deallocated.
> > That deallocates both me and my window.
> > But oops, that all happens in the middle of my window handling
> resignKeyWindow.
> > 
> > Alternatively we could autorelease ourselves in our (delegate method)
> > windowDidResignKey:, and not have an "owner".  That would also solve the
> > problem.  Would you prefer that model instead?  I dislike self-deletion (we
> own
> > ourselves?!?!?) but will switch over if you think it cleaner.
> 
> After discussing with pink I'm moving this logic to windowDidResignKey: (and
> adding comments) to make things more clear.