Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking close

Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
TEST=On OSX 10.9, have one browser tab, navigate to a page (e.g.
chrome://version), then press Backspace. The "Press Cmd+Left to go back"
bubble should appear. Wait for it to fade out completely. Then close the
tab via the tabstrip. Chrome shouldn't crash.

Committed: https://crrev.com/6777bcc56aff3a1dc27fb0adced9a462b22e4e33
Cr-Commit-Position: refs/heads/master@{#398890}

[tapted, 2016-06-09 02:01:09]

Description was changed from

==========
Neater


Cleanup, has a test


With a test


proper


a test?

BUG=616701
==========

to

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, before 10.11 bad things
can happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this
resulting, in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
==========

[tapted, 2016-06-09 02:01:10]

tapted@chromium.org changed reviewers:
+ rsesek@chromium.org

[tapted, 2016-06-09 02:02:00]

Pretty sure this will fix it!

[tapted, 2016-06-09 02:45:19]

On 2016/06/09 02:02:00, tapted wrote:
> Pretty sure this will fix it!

And it looks like the test (without the fix!) will fail on a 10.9 bot -
https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel...

I couldn't get a failure on a local 10.10 build. (checking 10.9 now).

But there are also some browser_tests failing here on the extra DCHECKs I put in
:( - investigating that now.

[tapted, 2016-06-09 03:07:49]

Description was changed from

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, before 10.11 bad things
can happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this
resulting, in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
==========

to

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
==========

[tapted, 2016-06-09 03:49:15]

On 2016/06/09 02:45:19, tapted wrote:
> On 2016/06/09 02:02:00, tapted wrote:
> > Pretty sure this will fix it!
> 
> And it looks like the test (without the fix!) will fail on a 10.9 bot -
>
https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel...
> 
> I couldn't get a failure on a local 10.10 build. (checking 10.9 now).

Verified on a local 10.9 build that the segfault was coming from
WidgetOwnerNSWindowAdapter::OnWindowWillClose() when the fix wasn't present.

> But there are also some browser_tests failing here on the extra DCHECKs I put
in
> :( - investigating that now.

Fixed these too \o/ . So this should be good to go.

[tapted, 2016-06-09 05:55:17]

Description was changed from

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
==========

to

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
TEST=On OSX 10.9, navigate to a page (e.g. chrome://version), then press
Backspace. The "Press Cmd+Left to go back" bubble should appear. Wait
for it to fade out completely. Then close all tab in the browser window.
Chrome shouldn't crash.
==========

[tapted, 2016-06-09 05:56:51]

Description was changed from

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
TEST=On OSX 10.9, navigate to a page (e.g. chrome://version), then press
Backspace. The "Press Cmd+Left to go back" bubble should appear. Wait
for it to fade out completely. Then close all tab in the browser window.
Chrome shouldn't crash.
==========

to

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
TEST=On OSX 10.9, have one browser tab, navigate to a page (e.g.
chrome://version), then press Backspace. The "Press Cmd+Left to go back"
bubble should appear. Wait for it to fade out completely. Then close the
tab via the tabstrip. Chrome shouldn't crash.
==========

[Robert Sesek, 2016-06-09 15:03:22]

The CQ bit was checked by rsesek@chromium.org

[Robert Sesek, 2016-06-09 15:03:23]

lgtm

[commit-bot: I haz the power, 2016-06-09 15:03:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2056593002/20001

[commit-bot: I haz the power, 2016-06-09 15:25:48]

Description was changed from

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
TEST=On OSX 10.9, have one browser tab, navigate to a page (e.g.
chrome://version), then press Backspace. The "Press Cmd+Left to go back"
bubble should appear. Wait for it to fade out completely. Then close the
tab via the tabstrip. Chrome shouldn't crash.
==========

to

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
TEST=On OSX 10.9, have one browser tab, navigate to a page (e.g.
chrome://version), then press Backspace. The "Press Cmd+Left to go back"
bubble should appear. Wait for it to fade out completely. Then close the
tab via the tabstrip. Chrome shouldn't crash.
==========

[commit-bot: I haz the power, 2016-06-09 15:25:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-09 15:25:56]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-09 15:26:57]

Description was changed from

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
TEST=On OSX 10.9, have one browser tab, navigate to a page (e.g.
chrome://version), then press Backspace. The "Press Cmd+Left to go back"
bubble should appear. Wait for it to fade out completely. Then close the
tab via the tabstrip. Chrome shouldn't crash.
==========

to

==========
Mac: Retain the child NSWindow in WidgetOwnerNSWindowAdapter before invoking
close

AppKit has gotten better at handling the last reference to an NSWindow
going away inside -[NSWindow close]. However, on 10.9, bad things can
happen if we don't retain the NSWindow being closed until after the
-[NSWindow close] call returns.

WidgetOwnerNSWindowAdapter::OnWindowWillClose() wasn't doing this. The
result: in some codepaths, a zombie access under the close call.

To fix, retain the window being closed.

BUG=616701
TEST=On OSX 10.9, have one browser tab, navigate to a page (e.g.
chrome://version), then press Backspace. The "Press Cmd+Left to go back"
bubble should appear. Wait for it to fade out completely. Then close the
tab via the tabstrip. Chrome shouldn't crash.

Committed: https://crrev.com/6777bcc56aff3a1dc27fb0adced9a462b22e4e33
Cr-Commit-Position: refs/heads/master@{#398890}
==========

[commit-bot: I haz the power, 2016-06-09 15:26:58]

Patchset 2 (id:??) landed as
https://crrev.com/6777bcc56aff3a1dc27fb0adced9a462b22e4e33
Cr-Commit-Position: refs/heads/master@{#398890}