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
diff --git a/content/renderer/media/android/stream_texture_factory_impl.cc b/content/renderer/media/android/stream_texture_factory_impl.cc
index bf95b14..a8b9a32 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 @@
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() {}
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 @@
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 ec54a8c..1057213 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 @@
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() {}
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 @@
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 @@
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 @@
// updateTexImage since after we received the first frame.
has_updated_ = true;
- base::AutoLock lock(client_lock_);
+ base::AutoLock lock(lock_);
if (client_)
client_->DidReceiveFrame();
}
