Hookup the MediaStream glue for Adding and Removing tracks to an existing MediaStream.

content/renderer/media/media_stream_dependency_factory.cc

Index: content/renderer/media/media_stream_dependency_factory.cc
diff --git a/content/renderer/media/media_stream_dependency_factory.cc b/content/renderer/media/media_stream_dependency_factory.cc
index 06943a6bb2da00c58b1fc460567b7e843a0d83f1..cb03e5809afa01fc512ba12fd2fe96e90320d689 100644
--- a/content/renderer/media/media_stream_dependency_factory.cc
+++ b/content/renderer/media/media_stream_dependency_factory.cc
@@ -311,75 +311,23 @@ void MediaStreamDependencyFactory::CreateNativeLocalMediaStream(
   std::string label = UTF16ToUTF8(description->label());
   scoped_refptr<webrtc::MediaStreamInterface> native_stream =
       CreateLocalMediaStream(label);
+  MediaStreamExtraData* extra_data = new MediaStreamExtraData(native_stream,
+                                                              true);
+  description->setExtraData(extra_data);
 
   // Add audio tracks.
   WebKit::WebVector<WebKit::WebMediaStreamTrack> audio_tracks;
-  description->audioSources(audio_tracks);
-
-  bool start_stream = false;
+  description->audioTracks(audio_tracks);
   for (size_t i = 0; i < audio_tracks.size(); ++i) {

[henrika (OOO until Aug 14), 2013/04/09 16:53:43]

Nice! Much better structure even if I find it a bit tricky to figure out what 

WebKit::WebVector<WebKit::WebMediaStreamTrack> audio_tracks;
 description->audioSources(audio_tracks);
description->audioTracks(audio_tracks);

actually does.

[perkj_chrome, 2013/04/10 19:58:00]

..
audioSources(audio_tracks) has changed name to audioTracks(audio_tracks);

-    WebKit::WebMediaStreamSource source = audio_tracks[i].source();
-
-    // See if we're adding a WebAudio MediaStream.
-    if (source.requiresAudioConsumer()) {
-      // TODO(crogers, xians): In reality we should be able to send a unique
-      // audio stream to each PeerConnection separately.  But currently WebRTC
-      // is only able to handle a global audio stream sent to ALL peers.
-
-      // Create a special source where default WebAudio constraints are used.
-      if (!CreateWebAudioSource(&source)) {
-        LOG(ERROR) << "Failed to create WebAudio source";
-        continue;
-      }
-    }
-
-    MediaStreamSourceExtraData* source_data =
-        static_cast<MediaStreamSourceExtraData*>(source.extraData());
-
-    if (!source_data) {
-      // TODO(perkj): Implement support for sources from
-      // remote MediaStreams.
-      NOTIMPLEMENTED();
-      continue;
-    }
-
-    scoped_refptr<webrtc::AudioTrackInterface> audio_track(
-        CreateLocalAudioTrack(UTF16ToUTF8(audio_tracks[i].id()),
-                              source_data->local_audio_source()));
-    native_stream->AddTrack(audio_track);
-    audio_track->set_enabled(audio_tracks[i].isEnabled());
-    start_stream = true;
-  }
-
-  if (start_stream && GetWebRtcAudioDevice()) {
-    WebRtcAudioCapturer* capturer = GetWebRtcAudioDevice()->capturer();
-    capturer->Start();
+    AddNativeLocalMediaTrack(*description, audio_tracks[i]);
   }
 
   // Add video tracks.
   WebKit::WebVector<WebKit::WebMediaStreamTrack> video_tracks;
-  description->videoSources(video_tracks);
+  description->videoTracks(video_tracks);
   for (size_t i = 0; i < video_tracks.size(); ++i) {
-    const WebKit::WebMediaStreamSource& source = video_tracks[i].source();
-    MediaStreamSourceExtraData* source_data =
-        static_cast<MediaStreamSourceExtraData*>(source.extraData());
-    if (!source_data || !source_data->video_source()) {
-      // TODO(perkj): Implement support for sources from remote MediaStreams.
-      NOTIMPLEMENTED();
-      continue;
-    }
-
-    scoped_refptr<webrtc::VideoTrackInterface> video_track(
-        CreateLocalVideoTrack(UTF16ToUTF8(video_tracks[i].id()),
-                              source_data->video_source()));
-
-    native_stream->AddTrack(video_track);
-    video_track->set_enabled(video_tracks[i].isEnabled());
+    AddNativeLocalMediaTrack(*description, video_tracks[i]);
   }
-
-  MediaStreamExtraData* extra_data = new MediaStreamExtraData(native_stream,
-                                                              true);
-  description->setExtraData(extra_data);
 }
 
 void MediaStreamDependencyFactory::CreateNativeLocalMediaStream(
@@ -392,6 +340,65 @@ void MediaStreamDependencyFactory::CreateNativeLocalMediaStream(
   extra_data->SetLocalStreamStopCallback(stream_stop);
 }
 
+bool MediaStreamDependencyFactory::AddNativeLocalMediaTrack(
+      const WebKit::WebMediaStream& stream,
+      const WebKit::WebMediaStreamTrack& track) {
+  MediaStreamExtraData* extra_data =
+     static_cast<MediaStreamExtraData*>(stream.extraData());
+  webrtc::MediaStreamInterface* native_stream = extra_data->stream();
+  if (!native_stream) {
+    NOTREACHED();

[tommi (sloooow) - chröme, 2013/04/09 13:28:49]

if this is something that will never happen, this if() clause is not needed. 
Instead you can just DCHECK that the native_stream pointer is valid.

[perkj_chrome, 2013/04/10 19:58:00]

Done.

+    return false;
+  }
+
+  WebKit::WebMediaStreamSource source = track.source();
+  MediaStreamSourceExtraData* source_data =
+      static_cast<MediaStreamSourceExtraData*>(source.extraData());
+
+  if (!source_data) {
+    if (!source.requiresAudioConsumer()) {
+      // TODO(perkj): Implement support for sources from
+      // remote MediaStreams.
+      NOTIMPLEMENTED();

[henrika (OOO until Aug 14), 2013/04/09 16:53:43]

What does the combo of NOTIMPLEMENTED() + return false do actually? What happens
in release and debug mode?

[perkj_chrome, 2013/04/10 19:58:00]

NOTIMPLEMENTED prints a warning in the log output.

+      return false;
+    } else
+    // We're adding a WebAudio MediaStream.

[tommi (sloooow) - chröme, 2013/04/09 13:28:49]

Is that the only possibility? Is there a way to verify that somehow?

[perkj_chrome, 2013/04/10 19:58:00]

source.requiresAudioConsumer() means WebAudio in Blinkish language. Switched if
case to make that more clear.

+    // TODO(crogers, xians): In reality we should be able to send a unique
+    // audio stream to each PeerConnection separately.  But currently WebRTC
+    // is only able to handle a global audio stream sent to ALL peers.
+    // Create a special source where default WebAudio constraints are used.

[henrika (OOO until Aug 14), 2013/04/09 16:53:43]

Nit, the TODO is not really connected to the audio constraint part. Perhaps
separate these comments?

[perkj_chrome, 2013/04/10 19:58:00]

Done.

+    if (CreateWebAudioSource(&source)) {
+      source_data =
+          static_cast<MediaStreamSourceExtraData*>(source.extraData());
+    } else {
+      LOG(ERROR) << "Failed to create WebAudio source";
+      return false;
+    }
+  }
+
+  if (source.type() == WebKit::WebMediaStreamSource::TypeAudio) {

[tommi (sloooow) - chröme, 2013/04/09 13:28:49]

nit: if type() can only ever be audio or video, then I'd prefer a dcheck instead
of if/else and three return statements.

[perkj_chrome, 2013/04/10 19:58:00]

Done.

+    scoped_refptr<webrtc::AudioTrackInterface> audio_track(
+        CreateLocalAudioTrack(UTF16ToUTF8(track.id()),
+                              source_data->local_audio_source()));
+    audio_track->set_enabled(track.isEnabled());
+    if (GetWebRtcAudioDevice()) {

[henrika (OOO until Aug 14), 2013/04/09 16:53:43]

Could you add a TODO(henrika,xians) here saying that we should refactor this
part and harmonize better with video (se below)?

+      WebRtcAudioCapturer* capturer = GetWebRtcAudioDevice()->capturer();
+      if (!capturer->is_recording())
+        capturer->Start();
+    }
+    return native_stream->AddTrack(audio_track);
+
+  } else if (source.type() == WebKit::WebMediaStreamSource::TypeVideo) {
+    scoped_refptr<webrtc::VideoTrackInterface> video_track(
+        CreateLocalVideoTrack(UTF16ToUTF8(track.id()),
+                              source_data->video_source()));
+    video_track->set_enabled(track.isEnabled());
+    return native_stream->AddTrack(video_track);
+  }
+  NOTREACHED();
+  return false;
+}
+
 bool MediaStreamDependencyFactory::CreatePeerConnectionFactory() {
   DVLOG(1) << "MediaStreamDependencyFactory::CreatePeerConnectionFactory()";
   if (!pc_factory_.get()) {