Patch refactors the GpuProcessHost::SurfaceRef class that was previously only used on GTK to ensure that browser process drawing surface wasn't torn down until the GPU process was done with it so that it can be used on Android too.

[Android notes...]
Ensure that we don't release the Java side resources used by SurfaceTextureBridge until the gpu process is completely finished with the GL surface. Otherwise we end up with a race
between the RenderView shutting down and releasing the Java Surface, yet the GPU process continuing to use it.
    
Bug: b/7697692
Bug: 119006

Review URL: https://chromiumcodereview.appspot.com/12989029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193907 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 132 insertions, 83 deletions

diff --git a/content/browser/gpu/gpu_process_host.cc b/content/browser/gpu/gpu_process_host.cc
index 49de984..2d2a5e2 100644
--- a/content/browser/gpu/gpu_process_host.cc
+++ b/content/browser/gpu/gpu_process_host.cc
@@ -19,7 +19,6 @@
 #include "content/browser/browser_child_process_host_impl.h"
 #include "content/browser/gpu/gpu_data_manager_impl.h"
 #include "content/browser/gpu/gpu_process_host_ui_shim.h"
-#include "content/browser/gpu/gpu_surface_tracker.h"
 #include "content/browser/gpu/shader_disk_cache.h"
 #include "content/browser/renderer_host/render_widget_helper.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
@@ -42,9 +41,6 @@
 #include "ipc/ipc_switches.h"
 #include "ui/gl/gl_switches.h"
 
