Use JavaRef for Android surface objects.

When referring to Android surface objects, use a JavaRef instead of a
bare jobject, to make sure we manage reference lifetimes correctly.

This changes the following interfaces:
- ScopedJavaSurface::AcquireExternalSurface
- GpuSurfaceTracker::SurfaceRecord::SurfaceRecord
- Compositor::SetSurface

Most of the callers of these functions already had JavaParamRef objects
and were relying on the implicit conversion to jobject, which is no
longer necessary with this change; the remaining callers have been
updated.

Change-Id: Ic667ad98974bbe8b0674afbce3c0eb6072001468
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2453270Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815150}

components/webxr/mailbox_to_surface_bridge_impl.cc

@@ -222,7 +222,7 @@ void MailboxToSurfaceBridgeImpl::CreateSurface(
   surface_ = std::make_unique<gl::ScopedJavaSurface>(surface_texture);
   surface_handle_ =
       tracker->AddSurfaceForNativeWidget(gpu::GpuSurfaceTracker::SurfaceRecord(
-          window, surface_->j_surface().obj(),
+          window, surface_->j_surface(),
           false /* can_be_used_with_surface_control */));
   // Unregistering happens in the destructor.
   ANativeWindow_release(window);

content/browser/android/dialog_overlay_impl.cc

@@ -258,7 +258,7 @@ static jint JNI_DialogOverlayImpl_RegisterSurface(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return gpu::GpuSurfaceTracker::Get()->AddSurfaceForNativeWidget(
       gpu::GpuSurfaceTracker::SurfaceRecord(
-          gfx::kNullAcceleratedWidget, surface.obj(),
+          gfx::kNullAcceleratedWidget, surface,
           false /* can_be_used_with_surface_control */));
 }
 

content/browser/renderer_host/compositor_impl_android.cc

@@ -365,7 +365,7 @@ void CompositorImpl::SetRootLayer(scoped_refptr<cc::Layer> root_layer) {
   }
 }
 
