Fix a race condition that SurfaceTextureBridge can get released before passing it to the mediaplayer

Use a scoped_refptr to protect SurfaceTextureBridge from being released when DestroyStreamTextureChromium and SetVideoSurface() get called at the same time on GPU and UI thread.


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

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

7 files changed, 55 insertions, 46 deletions

diff --git a/content/app/android/sandboxed_process_service.cc b/content/app/android/sandboxed_process_service.cc
index eb3c299..2b3b2006 100644
--- a/content/app/android/sandboxed_process_service.cc
+++ b/content/app/android/sandboxed_process_service.cc
@@ -30,14 +30,17 @@ class SurfaceTexturePeerSandboxedImpl : public content::SurfaceTexturePeer {
   virtual ~SurfaceTexturePeerSandboxedImpl() {
   }
 
-  virtual void EstablishSurfaceTexturePeer(base::ProcessHandle pid,
-                                           SurfaceTextureTarget type,
-                                           jobject j_surface_texture,
-                                           int primary_id,
-                                           int secondary_id) {
+  virtual void EstablishSurfaceTexturePeer(
+      base::ProcessHandle pid,
+      SurfaceTextureTarget type,
+      scoped_refptr<content::SurfaceTextureBridge> surface_texture_bridge,
+      int primary_id,
+      int secondary_id) {
     JNIEnv* env = base::android::AttachCurrentThread();
     content::Java_SandboxedProcessService_establishSurfaceTexturePeer(
-        env, service_, pid, type, j_surface_texture, primary_id, secondary_id);
+        env, service_, pid, type,
+        surface_texture_bridge->j_surface_texture().obj(), primary_id,
+        secondary_id);
     CheckException(env);
   }
 
diff --git a/content/browser/android/surface_texture_peer_browser_impl.cc b/content/browser/android/surface_texture_peer_browser_impl.cc
index e1d1884..4a412592 100644
--- a/content/browser/android/surface_texture_peer_browser_impl.cc
+++ b/content/browser/android/surface_texture_peer_browser_impl.cc
@@ -16,10 +16,11 @@ namespace content {
 
 // Pass a java surface object to the MediaPlayerBridge object
 // identified by render process handle, render view ID and player ID.
-static void SetSurfacePeer(jobject j_surface,
-                           base::ProcessHandle render_process_handle,
-                           int render_view_id,
-                           int player_id) {
+static void SetSurfacePeer(
+    scoped_refptr<SurfaceTextureBridge> surface_texture_bridge,
+    base::ProcessHandle render_process_handle,
+    int render_view_id,
+    int player_id) {
   int renderer_id = 0;
   RenderProcessHost::iterator it = RenderProcessHost::AllHostsIterator();
   while (!it.IsAtEnd()) {
@@ -40,13 +41,18 @@ static void SetSurfacePeer(jobject j_surface,
           host->media_player_manager()->GetPlayer(player_id);
       if (player &&
           player != host->media_player_manager()->GetFullscreenPlayer()) {
-        player->SetVideoSurface(j_surface);
+        base::android::ScopedJavaLocalRef<jclass> cls(
+            base::android::GetClass(env, "android/view/Surface"));
+        jmethodID constructor = GetMethodID(env, cls, "<init>",
+            "(Landroid/graphics/SurfaceTexture;)V");
+        ScopedJavaLocalRef<jobject> j_surface(env,
+            env->NewObject(cls.obj(), constructor,
+                           surface_texture_bridge->j_surface_texture().obj()));
+        player->SetVideoSurface(j_surface.obj());
+        ReleaseSurface(j_surface.obj());
       }
     }
   }
-
-  ReleaseSurface(j_surface);
-  env->DeleteGlobalRef(j_surface);
 }
 
 SurfaceTexturePeerBrowserImpl::SurfaceTexturePeerBrowserImpl(
@@ -62,30 +68,23 @@ SurfaceTexturePeerBrowserImpl::~SurfaceTexturePeerBrowserImpl() {
 void SurfaceTexturePeerBrowserImpl::EstablishSurfaceTexturePeer(
     base::ProcessHandle render_process_handle,
     SurfaceTextureTarget type,
-    jobject j_surface_texture,
+    scoped_refptr<SurfaceTextureBridge> surface_texture_bridge,
     int render_view_id,
     int player_id) {
-  if (j_surface_texture == NULL)
+  if (!surface_texture_bridge)
     return;
 
   JNIEnv* env = base::android::AttachCurrentThread();
   DCHECK(env);
   if (player_in_render_process_) {
     Java_BrowserProcessSurfaceTexture_establishSurfaceTexturePeer(
-        env, render_process_handle, type, j_surface_texture,
+        env, render_process_handle, type,
+        surface_texture_bridge->j_surface_texture().obj(),
         render_view_id, player_id);
   } else {
-    base::android::ScopedJavaLocalRef<jclass> cls(
-        base::android::GetClass(env, "android/view/Surface"));
-    jmethodID constructor = GetMethodID(env, cls, "<init>",
-        "(Landroid/graphics/SurfaceTexture;)V");
-    ScopedJavaLocalRef<jobject> tmp(env,
-        env->NewObject(cls.obj(), constructor, j_surface_texture));
-    jobject surface = env->NewGlobalRef(tmp.obj());
-
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
-        &SetSurfacePeer, surface, render_process_handle, render_view_id,
-        player_id));
+        &SetSurfacePeer, surface_texture_bridge, render_process_handle,
+        render_view_id, player_id));
   }
 }
 
diff --git a/content/browser/android/surface_texture_peer_browser_impl.h b/content/browser/android/surface_texture_peer_browser_impl.h
index d2685f8..9dd1c59 100644
--- a/content/browser/android/surface_texture_peer_browser_impl.h
+++ b/content/browser/android/surface_texture_peer_browser_impl.h
@@ -26,7 +26,7 @@ class SurfaceTexturePeerBrowserImpl : public SurfaceTexturePeer {
   virtual void EstablishSurfaceTexturePeer(
       base::ProcessHandle render_process_handle,
       SurfaceTextureTarget type,
-      jobject j_surface_texture,
+      scoped_refptr<SurfaceTextureBridge> surface_texture_bridge,
       int render_view_id,
       int player_id) OVERRIDE;
 
diff --git a/content/common/android/surface_texture_bridge.h b/content/common/android/surface_texture_bridge.h
index 5024234..97e8ef6 100644
--- a/content/common/android/surface_texture_bridge.h
+++ b/content/common/android/surface_texture_bridge.h
@@ -9,15 +9,16 @@
 
 #include "base/android/scoped_java_ref.h"
 #include "base/callback.h"
+#include "base/memory/ref_counted.h"
 
 namespace content {
 
 // This class serves as a bridge for native code to call java functions inside
 // android SurfaceTexture class.
-class SurfaceTextureBridge {
+class SurfaceTextureBridge
+    : public base::RefCountedThreadSafe<SurfaceTextureBridge>{
  public:
   explicit SurfaceTextureBridge(int texture_id);
-  ~SurfaceTextureBridge();
 
   // Set the listener callback, which will be invoked on the same thread that
   // is being called from here for registration.
@@ -45,6 +46,9 @@ class SurfaceTextureBridge {
   }
 
  private:
+  friend class base::RefCountedThreadSafe<SurfaceTextureBridge>;
+  ~SurfaceTextureBridge();
+
   const int texture_id_;
 
   // Java SurfaceTexture instance.
diff --git a/content/common/android/surface_texture_peer.h b/content/common/android/surface_texture_peer.h
index 0b079f0..4760127 100644
--- a/content/common/android/surface_texture_peer.h
+++ b/content/common/android/surface_texture_peer.h
@@ -5,9 +5,8 @@
 #ifndef CONTENT_COMMON_ANDROID_SURFACE_TEXTURE_PEER_H_
 #define CONTENT_COMMON_ANDROID_SURFACE_TEXTURE_PEER_H_
 
-#include <jni.h>
-
 #include "base/process.h"
+#include "content/common/android/surface_texture_bridge.h"
 
 namespace content {
 
@@ -26,11 +25,12 @@ class SurfaceTexturePeer {
 
   // Establish the producer end for the given surface texture in another
   // process.
-  virtual void EstablishSurfaceTexturePeer(base::ProcessHandle pid,
-                                           SurfaceTextureTarget type,
-                                           jobject j_surface_texture,
-                                           int primary_id,
-                                           int secondary_id) = 0;
+  virtual void EstablishSurfaceTexturePeer(
+      base::ProcessHandle pid,
+      SurfaceTextureTarget type,
+      scoped_refptr<SurfaceTextureBridge> surface_texture_bridge,
+      int primary_id,
+      int secondary_id) = 0;
 
  protected:
   SurfaceTexturePeer();
diff --git a/content/common/gpu/stream_texture_manager_android.cc b/content/common/gpu/stream_texture_manager_android.cc
index 06784d7..e073fbc 100644
--- a/content/common/gpu/stream_texture_manager_android.cc
+++ b/content/common/gpu/stream_texture_manager_android.cc
@@ -15,7 +15,7 @@ namespace content {
 
 StreamTextureManagerAndroid::StreamTextureAndroid::StreamTextureAndroid(
     GpuChannel* channel, int service_id)
-    : surface_texture_(new SurfaceTextureBridge(service_id)),
+    : surface_texture_bridge_(new SurfaceTextureBridge(service_id)),
       has_updated_(false),
       channel_(channel) {
   memset(current_matrix_, 0, sizeof(current_matrix_));
@@ -25,12 +25,12 @@ StreamTextureManagerAndroid::StreamTextureAndroid::~StreamTextureAndroid() {
 }
 
 void StreamTextureManagerAndroid::StreamTextureAndroid::Update() {
-  surface_texture_->UpdateTexImage();
+  surface_texture_bridge_->UpdateTexImage();
   if (matrix_callback_.is_null())
     return;
 
   float mtx[16];
-  surface_texture_->GetTransformMatrix(mtx);
+  surface_texture_bridge_->GetTransformMatrix(mtx);
 
   // Only query the matrix once we have bound a valid frame.
   if (has_updated_ && memcmp(current_matrix_, mtx, sizeof(mtx)) != 0) {
@@ -111,9 +111,10 @@ void StreamTextureManagerAndroid::RegisterStreamTextureProxy(
           base::Unretained(this),
           route_id);
     stream_texture->set_matrix_changed_callback(matrix_cb);
-    stream_texture->bridge()->SetFrameAvailableCallback(frame_cb);
-    stream_texture->bridge()->SetDefaultBufferSize(initial_size.width(),
-                                                   initial_size.height());
+    stream_texture->surface_texture_bridge()->SetFrameAvailableCallback(
+        frame_cb);
+    stream_texture->surface_texture_bridge()->SetDefaultBufferSize(
+        initial_size.width(), initial_size.height());
   }
 }
 
@@ -127,7 +128,7 @@ void StreamTextureManagerAndroid::EstablishStreamTexture(
     SurfaceTexturePeer::GetInstance()->EstablishSurfaceTexturePeer(
         process,
         type,
-        stream_texture->bridge()->j_surface_texture().obj(),
+        stream_texture->surface_texture_bridge(),
         primary_id,
         secondary_id);
   }
diff --git a/content/common/gpu/stream_texture_manager_android.h b/content/common/gpu/stream_texture_manager_android.h
index cbecdc5..f0529d3 100644
--- a/content/common/gpu/stream_texture_manager_android.h
+++ b/content/common/gpu/stream_texture_manager_android.h
@@ -58,7 +58,9 @@ class StreamTextureManagerAndroid : public gpu::StreamTextureManager {
 
     virtual void Update() OVERRIDE;
 
-    SurfaceTextureBridge* bridge() { return surface_texture_.get(); }
+    scoped_refptr<SurfaceTextureBridge> surface_texture_bridge() {
+        return surface_texture_bridge_;
+    }
 
     // Called when a new frame is available.
     void OnFrameAvailable(int route_id);
@@ -74,7 +76,7 @@ class StreamTextureManagerAndroid : public gpu::StreamTextureManager {
     }
 
    private:
-    scoped_ptr<SurfaceTextureBridge> surface_texture_;
+    scoped_refptr<SurfaceTextureBridge> surface_texture_bridge_;
 
     // Current transform matrix of the surface texture.
     float current_matrix_[16];