Avoid hitting the thread check when WebRtcAudioRenderer is going away.

Avoid hitting the thread check when WebRtcAudioRenderer is going away.

We need to reset the render_thread_checker and render_converter_ when the WebRtcAudioRenderer is going away.

NOTRY=true

BUG=264611
TEST=manual test webrtc with --enable-audio-track-processor

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255158

[no longer working on chromium, 2014-03-04 20:39:39]

Tommi, could you please review this CL?

Thanks,
SX

[tommi (sloooow) - chröme, 2014-03-04 20:48:38]

just documentatin requests

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_audio_processor.cc:215:
render_thread_checker_.DetachFromThread();
add some documentation here that explains what thread we're on and why it's safe
to delete this object without a lock.

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
File content/renderer/media/webrtc_audio_device_impl.cc (right):

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_impl.cc:178: // Notify the playout
sink on the change.
s/on/of

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_impl.cc:180: it !=
playout_sinks_.end(); ++it) {
indent

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
File content/renderer/media/webrtc_audio_device_impl.h (right):

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_impl.h:247: virtual void
OnPlayoutDataSourceChanged() = 0;
also document thread semantics

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

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_renderer.cc:429: // sure that no more
callback will get into the source.
nit:
// Stop the sink before calling RemoveAudioRenderer() to make
// sure that no more callbacks will be made while the renderer
// is being removed.

[no longer working on chromium, 2014-03-05 13:01:03]

Tommi, PTAL.

Thanks,
SX

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_audio_processor.cc:215:
render_thread_checker_.DetachFromThread();
On 2014/03/04 20:48:39, tommi wrote:
> add some documentation here that explains what thread we're on and why it's
safe
> to delete this object without a lock.

Done, I added a thread check here as well.

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
File content/renderer/media/webrtc_audio_device_impl.cc (right):

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_impl.cc:178: // Notify the playout
sink on the change.
On 2014/03/04 20:48:39, tommi wrote:
> s/on/of

Done.

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_impl.cc:180: it !=
playout_sinks_.end(); ++it) {
On 2014/03/04 20:48:39, tommi wrote:
> indent

Done.

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
File content/renderer/media/webrtc_audio_device_impl.h (right):

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_device_impl.h:247: virtual void
OnPlayoutDataSourceChanged() = 0;
On 2014/03/04 20:48:39, tommi wrote:
> also document thread semantics

Done.

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

https://codereview.chromium.org/178153007/diff/1/content/renderer/media/webrt...
content/renderer/media/webrtc_audio_renderer.cc:429: // sure that no more
callback will get into the source.
On 2014/03/04 20:48:39, tommi wrote:
> nit:
> // Stop the sink before calling RemoveAudioRenderer() to make
> // sure that no more callbacks will be made while the renderer
> // is being removed.

Done.

[tommi (sloooow) - chröme, 2014-03-05 14:09:41]

lgtm

[no longer working on chromium, 2014-03-05 15:10:48]

The CQ bit was checked by xians@chromium.org

[commit-bot: I haz the power, 2014-03-05 15:11:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xians@chromium.org/178153007/40001

[commit-bot: I haz the power, 2014-03-05 15:11:13]

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

[commit-bot: I haz the power, 2014-03-05 15:11:13]

Failed to apply patch for
content/renderer/media/media_stream_audio_processor.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/renderer/media/media_stream_audio_processor.cc
  Hunk #1 FAILED at 211.
  1 out of 1 hunk FAILED -- saving rejects to file
content/renderer/media/media_stream_audio_processor.cc.rej

Patch:       content/renderer/media/media_stream_audio_processor.cc
Index: content/renderer/media/media_stream_audio_processor.cc
diff --git a/content/renderer/media/media_stream_audio_processor.cc
b/content/renderer/media/media_stream_audio_processor.cc
index
7ae7d26f6d79b1c596d01b56306bc235a9e7c7ab..40d54c240be2e546872b850f07d3353f061758da
100644
--- a/content/renderer/media/media_stream_audio_processor.cc
+++ b/content/renderer/media/media_stream_audio_processor.cc
@@ -211,6 +211,14 @@ void
MediaStreamAudioProcessor::OnPlayoutData(media::AudioBus* audio_bus,
     audio_processing_->AnalyzeReverseStream(&render_frame_);
 }
 
+void MediaStreamAudioProcessor::OnPlayoutDataSourceChanged() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
+  // There is no need to hold a lock here since the caller guarantees that
+  // there is no more OnPlayoutData() callback on the render thread.
+  render_thread_checker_.DetachFromThread();
+  render_converter_.reset();
+}
+
 void MediaStreamAudioProcessor::GetStats(AudioProcessorStats* stats) {
   stats->typing_noise_detected =
       (base::subtle::Acquire_Load(&typing_detected_) != false);

[no longer working on chromium, 2014-03-05 18:56:57]

The CQ bit was checked by xians@chromium.org

[commit-bot: I haz the power, 2014-03-05 18:59:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xians@chromium.org/178153007/40001

[commit-bot: I haz the power, 2014-03-05 18:59:59]

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

[commit-bot: I haz the power, 2014-03-05 19:00:00]

Failed to apply patch for
content/renderer/media/media_stream_audio_processor.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/renderer/media/media_stream_audio_processor.cc
  Hunk #1 FAILED at 211.
  1 out of 1 hunk FAILED -- saving rejects to file
content/renderer/media/media_stream_audio_processor.cc.rej

Patch:       content/renderer/media/media_stream_audio_processor.cc
Index: content/renderer/media/media_stream_audio_processor.cc
diff --git a/content/renderer/media/media_stream_audio_processor.cc
b/content/renderer/media/media_stream_audio_processor.cc
index
7ae7d26f6d79b1c596d01b56306bc235a9e7c7ab..40d54c240be2e546872b850f07d3353f061758da
100644
--- a/content/renderer/media/media_stream_audio_processor.cc
+++ b/content/renderer/media/media_stream_audio_processor.cc
@@ -211,6 +211,14 @@ void
MediaStreamAudioProcessor::OnPlayoutData(media::AudioBus* audio_bus,
     audio_processing_->AnalyzeReverseStream(&render_frame_);
 }
 
+void MediaStreamAudioProcessor::OnPlayoutDataSourceChanged() {
+  DCHECK(main_thread_checker_.CalledOnValidThread());
+  // There is no need to hold a lock here since the caller guarantees that
+  // there is no more OnPlayoutData() callback on the render thread.
+  render_thread_checker_.DetachFromThread();
+  render_converter_.reset();
+}
+
 void MediaStreamAudioProcessor::GetStats(AudioProcessorStats* stats) {
   stats->typing_noise_detected =
       (base::subtle::Acquire_Load(&typing_detected_) != false);

[no longer working on chromium, 2014-03-05 19:35:42]

The CQ bit was checked by xians@chromium.org

[commit-bot: I haz the power, 2014-03-05 19:36:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xians@chromium.org/178153007/60001

[commit-bot: I haz the power, 2014-03-05 21:35:25]

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

[commit-bot: I haz the power, 2014-03-05 21:35:26]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[no longer working on chromium, 2014-03-05 21:48:36]

The CQ bit was checked by xians@chromium.org

[no longer working on chromium, 2014-03-05 21:49:02]

The CQ bit was unchecked by xians@chromium.org

[no longer working on chromium, 2014-03-05 21:49:54]

The CQ bit was checked by xians@chromium.org

[commit-bot: I haz the power, 2014-03-05 21:55:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xians@chromium.org/178153007/60001

[commit-bot: I haz the power, 2014-03-05 21:58:12]

Change committed as 255158