start/stop the source of the capturer when 1st audiotrack/last audiotrack is added/removed

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 fc0cc0dc80b80c74b11d7050a857a626d1cec1e2..f88f112b77edb7d18793e32650fc2a9a27247657 100644
--- a/content/renderer/media/media_stream_dependency_factory.cc
+++ b/content/renderer/media/media_stream_dependency_factory.cc
@@ -413,11 +413,9 @@ bool MediaStreamDependencyFactory::AddNativeMediaStreamTrack(
     scoped_refptr<webrtc::AudioTrackInterface> audio_track(
         CreateLocalAudioTrack(track_id, source_data->local_audio_source()));
     audio_track->set_enabled(track.isEnabled());
-    if (GetWebRtcAudioDevice()) {
-      WebRtcAudioCapturer* capturer = GetWebRtcAudioDevice()->capturer().get();
-      if (!capturer->is_recording())
-        capturer->Start();
-    }
+    if (GetWebRtcAudioDevice() && GetWebRtcAudioDevice()->capturer().get())

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

It is now a bit unclear why this if-statement is needed. It was clear in the old
revision but not now.

[no longer working on chromium, 2013/06/05 16:29:45]

Good catch, it is because we were using MockLocalAudioTrack on our unittest, and
it will crash when we cast it to WebRtcLocalAudioTrack.

I changed the unittest to create real WebRtcLocalAudioTrack instead, now the
checks are removed.

+      static_cast<WebRtcLocalAudioTrack*>(audio_track.get())->Start();

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

Can you add some comments here to explain what actually happens. I assume you
start the source (if not already started) which starts feeding the tracks.
Right?

[no longer working on chromium, 2013/06/05 16:29:45]

Done.

+
     return native_stream->AddTrack(audio_track.get());
   } else {
     scoped_refptr<webrtc::VideoTrackInterface> video_track(
@@ -690,13 +688,11 @@ void MediaStreamDependencyFactory::StopLocalAudioSource(
   MediaStreamExtraData* extra_data = static_cast<MediaStreamExtraData*>(
       description.extraData());
   if (extra_data && extra_data->is_local() && extra_data->stream().get() &&
-      !extra_data->stream()->GetAudioTracks().empty()) {
-    if (GetWebRtcAudioDevice()) {
-      scoped_refptr<WebRtcAudioCapturer> capturer =
-          GetWebRtcAudioDevice()->capturer();
-      if (capturer.get())
-        capturer->Stop();
-    }
+      !extra_data->stream()->GetAudioTracks().empty() &&
+      GetWebRtcAudioDevice() &&

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

Same comment as above. It is not clear why you must check for an existing ADM
here. Can't say by looking at the following code why this is needed.

[no longer working on chromium, 2013/06/05 16:29:45]

Removed.

+      GetWebRtcAudioDevice()->capturer().get()) {
+    static_cast<WebRtcLocalAudioTrack*>(

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

You call start when a track is added and stop when the source is stopped. What
happens when a track is removed then? Feels like there is an asymmetry here.

[no longer working on chromium, 2013/06/05 16:29:45]

StopLocalAudioSource is the wrong name, it should be renamed as
StopLocalAudioTrack, I will create an issue to Tommy.

+        extra_data->stream()->GetAudioTracks()[0].get())->Stop();
   }
 }