Fix data race in WebRtcAudioRenderer.

Fix data race in WebRtcAudioRenderer.

Accessing |max_render_time_| under |lock_| in Stop(). It's already accessed under the lock in the other place.

BUG=641339
Committed: https://crrev.com/c0a322db0a71e6d63f4905d4b55f27d83f55726d
Cr-Commit-Position: refs/heads/master@{#415617}

[Henrik Grunell, 2016-08-30 11:26:15]

Description was changed from

==========
Fix data race in WebRtcAudioRenderer.

Accessing |max_render_time_| under lock.

BUG=641339
==========

to

==========
Fix data race in WebRtcAudioRenderer.

Accessing |max_render_time_| under |lock_| in Stop(). It's already accessed
under the lock in the other place.

BUG=641339
==========

[Henrik Grunell, 2016-08-30 11:26:15]

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

[DaleCurtis, 2016-08-30 18:25:15]

lgtm

[miu, 2016-08-30 18:36:46]

Drive-by observation:

https://codereview.chromium.org/2292053002/diff/1/content/renderer/media/webr...
File content/renderer/media/webrtc_audio_renderer.cc (right):

https://codereview.chromium.org/2292053002/diff/1/content/renderer/media/webr...
content/renderer/media/webrtc_audio_renderer.cc:313: // If |max_render_time_| is
zero, no render call has been made.
It's not necessary to do this while holding the lock. Simply move this code to
any point after "source_ = NULL;" (and the lock released) and max_render_time_
won't be read/mutated by any other thread.

We know this because WebRtcAudioRenderer::SourceCallback() is the only method
that mutates max_render_time_ (at line 496). It only runs while the lock_ is
held, but it would not have been called (via WebRtcAudioRenderer::Render()) if
source_ was NULL (at line 406).

[Henrik Grunell, 2016-08-31 10:29:58]

https://codereview.chromium.org/2292053002/diff/1/content/renderer/media/webr...
File content/renderer/media/webrtc_audio_renderer.cc (right):

https://codereview.chromium.org/2292053002/diff/1/content/renderer/media/webr...
content/renderer/media/webrtc_audio_renderer.cc:313: // If |max_render_time_| is
zero, no render call has been made.
On 2016/08/30 18:36:46, miu wrote:
> It's not necessary to do this while holding the lock. Simply move this code to
> any point after "source_ = NULL;" (and the lock released) and max_render_time_
> won't be read/mutated by any other thread.
> 
> We know this because WebRtcAudioRenderer::SourceCallback() is the only method
> that mutates max_render_time_ (at line 496). It only runs while the lock_ is
> held, but it would not have been called (via WebRtcAudioRenderer::Render()) if
> source_ was NULL (at line 406).

That's a great point, thanks. Changed it.

[Henrik Grunell, 2016-08-31 10:30:15]

The CQ bit was checked by grunell@chromium.org

[Henrik Grunell, 2016-08-31 10:30:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/2292053002/#ps20001
(title: "Accessing after the lock is released.")

[commit-bot: I haz the power, 2016-08-31 10:30:41]

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, 2016-08-31 12:30:34]

Description was changed from

==========
Fix data race in WebRtcAudioRenderer.

Accessing |max_render_time_| under |lock_| in Stop(). It's already accessed
under the lock in the other place.

BUG=641339
==========

to

==========
Fix data race in WebRtcAudioRenderer.

Accessing |max_render_time_| under |lock_| in Stop(). It's already accessed
under the lock in the other place.

BUG=641339
==========

[commit-bot: I haz the power, 2016-08-31 12:30:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-31 12:32:29]

Description was changed from

==========
Fix data race in WebRtcAudioRenderer.

Accessing |max_render_time_| under |lock_| in Stop(). It's already accessed
under the lock in the other place.

BUG=641339
==========

to

==========
Fix data race in WebRtcAudioRenderer.

Accessing |max_render_time_| under |lock_| in Stop(). It's already accessed
under the lock in the other place.

BUG=641339

Committed: https://crrev.com/c0a322db0a71e6d63f4905d4b55f27d83f55726d
Cr-Commit-Position: refs/heads/master@{#415617}
==========

[commit-bot: I haz the power, 2016-08-31 12:32:30]

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