Connect webrtc MediaSourceInterface ready states with webkit WebMediaStreamSource

content/renderer/media/media_stream_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/media_stream_impl.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
@@ skipping 207 matching lines @@
       WebKit::WebMediaStreamRegistry::lookupMediaStreamDescriptor(url));
 
   if (descriptor.isNull() || !descriptor.extraData())
     return false;  // This is not a valid stream.
 
   webrtc::MediaStreamInterface* stream = GetNativeMediaStream(descriptor);
   return (stream &&
       (!stream->GetVideoTracks().empty() || !stream->GetAudioTracks().empty()));
 }
 
+// static
+void MediaStreamImpl::ReleaseSourceObservers(

[perkj_chrome, 2013/04/18 13:36:20]

Why does this method has to be static? 
You only call it from within UserMediaRequestInfo::~UserMediaRequestInfo() ?

This does exactly what I suggested and have the same problem- ie - if a track is
removed from |descriptor| the WebKitMediaStreamSource and its extra data field
is leaked.

Please add a todo with this - same as in your media_stream_source_observer.h

[tommi (sloooow) - chröme, 2013/04/23 09:50:55]

...and file a bug as well?

+    WebKit::WebMediaStream descriptor) {
+  if (descriptor.isNull())
+    return;
+
+  WebKit::WebVector<WebKit::WebMediaStreamTrack> audio_tracks;
+  descriptor.audioSources(audio_tracks);

[perkj_chrome, 2013/04/18 13:36:20]

use audioTracks

+  for (size_t i = 0; i < audio_tracks.size(); ++i) {
+    const WebKit::WebMediaStreamSource& source = audio_tracks[i].source();

[perkj_chrome, 2013/04/18 13:36:20]

Thinking about this- You can do 
source.setExtraData(NULL);
source.setReadyState(ReadyStateEnded) ended here.

To get a non const WebKitMediaStreamSource do 
WebKitMediaStreamSource source(audio_tracks[i].source());

[tommi (sloooow) - chröme, 2013/04/23 09:50:55]

If you create a new source object from another const one, then do they still
represent the same source?  I.e. changes in one will be reflected in the other?

+    content::MediaStreamSourceExtraData* source_data =
+        static_cast<content::MediaStreamSourceExtraData*>(source.extraData());
+    if (source_data) {
+      source_data->SetSourceObserver(NULL);
+    }
+  }
+
+  WebKit::WebVector<WebKit::WebMediaStreamTrack> video_tracks;
+  descriptor.videoSources(video_tracks);

[perkj_chrome, 2013/04/18 13:36:20]

use videoTracks

+  for (size_t i = 0; i < video_tracks.size(); ++i) {
+    const WebKit::WebMediaStreamSource& source = video_tracks[i].source();
+    content::MediaStreamSourceExtraData* source_data =
+        static_cast<content::MediaStreamSourceExtraData*>(source.extraData());
+    if (source_data) {
+      source_data->SetSourceObserver(NULL);
+    }
+  }
+}
+
 scoped_refptr<webkit_media::VideoFrameProvider>
 MediaStreamImpl::GetVideoFrameProvider(
     const GURL& url,
     const base::Closure& error_cb,
     const webkit_media::VideoFrameProvider::RepaintCB& repaint_cb) {
   DCHECK(CalledOnValidThread());
   WebKit::WebMediaStream descriptor(GetMediaStream(url));
 
   if (descriptor.isNull() || !descriptor.extraData())
     return NULL;  // This is not a valid stream.
@@ skipping 348 matching lines @@
 void MediaStreamExtraData::SetLocalStreamStopCallback(
     const StreamStopCallback& stop_callback) {
   stream_stop_callback_ = stop_callback;
 }
 
 void MediaStreamExtraData::OnLocalStreamStop() {
   if (!stream_stop_callback_.is_null())
     stream_stop_callback_.Run(stream_->label());
 }
 
+MediaStreamImpl::UserMediaRequestInfo::~UserMediaRequestInfo() {
+  ReleaseSourceObservers(descriptor);

[perkj_chrome, 2013/04/18 13:36:20]

Did I miss something or can you move the ReleaseSourceObservers implementation
to here?

+}
+
 }  // namespace content