work-in-progress patch to fix context lost black video

content/renderer/media/android/stream_texture_factory_synchronous_impl.cc

Index: content/renderer/media/android/stream_texture_factory_synchronous_impl.cc
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 20c8ee7ae1ab6f16ec19b736b342b4d222f35803..22584e340f6b6cf6a18b436d5683afe30d28ebf3 100644
--- a/content/renderer/media/android/stream_texture_factory_synchronous_impl.cc
+++ b/content/renderer/media/android/stream_texture_factory_synchronous_impl.cc
@@ -34,22 +34,24 @@ class StreamTextureProxyImpl
   virtual ~StreamTextureProxyImpl();
 
   // StreamTextureProxy implementation:
-  virtual void BindToCurrentThread(int32 stream_id) OVERRIDE;
-  virtual void SetClient(cc::VideoFrameProvider::Client* client) OVERRIDE;
+  virtual void BindToLoop(int32 stream_id,
+                          cc::VideoFrameProvider::Client* client,
+                          scoped_refptr<base::MessageLoopProxy> loop) OVERRIDE;
   virtual void Release() OVERRIDE;
 
  private:
+  void BindOnThread(int32 stream_id, cc::VideoFrameProvider::Client* client);
   void OnFrameAvailable();
 
+  base::Lock lock_;
   scoped_refptr<base::MessageLoopProxy> loop_;
-  base::Lock client_lock_;
   cc::VideoFrameProvider::Client* client_;
-  base::Closure callback_;
 
+  // Accessed on the |loop_| thread only.
+  base::Closure callback_;
   scoped_refptr<StreamTextureFactorySynchronousImpl::ContextProvider>
       context_provider_;
   scoped_refptr<gfx::SurfaceTexture> surface_texture_;
-
   float current_matrix_[16];
   bool has_updated_;
 
@@ -58,27 +60,59 @@ class StreamTextureProxyImpl
 
 StreamTextureProxyImpl::StreamTextureProxyImpl(
     StreamTextureFactorySynchronousImpl::ContextProvider* provider)
-    : context_provider_(provider), has_updated_(false) {
+    : client_(NULL), context_provider_(provider), has_updated_(false) {
   std::fill(current_matrix_, current_matrix_ + 16, 0);
 }
 
 StreamTextureProxyImpl::~StreamTextureProxyImpl() {}
 
 void StreamTextureProxyImpl::Release() {
-  SetClient(NULL);
-  if (loop_.get() && !loop_->BelongsToCurrentThread())
-    loop_->DeleteSoon(FROM_HERE, this);
-  else
+  base::AutoLock lock(lock_);
+  client_ = NULL;
+  if (!loop_.get() || loop_.get() != base::MessageLoopProxy::current() ||
+      !loop_->DeleteSoon(FROM_HERE, this)) {
     delete this;

[boliu, 2014/09/15 17:00:29]

Hmm, this crashes in lock because lock can't be deleted while it's held.

Technically lock only protects client_ and loop_. The fact that BindToLoop can't
be called from another thread is not guaranteed in this class. So could add a
AutoUnlock here. Barrrr

+  }
 }
 
-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) {
+  base::AutoLock lock(lock_);
+  DCHECK(loop.get());
+  loop_ = loop;
+
+  if (loop.get() != base::MessageLoopProxy::current()) {
+    // Unretained is safe here only because the object is deleted on |loop_|
+    // thread.
+    loop->PostTask(FROM_HERE,
+                   base::Bind(&StreamTextureProxyImpl::BindOnThread,
+                              base::Unretained(this),
+                              stream_id,
+                              client));
+  } else {
+    client_ = client;
+    surface_texture_ = context_provider_->GetSurfaceTexture(stream_id);
+    if (!surface_texture_) {
+      LOG(ERROR) << "Failed to get SurfaceTexture for stream.";
+      return;
+    }
+
+    callback_ =
+        base::Bind(&StreamTextureProxyImpl::OnFrameAvailable, AsWeakPtr());
+    surface_texture_->SetFrameAvailableCallback(callback_);
+  }
 }
 
-void StreamTextureProxyImpl::BindToCurrentThread(int stream_id) {
-  loop_ = base::MessageLoopProxy::current();
+void StreamTextureProxyImpl::BindOnThread(
+    int32 stream_id,
+    cc::VideoFrameProvider::Client* client) {
+  {
+    base::AutoLock lock(lock_);
+    client_ = client;
+  }
+
   surface_texture_ = context_provider_->GetSurfaceTexture(stream_id);
   if (!surface_texture_) {
     LOG(ERROR) << "Failed to get SurfaceTexture for stream.";
@@ -101,7 +135,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_);
     }
@@ -110,7 +144,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();
 }
@@ -182,4 +216,14 @@ gpu::gles2::GLES2Interface* StreamTextureFactorySynchronousImpl::ContextGL() {
   return context_provider_->ContextGL();
 }
 
+void StreamTextureFactorySynchronousImpl::AddObserver(
+    StreamTextureFactoryContextObserver* obs) {
+  context_provider_->AddObserver(obs);
+}
+
+void StreamTextureFactorySynchronousImpl::RemoveObserver(
+    StreamTextureFactoryContextObserver* obs) {
+  context_provider_->RemoveObserver(obs);
+}
+
 }  // namespace content