Destroy WebRTCStatsReportCallbackResolver on main thread, fixes crash.

Destroy WebRTCStatsReportCallbackResolver on main thread, fixes crash.

The WebRTCStatsReportCallbackResolver has a
Persistent<ScriptPromiseResolver> and needs to be destroyed on the main
thread (blink/javascript thread). The callback resolver is an
implementation of WebRTCStatsReportCallback which is referenced and
owned by GetRTCStatsCallback::callback_.

callback_.reset() added to OnStatsDeliveredOnMainThread.
Previously it was implicitly reset by the destructor, which could
occur either on the webrtc signaling thread or the main thread
depending on which thread held the last reference to the
GetRTCStatsCallback (race).

BUG=700068
Review-Url: https://codereview.chromium.org/2772283002
Cr-Commit-Position: refs/heads/master@{#459784}
Committed: https://chromium.googlesource.com/chromium/src/+/b39b8d6fb03d46dff5556e7a5c428ac9302f5579

[hbos_chromium, 2017-03-27 12:14:49]

The CQ bit was checked by hbos@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 12:15:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hbos_chromium, 2017-03-27 12:15:44]

hbos@chromium.org changed reviewers:
+ guidou@chromium.org

[hbos_chromium, 2017-03-27 12:15:44]

Please take a look, guidou.

[Guido Urdaneta, 2017-03-27 12:19:46]

https://codereview.chromium.org/2772283002/diff/1/content/renderer/media/rtc_...
File content/renderer/media/rtc_peer_connection_handler.cc (right):

https://codereview.chromium.org/2772283002/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_peer_connection_handler.cc:757: callback_.release();
I think you want .reset() here.
release() does not delete the pointer.

[hbos_chromium, 2017-03-27 12:21:03]

Description was changed from

==========
Destroy WebRTCStatsReportCallbackResolver on main thread, fixes crash.

The WebRTCStatsReportCallbackResolver has a
Persistent<ScriptPromiseResolver> and needs to be destroyed on the main
thread (blink/javascript thread). The callback resolver is an
implementation of WebRTCStatsReportCallback which is referenced and
owned by GetRTCStatsCallback::callback_.

callback_.release() added to OnStatsDeliveredOnMainThread.
Previously it was implicitly released by the destructor, which could
occur either on the webrtc signaling thread or the main thread
depending on which thread held the last reference to the
GetRTCStatsCallback (race).

BUG=700068
==========

to

==========
Destroy WebRTCStatsReportCallbackResolver on main thread, fixes crash.

The WebRTCStatsReportCallbackResolver has a
Persistent<ScriptPromiseResolver> and needs to be destroyed on the main
thread (blink/javascript thread). The callback resolver is an
implementation of WebRTCStatsReportCallback which is referenced and
owned by GetRTCStatsCallback::callback_.

callback_.reset() added to OnStatsDeliveredOnMainThread.
Previously it was implicitly reset by the destructor, which could
occur either on the webrtc signaling thread or the main thread
depending on which thread held the last reference to the
GetRTCStatsCallback (race).

BUG=700068
==========

[hbos_chromium, 2017-03-27 12:22:14]

The CQ bit was checked by hbos@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 12:22:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[hbos_chromium, 2017-03-27 12:22:45]

PTAL guidou

https://codereview.chromium.org/2772283002/diff/1/content/renderer/media/rtc_...
File content/renderer/media/rtc_peer_connection_handler.cc (right):

https://codereview.chromium.org/2772283002/diff/1/content/renderer/media/rtc_...
content/renderer/media/rtc_peer_connection_handler.cc:757: callback_.release();
On 2017/03/27 12:19:46, Guido Urdaneta wrote:
> I think you want .reset() here.
> release() does not delete the pointer.

Oops, yes, done.

[Guido Urdaneta, 2017-03-27 12:23:55]

lgtm

[hbos_chromium, 2017-03-27 12:29:19]

The CQ bit was checked by hbos@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-27 12:29:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-27 13:50:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-27 13:50:05]

Dry run: This issue passed the CQ dry run.

[Guido Urdaneta, 2017-03-27 14:56:44]

still lgtm

[hbos_chromium, 2017-03-27 14:57:23]

The CQ bit was checked by hbos@chromium.org

[commit-bot: I haz the power, 2017-03-27 14:57:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-27 15:05:07]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490626643975330,
"parent_rev": "60ae8b467eae2696d4df9f87616b381dcaaf02b8", "commit_rev":
"b39b8d6fb03d46dff5556e7a5c428ac9302f5579"}

[commit-bot: I haz the power, 2017-03-27 15:05:55]

Description was changed from

==========
Destroy WebRTCStatsReportCallbackResolver on main thread, fixes crash.

The WebRTCStatsReportCallbackResolver has a
Persistent<ScriptPromiseResolver> and needs to be destroyed on the main
thread (blink/javascript thread). The callback resolver is an
implementation of WebRTCStatsReportCallback which is referenced and
owned by GetRTCStatsCallback::callback_.

callback_.reset() added to OnStatsDeliveredOnMainThread.
Previously it was implicitly reset by the destructor, which could
occur either on the webrtc signaling thread or the main thread
depending on which thread held the last reference to the
GetRTCStatsCallback (race).

BUG=700068
==========

to

==========
Destroy WebRTCStatsReportCallbackResolver on main thread, fixes crash.

The WebRTCStatsReportCallbackResolver has a
Persistent<ScriptPromiseResolver> and needs to be destroyed on the main
thread (blink/javascript thread). The callback resolver is an
implementation of WebRTCStatsReportCallback which is referenced and
owned by GetRTCStatsCallback::callback_.

callback_.reset() added to OnStatsDeliveredOnMainThread.
Previously it was implicitly reset by the destructor, which could
occur either on the webrtc signaling thread or the main thread
depending on which thread held the last reference to the
GetRTCStatsCallback (race).

BUG=700068

Review-Url: https://codereview.chromium.org/2772283002
Cr-Commit-Position: refs/heads/master@{#459784}
Committed:
https://chromium.googlesource.com/chromium/src/+/b39b8d6fb03d46dff5556e7a5c42...
==========

[commit-bot: I haz the power, 2017-03-27 15:05:56]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/b39b8d6fb03d46dff5556e7a5c42...

[sof, 2017-03-28 06:40:54]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2017-03-28 06:40:55]

that heads off any finalization complexity, good fix.