content: Cleanup surface reference passing using Java IPC.

This moves the surface/surface-texture id lookup code into Java so
we don't have to enter native land when responding to related IPC
messages.

BUG=269808

Review URL: https://codereview.chromium.org/226103004

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

5 files changed, 107 insertions, 106 deletions

diff --git a/content/browser/android/child_process_launcher_android.cc b/content/browser/android/child_process_launcher_android.cc
index ca99a78..6c4c6af 100644
--- a/content/browser/android/child_process_launcher_android.cc
+++ b/content/browser/android/child_process_launcher_android.cc
@@ -153,24 +153,33 @@ void EstablishSurfacePeer(
       &SetSurfacePeer, jsurface, pid, primary_id, secondary_id));
 }
 
-jobject GetViewSurface(JNIEnv* env, jclass clazz, jint surface_id) {
-  // This is a synchronous call from the GPU process and is expected to be
-  // handled on a binder thread. Handling this on the UI thread will lead
-  // to deadlocks.
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
-  return CompositorImpl::GetSurface(surface_id).Release();
+void RegisterViewSurface(int surface_id, jobject j_surface) {
+  JNIEnv* env = AttachCurrentThread();
+  DCHECK(env);
+  Java_ChildProcessLauncher_registerViewSurface(env, surface_id, j_surface);
 }
 
-jobject GetSurfaceTextureSurface(JNIEnv* env,
-                                 jclass clazz,
-                                 jint surface_texture_id,
-                                 jint child_process_id) {
-  // This is a synchronous call from a renderer process and is expected to be
-  // handled on a binder thread. Handling this on the UI thread will lead
-  // to deadlocks.
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::UI));
-  return CompositorImpl::GetSurfaceTextureSurface(surface_texture_id,
-                                                  child_process_id).Release();
+void UnregisterViewSurface(int surface_id) {
+  JNIEnv* env = AttachCurrentThread();
+  DCHECK(env);
+  Java_ChildProcessLauncher_unregisterViewSurface(env, surface_id);
+}
+
+void RegisterChildProcessSurfaceTexture(int surface_texture_id,
+                                        int child_process_id,
+                                        jobject j_surface_texture) {
+  JNIEnv* env = AttachCurrentThread();
+  DCHECK(env);
+  Java_ChildProcessLauncher_registerSurfaceTexture(
+      env, surface_texture_id, child_process_id, j_surface_texture);
+}
+
+void UnregisterChildProcessSurfaceTexture(int surface_texture_id,
+                                          int child_process_id) {
+  JNIEnv* env = AttachCurrentThread();
+  DCHECK(env);
+  Java_ChildProcessLauncher_unregisterSurfaceTexture(
+      env, surface_texture_id, child_process_id);
 }
 
 jboolean IsSingleProcess(JNIEnv* env, jclass clazz) {
diff --git a/content/browser/android/child_process_launcher_android.h b/content/browser/android/child_process_launcher_android.h
index 452b9b6..1d48656 100644
--- a/content/browser/android/child_process_launcher_android.h
+++ b/content/browser/android/child_process_launcher_android.h
@@ -32,6 +32,17 @@ void StopChildProcess(base::ProcessHandle handle);
 
 bool IsChildProcessOomProtected(base::ProcessHandle handle);
 
+void RegisterViewSurface(int surface_id, jobject j_surface);
+
+void UnregisterViewSurface(int surface_id);
+
+void RegisterChildProcessSurfaceTexture(int surface_texture_id,
+                                        int child_process_id,
+                                        jobject j_surface_texture);
+
+void UnregisterChildProcessSurfaceTexture(int surface_texture_id,
+                                          int child_process_id);
+
 bool RegisterChildProcessLauncher(JNIEnv* env);
 
 }  // namespace content
diff --git a/content/browser/renderer_host/compositor_impl_android.cc b/content/browser/renderer_host/compositor_impl_android.cc
index db02a7c..dec2ff3 100644
--- a/content/browser/renderer_host/compositor_impl_android.cc
+++ b/content/browser/renderer_host/compositor_impl_android.cc
@@ -6,13 +6,12 @@
 
 #include <android/bitmap.h>
 #include <android/native_window_jni.h>
-#include <map>
 
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
 #include "base/command_line.h"
