Allow track adapters and adapter refs to be initialized explicitly.

Normally they are created on the signaling thread and initialized in a
PostTask to the main thread, so they are implicitly initialized if you
don't do anything. This is still true.

This CL introduced InitializeOnMainThread() which can be called
explicitly, meaning you don't have to wait for the posted task to run.

This is only relevant if the main thread is doing a blocking call to
the webrtc signaling thread and obtains the adapter earlier than is
possible with a PostTask. This needs to be doable when we have
transceivers and remote tracks are created when you synchronously add
tracks (addTrack, addStream, addTransceiver).

Bug: 777617
Change-Id: I22400466acb925221dff1a6f4db4eeb333936b66
Reviewed-on: https://chromium-review.googlesource.com/1113756
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572537}

content/renderer/media/webrtc/webrtc_media_stream_track_adapter.cc

@@ -63,22 +63,24 @@ WebRtcMediaStreamTrackAdapter::WebRtcMediaStreamTrackAdapter(
       remote_track_can_complete_initialization_(
           base::WaitableEvent::ResetPolicy::MANUAL,
           base::WaitableEvent::InitialState::NOT_SIGNALED),
-      is_initialized_(false) {
+      is_initialized_(false),
+      is_disposed_(false) {
   DCHECK(factory_);
   DCHECK(main_thread_);
 }
 
 WebRtcMediaStreamTrackAdapter::~WebRtcMediaStreamTrackAdapter() {
   DCHECK(!remote_track_can_complete_initialization_.IsSignaled());
-  DCHECK(!is_initialized_);
+  DCHECK(is_disposed_);
 }
 
 void WebRtcMediaStreamTrackAdapter::Dispose() {
   DCHECK(main_thread_->BelongsToCurrentThread());
-  if (!is_initialized_)
+  DCHECK(is_initialized_);
+  if (is_disposed_)
     return;
   remote_track_can_complete_initialization_.Reset();
-  is_initialized_ = false;
+  is_disposed_ = true;
   if (web_track_.Source().GetType() ==
       blink::WebMediaStreamSource::kTypeAudio) {
     if (local_track_audio_sink_)
@@ -99,10 +101,19 @@ bool WebRtcMediaStreamTrackAdapter::is_initialized() const {
   return is_initialized_;
 }
 
+void WebRtcMediaStreamTrackAdapter::InitializeOnMainThread() {
+  DCHECK(main_thread_->BelongsToCurrentThread());
+  if (is_initialized_)
+    return;
+  // TODO(hbos): Only ever initialize explicitly,
+  // remove EnsureTrackIsInitialized(). https://crbug.com/857458
+  EnsureTrackIsInitialized();
+}
+
 const blink::WebMediaStreamTrack& WebRtcMediaStreamTrackAdapter::web_track() {
   DCHECK(main_thread_->BelongsToCurrentThread());
-  DCHECK(!web_track_.IsNull());
   EnsureTrackIsInitialized();
+  DCHECK(!web_track_.IsNull());
   return web_track_;
 }
 
@@ -231,6 +242,7 @@ void WebRtcMediaStreamTrackAdapter::
 }
 
 void WebRtcMediaStreamTrackAdapter::EnsureTrackIsInitialized() {
+  DCHECK(main_thread_->BelongsToCurrentThread());
   if (is_initialized_)
     return;
 
@@ -242,7 +254,6 @@ void WebRtcMediaStreamTrackAdapter::EnsureTrackIsInitialized() {
 
 void WebRtcMediaStreamTrackAdapter::DisposeLocalAudioTrack() {
   DCHECK(main_thread_->BelongsToCurrentThread());
-  DCHECK(!is_initialized_);
   DCHECK(local_track_audio_sink_);
   DCHECK_EQ(web_track_.Source().GetType(),
             blink::WebMediaStreamSource::kTypeAudio);
@@ -256,7 +267,6 @@ void WebRtcMediaStreamTrackAdapter::DisposeLocalAudioTrack() {
 
 void WebRtcMediaStreamTrackAdapter::DisposeLocalVideoTrack() {
   DCHECK(main_thread_->BelongsToCurrentThread());
-  DCHECK(!is_initialized_);
   DCHECK(local_track_video_sink_);
   DCHECK_EQ(web_track_.Source().GetType(),
             blink::WebMediaStreamSource::kTypeVideo);
@@ -267,7 +277,6 @@ void WebRtcMediaStreamTrackAdapter::DisposeLocalVideoTrack() {
 
 void WebRtcMediaStreamTrackAdapter::DisposeRemoteAudioTrack() {
   DCHECK(main_thread_->BelongsToCurrentThread());
-  DCHECK(!is_initialized_);
   DCHECK(remote_audio_track_adapter_);
   DCHECK_EQ(web_track_.Source().GetType(),
             blink::WebMediaStreamSource::kTypeAudio);
@@ -280,7 +289,6 @@ void WebRtcMediaStreamTrackAdapter::DisposeRemoteAudioTrack() {
 
 void WebRtcMediaStreamTrackAdapter::DisposeRemoteVideoTrack() {
   DCHECK(main_thread_->BelongsToCurrentThread());
-  DCHECK(!is_initialized_);
   DCHECK(remote_video_track_adapter_);
   DCHECK_EQ(web_track_.Source().GetType(),
             blink::WebMediaStreamSource::kTypeVideo);
@@ -300,7 +308,7 @@ void WebRtcMediaStreamTrackAdapter::
 
 void WebRtcMediaStreamTrackAdapter::FinalizeRemoteTrackDisposingOnMainThread() {
   DCHECK(main_thread_->BelongsToCurrentThread());
-  DCHECK(!is_initialized_);
+  DCHECK(is_disposed_);
   remote_audio_track_adapter_ = nullptr;
   remote_video_track_adapter_ = nullptr;
   webrtc_track_ = nullptr;

content/renderer/media/webrtc/webrtc_media_stream_track_adapter.h

@@ -53,6 +53,7 @@ class CONTENT_EXPORT WebRtcMediaStreamTrackAdapter
   void Dispose();
 
   bool is_initialized() const;
+  void InitializeOnMainThread();
   // These methods must be called on the main thread.
   // TODO(hbos): Allow these methods to be called on any thread and make them
   // const. https://crbug.com/756436
@@ -116,6 +117,7 @@ class CONTENT_EXPORT WebRtcMediaStreamTrackAdapter
   // completed on the main thread.
   base::WaitableEvent remote_track_can_complete_initialization_;
   bool is_initialized_;
+  bool is_disposed_;
   blink::WebMediaStreamTrack web_track_;
   scoped_refptr<webrtc::MediaStreamTrackInterface> webrtc_track_;
   // If the track is local, a sink is added to the local webrtc track that is

content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.cc

@@ -59,6 +59,17 @@ WebRtcMediaStreamTrackAdapterMap::AdapterRef::Copy() const {
   return base::WrapUnique(new AdapterRef(map_, type_, adapter_));
 }
 
+void WebRtcMediaStreamTrackAdapterMap::AdapterRef::InitializeOnMainThread() {
+  adapter_->InitializeOnMainThread();
+  if (type_ == WebRtcMediaStreamTrackAdapterMap::AdapterRef::Type::kRemote) {
+    base::AutoLock scoped_lock(map_->lock_);
+    if (!map_->remote_track_adapters_.FindBySecondary(web_track().UniqueId())) {
+      map_->remote_track_adapters_.SetSecondaryKey(webrtc_track(),
+                                                   web_track().UniqueId());
+    }
+  }
+}
+
 WebRtcMediaStreamTrackAdapterMap::WebRtcMediaStreamTrackAdapterMap(
     PeerConnectionDependencyFactory* const factory,
     scoped_refptr<base::SingleThreadTaskRunner> main_thread)
@@ -182,13 +193,13 @@ WebRtcMediaStreamTrackAdapterMap::GetOrCreateRemoteTrackAdapter(
   // entry as its secondary key. This ensures that there is at least one
   // |AdapterRef| alive until after the adapter is initialized and its secondary
   // key is set.
+  auto adapter_ref = base::WrapUnique(
+      new AdapterRef(this, AdapterRef::Type::kRemote, new_adapter));
   main_thread_->PostTask(
       FROM_HERE,
       base::BindOnce(
-          &WebRtcMediaStreamTrackAdapterMap::OnRemoteTrackAdapterInitialized,
-          this,
-          base::WrapUnique(
-              new AdapterRef(this, AdapterRef::Type::kRemote, new_adapter))));
+          &WebRtcMediaStreamTrackAdapterMap::AdapterRef::InitializeOnMainThread,
+          std::move(adapter_ref)));
   return base::WrapUnique(
       new AdapterRef(this, AdapterRef::Type::kRemote, new_adapter));
 }
@@ -198,14 +209,4 @@ size_t WebRtcMediaStreamTrackAdapterMap::GetRemoteTrackCount() const {
   return remote_track_adapters_.PrimarySize();
 }
 
-void WebRtcMediaStreamTrackAdapterMap::OnRemoteTrackAdapterInitialized(
-    std::unique_ptr<AdapterRef> adapter_ref) {
-  DCHECK(adapter_ref->is_initialized());
-  {
-    base::AutoLock scoped_lock(lock_);
-    remote_track_adapters_.SetSecondaryKey(adapter_ref->webrtc_track(),
-                                           adapter_ref->web_track().UniqueId());
-  }
-}
-
 }  // namespace content

content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map.h

@@ -38,6 +38,7 @@ class CONTENT_EXPORT WebRtcMediaStreamTrackAdapterMap
 
     std::unique_ptr<AdapterRef> Copy() const;
     bool is_initialized() const { return adapter_->is_initialized(); }
+    void InitializeOnMainThread();
     const blink::WebMediaStreamTrack& web_track() const {
       return adapter_->web_track();
     }
@@ -137,8 +138,6 @@ class CONTENT_EXPORT WebRtcMediaStreamTrackAdapterMap
   // Invoke on the main thread.
   virtual ~WebRtcMediaStreamTrackAdapterMap();
 
-  void OnRemoteTrackAdapterInitialized(std::unique_ptr<AdapterRef> adapter_ref);
-
   // Pointer to a |PeerConnectionDependencyFactory| owned by the |RenderThread|.
   // It's valid for the lifetime of |RenderThread|.
   PeerConnectionDependencyFactory* const factory_;

content/renderer/media/webrtc/webrtc_media_stream_track_adapter_map_unittest.cc

@@ -53,8 +53,8 @@ class WebRtcMediaStreamTrackAdapterMapTest : public ::testing::Test {
   }
 
   std::unique_ptr<WebRtcMediaStreamTrackAdapterMap::AdapterRef>
-  GetOrCreateRemoteTrackAdapter(
-      webrtc::MediaStreamTrackInterface* webrtc_track) {
+  GetOrCreateRemoteTrackAdapter(webrtc::MediaStreamTrackInterface* webrtc_track,
+                                bool wait_for_initialization = true) {
     DCHECK(main_thread_->BelongsToCurrentThread());
     std::unique_ptr<WebRtcMediaStreamTrackAdapterMap::AdapterRef> adapter;
     signaling_thread()->PostTask(
@@ -63,7 +63,13 @@ class WebRtcMediaStreamTrackAdapterMapTest : public ::testing::Test {
                            GetOrCreateRemoteTrackAdapterOnSignalingThread,
                        base::Unretained(this), base::Unretained(webrtc_track),
                        &adapter));
-    RunMessageLoopsUntilIdle();
+    RunMessageLoopsUntilIdle(wait_for_initialization);
+    DCHECK(adapter);
+    if (wait_for_initialization) {
+      DCHECK(adapter->is_initialized());
+    } else {
+      DCHECK(!adapter->is_initialized());
+    }
     return adapter;
   }
 
@@ -76,7 +82,7 @@ class WebRtcMediaStreamTrackAdapterMapTest : public ::testing::Test {
 
   // Runs message loops on the webrtc signaling thread and the main thread until
   // idle.
-  void RunMessageLoopsUntilIdle() {
+  void RunMessageLoopsUntilIdle(bool run_loop_on_main_thread = true) {
     DCHECK(main_thread_->BelongsToCurrentThread());
     base::WaitableEvent waitable_event(
         base::WaitableEvent::ResetPolicy::MANUAL,
@@ -86,7 +92,8 @@ class WebRtcMediaStreamTrackAdapterMapTest : public ::testing::Test {
                                       RunMessageLoopUntilIdleOnSignalingThread,
                                   base::Unretained(this), &waitable_event));
     waitable_event.Wait();
-    base::RunLoop().RunUntilIdle();
+    if (run_loop_on_main_thread)
+      base::RunLoop().RunUntilIdle();
   }
 
   void RunMessageLoopUntilIdleOnSignalingThread(
@@ -167,6 +174,29 @@ TEST_F(WebRtcMediaStreamTrackAdapterMapTest, AddAndRemoveRemoteTrackAdapter) {
   RunMessageLoopsUntilIdle();
 }
 
+TEST_F(WebRtcMediaStreamTrackAdapterMapTest,
+       InitializeRemoteTrackAdapterExplicitly) {
+  scoped_refptr<MockWebRtcAudioTrack> webrtc_track =
+      MockWebRtcAudioTrack::Create("remote_track");
+  std::unique_ptr<WebRtcMediaStreamTrackAdapterMap::AdapterRef> adapter_ref =
+      GetOrCreateRemoteTrackAdapter(webrtc_track.get(), false);
+  EXPECT_FALSE(adapter_ref->is_initialized());
+  adapter_ref->InitializeOnMainThread();
+  EXPECT_TRUE(adapter_ref->is_initialized());
+
+  EXPECT_EQ(1u, map_->GetRemoteTrackCount());
+  // Ensure the implicit initialization's posted task is run after it is already
+  // initialized.
+  RunMessageLoopsUntilIdle();
+  // Destroying all references to the adapter should remove it from the map and
+  // dispose it.
+  adapter_ref.reset();
+  EXPECT_EQ(0u, map_->GetRemoteTrackCount());
+  EXPECT_EQ(nullptr, map_->GetRemoteTrackAdapter(webrtc_track.get()));
+  // Allow the disposing of track to occur.
+  RunMessageLoopsUntilIdle();
+}
+
 TEST_F(WebRtcMediaStreamTrackAdapterMapTest,
        LocalAndRemoteTrackAdaptersWithSameID) {
   // Local and remote tracks should be able to use the same id without conflict.

content/renderer/media/webrtc/webrtc_media_stream_track_adapter_unittest.cc

@@ -37,7 +37,6 @@ class WebRtcMediaStreamTrackAdapterTest : public ::testing::Test {
     if (track_adapter_) {
       EXPECT_TRUE(track_adapter_->is_initialized());
       track_adapter_->Dispose();
-      EXPECT_FALSE(track_adapter_->is_initialized());
       track_adapter_ = nullptr;
       RunMessageLoopsUntilIdle();
     }
@@ -80,9 +79,9 @@ class WebRtcMediaStreamTrackAdapterTest : public ::testing::Test {
         dependency_factory_.get(), main_thread_, webrtc_track);
   }
 
-  // Runs message loops on the webrtc signaling thread and the main thread until
-  // idle.
-  void RunMessageLoopsUntilIdle() {
+  // Runs message loops on the webrtc signaling thread and optionally the main
+  // thread until idle.
+  void RunMessageLoopsUntilIdle(bool run_loop_on_main_thread = true) {
     base::WaitableEvent waitable_event(
         base::WaitableEvent::ResetPolicy::MANUAL,
         base::WaitableEvent::InitialState::NOT_SIGNALED);
@@ -91,7 +90,8 @@ class WebRtcMediaStreamTrackAdapterTest : public ::testing::Test {
                                       RunMessageLoopUntilIdleOnSignalingThread,
                                   base::Unretained(this), &waitable_event));
     waitable_event.Wait();
-    base::RunLoop().RunUntilIdle();
+    if (run_loop_on_main_thread)
+      base::RunLoop().RunUntilIdle();
   }
 
   void RunMessageLoopUntilIdleOnSignalingThread(
@@ -157,6 +157,7 @@ TEST_F(WebRtcMediaStreamTrackAdapterTest, RemoteAudioTrack) {
       base::BindOnce(
           &WebRtcMediaStreamTrackAdapterTest::CreateRemoteTrackAdapter,
           base::Unretained(this), base::Unretained(webrtc_track.get())));
+  // The adapter is initialized implicitly in a PostTask, allow it to run.
   RunMessageLoopsUntilIdle();
   DCHECK(track_adapter_);
   EXPECT_TRUE(track_adapter_->is_initialized());
@@ -181,6 +182,7 @@ TEST_F(WebRtcMediaStreamTrackAdapterTest, RemoteVideoTrack) {
       base::BindOnce(
           &WebRtcMediaStreamTrackAdapterTest::CreateRemoteTrackAdapter,
           base::Unretained(this), base::Unretained(webrtc_track.get())));
+  // The adapter is initialized implicitly in a PostTask, allow it to run.
   RunMessageLoopsUntilIdle();
   DCHECK(track_adapter_);
   EXPECT_TRUE(track_adapter_->is_initialized());
@@ -197,4 +199,33 @@ TEST_F(WebRtcMediaStreamTrackAdapterTest, RemoteVideoTrack) {
       track_adapter_->GetRemoteVideoTrackAdapterForTesting()->initialized());
 }
 
+TEST_F(WebRtcMediaStreamTrackAdapterTest, RemoteTrackExplicitlyInitialized) {
+  scoped_refptr<MockWebRtcAudioTrack> webrtc_track =
+      MockWebRtcAudioTrack::Create("remote_audio_track");
+  dependency_factory_->GetWebRtcSignalingThread()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &WebRtcMediaStreamTrackAdapterTest::CreateRemoteTrackAdapter,
+          base::Unretained(this), base::Unretained(webrtc_track.get())));
+  // Wait for the CreateRemoteTrackAdapter() call, but don't run the main thread
+  // loop that would have implicitly initialized the adapter.
+  RunMessageLoopsUntilIdle(false);
+  DCHECK(track_adapter_);
+  EXPECT_FALSE(track_adapter_->is_initialized());
+  // Explicitly initialize before the main thread loop has a chance to run.
+  track_adapter_->InitializeOnMainThread();
+  EXPECT_TRUE(track_adapter_->is_initialized());
+  EXPECT_TRUE(!track_adapter_->web_track().IsNull());
+  EXPECT_EQ(track_adapter_->web_track().Source().GetType(),
+            blink::WebMediaStreamSource::kTypeAudio);
+  EXPECT_TRUE(track_adapter_->webrtc_track());
+  EXPECT_EQ(track_adapter_->webrtc_track()->kind(),
+            webrtc::MediaStreamTrackInterface::kAudioKind);
+  EXPECT_EQ(track_adapter_->webrtc_track()->id().c_str(),
+            track_adapter_->web_track().Id());
+  EXPECT_TRUE(track_adapter_->GetRemoteAudioTrackAdapterForTesting());
+  EXPECT_TRUE(
+      track_adapter_->GetRemoteAudioTrackAdapterForTesting()->initialized());
+}
+
 }  // namespace content