diff options
author | reveman@chromium.org <reveman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-06 04:29:01 +0000 |
---|---|---|
committer | reveman@chromium.org <reveman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-06 04:29:01 +0000 |
commit | 354c448d265435890a34f364562be49315f5af1c (patch) | |
tree | a4e35233ce56a0f64b206fb1a812dcae166b42dc | |
parent | 0485cf05e88915e1b175a02d60b54e5c1af31a44 (diff) | |
download | chromium_src-354c448d265435890a34f364562be49315f5af1c.zip chromium_src-354c448d265435890a34f364562be49315f5af1c.tar.gz chromium_src-354c448d265435890a34f364562be49315f5af1c.tar.bz2 |
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(); |