Split libjingle's signaling thread from the UI thread

content/renderer/media/webrtc/media_stream_remote_video_source.cc

Index: content/renderer/media/webrtc/media_stream_remote_video_source.cc
diff --git a/content/renderer/media/webrtc/media_stream_remote_video_source.cc b/content/renderer/media/webrtc/media_stream_remote_video_source.cc
index 02bd5092dd3b449ac29204290d170172a17c83bb..33bcfc82773a1f9dc4e0975547c7b866c83d1db8 100644
--- a/content/renderer/media/webrtc/media_stream_remote_video_source.cc
+++ b/content/renderer/media/webrtc/media_stream_remote_video_source.cc
@@ -6,8 +6,10 @@
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
+#include "base/debug/trace_event.h"
 #include "base/location.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/threading/thread_checker.h"
 #include "content/renderer/media/native_handle_impl.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/video_frame.h"
@@ -71,6 +73,7 @@ RemoteVideoSourceDelegate::SetSize(int width, int height) {
 void MediaStreamRemoteVideoSource::
 RemoteVideoSourceDelegate::RenderFrame(
     const cricket::VideoFrame* frame) {
+  TRACE_EVENT0("webrtc", "RemoteVideoSourceDelegate::RenderFrame");
   base::TimeDelta timestamp = base::TimeDelta::FromMicroseconds(
       frame->GetElapsedTime() / rtc::kNumNanosecsPerMicrosec);
 
@@ -120,19 +123,66 @@ RemoteVideoSourceDelegate::DoRenderFrameOnIOThread(
     scoped_refptr<media::VideoFrame> video_frame,
     const media::VideoCaptureFormat& format) {
   DCHECK(io_message_loop_->BelongsToCurrentThread());
+  TRACE_EVENT0("webrtc", "RemoteVideoSourceDelegate::DoRenderFrameOnIOThread");
   // TODO(hclam): Give the estimated capture time.
   frame_callback_.Run(video_frame, format, base::TimeTicks());
 }
 
+MediaStreamRemoteVideoSource::Observer::Observer(

[perkj_chrome, 2014/10/30 12:42:36]

I think we should try to create this object on the main render thread and only
have Observer::OnChanged() called on the signaling thread.
 

[tommi (sloooow) - chröme, 2014/10/30 20:37:36]

Discussed offline

+    const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
+    webrtc::VideoTrackInterface* track)
+    : main_thread_(main_thread), track_(track), state_(track->state()) {
+  track->RegisterObserver(this);
+}
+
+const scoped_refptr<webrtc::VideoTrackInterface>&
+MediaStreamRemoteVideoSource::Observer::track() {
+  DCHECK(main_thread_->BelongsToCurrentThread());
+  return track_;
+}
+
+webrtc::MediaStreamTrackInterface::TrackState
+MediaStreamRemoteVideoSource::Observer::state() const {
+  DCHECK(main_thread_->BelongsToCurrentThread());
+  return state_;
+}
+
+MediaStreamRemoteVideoSource::Observer::~Observer() {
+  DCHECK(main_thread_->BelongsToCurrentThread());
+  track_->UnregisterObserver(this);
+}
+
+void MediaStreamRemoteVideoSource::Observer::SetSource(
+    const base::WeakPtr<MediaStreamRemoteVideoSource>& source) {
+  DCHECK(main_thread_->BelongsToCurrentThread());
+  DCHECK(!source_);
+  source_ = source;
+}
+
+void MediaStreamRemoteVideoSource::Observer::OnChanged() {
+  webrtc::MediaStreamTrackInterface::TrackState state = track_->state();
+  main_thread_->PostTask(FROM_HERE,
+      base::Bind(&MediaStreamRemoteVideoSource::Observer::OnChangedImpl,
+          this, state));
+}
+
+void MediaStreamRemoteVideoSource::Observer::OnChangedImpl(
+    webrtc::MediaStreamTrackInterface::TrackState state) {
+  DCHECK(main_thread_->BelongsToCurrentThread());
+  DLOG_IF(ERROR, !source_) << "Dropping a state change event. " << state;

[perkj_chrome, 2014/10/30 12:42:35]

Can this happen before |source_| have been set? 

[tommi (sloooow) - chröme, 2014/10/30 20:37:36]

Only if a unittest is doing something funky.  Changed it to be a DCHECK.

+  if (source_ && state != state_)
+    source_->OnChanged(state);
+  state_ = state;
+}
+
 MediaStreamRemoteVideoSource::MediaStreamRemoteVideoSource(
-    webrtc::VideoTrackInterface* remote_track)
-    : remote_track_(remote_track),
-      last_state_(remote_track->state()) {
-  remote_track_->RegisterObserver(this);
+    const scoped_refptr<MediaStreamRemoteVideoSource::Observer>& observer)
+    : observer_(observer), weak_factory_(this) {
+  observer->SetSource(weak_factory_.GetWeakPtr());
 }
 
 MediaStreamRemoteVideoSource::~MediaStreamRemoteVideoSource() {
-  remote_track_->UnregisterObserver(this);
+  DCHECK(CalledOnValidThread());
 }
 
 void MediaStreamRemoteVideoSource::GetCurrentSupportedFormats(
@@ -140,7 +190,7 @@ void MediaStreamRemoteVideoSource::GetCurrentSupportedFormats(
     int max_requested_height,
     double max_requested_frame_rate,
     const VideoCaptureDeviceFormatsCB& callback) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(CalledOnValidThread());
   media::VideoCaptureFormats formats;
   // Since the remote end is free to change the resolution at any point in time
   // the supported formats are unknown.
@@ -150,17 +200,17 @@ void MediaStreamRemoteVideoSource::GetCurrentSupportedFormats(
 void MediaStreamRemoteVideoSource::StartSourceImpl(
     const media::VideoCaptureFormat& format,
     const VideoCaptureDeliverFrameCB& frame_callback) {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(CalledOnValidThread());
   DCHECK(!delegate_.get());
   delegate_ = new RemoteVideoSourceDelegate(io_message_loop(), frame_callback);
-  remote_track_->AddRenderer(delegate_.get());
+  observer_->track()->AddRenderer(delegate_.get());
   OnStartDone(MEDIA_DEVICE_OK);
 }
 
 void MediaStreamRemoteVideoSource::StopSourceImpl() {
-  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(CalledOnValidThread());
   DCHECK(state() != MediaStreamVideoSource::ENDED);
-  remote_track_->RemoveRenderer(delegate_.get());
+  observer_->track()->RemoveRenderer(delegate_.get());
 }
 
 webrtc::VideoRendererInterface*
@@ -168,26 +218,23 @@ MediaStreamRemoteVideoSource::RenderInterfaceForTest() {
   return delegate_.get();
 }
 
-void MediaStreamRemoteVideoSource::OnChanged() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  webrtc::MediaStreamTrackInterface::TrackState state = remote_track_->state();
-  if (state != last_state_) {
-    last_state_ = state;
-    switch (state) {
-      case webrtc::MediaStreamTrackInterface::kInitializing:
-        // Ignore the kInitializing state since there is no match in
-        // WebMediaStreamSource::ReadyState.
-        break;
-      case webrtc::MediaStreamTrackInterface::kLive:
-        SetReadyState(blink::WebMediaStreamSource::ReadyStateLive);
-        break;
-      case webrtc::MediaStreamTrackInterface::kEnded:
-        SetReadyState(blink::WebMediaStreamSource::ReadyStateEnded);
-        break;
-      default:
-        NOTREACHED();
-        break;
-    }
+void MediaStreamRemoteVideoSource::OnChanged(
+    webrtc::MediaStreamTrackInterface::TrackState state) {
+  DCHECK(CalledOnValidThread());
+  switch (state) {
+    case webrtc::MediaStreamTrackInterface::kInitializing:
+      // Ignore the kInitializing state since there is no match in
+      // WebMediaStreamSource::ReadyState.
+      break;
+    case webrtc::MediaStreamTrackInterface::kLive:
+      SetReadyState(blink::WebMediaStreamSource::ReadyStateLive);
+      break;
+    case webrtc::MediaStreamTrackInterface::kEnded:
+      SetReadyState(blink::WebMediaStreamSource::ReadyStateEnded);
+      break;
+    default:
+      NOTREACHED();
+      break;
   }
 }