RemoteMediaStreamImpl using WebRtcMediaStreamTrackMap.

content/renderer/media/remote_media_stream_impl.cc

Index: content/renderer/media/remote_media_stream_impl.cc
diff --git a/content/renderer/media/remote_media_stream_impl.cc b/content/renderer/media/remote_media_stream_impl.cc
index 14966e488d5ce48b51f4eb34d36db858aecc9b9a..38b7db9643d4250aa0f4e8b1c20ca4a55167d6c5 100644
--- a/content/renderer/media/remote_media_stream_impl.cc
+++ b/content/renderer/media/remote_media_stream_impl.cc
@@ -23,33 +23,41 @@
 #include "third_party/WebKit/public/platform/WebString.h"
 
 namespace content {
-namespace {
 
-template <typename WebRtcTrackVector, typename AdapterType>
-void CreateAdaptersForTracks(
-    const WebRtcTrackVector& tracks,
-    std::vector<scoped_refptr<AdapterType>>* observers,
-    const scoped_refptr<base::SingleThreadTaskRunner>& main_thread) {
-  for (auto& track : tracks)
-    observers->push_back(new AdapterType(main_thread, track));
-}
+namespace {
 
-template<typename VectorType>
-bool IsTrackInVector(const VectorType& v, const std::string& id) {
-  for (const auto& t : v) {
-    if (t->id() == id)
-      return true;
+// Gets the adapters for the tracks that are members of the webrtc stream.
+// Invoke on webrtc signaling thread. New adapters are initialized in a post
+// to the main thread after which their |web_track| becomes available.
+RemoteMediaStreamImpl::AdapterRefMap GetAdapterRefMapFromWebRtcStream(
+    const scoped_refptr<WebRtcMediaStreamTrackAdapterMap>& track_adapter_map,
+    webrtc::MediaStreamInterface* webrtc_stream) {
+  RemoteMediaStreamImpl::AdapterRefMap adapter_refs;
+  for (auto& webrtc_audio_track : webrtc_stream->GetAudioTracks()) {
+    adapter_refs.insert(
+        std::make_pair(webrtc_audio_track->id(),
+                       track_adapter_map->GetOrCreateRemoteTrackAdapter(
+                           webrtc_audio_track.get())));
   }
-  return false;
+  for (auto& webrtc_video_track : webrtc_stream->GetVideoTracks()) {
+    adapter_refs.insert(
+        std::make_pair(webrtc_video_track->id(),
+                       track_adapter_map->GetOrCreateRemoteTrackAdapter(
+                           webrtc_video_track.get())));
+  }
+  return adapter_refs;
 }
+
 }  // namespace
 
 RemoteMediaStreamImpl::Observer::Observer(
     const base::WeakPtr<RemoteMediaStreamImpl>& media_stream,
     const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
+    const scoped_refptr<WebRtcMediaStreamTrackAdapterMap>& track_adapter_map,
     webrtc::MediaStreamInterface* webrtc_stream)
     : media_stream_(media_stream),
       main_thread_(main_thread),
+      track_adapter_map_(track_adapter_map),
       webrtc_stream_(webrtc_stream) {
   webrtc_stream_->RegisterObserver(this);
 }
@@ -59,10 +67,15 @@ RemoteMediaStreamImpl::Observer::~Observer() {
 }
 
 void RemoteMediaStreamImpl::Observer::InitializeOnMainThread(
-    const std::string& label) {
+    const std::string& label,
+    AdapterRefMap adapter_refs,
+    size_t audio_track_count,
+    size_t video_track_count) {
   DCHECK(main_thread_->BelongsToCurrentThread());
-  if (media_stream_)
-    media_stream_->InitializeOnMainThread(label);
+  if (media_stream_) {
+    media_stream_->InitializeOnMainThread(label, std::move(adapter_refs),
+                                          audio_track_count, video_track_count);
+  }
 }
 
 void RemoteMediaStreamImpl::Observer::Unregister() {
@@ -74,128 +87,96 @@ void RemoteMediaStreamImpl::Observer::Unregister() {
 }
 
 void RemoteMediaStreamImpl::Observer::OnChanged() {
-  std::unique_ptr<RemoteAudioTrackAdapters> audio(
-      new RemoteAudioTrackAdapters());
-  std::unique_ptr<RemoteVideoTrackAdapters> video(
-      new RemoteVideoTrackAdapters());
-
-  CreateAdaptersForTracks(
-      webrtc_stream_->GetAudioTracks(), audio.get(), main_thread_);
-  CreateAdaptersForTracks(
-      webrtc_stream_->GetVideoTracks(), video.get(), main_thread_);
-
-  main_thread_->PostTask(FROM_HERE,
-      base::Bind(&RemoteMediaStreamImpl::Observer::OnChangedOnMainThread,
-      this, base::Passed(&audio), base::Passed(&video)));
+  AdapterRefMap new_adapter_refs = GetAdapterRefMapFromWebRtcStream(

[Guido Urdaneta, 2017/06/12 14:25:28]

Is there a race if you access track_adapter_map_ from this thread?

[hbos_chromium, 2017/06/12 15:05:20]

No, track_adapter_map_ is thread-safe.

[Guido Urdaneta, 2017/06/12 15:18:34]

Acknowledged.

+      track_adapter_map_, webrtc_stream_.get());
+  main_thread_->PostTask(
+      FROM_HERE,
+      base::Bind(&RemoteMediaStreamImpl::Observer::OnChangedOnMainThread, this,
+                 base::Passed(&new_adapter_refs)));
 }
 
 void RemoteMediaStreamImpl::Observer::OnChangedOnMainThread(
-    std::unique_ptr<RemoteAudioTrackAdapters> audio_tracks,
-    std::unique_ptr<RemoteVideoTrackAdapters> video_tracks) {
+    AdapterRefMap new_adapter_refs) {
   DCHECK(main_thread_->BelongsToCurrentThread());
   if (media_stream_)
-    media_stream_->OnChanged(std::move(audio_tracks), std::move(video_tracks));
+    media_stream_->OnChanged(std::move(new_adapter_refs));
 }
 
 // Called on the signaling thread.
 RemoteMediaStreamImpl::RemoteMediaStreamImpl(
     const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
+    const scoped_refptr<WebRtcMediaStreamTrackAdapterMap>& track_adapter_map,
     webrtc::MediaStreamInterface* webrtc_stream)
     : signaling_thread_(base::ThreadTaskRunnerHandle::Get()),
+      track_adapter_map_(track_adapter_map),
       weak_factory_(this) {

[Guido Urdaneta, 2017/06/12 14:25:28]

If necessary, can you add a check to ensure this runs on a correct thread?

[hbos_chromium, 2017/06/12 15:05:20]

Added a DCHECK that we are not on the main thread, that should suffice.

[Guido Urdaneta, 2017/06/12 15:18:34]

Acknowledged.

+  DCHECK(track_adapter_map_);
   observer_ = new RemoteMediaStreamImpl::Observer(
-      weak_factory_.GetWeakPtr(), main_thread, webrtc_stream);
-  CreateAdaptersForTracks(webrtc_stream->GetAudioTracks(),
-      &audio_track_observers_, main_thread);
-  CreateAdaptersForTracks(webrtc_stream->GetVideoTracks(),
-      &video_track_observers_, main_thread);
+      weak_factory_.GetWeakPtr(), main_thread, track_adapter_map_,
+      webrtc_stream);
 
-  main_thread->PostTask(FROM_HERE,
+  AdapterRefMap adapter_refs =
+      GetAdapterRefMapFromWebRtcStream(track_adapter_map_, webrtc_stream);
+  main_thread->PostTask(
+      FROM_HERE,
       base::Bind(&RemoteMediaStreamImpl::Observer::InitializeOnMainThread,
-          observer_, webrtc_stream->label()));
+                 observer_, webrtc_stream->label(), base::Passed(&adapter_refs),
+                 webrtc_stream->GetAudioTracks().size(),
+                 webrtc_stream->GetVideoTracks().size()));
 }
 
 RemoteMediaStreamImpl::~RemoteMediaStreamImpl() {
   DCHECK(observer_->main_thread()->BelongsToCurrentThread());
-  for (auto& track : audio_track_observers_)
-    track->Unregister();
   observer_->Unregister();
+  OnChanged(AdapterRefMap());
 }
 
-void RemoteMediaStreamImpl::InitializeOnMainThread(const std::string& label) {
+void RemoteMediaStreamImpl::InitializeOnMainThread(const std::string& label,
+                                                   AdapterRefMap adapter_refs,
+                                                   size_t audio_track_count,
+                                                   size_t video_track_count) {
   DCHECK(observer_->main_thread()->BelongsToCurrentThread());
-  blink::WebVector<blink::WebMediaStreamTrack> webkit_audio_tracks(
-        audio_track_observers_.size());
-  for (size_t i = 0; i < audio_track_observers_.size(); ++i) {
-    audio_track_observers_[i]->Initialize();
-    webkit_audio_tracks[i] = *audio_track_observers_[i]->web_track();
-  }
-
-  blink::WebVector<blink::WebMediaStreamTrack> webkit_video_tracks(
-        video_track_observers_.size());
-  for (size_t i = 0; i < video_track_observers_.size(); ++i) {
-    video_track_observers_[i]->Initialize();
-    webkit_video_tracks[i] = *video_track_observers_[i]->web_track();
+  DCHECK_EQ(audio_track_count + video_track_count, adapter_refs.size());
+
+  adapter_refs_ = std::move(adapter_refs);
+  blink::WebVector<blink::WebMediaStreamTrack> web_audio_tracks(
+      audio_track_count);
+  blink::WebVector<blink::WebMediaStreamTrack> web_video_tracks(
+      video_track_count);
+  size_t audio_i = 0;
+  size_t video_i = 0;
+  for (const auto& it : adapter_refs_) {
+    const blink::WebMediaStreamTrack& web_track = it.second->web_track();
+    if (web_track.Source().GetType() == blink::WebMediaStreamSource::kTypeAudio)
+      web_audio_tracks[audio_i++] = web_track;
+    else
+      web_video_tracks[video_i++] = web_track;
   }
 
-  webkit_stream_.Initialize(blink::WebString::FromUTF8(label),
-                            webkit_audio_tracks, webkit_video_tracks);
+  webkit_stream_.Initialize(blink::WebString::FromUTF8(label), web_audio_tracks,
+                            web_video_tracks);
   webkit_stream_.SetExtraData(new MediaStream());
 }
 
-void RemoteMediaStreamImpl::OnChanged(
-    std::unique_ptr<RemoteAudioTrackAdapters> audio_tracks,
-    std::unique_ptr<RemoteVideoTrackAdapters> video_tracks) {
-  // Find removed tracks.
-  auto audio_it = audio_track_observers_.begin();
-  while (audio_it != audio_track_observers_.end()) {
-    if (!IsTrackInVector(*audio_tracks.get(), (*audio_it)->id())) {
-      (*audio_it)->Unregister();
-      webkit_stream_.RemoveTrack(*(*audio_it)->web_track());
-      audio_it = audio_track_observers_.erase(audio_it);
-    } else {
-      ++audio_it;
-    }
-  }
+void RemoteMediaStreamImpl::OnChanged(AdapterRefMap new_adapter_refs) {
+  DCHECK(observer_->main_thread()->BelongsToCurrentThread());
 
-  auto video_it = video_track_observers_.begin();
-  while (video_it != video_track_observers_.end()) {
-    if (!IsTrackInVector(*video_tracks.get(), (*video_it)->id())) {
-      webkit_stream_.RemoveTrack(*(*video_it)->web_track());
-      video_it = video_track_observers_.erase(video_it);
+  // Find removed tracks.
+  for (auto it = adapter_refs_.begin(); it != adapter_refs_.end();) {
+    if (new_adapter_refs.find(it->first) == new_adapter_refs.end()) {
+      webkit_stream_.RemoveTrack(it->second->web_track());
+      it = adapter_refs_.erase(it);
     } else {
-      ++video_it;
+      ++it;
     }
   }
-
   // Find added tracks.
-  for (auto& track : *audio_tracks.get()) {
-    if (!IsTrackInVector(audio_track_observers_, track->id())) {
-      track->Initialize();
-      audio_track_observers_.push_back(track);
-      webkit_stream_.AddTrack(*track->web_track());
-      // Set the track to null to avoid unregistering it below now that it's
-      // been associated with a media stream.
-      track = nullptr;
+  for (auto& it : new_adapter_refs) {
+    if (adapter_refs_.find(it.first) == adapter_refs_.end()) {
+      webkit_stream_.AddTrack(it.second->web_track());
+      adapter_refs_.insert(std::make_pair(it.first, std::move(it.second)));
     }
   }
-
-  // Find added video tracks.
-  for (const auto& track : *video_tracks.get()) {
-    if (!IsTrackInVector(video_track_observers_, track->id())) {
-      track->Initialize();
-      video_track_observers_.push_back(track);
-      webkit_stream_.AddTrack(*track->web_track());
-    }
-  }
-
-  // Unregister all the audio track observers that were not used.
-  // We need to do this before destruction since the observers can't unregister
-  // from within the dtor due to a race.
-  for (auto& track : *audio_tracks.get()) {
-    if (track.get())
-      track->Unregister();
-  }
 }
 
 }  // namespace content