RL: Stop source and fire MediaStreamTrack ended event if missing audio input callbacks are detected.

Re-land: Stop source and fire MediaStreamTrack ended event if missing audio input callbacks are detected.

This way the javascript application is informed when there's a problem with the input device. Since the detection is in the renderer process we would also catch any other issues in the pipeline, including on the process border (i.e. the socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so that other components down the stack have a chance to defer start if needed and report startup success stats. Specifically the Mac implementation. See also code comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with https://codereview.chromium.org/2891303002/ and using a WebRTC test page with the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001 (http://crrev.com/2888383002#ps300001)

BUG=722335, webrtc:4799
TEST=See above.
TBR=ossu@chromium.org, tommi@chromium.org, isherman@chromium.org

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2899413004
Cr-Commit-Position: refs/heads/master@{#474628}
Committed: https://chromium.googlesource.com/chromium/src/+/021d9e05dc8384ba3d5f7c1c1834ee0bf9fd3e5c

[Henrik Grunell, 2017-05-25 05:52:48]

Description was changed from

==========
Re-land: Stop source and fire MediaStreamTrack ended event if missing audio
input callbacks are detected.

This way the javascript application is informed when there's a problem with the
input device. Since the detection is in the renderer process we would also catch
any other issues in the pipeline, including on the process border (i.e. the
socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device
which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source
and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so
that other components down the stack have a chance to defer start if needed and
report startup success stats. Specifically the Mac implementation. See also code
comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with
https://codereview.chromium.org/2891303002/ and using a WebRTC test page with
the MediaStreamTrack.onended set to log that the event was fired.

BUG=722335
TEST=See above.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2888383002
Cr-Commit-Position: refs/heads/master@{#474383}
Committed:
https://chromium.googlesource.com/chromium/src/+/dc44616c932d390c6ee11c2eb593...

patch from issue 2888383002 at patchset 300001
(http://crrev.com/2888383002#ps300001)
==========

to

==========
Re-land: Stop source and fire MediaStreamTrack ended event if missing audio
input callbacks are detected.

This way the javascript application is informed when there's a problem with the
input device. Since the detection is in the renderer process we would also catch
any other issues in the pipeline, including on the process border (i.e. the
socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device
which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source
and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so
that other components down the stack have a chance to defer start if needed and
report startup success stats. Specifically the Mac implementation. See also code
comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with
https://codereview.chromium.org/2891303002/ and using a WebRTC test page with
the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001
(http://crrev.com/2888383002#ps300001)

BUG=722335
TEST=See above.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Henrik Grunell, 2017-05-25 06:15:54]

Description was changed from

==========
Re-land: Stop source and fire MediaStreamTrack ended event if missing audio
input callbacks are detected.

This way the javascript application is informed when there's a problem with the
input device. Since the detection is in the renderer process we would also catch
any other issues in the pipeline, including on the process border (i.e. the
socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device
which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source
and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so
that other components down the stack have a chance to defer start if needed and
report startup success stats. Specifically the Mac implementation. See also code
comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with
https://codereview.chromium.org/2891303002/ and using a WebRTC test page with
the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001
(http://crrev.com/2888383002#ps300001)

BUG=722335
TEST=See above.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Re-land: Stop source and fire MediaStreamTrack ended event if missing audio
input callbacks are detected.

This way the javascript application is informed when there's a problem with the
input device. Since the detection is in the renderer process we would also catch
any other issues in the pipeline, including on the process border (i.e. the
socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device
which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source
and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so
that other components down the stack have a chance to defer start if needed and
report startup success stats. Specifically the Mac implementation. See also code
comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with
https://codereview.chromium.org/2891303002/ and using a WebRTC test page with
the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001
(http://crrev.com/2888383002#ps300001)

BUG=722335
TEST=See above.
TBR=ossu@chromium.org, tommi@chromium.org, isherman@chromium.org

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Henrik Grunell, 2017-05-25 06:15:54]

grunell@chromium.org changed reviewers:
+ isherman@chromium.org, ossu@chromium.org, tommi@chromium.org

[Henrik Grunell, 2017-05-25 09:07:01]

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

[commit-bot: I haz the power, 2017-05-25 09:07:16]

Dry run: CQ is trying da patch.

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

[Henrik Grunell, 2017-05-25 09:32:20]

Created re-land CL. In the original CL the timer was deleted in the AID dtor,
which obviously was called on another thread than created on -thread check hit.

In this CL I'm creating the timer in Start() and deleting it in Stop(). Stopping
fully before deleting AID is already enforced.

Whether it's OK to delete AID on another thread I don't have time to verify now,
but I've made a note and will revisit next week.

[Henrik Grunell, 2017-05-25 09:55:35]

On 2017/05/25 09:32:20, Henrik Grunell wrote:
> Created re-land CL. In the original CL the timer was deleted in the AID dtor,
> which obviously was called on another thread than created on -thread check
hit.
> 
> In this CL I'm creating the timer in Start() and deleting it in Stop().
Stopping
> fully before deleting AID is already enforced.
> 
> Whether it's OK to delete AID on another thread I don't have time to verify
now,
> but I've made a note and will revisit next week.

And this hit only happened in a WebRTC test that is not run in the ordinary
Chromium waterfall or try bots. I've now run all WebRTC tests that I could
locally.

[commit-bot: I haz the power, 2017-05-25 10:58:43]

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

[commit-bot: I haz the power, 2017-05-25 10:58:44]

Dry run: This issue passed the CQ dry run.

[Henrik Grunell, 2017-05-25 11:25:47]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2017-05-25 11:26:05]

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-05-25 11:30:31]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495711547306510,
"parent_rev": "fd9e94be64898b22c757a5c0567ac8bb396e5ed9", "commit_rev":
"021d9e05dc8384ba3d5f7c1c1834ee0bf9fd3e5c"}

[commit-bot: I haz the power, 2017-05-25 11:30:45]

Description was changed from

==========
Re-land: Stop source and fire MediaStreamTrack ended event if missing audio
input callbacks are detected.

This way the javascript application is informed when there's a problem with the
input device. Since the detection is in the renderer process we would also catch
any other issues in the pipeline, including on the process border (i.e. the
socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device
which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source
and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so
that other components down the stack have a chance to defer start if needed and
report startup success stats. Specifically the Mac implementation. See also code
comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with
https://codereview.chromium.org/2891303002/ and using a WebRTC test page with
the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001
(http://crrev.com/2888383002#ps300001)

BUG=722335
TEST=See above.
TBR=ossu@chromium.org, tommi@chromium.org, isherman@chromium.org

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Re-land: Stop source and fire MediaStreamTrack ended event if missing audio
input callbacks are detected.

This way the javascript application is informed when there's a problem with the
input device. Since the detection is in the renderer process we would also catch
any other issues in the pipeline, including on the process border (i.e. the
socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device
which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source
and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so
that other components down the stack have a chance to defer start if needed and
report startup success stats. Specifically the Mac implementation. See also code
comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with
https://codereview.chromium.org/2891303002/ and using a WebRTC test page with
the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001
(http://crrev.com/2888383002#ps300001)

BUG=722335
TEST=See above.
TBR=ossu@chromium.org, tommi@chromium.org, isherman@chromium.org

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2899413004
Cr-Commit-Position: refs/heads/master@{#474628}
Committed:
https://chromium.googlesource.com/chromium/src/+/021d9e05dc8384ba3d5f7c1c1834...
==========

[commit-bot: I haz the power, 2017-05-25 11:30:46]

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

[Ilya Sherman, 2017-05-25 22:03:22]

On 2017/05/25 11:30:46, commit-bot: I haz the power wrote:
> Committed patchset #3 (id:40001) as
>
https://chromium.googlesource.com/chromium/src/+/021d9e05dc8384ba3d5f7c1c1834...

Hmm, it looks like this wasn't reviewed by anyone?  I think it's fine to TBR the
original CL's reviewers as you did, but please do have at least one person, who
is familiar with the code, review the diff between the previously-reverted CL
and the latest iteration.

[tommi (sloooow) - chröme, 2017-05-26 08:59:02]

in the spirit of better late than never, lgtm :)

[ossu-chromium, 2017-05-26 11:51:33]

lgtm from me as well!

[Henrik Grunell, 2017-05-26 21:01:02]

On 2017/05/25 22:03:22, Ilya Sherman wrote:
> On 2017/05/25 11:30:46, commit-bot: I haz the power wrote:
> > Committed patchset #3 (id:40001) as
> >
>
https://chromium.googlesource.com/chromium/src/+/021d9e05dc8384ba3d5f7c1c1834...
> 
> Hmm, it looks like this wasn't reviewed by anyone?  I think it's fine to TBR
the
> original CL's reviewers as you did, but please do have at least one person,
who
> is familiar with the code, review the diff between the previously-reverted CL
> and the latest iteration.

You're right, the fix was too large to land before any review.

[Henrik Grunell, 2017-06-05 08:48:30]

Description was changed from

==========
Re-land: Stop source and fire MediaStreamTrack ended event if missing audio
input callbacks are detected.

This way the javascript application is informed when there's a problem with the
input device. Since the detection is in the renderer process we would also catch
any other issues in the pipeline, including on the process border (i.e. the
socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device
which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source
and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so
that other components down the stack have a chance to defer start if needed and
report startup success stats. Specifically the Mac implementation. See also code
comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with
https://codereview.chromium.org/2891303002/ and using a WebRTC test page with
the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001
(http://crrev.com/2888383002#ps300001)

BUG=722335
TEST=See above.
TBR=ossu@chromium.org, tommi@chromium.org, isherman@chromium.org

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2899413004
Cr-Commit-Position: refs/heads/master@{#474628}
Committed:
https://chromium.googlesource.com/chromium/src/+/021d9e05dc8384ba3d5f7c1c1834...
==========

to

==========
Re-land: Stop source and fire MediaStreamTrack ended event if missing audio
input callbacks are detected.

This way the javascript application is informed when there's a problem with the
input device. Since the detection is in the renderer process we would also catch
any other issues in the pipeline, including on the process border (i.e. the
socket pair), should it render the same type of symptom.

* Detection is done in AudioInputDevice as this corresponds to a physical device
which is what we want to detect missing callbacks for.
* When detected we call CaptureError() on the capturer, which stops the source
and puts the track in ended state and fires the ended event.
* The time with missing callbacks before detecting is chosen to 12 seconds, so
that other components down the stack have a chance to defer start if needed and
report startup success stats. Specifically the Mac implementation. See also code
comment in the CL.
* UMA stats is added; reporting a boolean when stopping.

Tested manually by altering a Linux build with
https://codereview.chromium.org/2891303002/ and using a WebRTC test page with
the MediaStreamTrack.onended set to log that the event was fired.

Original patch (1) from issue 2888383002 at patchset 300001
(http://crrev.com/2888383002#ps300001)

BUG=722335,webrtc:4799
TEST=See above.
TBR=ossu@chromium.org, tommi@chromium.org, isherman@chromium.org

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2899413004
Cr-Commit-Position: refs/heads/master@{#474628}
Committed:
https://chromium.googlesource.com/chromium/src/+/021d9e05dc8384ba3d5f7c1c1834...
==========