-#include "base/containers/scoped_ptr_hash_map.h"
+#include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
@@ -28,6 +27,7 @@
 #include "cc/resources/scoped_ui_resource.h"
 #include "cc/resources/ui_resource_bitmap.h"
 #include "cc/trees/layer_tree_host.h"
+#include "content/browser/android/child_process_launcher_android.h"
 #include "content/browser/gpu/browser_gpu_channel_host_factory.h"
 #include "content/browser/gpu/gpu_surface_tracker.h"
 #include "content/common/gpu/client/command_buffer_proxy_impl.h"
@@ -46,7 +46,6 @@
 #include "ui/gfx/android/device_display_info.h"
 #include "ui/gfx/android/java_bitmap.h"
 #include "ui/gfx/frame_time.h"
-#include "ui/gl/android/scoped_java_surface.h"
 #include "ui/gl/android/surface_texture.h"
 #include "ui/gl/android/surface_texture_tracker.h"
 #include "webkit/common/gpu/context_provider_in_process.h"
@@ -148,9 +147,13 @@ class SurfaceTextureTrackerImpl : public gfx::SurfaceTextureTracker {
       int primary_id,
       int secondary_id) OVERRIDE {
     base::AutoLock lock(surface_textures_lock_);
-    scoped_ptr<SurfaceTextureInfo> info = surface_textures_.take_and_erase(
-        SurfaceTextureMapKey(primary_id, secondary_id));
-    return info ? info->surface_texture : NULL;
+    SurfaceTextureMapKey key(primary_id, secondary_id);
+    SurfaceTextureMap::iterator it = surface_textures_.find(key);
+    if (it == surface_textures_.end())
+      return scoped_refptr<gfx::SurfaceTexture>();
+    scoped_refptr<gfx::SurfaceTexture> surface_texture = it->second;
+    surface_textures_.erase(it);
+    return surface_texture;
   }
 
   int AddSurfaceTexture(gfx::SurfaceTexture* surface_texture,
@@ -163,8 +166,11 @@ class SurfaceTextureTrackerImpl : public gfx::SurfaceTextureTracker {
     base::AutoLock lock(surface_textures_lock_);
     SurfaceTextureMapKey key(surface_texture_id, child_process_id);
     DCHECK(surface_textures_.find(key) == surface_textures_.end());
-    surface_textures_.set(
-        key, make_scoped_ptr(new SurfaceTextureInfo(surface_texture)));
+    surface_textures_[key] = surface_texture;
+    content::RegisterChildProcessSurfaceTexture(
+        surface_texture_id,
+        child_process_id,
+        surface_texture->j_surface_texture().obj());
     return surface_texture_id;
   }
 
@@ -173,36 +179,20 @@ class SurfaceTextureTrackerImpl : public gfx::SurfaceTextureTracker {
     base::AutoLock lock(surface_textures_lock_);
     SurfaceTextureMap::iterator it = surface_textures_.begin();
     while (it != surface_textures_.end()) {
-      if (it->first.second == child_process_id)
+      if (it->first.second == child_process_id) {
+        content::UnregisterChildProcessSurfaceTexture(it->first.first,
+                                                      it->first.second);
         surface_textures_.erase(it++);
-      else
+      } else {
         ++it;
+      }
     }
   }
 
-  base::android::ScopedJavaLocalRef<jobject> GetSurface(
-      int surface_texture_id,
-      int child_process_id) const {
-    base::AutoLock lock(surface_textures_lock_);
-    SurfaceTextureMap::const_iterator it = surface_textures_.find(
-        SurfaceTextureMapKey(surface_texture_id, child_process_id));
-    return it == surface_textures_.end()
-               ? base::android::ScopedJavaLocalRef<jobject>()
-               : base::android::ScopedJavaLocalRef<jobject>(
-                     it->second->surface.j_surface());
-  }
-
  private:
-  struct SurfaceTextureInfo {
-    explicit SurfaceTextureInfo(gfx::SurfaceTexture* surface_texture)
-        : surface_texture(surface_texture), surface(surface_texture) {}
-
-    scoped_refptr<gfx::SurfaceTexture> surface_texture;
-    gfx::ScopedJavaSurface surface;
-  };
-
   typedef std::pair<int, int> SurfaceTextureMapKey;
-  typedef base::ScopedPtrHashMap<SurfaceTextureMapKey, SurfaceTextureInfo>
+  typedef base::hash_map<SurfaceTextureMapKey,
+                         scoped_refptr<gfx::SurfaceTexture> >
       SurfaceTextureMap;
   SurfaceTextureMap surface_textures_;
   mutable base::Lock surface_textures_lock_;
@@ -218,12 +208,6 @@ static bool g_initialized = false;
 
 namespace content {
 
-typedef std::map<int, base::android::ScopedJavaGlobalRef<jobject> >
-    SurfaceMap;
-static base::LazyInstance<SurfaceMap>
-    g_surface_map = LAZY_INSTANCE_INITIALIZER;
-static base::LazyInstance<base::Lock> g_surface_map_lock;
-
 // static
 Compositor* Compositor::Create(CompositorClient* client,
                                gfx::NativeWindow root_window) {
@@ -245,35 +229,6 @@ bool CompositorImpl::IsInitialized() {
 }
 
 // static
-base::android::ScopedJavaLocalRef<jobject> CompositorImpl::GetSurface(
-    int surface_id) {
-  base::AutoLock lock(g_surface_map_lock.Get());
-  SurfaceMap* surfaces = g_surface_map.Pointer();
-  SurfaceMap::iterator it = surfaces->find(surface_id);
-  base::android::ScopedJavaLocalRef<jobject> jsurface(
-      it == surfaces->end()
-          ? base::android::ScopedJavaLocalRef<jobject>()
-          : base::android::ScopedJavaLocalRef<jobject>(it->second));
-
-  LOG_IF(WARNING, !jsurface.is_null()) << "No surface for surface id "
-                                       << surface_id;
-  return jsurface;
-}
-
-// static
-base::android::ScopedJavaLocalRef<jobject>
-CompositorImpl::GetSurfaceTextureSurface(int surface_texture_id,
-                                         int child_process_id) {
-  base::android::ScopedJavaLocalRef<jobject> jsurface(
-      g_surface_texture_tracker.Pointer()->GetSurface(surface_texture_id,
-                                                      child_process_id));
-
-  LOG_IF(WARNING, jsurface.is_null()) << "No surface for surface texture id "
-                                      << surface_texture_id;
-  return jsurface;
-}
-
-// static
 int CompositorImpl::CreateSurfaceTexture(int child_process_id) {
   // Note: this needs to be 0 as the surface texture implemenation will take
   // ownership of the texture and call glDeleteTextures when the GPU service
@@ -351,12 +306,8 @@ void CompositorImpl::SetSurface(jobject surface) {
   base::android::ScopedJavaLocalRef<jobject> j_surface(env, surface);
 
   // First, cleanup any existing surface references.
-  if (surface_id_) {
-    DCHECK(g_surface_map.Get().find(surface_id_) !=
-           g_surface_map.Get().end());
-    base::AutoLock lock(g_surface_map_lock.Get());
-    g_surface_map.Get().erase(surface_id_);
-  }
+  if (surface_id_)
+    content::UnregisterViewSurface(surface_id_);
   SetWindowSurface(NULL);
 
   // Now, set the new surface if we have one.
@@ -366,10 +317,7 @@ void CompositorImpl::SetSurface(jobject surface) {
   if (window) {
     SetWindowSurface(window);
     ANativeWindow_release(window);
-    {
-      base::AutoLock lock(g_surface_map_lock.Get());
-      g_surface_map.Get().insert(std::make_pair(surface_id_, j_surface));
-    }
+    content::RegisterViewSurface(surface_id_, j_surface.obj());
   }
 }
 
diff --git a/content/browser/renderer_host/compositor_impl_android.h b/content/browser/renderer_host/compositor_impl_android.h
index 56292ef..b825a31 100644
--- a/content/browser/renderer_host/compositor_impl_android.h
+++ b/content/browser/renderer_host/compositor_impl_android.h
@@ -5,7 +5,6 @@
 #ifndef CONTENT_BROWSER_RENDERER_HOST_COMPOSITOR_IMPL_ANDROID_H_
 #define CONTENT_BROWSER_RENDERER_HOST_COMPOSITOR_IMPL_ANDROID_H_
 
-#include "base/android/scoped_java_ref.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/containers/scoped_ptr_hash_map.h"
@@ -47,14 +46,6 @@ class CONTENT_EXPORT CompositorImpl
 
   static bool IsInitialized();
 
-  // Returns the Java Surface object for a given view surface id.
-  static base::android::ScopedJavaLocalRef<jobject> GetSurface(int surface_id);
-
-  // Returns the Java Surface object for a given surface texture id.
-  static base::android::ScopedJavaLocalRef<jobject> GetSurfaceTextureSurface(
-      int surface_texture_id,
-      int child_process_id);
-
   // Creates a surface texture and returns a surface texture id. Returns -1 on
   // failure.
   static int CreateSurfaceTexture(int child_process_id);
diff --git a/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java b/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
index d88305c..613d12d 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java
@@ -5,7 +5,9 @@
 package org.chromium.content.browser;
 
 import android.content.Context;
+import android.graphics.SurfaceTexture;
 import android.util.Log;
+import android.util.Pair;
 import android.view.Surface;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -214,6 +216,14 @@ public class ChildProcessLauncher {
     // Manages oom bindings used to bind chind services.
     private static BindingManager sBindingManager = BindingManagerImpl.createBindingManager();
 
+    // Map from surface id to Surface.
+    private static Map<Integer, Surface> sViewSurfaceMap =
+            new ConcurrentHashMap<Integer, Surface>();
+
+    // Map from surface texture id to Surface.
+    private static Map<Pair<Integer, Integer>, Surface> sSurfaceTextureSurfaceMap =
+            new ConcurrentHashMap<Pair<Integer, Integer>, Surface>();
+
     static BindingManager getBindingManager() {
         return sBindingManager;
     }
@@ -228,6 +238,29 @@ public class ChildProcessLauncher {
         return sBindingManager.isOomProtected(pid);
     }
 
+    @CalledByNative
+    private static void registerViewSurface(int surfaceId, Surface surface) {
+        sViewSurfaceMap.put(surfaceId, surface);
+    }
+
+    @CalledByNative
+    private static void unregisterViewSurface(int surfaceId) {
+        sViewSurfaceMap.remove(surfaceId);
+    }
+
+    @CalledByNative
+    private static void registerSurfaceTexture(
+            int surfaceTextureId, int childProcessId, SurfaceTexture surfaceTexture) {
+        Pair<Integer, Integer> key = new Pair<Integer, Integer>(surfaceTextureId, childProcessId);
+        sSurfaceTextureSurfaceMap.put(key, new Surface(surfaceTexture));
+    }
+
+    @CalledByNative
+    private static void unregisterSurfaceTexture(int surfaceTextureId, int childProcessId) {
+        Pair<Integer, Integer> key = new Pair<Integer, Integer>(surfaceTextureId, childProcessId);
+        sSurfaceTextureSurfaceMap.remove(key);
+    }
+
     /**
      * Called when the embedding application is sent to background.
      */
@@ -423,7 +456,12 @@ public class ChildProcessLauncher {
                     return null;
                 }
 
-                return nativeGetViewSurface(surfaceId);
+                Surface surface = sViewSurfaceMap.get(surfaceId);
+                if (surface == null) {
+                    Log.e(TAG, "Invalid surfaceId.");
+                    return null;
+                }
+                return surface;
             }
 
             @Override
@@ -438,7 +476,14 @@ public class ChildProcessLauncher {
                     return null;
                 }
 
-                return nativeGetSurfaceTextureSurface(primaryId, secondaryId);
+                Pair<Integer, Integer> key = new Pair<Integer, Integer>(primaryId, secondaryId);
+                // Note: This removes the surface and passes the ownership to the caller.
+                Surface surface = sSurfaceTextureSurfaceMap.remove(key);
+                if (surface == null) {
+                    Log.e(TAG, "Invalid Id for surface texture.");
+                    return null;
+                }
+                return surface;
             }
         };
     }
@@ -451,9 +496,6 @@ public class ChildProcessLauncher {
     }
 
     private static native void nativeOnChildProcessStarted(long clientContext, int pid);
-    private static native Surface nativeGetViewSurface(int surfaceId);
-    private static native Surface nativeGetSurfaceTextureSurface(
-            int surfaceTextureId, int childProcessId);
     private static native void nativeEstablishSurfacePeer(
             int pid, Surface surface, int primaryID, int secondaryID);
     private static native boolean nativeIsSingleProcess();