swap the renderer_ reference to local before calling Stop()

content/renderer/media/webrtc_audio_device_impl.cc

Index: content/renderer/media/webrtc_audio_device_impl.cc
diff --git a/content/renderer/media/webrtc_audio_device_impl.cc b/content/renderer/media/webrtc_audio_device_impl.cc
index e65ca33607c39abedb1e600fb9e10f2148df403b..d608a5d31e5dce6dd3e84b9b8e56214695b60e42 100644
--- a/content/renderer/media/webrtc_audio_device_impl.cc
+++ b/content/renderer/media/webrtc_audio_device_impl.cc
@@ -259,8 +259,11 @@ int32_t WebRtcAudioDeviceImpl::Terminate() {
 
   // It is necessary to stop the |renderer_| before going away.
   if (renderer_) {
-    renderer_->Stop();
-    renderer_ = NULL;
+    // WebRtcAudioRenderer::Stop() will trigger RemoveAudioRenderer(), so we

[henrika (OOO until Aug 14), 2013/05/27 10:06:00]

I am stupid, but the example says:

{
//     scoped_refptr<MyFoo> a = new MyFoo();
//     scoped_refptr<MyFoo> b;
//
//     b.swap(a);
//     // now, |b| references the MyFoo object, and |a| references NULL.
//   }

[henrika (OOO until Aug 14), 2013/05/27 10:11:21]

OK, checking the swap implementation it looks like:

renderer_ will be NULL, and
renderer will be set to valid renderer_

but it is still not clear what that solves.

+    // have to swap the member reference to a local reference here.
+    scoped_refptr<WebRtcAudioRenderer> renderer;
+    renderer_.swap(renderer);

[tommi (sloooow) - chröme, 2013/05/27 10:48:13]

I don't think swap() is the right thing here because it explicitly sets
renderer_ to NULL even before RemoveAudioRenderer() is called.  If you run the
code (and assuming that RemoveAudioRenderer() is called as you say), then you
should hit a DCHECK.

Instead, what about doing this instead:

if (renderer_) {
  // Grab a local reference while we call Stop().
  scoped_refptr<WebRtcAudioRenderer> local_renderer(renderer_);
  local_renderer->Stop();
  // The above call to Stop() should have triggered a call to
RemoveAudioRenderer
  // which clears our reference to the audio renderer.
  DCHECK(!renderer_.get());
}

[no longer working on chromium, 2013/05/27 11:15:41]

Done.

+    renderer->Stop();
   }
 
   if (capturer_) {