From ee22d8807a7a7bbb69dcbb7a37307eadb57a7b2b Mon Sep 17 00:00:00 2001
From: "perkj@chromium.org"
 <perkj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 9 May 2014 11:07:47 +0000
Subject: [PATCH] Make sure webrtc::VideoSourceInterface is released on the
 main render thread. webrtc::VideoSourceInterface must be released on the main
 render thread since it needs a libjingle thread wrapper. Fix problem with
 where a const scoped_refptr<webrtc::VideoSourceInterface>& source must be
 released on the main render thread. The problem was introduced in
 https://codereview.chromium.org/252393006/ where the video frames are now
 delivered on the IO-thread that may result in that that
 WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter that holds the last
 reference to it is destroyed on the IO-thread.

TEST= http://googlechrome.github.io/webrtc/samples/web/content/constraints/ click Ge media and click Connect. When video is flowing, refresh the page.
R=tommi@chromium.org

Review URL: https://codereview.chromium.org/279533003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269222 0039d316-1c4b-4281-b951-d872f2087c98
---
 .../media/rtc_peer_connection_handler_unittest.cc  |  1 +
 .../webrtc/webrtc_media_stream_adapter_unittest.cc |  2 ++
 .../media/webrtc/webrtc_video_track_adapter.cc     | 22 +++++++++++++++++++++-
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/content/renderer/media/rtc_peer_connection_handler_unittest.cc b/content/renderer/media/rtc_peer_connection_handler_unittest.cc
index 93e5b237688d..85e8b199fc30 100644
--- a/content/renderer/media/rtc_peer_connection_handler_unittest.cc
+++ b/content/renderer/media/rtc_peer_connection_handler_unittest.cc
@@ -288,6 +288,7 @@ class RTCPeerConnectionHandlerTest : public ::testing::Test {
   }
 
   scoped_ptr<ChildProcess> child_process_;
+  base::MessageLoopForUI message_loop_;
   scoped_ptr<MockWebRTCPeerConnectionHandlerClient> mock_client_;
   scoped_ptr<MockMediaStreamDependencyFactory> mock_dependency_factory_;
   scoped_ptr<NiceMock<MockPeerConnectionTracker> > mock_tracker_;
diff --git a/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc b/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc
index 5a5b9bff01b1..a87fa93f9593 100644
--- a/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc
+++ b/content/renderer/media/webrtc/webrtc_media_stream_adapter_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "content/child/child_process.h"
 #include "content/renderer/media/media_stream.h"
 #include "content/renderer/media/media_stream_audio_source.h"
@@ -92,6 +93,7 @@ class WebRtcMediaStreamAdapterTest : public ::testing::Test {
 
  protected:
   scoped_ptr<ChildProcess> child_process_;
+  base::MessageLoopForUI message_loop_;
   scoped_ptr<MockMediaStreamDependencyFactory> dependency_factory_;
   scoped_ptr<WebRtcMediaStreamAdapter> adapter_;
 };
diff --git a/content/renderer/media/webrtc/webrtc_video_track_adapter.cc b/content/renderer/media/webrtc/webrtc_video_track_adapter.cc
index 0fdef64f2eb5..55ee5d45d3d4 100644
--- a/content/renderer/media/webrtc/webrtc_video_track_adapter.cc
+++ b/content/renderer/media/webrtc/webrtc_video_track_adapter.cc
@@ -22,6 +22,12 @@ bool ConstraintKeyExists(const blink::WebMediaConstraints& constraints,
 
 namespace content {
 
+// Used to make sure |source| is released on the main render thread.
+void ReleaseWebRtcSourceOnMainRenderThread(
+    webrtc::VideoSourceInterface* source) {
+  source->Release();
+}
+
 // Simple help class used for receiving video frames on the IO-thread from
 // a MediaStreamVideoTrack and forward the frames to a
 // WebRtcVideoCapturerAdapter that implements a video capturer for libjingle.
@@ -39,8 +45,12 @@ class WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter
   friend class base::RefCountedThreadSafe<WebRtcVideoSourceAdapter>;
   virtual ~WebRtcVideoSourceAdapter();
 
+  scoped_refptr<base::MessageLoopProxy> render_thread_message_loop_;
   // Used to DCHECK that frames are called on the IO-thread.
   base::ThreadChecker io_thread_checker_;
+
+  // |video_source_| is a libjingle object that must be released on the main
+  // render thread.
   scoped_refptr<webrtc::VideoSourceInterface> video_source_;
   // |capture_adapter_| is owned by |video_source_|
   WebRtcVideoCapturerAdapter* capture_adapter_;
@@ -49,12 +59,22 @@ class WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter
 WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter::WebRtcVideoSourceAdapter(
     const scoped_refptr<webrtc::VideoSourceInterface>& source,
     WebRtcVideoCapturerAdapter* capture_adapter)
-    : video_source_(source),
+    : render_thread_message_loop_(base::MessageLoopProxy::current()),
+      video_source_(source),
       capture_adapter_(capture_adapter) {
   io_thread_checker_.DetachFromThread();
 }
 
 WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter::~WebRtcVideoSourceAdapter() {
+  if (!render_thread_message_loop_->BelongsToCurrentThread()) {
+    webrtc::VideoSourceInterface* source = video_source_.get();
+    source->AddRef();
+    video_source_ = NULL;
+    render_thread_message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&ReleaseWebRtcSourceOnMainRenderThread,
+                   base::Unretained(source)));
+  }
 }
 
 void WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter::OnVideoFrameOnIO(
-- 
2.11.4.GIT