I vaguely remember locks are held by the caller class, so 1) and 2) cannot run simultaneously.

And |null_audio_sink_| paused before |sink_| starts playing should be fine. |null_audio_sink| is filled with empty data, and it simply discards them.

it is also fine that |sink_| is paused before |null_audio_sink_| is started. The null audio sink consumes nothing, if there is non empty data coming,  if will be copied to the buffer_after_silence_

Render() is a callback call, the callers classes are |sink_| and |null_audio_sink_|, so there is no lock shared and (1) and (2) can happen simultaneously.

If client_callback_->render(..) (https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/media/renderer_webaudiodevice_impl.cc&q=RendererWebAudioDeviceI&sq=package:chromium&l=123) have a lock, than we are lucky to not corrupt data being rendered, but we can't rely on that lock: Render() pipeline is traditionally not re-enterable, so that's an implicit contract. If there are some locks there - it's not because the code expects concurrent calls to the render method, but because of protecting some control data/state. So those lock can go away if the design changes. I looked up 3 layers up and haven't found any locks in the current code (probably I missed something though). So calling client_callback_->render(..) concurrently looks very dangerous.

It's always fine when one sinks pauses  before another starts playing - that's exactly what we need. The problems is when one sink pauses _after_ another starts playing. For example, if (null_audio_sink_) has not stopped immediatly after receiving |first_buffer_after_silence_|, and the next callback comes from the null sink as well, it means |first_buffer_after_silence_| is rendered to the null sink (here https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/media/renderer_webaudiodevice_impl.cc&q=RendererWebAudioDeviceI&sq=package:chromium&l=107).

hmm, if (null_audio_sink_) has not stopped immediatly after receiving |first_buffer_after_silence_|, it will still set |is_using_null_audio_sink_| to false.

And if the next callback is still from null sink, we should hit the DCHECK here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/media/renderer_webaudiodevice_impl.cc&q=RendererWebAudioDeviceI&sq=package:chromium&l=108

we don't hit the DCHECK probably because the silent buffer before |first_buffer_after_silence_| always contain some data. So it takes the null_sink some time to consume data before calling back Render(). And before that happens, it have already received the Stop() call.

null sink does nothing, it's Render() method who does the work. And its sets both here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/media/renderer_webaudiodevice_impl.cc&q=RendererWebAudioDeviceI&sq=package:chromium&l=134
      is_using_null_audio_sink_ = false;
      is_first_buffer_after_silence_ = true;
