Speculative fix for ui::DisplayLinkMac::StopDisplayLink crashes

Speculative fix for ui::DisplayLinkMac::StopDisplayLink crashes

The CVDisplayLink is tied to a ScopedTypeRef data member of DisplayLinkMac.
However, there may be other references. Since there's no guarantee that
~DisplayLinkMac() will invoke the last CVDisplayLinkRelease(..) and clear
the callback automatically, clear it explicitly in ~DisplayLinkMac.

BUG=564780
Committed: https://crrev.com/2286ddf0119516f7a93feaff4f8ce2edff8c4543
Cr-Commit-Position: refs/heads/master@{#371648}

[tapted, 2016-01-25 04:59:00]

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

[tapted, 2016-01-25 04:59:01]

Hi Chris - WDYT? I've verified that doing this correctly reports kCGErrorSuccess
when closing a window (i.e. Quartz is fine taking nullptrs here).

(Meant to send this out earlier.. turns out branch-point-Friday is a bad day to
plan stuff :p).

[ccameron, 2016-01-25 18:04:18]

Thanks! lgtm

https://codereview.chromium.org/1626163002/diff/1/ui/accelerated_widget_mac/d...
File ui/accelerated_widget_mac/display_link_mac.cc (right):

https://codereview.chromium.org/1626163002/diff/1/ui/accelerated_widget_mac/d...
ui/accelerated_widget_mac/display_link_mac.cc:91: // destructor completes.
Ensure the callback is cleared out regardless.
Maybe reference a bug # for the crashes?

[tapted, 2016-01-26 23:36:50]

https://codereview.chromium.org/1626163002/diff/1/ui/accelerated_widget_mac/d...
File ui/accelerated_widget_mac/display_link_mac.cc (right):

https://codereview.chromium.org/1626163002/diff/1/ui/accelerated_widget_mac/d...
ui/accelerated_widget_mac/display_link_mac.cc:91: // destructor completes.
Ensure the callback is cleared out regardless.
On 2016/01/25 18:04:17, ccameron wrote:
> Maybe reference a bug # for the crashes?

Done.

[tapted, 2016-01-26 23:36:57]

The CQ bit was checked by tapted@chromium.org

[tapted, 2016-01-26 23:36:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
ccameron@chromium.org
Link to the patchset: https://codereview.chromium.org/1626163002/#ps20001
(title: "reference bug")

[commit-bot: I haz the power, 2016-01-26 23:37:35]

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

[commit-bot: I haz the power, 2016-01-27 00:09:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-27 00:11:26]

Description was changed from

==========
Speculative fix for ui::DisplayLinkMac::StopDisplayLink crashes

The CVDisplayLink is tied to a ScopedTypeRef data member of DisplayLinkMac.
However, there may be other references. Since there's no guarantee that
~DisplayLinkMac() will invoke the last CVDisplayLinkRelease(..) and clear
the callback automatically, clear it explicitly in ~DisplayLinkMac.

BUG=564780
==========

to

==========
Speculative fix for ui::DisplayLinkMac::StopDisplayLink crashes

The CVDisplayLink is tied to a ScopedTypeRef data member of DisplayLinkMac.
However, there may be other references. Since there's no guarantee that
~DisplayLinkMac() will invoke the last CVDisplayLinkRelease(..) and clear
the callback automatically, clear it explicitly in ~DisplayLinkMac.

BUG=564780

Committed: https://crrev.com/2286ddf0119516f7a93feaff4f8ce2edff8c4543
Cr-Commit-Position: refs/heads/master@{#371648}
==========

[commit-bot: I haz the power, 2016-01-27 00:11:26]

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

[pdr., 2016-01-27 04:26:16]

On 2016/01/27 at 00:11:26, commit-bot wrote:
> Patchset 2 (id:??) landed as
https://crrev.com/2286ddf0119516f7a93feaff4f8ce2edff8c4543
> Cr-Commit-Position: refs/heads/master@{#371648}

I think this broke the mac build:
../../ui/accelerated_widget_mac/display_link_mac.cc:94:69: error: null passed to
a callee that requires a non-null argument [-Werror,-Wnonnull]
      CVDisplayLinkSetOutputCallback(display_link_, nullptr, nullptr);
                                                    ~~~~~~~         ^

Let me see if there's an easy fix (the mac bot is down). If not, I'll roll this
out.

[pdr., 2016-01-27 04:30:07]

On 2016/01/27 at 04:26:16, pdr wrote:
> On 2016/01/27 at 00:11:26, commit-bot wrote:
> > Patchset 2 (id:??) landed as
https://crrev.com/2286ddf0119516f7a93feaff4f8ce2edff8c4543
> > Cr-Commit-Position: refs/heads/master@{#371648}
> 
> I think this broke the mac build:
> ../../ui/accelerated_widget_mac/display_link_mac.cc:94:69: error: null passed
to a callee that requires a non-null argument [-Werror,-Wnonnull]
>       CVDisplayLinkSetOutputCallback(display_link_, nullptr, nullptr);
>                                                     ~~~~~~~         ^
> 
> Let me see if there's an easy fix (the mac bot is down). If not, I'll roll
this out.

I'm afraid I don't know how to fix this. I'll roll this out for now so the build
is fixed.

[pdr., 2016-01-27 04:30:28]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1637313002/ by pdr@chromium.org.

The reason for reverting is: Broke compile:
../../ui/accelerated_widget_mac/display_link_mac.cc:94:69: error: null passed to
a callee that requires a non-null argument [-Werror,-Wnonnull]
      CVDisplayLinkSetOutputCallback(display_link_, nullptr, nullptr);
                                                    ~~~~~~~         ^
.

[tapted, 2016-01-27 04:35:10]

On 2016/01/27 04:30:07, pdr wrote:
> On 2016/01/27 at 04:26:16, pdr wrote:
> > On 2016/01/27 at 00:11:26, commit-bot wrote:
> > > Patchset 2 (id:??) landed as
> https://crrev.com/2286ddf0119516f7a93feaff4f8ce2edff8c4543
> > > Cr-Commit-Position: refs/heads/master@{#371648}
> > 
> > I think this broke the mac build:
> > ../../ui/accelerated_widget_mac/display_link_mac.cc:94:69: error: null
passed
> to a callee that requires a non-null argument [-Werror,-Wnonnull]
> >       CVDisplayLinkSetOutputCallback(display_link_, nullptr, nullptr);
> >                                                     ~~~~~~~         ^
> > 
> > Let me see if there's an easy fix (the mac bot is down). If not, I'll roll
> this out.
> 
> I'm afraid I don't know how to fix this. I'll roll this out for now so the
build
> is fixed.

sgtm - I'm investigating now. Is there a link to a bot or is that an error you
got locally? (It's super-weird - probably a particular SDK thing which I can
probably repro by updating.. something.)