Add a NOTREACHED and uma stat when the audio callback fails

Add a NOTREACHED and uma stat when the audio callback fails.

It is used to catch problem where the audio fails silently.

BUG=428706
Committed: https://crrev.com/07a90a91a98021cd80bd10122dcdd2493d0362d1
Cr-Commit-Position: refs/heads/master@{#302104}

[no longer working on chromium, 2014-10-30 13:37:27]

xians@chromium.org changed reviewers:
+ asvitkine@chromium.org, tommi@chromium.org

[no longer working on chromium, 2014-10-30 13:38:04]

Please review. Thanks.

[Alexei Svitkine (slow), 2014-10-30 13:51:09]

https://codereview.chromium.org/685403002/diff/1/media/audio/mac/audio_low_la...
File media/audio/mac/audio_low_latency_input_mac.cc (right):

https://codereview.chromium.org/685403002/diff/1/media/audio/mac/audio_low_la...
media/audio/mac/audio_low_latency_input_mac.cc:502: NOTREACHED();
Why NOTREACHED()? If it's possible the OS API can return an error, then it's not
notreached...

https://codereview.chromium.org/685403002/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/685403002/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:11699: +    Erroe codes that
AudioUnitRender() returns in AUAudioInputStream on Mac.
Nit: Error.

[no longer working on chromium, 2014-10-30 13:56:57]

https://codereview.chromium.org/685403002/diff/1/media/audio/mac/audio_low_la...
File media/audio/mac/audio_low_latency_input_mac.cc (right):

https://codereview.chromium.org/685403002/diff/1/media/audio/mac/audio_low_la...
media/audio/mac/audio_low_latency_input_mac.cc:502: NOTREACHED();
On 2014/10/30 13:51:09, Alexei Svitkine wrote:
> Why NOTREACHED()? If it's possible the OS API can return an error, then it's
not
> notreached...

The error is unexpected to Chrome, when this happens, the audio fails silently,
which is the worst situation as we don't know what has happened. So we want to
crash in debug mode.
NOTREACHED() == DCHECK(false)

https://codereview.chromium.org/685403002/diff/1/tools/metrics/histograms/his...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/685403002/diff/1/tools/metrics/histograms/his...
tools/metrics/histograms/histograms.xml:11699: +    Erroe codes that
AudioUnitRender() returns in AUAudioInputStream on Mac.
On 2014/10/30 13:51:09, Alexei Svitkine wrote:
> Nit: Error.

Done.

[Alexei Svitkine (slow), 2014-10-30 13:58:18]

lgtm, I guess the NOTREACHED is OK if it's temporary to help diagnose issues.

[tommi (sloooow) - chröme, 2014-10-30 14:03:16]

lgtm

[no longer working on chromium, 2014-10-30 14:04:33]

The CQ bit was checked by xians@chromium.org

[no longer working on chromium, 2014-10-30 14:04:45]

On 2014/10/30 14:03:16, tommi wrote:
> lgtm

Thanks guys.

[commit-bot: I haz the power, 2014-10-30 14:05:41]

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

[commit-bot: I haz the power, 2014-10-30 17:55:30]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-30 17:56:02]

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

[DaleCurtis, 2014-10-30 23:24:52]

dalecurtis@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2014-10-30 23:24:53]

This stat has no balancing factory, so we can't determine what percentages of
clients are hitting this out of successful render calls.

[Alexei Svitkine (slow), 2014-10-31 12:42:39]

Good point. Maybe log the call's result even on success?

On Thu, Oct 30, 2014 at 7:24 PM, <dalecurtis@chromium.org> wrote:

> This stat has no balancing factory, so we can't determine what percentages
> of
> clients are hitting this out of successful render calls.
>
> https://codereview.chromium.org/685403002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[DaleCurtis, 2014-10-31 18:12:52]

Yeah, this is high frequency though, up to every ~2ms, so it may be better to
manually convert to a percentage and log success and failures at destruction
time instead.

[tommi (sloooow) - chröme, 2014-10-31 18:51:55]

The goal was to know what errors are the most common. We have other stats
that we can use to figure out rough percentages.

On Fri, Oct 31, 2014, 19:12 null <dalecurtis@chromium.org> wrote:

> Yeah, this is high frequency though, up to every ~2ms, so it may be better
> to
> manually convert to a percentage and log success and failures at
> destruction
> time instead.
>
> https://codereview.chromium.org/685403002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[no longer working on chromium, 2014-10-31 20:55:44]

I agree with Tommi that the goal is to understand which errors we run into, and
for the long term, we should put a CHECK there after making sure it won't fail
any more.