-void CompositorImpl::SetSurface(jobject surface,
+void CompositorImpl::SetSurface(const base::android::JavaRef<jobject>& surface,
                                 bool can_be_used_with_surface_control) {
   can_be_used_with_surface_control &=
       !root_window_->ApplyDisableSurfaceControlWorkaround();
@@ -388,7 +388,7 @@ void CompositorImpl::SetSurface(jobject surface,
     // ANativeWindow_fromSurface are released immediately. This is needed as a
     // workaround for https://code.google.com/p/android/issues/detail?id=68174
     base::android::ScopedJavaLocalFrame scoped_local_reference_frame(env);
-    window = ANativeWindow_fromSurface(env, surface);
+    window = ANativeWindow_fromSurface(env, surface.obj());
   }
 
   if (window) {

content/browser/renderer_host/compositor_impl_android.h

@@ -93,7 +93,7 @@ class CONTENT_EXPORT CompositorImpl
   // Compositor implementation.
   void SetRootWindow(gfx::NativeWindow root_window) override;
   void SetRootLayer(scoped_refptr<cc::Layer> root) override;
-  void SetSurface(jobject surface,
+  void SetSurface(const base::android::JavaRef<jobject>& surface,
                   bool can_be_used_with_surface_control) override;
   void SetBackgroundColor(int color) override;
   void SetWindowBounds(const gfx::Size& size) override;

content/public/browser/android/compositor.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_PUBLIC_BROWSER_ANDROID_COMPOSITOR_H_
 #define CONTENT_PUBLIC_BROWSER_ANDROID_COMPOSITOR_H_
 
+#include "base/android/scoped_java_ref.h"
 #include "base/callback.h"
 #include "cc/resources/ui_resource_bitmap.h"
 #include "cc/trees/layer_tree_host_client.h"
@@ -72,7 +73,7 @@ class CONTENT_EXPORT Compositor {
   virtual const gfx::Size& GetWindowBounds() = 0;
 
   // Set the output surface which the compositor renders into.
-  virtual void SetSurface(jobject surface,
+  virtual void SetSurface(const base::android::JavaRef<jobject>& surface,
                           bool can_be_used_with_surface_control) = 0;
 
   // Set the background color used by the layer tree host.

gpu/command_buffer/service/image_reader_gl_owner.cc

@@ -240,7 +240,8 @@ gl::ScopedJavaSurface ImageReaderGLOwner::CreateJavaSurface() const {
 
   // Get the java surface object from the Android native window.
   JNIEnv* env = base::android::AttachCurrentThread();
-  jobject j_surface = loader_.ANativeWindow_toSurface(env, window);
+  auto j_surface = base::android::ScopedJavaLocalRef<jobject>::Adopt(
+      env, loader_.ANativeWindow_toSurface(env, window));
   DCHECK(j_surface);
 
   // Get the scoped java surface that is owned externally.

gpu/ipc/common/gpu_surface_tracker.cc

@@ -17,14 +17,14 @@ namespace gpu {
 #if defined(OS_ANDROID)
 GpuSurfaceTracker::SurfaceRecord::SurfaceRecord(
     gfx::AcceleratedWidget widget,
-    jobject j_surface,
+    const base::android::JavaRef<jobject>& j_surface,
     bool can_be_used_with_surface_control)
     : widget(widget),
       can_be_used_with_surface_control(can_be_used_with_surface_control) {
-  // TODO(liberato): It would be nice to assert |surface != nullptr|, but we
+  // TODO(liberato): It would be nice to assert |surface|, but we
   // can't.  in_process_context_factory.cc (for tests) actually calls us without
   // a Surface from java.  Presumably, nobody uses it.  crbug.com/712717 .
-  if (j_surface != nullptr)
+  if (j_surface)
     surface = gl::ScopedJavaSurface::AcquireExternalSurface(j_surface);
 }
 #else   // defined(OS_ANDROID)
@@ -98,8 +98,7 @@ gl::ScopedJavaSurface GpuSurfaceTracker::AcquireJavaSurface(
 
   *can_be_used_with_surface_control =
       it->second.can_be_used_with_surface_control;
-  return gl::ScopedJavaSurface::AcquireExternalSurface(
-      j_surface.j_surface().obj());
+  return gl::ScopedJavaSurface::AcquireExternalSurface(j_surface.j_surface());
 }
 #endif
 

gpu/ipc/common/gpu_surface_tracker.h

@@ -12,11 +12,16 @@
 #include "base/macros.h"
 #include "base/memory/singleton.h"
 #include "base/synchronization/lock.h"
+#include "build/build_config.h"
 #include "gpu/gpu_export.h"
 #include "gpu/ipc/common/gpu_surface_lookup.h"
 #include "gpu/ipc/common/surface_handle.h"
 #include "ui/gfx/native_widget_types.h"
 
+#if defined(OS_ANDROID)
+#include "base/android/scoped_java_ref.h"
+#endif
+
 namespace gpu {
 
 // This class is used on Android and Mac, and is responsible for tracking native
@@ -37,7 +42,7 @@ class GPU_EXPORT GpuSurfaceTracker : public gpu::GpuSurfaceLookup {
   struct SurfaceRecord {
 #if defined(OS_ANDROID)
     SurfaceRecord(gfx::AcceleratedWidget widget,
-                  jobject j_surface,
+                  const base::android::JavaRef<jobject>& j_surface,
                   bool can_be_used_with_surface_control);
 #else   // defined(OS_ANDROID)
     explicit SurfaceRecord(gfx::AcceleratedWidget widget);

media/mojo/clients/mojo_android_overlay_unittest.cc

@@ -139,7 +139,7 @@ class MojoAndroidOverlayTest : public ::testing::Test {
     surface_ = gl::ScopedJavaSurface(surface_texture_.get());
     surface_key_ = gpu::GpuSurfaceTracker::Get()->AddSurfaceForNativeWidget(
         gpu::GpuSurfaceTracker::SurfaceRecord(
-            gfx::kNullAcceleratedWidget, surface_.j_surface().obj(),
+            gfx::kNullAcceleratedWidget, surface_.j_surface(),
             false /* can_be_used_with_surface_control */));
 
     mock_provider_.client_->OnSurfaceReady(surface_key_);

ui/gl/android/scoped_java_surface.cc

@@ -68,11 +68,9 @@ bool ScopedJavaSurface::IsValid() const {
 }
 
 // static
-ScopedJavaSurface ScopedJavaSurface::AcquireExternalSurface(jobject surface) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  ScopedJavaLocalRef<jobject> surface_ref;
-  surface_ref.Reset(env, surface);
-  ScopedJavaSurface scoped_surface(surface_ref);
+ScopedJavaSurface ScopedJavaSurface::AcquireExternalSurface(
+    const base::android::JavaRef<jobject>& surface) {
+  ScopedJavaSurface scoped_surface(surface);
   scoped_surface.auto_release_ = false;
   scoped_surface.is_protected_ = true;
   return scoped_surface;

ui/gl/android/scoped_java_surface.h

@@ -36,7 +36,8 @@ class GL_EXPORT ScopedJavaSurface {
 
   // Creates a ScopedJavaSurface that is owned externally, i.e.,
   // someone else is responsible to call Surface.release().
-  static ScopedJavaSurface AcquireExternalSurface(jobject surface);
+  static ScopedJavaSurface AcquireExternalSurface(
+      const base::android::JavaRef<jobject>& surface);
 
   ~ScopedJavaSurface();