Add a hang monitor for the audio thread.

Add a hang monitor for the audio thread.

We've seen a bunch of reports in the field that the audio thread is
getting hung for significant (perhaps indefinite) amounts of time;
to help debug these reports, add a hang monitor which will crash the
browser process and ensure we get a thread dump of what's going on.

The hang timer is set extremely conservatively right now, at 1 minute,
which is insanely long for audio tasks. I'll monitor crash reports and
adjust it as necessary to find culprits.

The hang monitor uses the browser I/O thread for hang checks, which is
similar to what other hung task monitors seem to do. To do this I had
to move AudioManager and MidiManager creation to after the browser
threads have been spun up. Which shouldn't be an issue...

BUG=422522, 469827
TEST=hang time exceeded ==> crash.

Committed: https://crrev.com/fecee0475788ffe261a233d1cc6d2d885ef7269e
Cr-Commit-Position: refs/heads/master@{#322276}

[DaleCurtis, 2015-03-24 23:13:41]

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

[DaleCurtis, 2015-03-24 23:14:33]

cc:tommi,henrika as FYI.

[henrika (OOO until Aug 14), 2015-03-25 13:25:55]

Great stuff Dale. Would it be beneficial to add UMA to this?

[nasko, 2015-03-25 17:28:00]

I have just a couple of questions, otherwise looks good.

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.cc
File media/audio/audio_manager.cc (right):

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.c...
media/audio/audio_manager.cc:28: CHECK(!monitor_task_runner_);
nit: Can we also check that this is called on the expected thread? Your comment
says it is on the IO thread, but you mentioned that on OSX it behaves
differently in the previous discussion.

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.c...
media/audio/audio_manager.cc:40: base::Unretained(this)),
Are we guaranteed that this object will never be null?

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.h
File media/audio/audio_manager.h (right):

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.h...
media/audio/audio_manager.h:39: // a hang is detected, the process will be
crashed.
nit: s/crashed/terminated/.

[DaleCurtis, 2015-03-25 20:28:56]

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.cc
File media/audio/audio_manager.cc (right):

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.c...
media/audio/audio_manager.cc:28: CHECK(!monitor_task_runner_);
On 2015/03/25 17:28:00, nasko wrote:
> nit: Can we also check that this is called on the expected thread? Your
comment
> says it is on the IO thread, but you mentioned that on OSX it behaves
> differently in the previous discussion.

Sent you a chat about this, because I'm not clear on exactly what you want to
verify here. The IO thread is just used to ensure we have an unblocked thread to
check on the audio thread (which may be its own thread or reuse the UI thread on
OSX).

On OSX this would essentially end up being a hang monitor for the UI thread;
which seems unnecessary since there's probably already one. So I'll probably add
a !OSX condition below.

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.c...
media/audio/audio_manager.cc:40: base::Unretained(this)),
On 2015/03/25 17:28:00, nasko wrote:
> Are we guaranteed that this object will never be null?

Yes, it's a LazyInstance::Leaky, which means it'll be leaked at shutdown.

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.h
File media/audio/audio_manager.h (right):

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.h...
media/audio/audio_manager.h:39: // a hang is detected, the process will be
crashed.
On 2015/03/25 17:28:00, nasko wrote:
> nit: s/crashed/terminated/.

Will change in followup patch set after discussion concludes.

[DaleCurtis, 2015-03-25 20:29:14]

henrika: No need to UMA, this is going to crash the entire browser so we'll get
a dump :)

[nasko, 2015-03-25 20:52:23]

LGTM

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.cc
File media/audio/audio_manager.cc (right):

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.c...
media/audio/audio_manager.cc:28: CHECK(!monitor_task_runner_);
On 2015/03/25 20:28:56, DaleCurtis wrote:
> On 2015/03/25 17:28:00, nasko wrote:
> > nit: Can we also check that this is called on the expected thread? Your
> comment
> > says it is on the IO thread, but you mentioned that on OSX it behaves
> > differently in the previous discussion.
> 
> Sent you a chat about this, because I'm not clear on exactly what you want to
> verify here. The IO thread is just used to ensure we have an unblocked thread
to
> check on the audio thread (which may be its own thread or reuse the UI thread
on
> OSX).
> 
> On OSX this would essentially end up being a hang monitor for the UI thread;
> which seems unnecessary since there's probably already one. So I'll probably
add
> a !OSX condition below.

Clarified over chat. Since this method isn't expected to execute on one specific
thread (platform specific details), we can't enforce a CURRENTLY_ON style check.

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.c...
media/audio/audio_manager.cc:40: base::Unretained(this)),
On 2015/03/25 20:28:56, DaleCurtis wrote:
> On 2015/03/25 17:28:00, nasko wrote:
> > Are we guaranteed that this object will never be null?
> 
> Yes, it's a LazyInstance::Leaky, which means it'll be leaked at shutdown.

Acknowledged.

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.h
File media/audio/audio_manager.h (right):

https://codereview.chromium.org/1036703002/diff/1/media/audio/audio_manager.h...
media/audio/audio_manager.h:39: // a hang is detected, the process will be
crashed.
On 2015/03/25 20:28:56, DaleCurtis wrote:
> On 2015/03/25 17:28:00, nasko wrote:
> > nit: s/crashed/terminated/.
> 
> Will change in followup patch set after discussion concludes.

No need to change. After chatting, it is actually more correct to have crash
here than terminate.
Thanks!

[DaleCurtis, 2015-03-25 22:28:01]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2015-03-25 22:28:02]

The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1036703002/#ps20001
(title: "Disable for OSX.")

[commit-bot: I haz the power, 2015-03-25 22:28:35]

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

[commit-bot: I haz the power, 2015-03-25 23:45:27]

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

[commit-bot: I haz the power, 2015-03-25 23:45:27]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[DaleCurtis, 2015-03-26 00:01:14]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2015-03-26 00:02:43]

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

[commit-bot: I haz the power, 2015-03-26 00:37:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-26 00:38:42]

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