So DCHECK will never be hit (it's actually redundant).
|is_using_null_audio_sink_| does not mean that Render() is call from null sink, it only means the flag was set in Render() call.

See my comments about threading: Play/Pause are not synchronous calls. They are executed on IO thread. So, on IO thread first |sink_| starts, then |null_audio_sink_| is paused.

I looked at the code, but there doesn't seem to me any race issues. Here is what happens when audio transitions from silence to non-silence:

1.FakeAudioWorker::Worker::DoRead() calls NullAudioSink::CallRender()
2.NullAudioSink::CallRender() calls RendererWebAudioDeviceImpl::Render(), and finds non-silent audio from the data.
3. first_buffer_after_silence_ is set to true, a task is posted to pause the null audio sink on main thread,  and sink_->play() is called.
4. FakeAudioWorker::Worker::DoRead() now posts a task to schedule the next DoRead() on main thread.

Since the task posted in 3 is before the task posted in 4, NullAudioSink::Pause() always executes before the next FakeAudioWorker::Worker::DoRead() is executed. As a result, there shouldn't be another call to RendererWebAudioDeviceImpl::Render() from null_audio_sink

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b770266d26fc09ab8fb746562d008c5d436e4784

commit b770266d26fc09ab8fb746562d008c5d436e4784
Author: dalecurtis <dalecurtis@chromium.org>
Date: Tue Sep 27 04:54:40 2016

Break out WebAudio suspension code into new class. Add tests.

We've had enough problems with this code over the past year that it needs
to be extracted into something less burdensome on the WebAudio code. It
also needs some better testing.

This creates a new class called SilentSinkSuspender which is only used on
Android today and adds tests for it. It also changes the implementation
of suspension from directly calling into the sink to instead post tasks
to a provided task runner (render thread).

Notably, fake render callbacks are now driven on the media thread instead
of the render thread since WebAudio and Oilpan have a plethora of checks
around what is and isn't run on the main thread. This also means more than
one Render() may occur during transition. Too many of these can disrupt
the WebAudio clock, but given the 30 second timeout this should be pretty
rare in actual WebAudio usage situations.

BUG= 649018 ,  614978 
TEST=new unittests, no more deadlock upon idle timeout.

Review-Url: https://codereview.chromium.org/2365723003
Cr-Commit-Position: refs/heads/master@{#421108}

[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/content/renderer/media/renderer_webaudiodevice_impl.h
[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/BUILD.gn
[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/fake_audio_render_callback.cc
[modify] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/fake_audio_render_callback.h
[add] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/silent_sink_suspender.cc
[add] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/silent_sink_suspender.h
[add] https://crrev.com/b770266d26fc09ab8fb746562d008c5d436e4784/media/base/silent_sink_suspender_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/942fee339500a935be00ac81d3a71e4991aa6666

commit 942fee339500a935be00ac81d3a71e4991aa6666
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Sep 28 21:30:19 2016

Merge M54: "Break out WebAudio suspension code into new class. Add tests."

We've had enough problems with this code over the past year that it needs
to be extracted into something less burdensome on the WebAudio code. It
also needs some better testing.

This creates a new class called SilentSinkSuspender which is only used on
Android today and adds tests for it. It also changes the implementation
of suspension from directly calling into the sink to instead post tasks
to a provided task runner (render thread).

Notably, fake render callbacks are now driven on the media thread instead
of the render thread since WebAudio and Oilpan have a plethora of checks
around what is and isn't run on the main thread. This also means more than
one Render() may occur during transition. Too many of these can disrupt
the WebAudio clock, but given the 30 second timeout this should be pretty
rare in actual WebAudio usage situations.

BUG= 649018 ,  614978 
TEST=new unittests, no more deadlock upon idle timeout.

Review-Url: https://codereview.chromium.org/2365723003
Cr-Commit-Position: refs/heads/master@{#421108}
(cherry picked from commit b770266d26fc09ab8fb746562d008c5d436e4784)

Review URL: https://codereview.chromium.org/2382473002 .

Cr-Commit-Position: refs/branch-heads/2840@{#568}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/content/renderer/media/renderer_webaudiodevice_impl.h
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/BUILD.gn
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/fake_audio_render_callback.cc
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/fake_audio_render_callback.h
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender.cc
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender.h
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender_unittest.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/942fee339500a935be00ac81d3a71e4991aa6666

commit 942fee339500a935be00ac81d3a71e4991aa6666
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Wed Sep 28 21:30:19 2016

Merge M54: "Break out WebAudio suspension code into new class. Add tests."

We've had enough problems with this code over the past year that it needs
to be extracted into something less burdensome on the WebAudio code. It
also needs some better testing.

This creates a new class called SilentSinkSuspender which is only used on
Android today and adds tests for it. It also changes the implementation
of suspension from directly calling into the sink to instead post tasks
to a provided task runner (render thread).

Notably, fake render callbacks are now driven on the media thread instead
of the render thread since WebAudio and Oilpan have a plethora of checks
around what is and isn't run on the main thread. This also means more than
one Render() may occur during transition. Too many of these can disrupt
the WebAudio clock, but given the 30 second timeout this should be pretty
rare in actual WebAudio usage situations.

BUG= 649018 ,  614978 
TEST=new unittests, no more deadlock upon idle timeout.

Review-Url: https://codereview.chromium.org/2365723003
Cr-Commit-Position: refs/heads/master@{#421108}
(cherry picked from commit b770266d26fc09ab8fb746562d008c5d436e4784)

Review URL: https://codereview.chromium.org/2382473002 .

Cr-Commit-Position: refs/branch-heads/2840@{#568}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/content/renderer/media/renderer_webaudiodevice_impl.cc
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/content/renderer/media/renderer_webaudiodevice_impl.h
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/BUILD.gn
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/fake_audio_render_callback.cc
[modify] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/fake_audio_render_callback.h
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender.cc
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender.h
[add] https://crrev.com/942fee339500a935be00ac81d3a71e4991aa6666/media/base/silent_sink_suspender_unittest.cc