diff options
author | Bo Liu <boliu@chromium.org> | 2014-09-17 18:41:32 -0700 |
---|---|---|
committer | Bo Liu <boliu@google.com> | 2014-09-18 02:43:52 +0000 |
commit | afd2a9f89449292aaeadb833e40f72ebbbe23d45 (patch) | |
tree | dbaa9181d60ac3290ab5636bbf93eb56c84237c0 | |
parent | afd392134d586de79b53a70bf2a5927287fd3447 (diff) | |
download | chromium_org-afd2a9f89449292aaeadb833e40f72ebbbe23d45.tar.gz |
Cherry-pick: Fix threading issue in StreamTextureProxyImpl
Cherry-pick of https://codereview.chromium.org/575293003/ PS2
BindToLoop need to set |loop_| synchronously to prevent delete from
destroying the object on the wrong thread.
BUG: 17354106
Change-Id: I3168bb9c27fa135e0c98eb018610218e65455f0a
-rw-r--r-- | content/renderer/media/android/stream_texture_factory_impl.cc | 42 | ||||
-rw-r--r-- | content/renderer/media/android/stream_texture_factory_synchronous_impl.cc | 43 |
2 files changed, 40 insertions, 45 deletions
diff --git a/content/renderer/media/android/stream_texture_factory_impl.cc b/content/renderer/media/android/stream_texture_factory_impl.cc index fba6296b45..007f3e9d76 100644 --- a/content/renderer/media/android/stream_texture_factory_impl.cc +++ b/content/renderer/media/android/stream_texture_factory_impl.cc @@ -32,15 +32,13 @@ class StreamTextureProxyImpl : public StreamTextureProxy, virtual void OnMatrixChanged(const float matrix[16]) OVERRIDE; private: - void SetClient(cc::VideoFrameProvider::Client* client); - void BindOnThread(int32 stream_id, - scoped_refptr<base::MessageLoopProxy> loop); + void BindOnThread(int32 stream_id); const scoped_ptr<StreamTextureHost> host_; - scoped_refptr<base::MessageLoopProxy> loop_; - base::Lock client_lock_; + base::Lock lock_; cc::VideoFrameProvider::Client* client_; + scoped_refptr<base::MessageLoopProxy> loop_; DISALLOW_IMPLICIT_CONSTRUCTORS(StreamTextureProxyImpl); }; @@ -51,28 +49,33 @@ StreamTextureProxyImpl::StreamTextureProxyImpl(StreamTextureHost* host) StreamTextureProxyImpl::~StreamTextureProxyImpl() {} void StreamTextureProxyImpl::Release() { + { + base::AutoLock lock(lock_); + client_ = NULL; + } // Assumes this is the last reference to this object. So no need to acquire // lock. - SetClient(NULL); if (!loop_.get() || loop_->BelongsToCurrentThread() || !loop_->DeleteSoon(FROM_HERE, this)) { delete this; } } -void StreamTextureProxyImpl::SetClient(cc::VideoFrameProvider::Client* client) { - base::AutoLock lock(client_lock_); - client_ = client; -} - void StreamTextureProxyImpl::BindToLoop( int32 stream_id, cc::VideoFrameProvider::Client* client, scoped_refptr<base::MessageLoopProxy> loop) { DCHECK(loop); - SetClient(client); + + { + base::AutoLock lock(lock_); + DCHECK(!loop_ || (loop == loop_)); + loop_ = loop; + client_ = client; + } + if (loop->BelongsToCurrentThread()) { - BindOnThread(stream_id, loop); + BindOnThread(stream_id); return; } // Unretained is safe here only because the object is deleted on |loop_| @@ -80,26 +83,21 @@ void StreamTextureProxyImpl::BindToLoop( loop->PostTask(FROM_HERE, base::Bind(&StreamTextureProxyImpl::BindOnThread, base::Unretained(this), - stream_id, - loop)); + stream_id)); } -void StreamTextureProxyImpl::BindOnThread( - int32 stream_id, - scoped_refptr<base::MessageLoopProxy> loop) { - DCHECK(!loop_ || (loop == loop_)); - loop_ = loop; +void StreamTextureProxyImpl::BindOnThread(int32 stream_id) { host_->BindToCurrentThread(stream_id, this); } void StreamTextureProxyImpl::OnFrameAvailable() { - base::AutoLock lock(client_lock_); + base::AutoLock lock(lock_); if (client_) client_->DidReceiveFrame(); } void StreamTextureProxyImpl::OnMatrixChanged(const float matrix[16]) { - base::AutoLock lock(client_lock_); + base::AutoLock lock(lock_); if (client_) client_->DidUpdateMatrix(matrix); } diff --git a/content/renderer/media/android/stream_texture_factory_synchronous_impl.cc b/content/renderer/media/android/stream_texture_factory_synchronous_impl.cc index 2beaf3b956..5d57545f11 100644 --- a/content/renderer/media/android/stream_texture_factory_synchronous_impl.cc +++ b/content/renderer/media/android/stream_texture_factory_synchronous_impl.cc @@ -40,16 +40,14 @@ class StreamTextureProxyImpl virtual void Release() OVERRIDE; private: - void SetClient(cc::VideoFrameProvider::Client* client); - void BindOnThread(int32 stream_id, - scoped_refptr<base::MessageLoopProxy> loop); + void BindOnThread(int32 stream_id); void OnFrameAvailable(); - base::Lock client_lock_; + base::Lock lock_; cc::VideoFrameProvider::Client* client_; + scoped_refptr<base::MessageLoopProxy> loop_; // Accessed on the |loop_| thread only. - scoped_refptr<base::MessageLoopProxy> loop_; base::Closure callback_; scoped_refptr<StreamTextureFactorySynchronousImpl::ContextProvider> context_provider_; @@ -69,28 +67,33 @@ StreamTextureProxyImpl::StreamTextureProxyImpl( StreamTextureProxyImpl::~StreamTextureProxyImpl() {} void StreamTextureProxyImpl::Release() { + { + base::AutoLock lock(lock_); + client_ = NULL; + } // Assumes this is the last reference to this object. So no need to acquire // lock. - SetClient(NULL); if (!loop_.get() || loop_->BelongsToCurrentThread() || !loop_->DeleteSoon(FROM_HERE, this)) { delete this; } } -void StreamTextureProxyImpl::SetClient(cc::VideoFrameProvider::Client* client) { - base::AutoLock lock(client_lock_); - client_ = client; -} - void StreamTextureProxyImpl::BindToLoop( int32 stream_id, cc::VideoFrameProvider::Client* client, scoped_refptr<base::MessageLoopProxy> loop) { DCHECK(loop); - SetClient(client); + + { + base::AutoLock lock(lock_); + DCHECK(!loop_ || (loop == loop_)); + loop_ = loop; + client_ = client; + } + if (loop->BelongsToCurrentThread()) { - BindOnThread(stream_id, loop); + BindOnThread(stream_id); return; } // Unretained is safe here only because the object is deleted on |loop_| @@ -98,16 +101,10 @@ void StreamTextureProxyImpl::BindToLoop( loop->PostTask(FROM_HERE, base::Bind(&StreamTextureProxyImpl::BindOnThread, base::Unretained(this), - stream_id, - loop)); + stream_id)); } -void StreamTextureProxyImpl::BindOnThread( - int32 stream_id, - scoped_refptr<base::MessageLoopProxy> loop) { - DCHECK(!loop_ || (loop == loop_)); - loop_ = loop; - +void StreamTextureProxyImpl::BindOnThread(int32 stream_id) { surface_texture_ = context_provider_->GetSurfaceTexture(stream_id); if (!surface_texture_) { LOG(ERROR) << "Failed to get SurfaceTexture for stream."; @@ -130,7 +127,7 @@ void StreamTextureProxyImpl::OnFrameAvailable() { if (memcmp(current_matrix_, matrix, sizeof(matrix)) != 0) { memcpy(current_matrix_, matrix, sizeof(matrix)); - base::AutoLock lock(client_lock_); + base::AutoLock lock(lock_); if (client_) client_->DidUpdateMatrix(current_matrix_); } @@ -139,7 +136,7 @@ void StreamTextureProxyImpl::OnFrameAvailable() { // updateTexImage since after we received the first frame. has_updated_ = true; - base::AutoLock lock(client_lock_); + base::AutoLock lock(lock_); if (client_) client_->DidReceiveFrame(); } |