-#if defined(TOOLKIT_GTK)
-#include "ui/gfx/gtk_native_view_id_manager.h"
-#endif
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
@@ -74,14 +70,6 @@ enum GPUProcessLifetimeEvent {
 // only be accessed from the IO thread.
 GpuProcessHost* g_gpu_process_hosts[GpuProcessHost::GPU_PROCESS_KIND_COUNT];
 
-#if defined(TOOLKIT_GTK)
-
-void ReleasePermanentXIDDispatcher(gfx::PluginWindowHandle surface) {
-  GtkNativeViewManager* manager = GtkNativeViewManager::GetInstance();
-  manager->ReleasePermanentXID(surface);
-}
-
-#endif
 
 void SendGpuProcessMessage(GpuProcessHost::GpuProcessKind kind,
                            CauseForGpuLaunch cause,
@@ -290,33 +278,6 @@ class GpuSandboxedProcessLauncherDelegate
 
 }  // anonymous namespace
 
-#if defined(TOOLKIT_GTK)
-// Used to put a lock on surfaces so that the window to which the GPU
-// process is drawing to doesn't disappear while it is drawing when
-// a WebContents is closed.
-class GpuProcessHost::SurfaceRef {
- public:
-  explicit SurfaceRef(gfx::PluginWindowHandle surface);
-  ~SurfaceRef();
- private:
-  gfx::PluginWindowHandle surface_;
-};
-
-GpuProcessHost::SurfaceRef::SurfaceRef(gfx::PluginWindowHandle surface)
-    : surface_(surface) {
-  GtkNativeViewManager* manager = GtkNativeViewManager::GetInstance();
-  if (!manager->AddRefPermanentXID(surface_)) {
-    LOG(ERROR) << "Surface " << surface << " cannot be referenced.";
-  }
-}
-
-GpuProcessHost::SurfaceRef::~SurfaceRef() {
-  BrowserThread::PostTask(BrowserThread::UI,
-                          FROM_HERE,
-                          base::Bind(&ReleasePermanentXIDDispatcher, surface_));
-}
-#endif  // defined(TOOLKIT_GTK)
-
 // This class creates a GPU thread (instead of a GPU process), when running
 // with --in-process-gpu or --single-process.
 class GpuMainThread : public base::Thread {
@@ -772,26 +733,12 @@ void GpuProcessHost::CreateViewCommandBuffer(
 
   DCHECK(CalledOnValidThread());
 
-#if defined(TOOLKIT_GTK)
-  // There should only be one such command buffer (for the compositor).  In
-  // practice, if the GPU process lost a context, GraphicsContext3D with
-  // associated command buffer and view surface will not be gone until new
-  // one is in place and all layers are reattached.
-  linked_ptr<SurfaceRef> surface_ref;
-  SurfaceRefMap::iterator it = surface_refs_.find(surface_id);
-  if (it != surface_refs_.end())
-    surface_ref = (*it).second;
-  else
-    surface_ref.reset(new SurfaceRef(compositing_surface.handle));
-#endif  // defined(TOOLKIT_GTK)
-
   if (!compositing_surface.is_null() &&
       Send(new GpuMsg_CreateViewCommandBuffer(
           compositing_surface, surface_id, client_id, init_params))) {
     create_command_buffer_requests_.push(callback);
-#if defined(TOOLKIT_GTK)
-    surface_refs_.insert(std::make_pair(surface_id, surface_ref));
-#endif
+    surface_refs_.insert(std::make_pair(surface_id,
+        GpuSurfaceTracker::GetInstance()->GetSurfaceRefForSurface(surface_id)));
   } else {
     callback.Run(MSG_ROUTING_NONE);
   }
@@ -865,12 +812,10 @@ void GpuProcessHost::OnCommandBufferCreated(const int32 route_id) {
 
 void GpuProcessHost::OnDestroyCommandBuffer(int32 surface_id) {
   TRACE_EVENT0("gpu", "GpuProcessHost::OnDestroyCommandBuffer");
-
-#if defined(TOOLKIT_GTK)
   SurfaceRefMap::iterator it = surface_refs_.find(surface_id);
-  if (it != surface_refs_.end())
+  if (it != surface_refs_.end()) {
     surface_refs_.erase(it);
-#endif  // defined(TOOLKIT_GTK)
+  }
 }
 
 void GpuProcessHost::OnImageCreated(const gfx::Size size) {
diff --git a/content/browser/gpu/gpu_process_host.h b/content/browser/gpu/gpu_process_host.h
index 405918f6..4d99c9e 100644
--- a/content/browser/gpu/gpu_process_host.h
+++ b/content/browser/gpu/gpu_process_host.h
@@ -12,10 +12,10 @@
 
 #include "base/callback.h"
 #include "base/hash_tables.h"
-#include "base/memory/linked_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/time.h"
+#include "content/browser/gpu/gpu_surface_tracker.h"
 #include "content/common/content_export.h"
 #include "content/common/gpu/gpu_memory_uma_stats.h"
 #include "content/common/gpu/gpu_process_launch_causes.h"
@@ -202,18 +202,6 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate,
   // The pending create image requests we need to reply to.
   std::queue<CreateImageCallback> create_image_requests_;
 
-#if defined(TOOLKIT_GTK)
-  // Encapsulates surfaces that we lock when creating view command buffers.
-  // We release this lock once the command buffer (or associated GPU process)
-  // is destroyed. This prevents the browser from destroying the surface
-  // while the GPU process is drawing to it.
-
-  // Multimap is used to simulate reference counting, see comment in
-  // GpuProcessHostUIShim::CreateViewCommandBuffer.
-  class SurfaceRef;
-  typedef std::multimap<int, linked_ptr<SurfaceRef> > SurfaceRefMap;
-  SurfaceRefMap surface_refs_;
-#endif
 
   // Qeueud messages to send when the process launches.
   std::queue<IPC::Message*> queued_messages_;
@@ -272,6 +260,13 @@ class GpuProcessHost : public BrowserChildProcessHostDelegate,
 
   std::string shader_prefix_key_;
 
+  // Keep an extra reference to the SurfaceRef stored in the GpuSurfaceTracker
+  // in this map so that we don't destroy it whilst the GPU process is
+  // drawing to it.
+  typedef std::multimap<int, scoped_refptr<GpuSurfaceTracker::SurfaceRef> >
+      SurfaceRefMap;
+  SurfaceRefMap surface_refs_;
+
   DISALLOW_COPY_AND_ASSIGN(GpuProcessHost);
 };
 
diff --git a/content/browser/gpu/gpu_surface_tracker.cc b/content/browser/gpu/gpu_surface_tracker.cc
index 2c4873a..ce1e956 100644
--- a/content/browser/gpu/gpu_surface_tracker.cc
+++ b/content/browser/gpu/gpu_surface_tracker.cc
@@ -10,8 +10,57 @@
 
 #include "base/logging.h"
 
+#if defined(TOOLKIT_GTK)
+#include "base/bind.h"
+#include "content/public/browser/browser_thread.h"
+#include "ui/gfx/gtk_native_view_id_manager.h"
+#endif  // defined(TOOLKIT_GTK)
+
 namespace content {
 
+namespace {
+#if defined(TOOLKIT_GTK)
+
+void ReleasePermanentXIDDispatcher(
+    const gfx::PluginWindowHandle& surface) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  GtkNativeViewManager* manager = GtkNativeViewManager::GetInstance();
+  manager->ReleasePermanentXID(surface);
+}
+
+// Implementation of SurfaceRef that allows GTK to ref and unref the
+// surface with the GtkNativeViewManager.
+class SurfaceRefPluginWindow : public GpuSurfaceTracker::SurfaceRef {
+  public:
+   SurfaceRefPluginWindow(const gfx::PluginWindowHandle& surface_ref);
+  private:
+   virtual ~SurfaceRefPluginWindow();
+   gfx::PluginWindowHandle surface_;
+};
+
+SurfaceRefPluginWindow::SurfaceRefPluginWindow(
+    const gfx::PluginWindowHandle& surface)
+    : surface_(surface) {
+  if (surface_ != gfx::kNullPluginWindow) {
+    GtkNativeViewManager* manager = GtkNativeViewManager::GetInstance();
+    if (!manager->AddRefPermanentXID(surface_)) {
+      LOG(ERROR) << "Surface " << surface << " cannot be referenced.";
+    }
+  }
+}
+
+SurfaceRefPluginWindow::~SurfaceRefPluginWindow() {
+  if (surface_ != gfx::kNullPluginWindow) {
+    BrowserThread::PostTask(BrowserThread::UI,
+                            FROM_HERE,
+                            base::Bind(&ReleasePermanentXIDDispatcher,
+                                       surface_));
+  }
+}
+#endif  // defined(TOOLKIT_GTK)
+}  // anonymous
+
 GpuSurfaceTracker::GpuSurfaceTracker()
     : next_surface_id_(1) {
   GpuSurfaceLookup::InitInstance(this);
@@ -28,13 +77,10 @@ GpuSurfaceTracker* GpuSurfaceTracker::GetInstance() {
 int GpuSurfaceTracker::AddSurfaceForRenderer(int renderer_id,
                                              int render_widget_id) {
   base::AutoLock lock(lock_);
-  SurfaceInfo info = {
-    renderer_id,
-    render_widget_id,
-    gfx::kNullAcceleratedWidget
-  };
   int surface_id = next_surface_id_++;
-  surface_map_[surface_id] = info;
+  surface_map_[surface_id] =
+      SurfaceInfo(renderer_id, render_widget_id, gfx::kNullAcceleratedWidget,
+                  gfx::GLSurfaceHandle(), NULL);
   return surface_id;
 }
 
@@ -55,9 +101,9 @@ int GpuSurfaceTracker::LookupSurfaceForRenderer(int renderer_id,
 int GpuSurfaceTracker::AddSurfaceForNativeWidget(
     gfx::AcceleratedWidget widget) {
   base::AutoLock lock(lock_);
-  SurfaceInfo info = { 0, 0, widget };
   int surface_id = next_surface_id_++;
-  surface_map_[surface_id] = info;
+  surface_map_[surface_id] =
+      SurfaceInfo(0, 0, widget, gfx::GLSurfaceHandle(), NULL);
   return surface_id;
 }
 
@@ -86,6 +132,9 @@ void GpuSurfaceTracker::SetSurfaceHandle(int surface_id,
   DCHECK(surface_map_.find(surface_id) != surface_map_.end());
   SurfaceInfo& info = surface_map_[surface_id];
   info.handle = handle;
+#if defined(TOOLKIT_GTK)
+  info.surface_ref = new SurfaceRefPluginWindow(handle.handle);
+#endif  // defined(TOOLKIT_GTK)
 }
 
 gfx::GLSurfaceHandle GpuSurfaceTracker::GetSurfaceHandle(int surface_id) {
@@ -111,11 +160,14 @@ gfx::AcceleratedWidget GpuSurfaceTracker::AcquireNativeWidget(int surface_id) {
 }
 
 void GpuSurfaceTracker::SetNativeWidget(
-    int surface_id, gfx::AcceleratedWidget widget) {
+    int surface_id, gfx::AcceleratedWidget widget,
+    SurfaceRef* surface_ref) {
   base::AutoLock lock(lock_);
   SurfaceMap::iterator it = surface_map_.find(surface_id);
   DCHECK(it != surface_map_.end());
-  it->second.native_widget = widget;
+  SurfaceInfo& info = it->second;
+  info.native_widget = widget;
+  info.surface_ref = surface_ref;
 }
 
 std::size_t GpuSurfaceTracker::GetSurfaceCount() {
@@ -123,4 +175,24 @@ std::size_t GpuSurfaceTracker::GetSurfaceCount() {
   return surface_map_.size();
 }
 
+GpuSurfaceTracker::SurfaceInfo::SurfaceInfo()
+   : renderer_id(0),
+     render_widget_id(0),
+     native_widget(gfx::kNullAcceleratedWidget) { }
+
+GpuSurfaceTracker::SurfaceInfo::SurfaceInfo(
+    int renderer_id,
+    int render_widget_id,
+    const gfx::AcceleratedWidget& native_widget,
+    const gfx::GLSurfaceHandle& handle,
+    const scoped_refptr<SurfaceRef>& surface_ref)
+    : renderer_id(renderer_id),
+      render_widget_id(render_widget_id),
+      native_widget(native_widget),
+      handle(handle),
+      surface_ref(surface_ref) { }
+
+GpuSurfaceTracker::SurfaceInfo::~SurfaceInfo() { }
+
+
 }  // namespace content
diff --git a/content/browser/gpu/gpu_surface_tracker.h b/content/browser/gpu/gpu_surface_tracker.h
index ddf0a66..de9d666 100644
--- a/content/browser/gpu/gpu_surface_tracker.h
+++ b/content/browser/gpu/gpu_surface_tracker.h
@@ -8,6 +8,7 @@
 #include <map>
 
 #include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/singleton.h"
 #include "base/synchronization/lock.h"
 #include "content/common/gpu/gpu_surface_lookup.h"
@@ -27,6 +28,27 @@ namespace content {
 // it is unamibiguously identified.
 class GpuSurfaceTracker : public GpuSurfaceLookup {
  public:
+  // Base class for reference counting surfaces. We store a
+  // reference to an instance of this class in the surface_map_
+  // and GpuProcessHost (if the GPU process is drawing to
+  // the surface with a Command Buffer). The reference count ensures that
+  // we don't destroy the object until it's released from both places.
+  //
+  // This is especially important on Android and GTK where the surface must
+  // not be destroyed when the WebContents is closed if the GPU is still
+  // drawing to it. Those platforms extend this class with the functionality
+  // they need to implement on tear down (see SurfaceRefPluginWindow for GTK and
+  // SurfaceRefAndroid for Android).
+  class SurfaceRef : public base::RefCountedThreadSafe<SurfaceRef> {
+   protected:
+    SurfaceRef() { }
+    virtual ~SurfaceRef() { }
+
+   private:
+    friend class base::RefCountedThreadSafe<SurfaceRef>;
+    DISALLOW_COPY_AND_ASSIGN(SurfaceRef);
+  };
+
   // GpuSurfaceLookup implementation:
   // Returns the native widget associated with a given surface_id.
   virtual gfx::AcceleratedWidget AcquireNativeWidget(int surface_id) OVERRIDE;
@@ -62,7 +84,10 @@ class GpuSurfaceTracker : public GpuSurfaceLookup {
   void SetSurfaceHandle(int surface_id, const gfx::GLSurfaceHandle& handle);
 
   // Sets the native widget associated with the surface_id.
-  void SetNativeWidget(int surface_id, gfx::AcceleratedWidget widget);
+  void SetNativeWidget(
+      int surface_id,
+      gfx::AcceleratedWidget widget,
+      SurfaceRef* surface_ref);
 
   // Gets the native handle for the given surface.
   // Note: This is an O(log N) lookup.
@@ -75,12 +100,24 @@ class GpuSurfaceTracker : public GpuSurfaceLookup {
   // named that way for the implementation of Singleton.
   static GpuSurfaceTracker* GetInstance();
 
+  scoped_refptr<SurfaceRef> GetSurfaceRefForSurface(int surface_id) {
+    return surface_map_[surface_id].surface_ref;
+  }
+
  private:
   struct SurfaceInfo {
+    SurfaceInfo();
+    SurfaceInfo(int renderer_id,
+                int render_widget_id,
+                const gfx::AcceleratedWidget& native_widget,
+                const gfx::GLSurfaceHandle& handle,
+                const scoped_refptr<SurfaceRef>& surface_ref);
+    ~SurfaceInfo();
     int renderer_id;
     int render_widget_id;
     gfx::AcceleratedWidget native_widget;
     gfx::GLSurfaceHandle handle;
+    scoped_refptr<SurfaceRef> surface_ref;
   };
   typedef std::map<int, SurfaceInfo> SurfaceMap;