aw: Avoid uncontrolled video context destruction

content/browser/android/in_process/synchronous_compositor_factory_impl.cc

Index: content/browser/android/in_process/synchronous_compositor_factory_impl.cc
diff --git a/content/browser/android/in_process/synchronous_compositor_factory_impl.cc b/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
index 006a3c6e29efff014b75a4570f001feb709756f2..e624e9b53f01d80d69e0fe7b3c17f399c0cb489a 100644
--- a/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
+++ b/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
@@ -232,11 +232,6 @@ void SynchronousCompositorFactoryImpl::CompositorReleasedHardwareDraw() {
   base::AutoLock lock(num_hardware_compositor_lock_);
   DCHECK_GT(num_hardware_compositors_, 0u);
   num_hardware_compositors_--;
-  if (num_hardware_compositors_ == 0) {
-    // Nullify the video_context_provider_ now so that it is not null only if
-    // there is at least 1 hardware compositor
-    video_context_provider_ = NULL;
-  }
 }
 
 bool SynchronousCompositorFactoryImpl::CanCreateMainThreadContext() {
@@ -246,13 +241,16 @@ bool SynchronousCompositorFactoryImpl::CanCreateMainThreadContext() {
 
 scoped_refptr<StreamTextureFactorySynchronousImpl::ContextProvider>
 SynchronousCompositorFactoryImpl::TryCreateStreamTextureFactory() {
-  scoped_refptr<StreamTextureFactorySynchronousImpl::ContextProvider>
-      context_provider;
-  // This check only guarantees the main thread context is created after
-  // a compositor did successfully initialize hardware draw in the past.
-  // When all compositors have released hardware draw, main thread context
-  // creation is guaranteed to fail.
-  if (CanCreateMainThreadContext() && !video_context_provider_) {
+  // Always fail creation even if |video_context_provider_| is not NULL.
+  // This is to avoid synchronous calls that may deadlock. Setting
+  // |video_context_provider_| to null is also not safe since it makes
+  // synchronous destruction uncontrolled and possibly deadlock.
+  if (!CanCreateMainThreadContext()) {
+    return
+        scoped_refptr<StreamTextureFactorySynchronousImpl::ContextProvider>();

[hush (inactive), 2014/08/22 17:49:38]

can we just return null here?

[boliu, 2014/08/22 17:54:17]

Hmm, compiler seems happy, but not sure if it fits chromium style

[boliu, 2014/08/22 18:14:30]

Took a cursory look and didn't find any instance of "return NULL" used on a
ref-counted type.

[hush (inactive), 2014/08/22 18:26:43]

alright.
lgtm
sorry that I broke this thing...
On 2014/08/22 18:14:30, boliu wrote:

+  }
+
+  if (!video_context_provider_) {
     DCHECK(service_);
     DCHECK(share_context_.get());