WebMediaPlayerImpl needs to own the audio source provider.

webkit/media/webmediaplayer_impl.h

Index: webkit/media/webmediaplayer_impl.h
diff --git a/webkit/media/webmediaplayer_impl.h b/webkit/media/webmediaplayer_impl.h
index c19e00650251e948868c24e6e66f41d8cf59c3f4..5058b9a1e54b6e78e5e357158303733823418c5f 100644
--- a/webkit/media/webmediaplayer_impl.h
+++ b/webkit/media/webmediaplayer_impl.h
@@ -74,6 +74,7 @@ class WebFrame;
 
 namespace media {
 class MediaLog;
+class AudioRendererSink;

[scherkus (not reviewing), 2012/07/09 23:13:23]

a->z ordering

[Raymond Toy, 2012/07/10 02:51:39]

Done.

 }
 
 namespace webkit_media {
@@ -102,11 +103,14 @@ class WebMediaPlayerImpl
   // filter if they wish to hear any sound coming out the speakers, otherwise
   // audio data is discarded and media plays back based on wall clock time.
   //
+  // When calling this, the audio_source_provider and
+  // audio_renderer_sink arguments should be the same object.

[scherkus (not reviewing), 2012/07/09 23:13:23]

add || around variable names referenced in comments

add a TODO(scherkus) saying "Remove WebAudioSourceProvider parameter once we
refactor RenderAudioSourceProvider to live under webkit/media/ instead of
content/renderer/, see http://crbug.com/136442"

[Raymond Toy, 2012/07/10 02:51:39]

Done.

   WebMediaPlayerImpl(WebKit::WebFrame* frame,
                      WebKit::WebMediaPlayerClient* client,
                      base::WeakPtr<WebMediaPlayerDelegate> delegate,
                      media::FilterCollection* collection,
                      WebKit::WebAudioSourceProvider* audio_source_provider,
+                     media::AudioRendererSink* audio_renderer_sink,
                      media::MessageLoopFactory* message_loop_factory,
                      MediaStreamClient* media_stream_client,
                      media::MediaLog* media_log);
@@ -337,6 +341,11 @@ class WebMediaPlayerImpl
 
   WebKit::WebAudioSourceProvider* audio_source_provider_;
 
+  // We need to own the audio_renderer_sink to prevent the sink from
+  // getting deleted before we are finished with it.  This should be
+  // the same as audio_source_provider_.

[scherkus (not reviewing), 2012/07/09 23:13:23]

I'd remove the comment -- it's OK to assume this class should hold onto a
pointer (i.e,. this comment will quickly become stale)

In fact it's better to DCHECK this inside the .cc + comment there

[Raymond Toy, 2012/07/10 02:51:39]

Comment removed.  But see comment for .cc where we can't DCHECK_EQ the pointers.

+  scoped_refptr<media::AudioRendererSink> audio_renderer_sink_;
+
   bool is_local_source_;
 
   DISALLOW_COPY_AND_ASSIGN(WebMediaPlayerImpl);