[Android WebView] Clean up global GL resource pt.1

content/browser/android/in_process/synchronous_compositor_impl.cc

Index: content/browser/android/in_process/synchronous_compositor_impl.cc
diff --git a/content/browser/android/in_process/synchronous_compositor_impl.cc b/content/browser/android/in_process/synchronous_compositor_impl.cc
index 97bbfe906a3f46f223cab780611eb85ecfa9783f..ab59d627ea8c3355fa9479e06763f036abbe1388 100644
--- a/content/browser/android/in_process/synchronous_compositor_impl.cc
+++ b/content/browser/android/in_process/synchronous_compositor_impl.cc
@@ -76,7 +76,8 @@ class VideoContextProvider
 class SynchronousCompositorFactoryImpl : public SynchronousCompositorFactory {
  public:
   SynchronousCompositorFactoryImpl()
-      : wrapped_gl_context_for_main_thread_(NULL) {
+      : wrapped_gl_context_for_main_thread_(NULL),
+        num_hardware_compositors_(0) {
     SynchronousCompositorFactory::SetInstance(this);
   }
 
@@ -137,16 +138,20 @@ class SynchronousCompositorFactoryImpl : public SynchronousCompositorFactory {
 
   virtual scoped_refptr<cc::ContextProvider>
   GetOffscreenContextProviderForMainThread() OVERRIDE {
-    if (!offscreen_context_for_main_thread_.get() ||
-        offscreen_context_for_main_thread_->DestroyedOnMainThread()) {
+    bool can_create_context = CanCreateMainThreadContext();
+    if (can_create_context &&
+        (!offscreen_context_for_main_thread_.get() ||
+         offscreen_context_for_main_thread_->DestroyedOnMainThread())) {
       offscreen_context_for_main_thread_ =
           webkit::gpu::ContextProviderInProcess::Create(
               CreateOffscreenContext());
-      if (offscreen_context_for_main_thread_.get() &&
-          !offscreen_context_for_main_thread_->BindToCurrentThread()) {
-        offscreen_context_for_main_thread_ = NULL;
-        wrapped_gl_context_for_main_thread_ = NULL;
-      }
+    }
+
+    if (!can_create_context ||

[joth, 2013/09/05 20:47:12]

splicing the can_create_context into both expressions makes it hard to follow
the possible different paths here.

what you really want is a goto. (or moreover, extract a function).

As it is, maybe

bool fail = !CanCreateMainThreadContext();
if (!fail) {
  if (!offscreen_context_for_main_thread_.get() ||
offscreen_context_for_main_thread_->DestroyedOnMainThread()) {
  offscreen_context_for_main_thread_ = create...
  fail = !offscreen_context_for_main_thread_.get() ||
!offscreen_context_for_main_thread_->BindToCurrentThread();
}
}

if (fail) {
  offscreen_context_for_main_thread_ = NULL;
  wrapped_gl_context_for_main_thread_ = NULL;
}

[boliu, 2013/09/05 23:21:25]

Done.

+        (offscreen_context_for_main_thread_.get() &&
+         !offscreen_context_for_main_thread_->BindToCurrentThread())) {
+      offscreen_context_for_main_thread_ = NULL;
+      wrapped_gl_context_for_main_thread_ = NULL;
     }
     return offscreen_context_for_main_thread_;
   }
@@ -159,7 +164,7 @@ class SynchronousCompositorFactoryImpl : public SynchronousCompositorFactory {
   // any thread and is lightweight.
   virtual scoped_refptr<cc::ContextProvider>
       GetOffscreenContextProviderForCompositorThread() OVERRIDE {
-    base::AutoLock lock(offscreen_context_for_compositor_thread_creation_lock_);
+    base::AutoLock lock(offscreen_context_for_compositor_thread_lock_);
     if (!offscreen_context_for_compositor_thread_.get() ||
         offscreen_context_for_compositor_thread_->DestroyedOnMainThread()) {
       offscreen_context_for_compositor_thread_ =
@@ -170,27 +175,73 @@ class SynchronousCompositorFactoryImpl : public SynchronousCompositorFactory {
 
   virtual scoped_ptr<StreamTextureFactory> CreateStreamTextureFactory(
       int view_id) OVERRIDE {
-    scoped_refptr<VideoContextProvider> context_provider =
-        new VideoContextProvider(offscreen_context_for_main_thread_,
-                                 wrapped_gl_context_for_main_thread_);
+    scoped_refptr<VideoContextProvider> context_provider;
+    if (CanCreateMainThreadContext()) {
+      context_provider =
+          new VideoContextProvider(offscreen_context_for_main_thread_,
+                                   wrapped_gl_context_for_main_thread_);
+    }
     return make_scoped_ptr(new StreamTextureFactorySynchronousImpl(
                                context_provider.get(), view_id))
         .PassAs<StreamTextureFactory>();
   }
 
+  void CompositorInitializedHardwareDraw(SynchronousCompositorImpl* compositor);
+  void CompositorReleasedHardwareDraw(SynchronousCompositorImpl* compositor);
+
  private:
+  void ReleaseGlobalHardwareResources();
+  bool CanCreateMainThreadContext();
+
   SynchronousInputEventFilter synchronous_input_event_filter_;
 
-  // Only guards construction of |offscreen_context_for_compositor_thread_|,
-  // not usage.
-  base::Lock offscreen_context_for_compositor_thread_creation_lock_;
+  // Only guards construction and destruction of
+  // |offscreen_context_for_compositor_thread_|, not usage.
+  base::Lock offscreen_context_for_compositor_thread_lock_;
   scoped_refptr<cc::ContextProvider> offscreen_context_for_main_thread_;
   // This is a pointer to the context owned by
   // |offscreen_context_for_main_thread_|.
   gpu::GLInProcessContext* wrapped_gl_context_for_main_thread_;
   scoped_refptr<cc::ContextProvider> offscreen_context_for_compositor_thread_;
+
+  base::Lock num_hardware_compositor_lock_;

[joth, 2013/09/05 20:47:12]

comment that num_hardware_compositors_ is always updated on the UI thread, but
maybe read on another thread.

(also should have a comment here or in the release function to acknowledge
possible race in reading this while the last compositor is closing)

[boliu, 2013/09/05 23:21:25]

Both done

+  unsigned int num_hardware_compositors_;
 };
 
+void SynchronousCompositorFactoryImpl::CompositorInitializedHardwareDraw(
+    SynchronousCompositorImpl* compositor) {
+  base::AutoLock lock(num_hardware_compositor_lock_);
+  num_hardware_compositors_++;
+}
+
+void SynchronousCompositorFactoryImpl::CompositorReleasedHardwareDraw(
+    SynchronousCompositorImpl* compositor) {
+  bool should_release_resources = false;
+  {
+    base::AutoLock lock(num_hardware_compositor_lock_);
+    DCHECK_GT(num_hardware_compositors_, 0u);
+    num_hardware_compositors_--;
+    should_release_resources = num_hardware_compositors_ == 0u;
+  }
+  if (should_release_resources)
+    ReleaseGlobalHardwareResources();
+}
+
+void SynchronousCompositorFactoryImpl::ReleaseGlobalHardwareResources() {
+  {
+    base::AutoLock lock(offscreen_context_for_compositor_thread_lock_);
+    offscreen_context_for_compositor_thread_ = NULL;
+  }
+
+  // TODO(boliu): Properly clean up command buffer server of main thread
+  // context here.
+}
+
+bool SynchronousCompositorFactoryImpl::CanCreateMainThreadContext() {
+  base::AutoLock lock(num_hardware_compositor_lock_);
+  return num_hardware_compositors_ > 0;
+}
+
 base::LazyInstance<SynchronousCompositorFactoryImpl>::Leaky g_factory =
     LAZY_INSTANCE_INITIALIZER;
 
@@ -241,15 +292,19 @@ bool SynchronousCompositorImpl::InitializeHwDraw(
     scoped_refptr<gfx::GLSurface> surface) {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
-  return output_surface_->InitializeHwDraw(
+  bool success = output_surface_->InitializeHwDraw(
       surface,
       g_factory.Get().GetOffscreenContextProviderForCompositorThread());
+  if (success)
+    g_factory.Get().CompositorInitializedHardwareDraw(this);
+  return success;
 }
 
 void SynchronousCompositorImpl::ReleaseHwDraw() {
   DCHECK(CalledOnValidThread());
   DCHECK(output_surface_);
-  return output_surface_->ReleaseHwDraw();
+  output_surface_->ReleaseHwDraw();
+  g_factory.Get().CompositorReleasedHardwareDraw(this);
 }
 
 bool SynchronousCompositorImpl::DemandDrawHw(