Android: Remove finalizer in SurfaceTextureListener

Surfaces already have a destroy(), so just destroy the listener from
there rather than relying on GC.

This also simplifies the JNI bindings by merging the 2 SurfaceTexture
classes into one.

Bug: 40286193
Change-Id: I36f920c842ca964a034aae629a3ea7337c3d1f31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5898394
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Owners-Override: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1362800}

gpu/BUILD.gn

@@ -675,10 +675,7 @@ test("gpu_unittests") {
   }
 
   if (is_android) {
-    deps += [
-      "//ui/android:ui_java",
-      "//ui/gl:gl_jni_headers",
-    ]
+    deps += [ "//ui/android:ui_java" ]
   }
 
   if (use_dawn) {

media/gpu/BUILD.gn

@@ -180,7 +180,6 @@ component("gpu") {
       "//media/mojo:buildflags",
       "//services/service_manager/public/cpp:cpp",
       "//third_party/libgav1:libgav1_parser",
-      "//ui/gl:gl_jni_headers",
     ]
     if (enable_vulkan) {
       deps += [ "//gpu/vulkan:vulkan" ]

ui/android/BUILD.gn

@@ -130,7 +130,6 @@ generate_jni("ui_android_jni_headers") {
     "java/src/org/chromium/ui/dragdrop/DropDataAndroid.java",
     "java/src/org/chromium/ui/events/devices/InputDeviceObserver.java",
     "java/src/org/chromium/ui/gfx/ViewConfigurationHelper.java",
-    "java/src/org/chromium/ui/gl/SurfaceTextureListener.java",
     "java/src/org/chromium/ui/modaldialog/ModalDialogManagerBridge.java",
     "java/src/org/chromium/ui/resources/HandleViewResources.java",
     "java/src/org/chromium/ui/resources/ResourceFactory.java",
@@ -356,9 +355,8 @@ android_library("ui_no_recycler_view_java") {
     "java/src/org/chromium/ui/gfx/Animation.java",
     "java/src/org/chromium/ui/gfx/BitmapHelper.java",
     "java/src/org/chromium/ui/gfx/ViewConfigurationHelper.java",
+    "java/src/org/chromium/ui/gl/ChromeSurfaceTexture.java",
     "java/src/org/chromium/ui/gl/ScopedJavaSurfaceControl.java",
-    "java/src/org/chromium/ui/gl/SurfaceTextureListener.java",
-    "java/src/org/chromium/ui/gl/SurfaceTexturePlatformWrapper.java",
     "java/src/org/chromium/ui/interpolators/AndroidxInterpolators.java",
     "java/src/org/chromium/ui/interpolators/Interpolators.java",
     "java/src/org/chromium/ui/listmenu/BasicListMenu.java",
@@ -478,6 +476,7 @@ android_library("ui_no_recycler_view_java") {
     "//ui/base:select_file_dialog_jni_headers_java",
     "//ui/base/cursor/mojom:cursor_type_java",
     "//ui/base/ime/mojom:mojom_java",
+    "//ui/gl:gl_jni_headers_java",
     "//url:gurl_java",
   ]
 

ui/android/java/src/org/chromium/ui/gl/ChromeSurfaceTexture.java

@@ -0,0 +1,92 @@
+// Copyright 2013 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.ui.gl;
+
+import android.graphics.SurfaceTexture;
+import android.util.Log;
+
+import org.jni_zero.CalledByNative;
+import org.jni_zero.JNINamespace;
+import org.jni_zero.NativeMethods;
+
+/** Exposes SurfaceTexture APIs to native. */
+@JNINamespace("gl")
+class ChromeSurfaceTexture extends SurfaceTexture
+        implements SurfaceTexture.OnFrameAvailableListener {
+    private static final String TAG = "SurfaceTexture";
+
+    // Used to determine the class instance to dispatch the native call to.
+    private long mNativeSurfaceTextureListener;
+
+    @CalledByNative
+    ChromeSurfaceTexture(int textureId) {
+        super(textureId);
+    }
+
+    @Override
+    public void onFrameAvailable(SurfaceTexture surfaceTexture) {
+        if (mNativeSurfaceTextureListener != 0) {
+            ChromeSurfaceTextureJni.get().frameAvailable(mNativeSurfaceTextureListener);
+        }
+    }
+
+    @CalledByNative
+    public void destroy() {
+        if (mNativeSurfaceTextureListener != 0) {
+            ChromeSurfaceTextureJni.get().destroy(mNativeSurfaceTextureListener);
+            mNativeSurfaceTextureListener = 0;
+        }
+        release();
+    }
+
+    @CalledByNative
+    public void setNativeListener(long nativeSurfaceTextureListener) {
+        assert mNativeSurfaceTextureListener == 0;
+        mNativeSurfaceTextureListener = nativeSurfaceTextureListener;
+        setOnFrameAvailableListener(this);
+    }
+
+    @CalledByNative
+    @Override
+    public void updateTexImage() {
+        try {
+            super.updateTexImage();
+        } catch (RuntimeException e) {
+            Log.e(TAG, "Error calling updateTexImage", e);
+        }
+    }
+
+    @CalledByNative
+    @Override
+    public void getTransformMatrix(float[] matrix) {
+        super.getTransformMatrix(matrix);
+    }
+
+    @CalledByNative
+    @Override
+    public void attachToGLContext(int texName) {
+        super.attachToGLContext(texName);
+    }
+
+    @CalledByNative
+    @Override
+    public void detachFromGLContext() {
+        super.detachFromGLContext();
+    }
+
+    @CalledByNative
+    @Override
+    public void setDefaultBufferSize(int width, int height) {
+        super.setDefaultBufferSize(width, height);
+    }
+
+    @NativeMethods
+    interface Natives {
+        // These are methods on SurfaceTextureListener.
+        void frameAvailable(long nativeSurfaceTextureListener);
+
+        void destroy(long nativeSurfaceTextureListener);
+    }
+}

ui/android/java/src/org/chromium/ui/gl/SurfaceTextureListener.java

@@ -1,46 +0,0 @@
-// Copyright 2013 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package org.chromium.ui.gl;
-
-import android.graphics.SurfaceTexture;
-
-import org.jni_zero.JNINamespace;
-import org.jni_zero.NativeMethods;
-
-/** Listener to an android SurfaceTexture object for frame availability. */
-@JNINamespace("gl")
-class SurfaceTextureListener implements SurfaceTexture.OnFrameAvailableListener {
-    // Used to determine the class instance to dispatch the native call to.
-    private final long mNativeSurfaceTextureListener;
-
-    SurfaceTextureListener(long nativeSurfaceTextureListener) {
-        assert nativeSurfaceTextureListener != 0;
-        mNativeSurfaceTextureListener = nativeSurfaceTextureListener;
-    }
-
-    @Override
-    public void onFrameAvailable(SurfaceTexture surfaceTexture) {
-        SurfaceTextureListenerJni.get()
-                .frameAvailable(mNativeSurfaceTextureListener, SurfaceTextureListener.this);
-    }
-
-    @Override
-    @SuppressWarnings("Finalize")  // TODO(crbug.com/40286193): Remove in favor of destroy().
-    protected void finalize() throws Throwable {
-        try {
-            SurfaceTextureListenerJni.get()
-                    .destroy(mNativeSurfaceTextureListener, SurfaceTextureListener.this);
-        } finally {
-            super.finalize();
-        }
-    }
-
-    @NativeMethods
-    interface Natives {
-        void frameAvailable(long nativeSurfaceTextureListener, SurfaceTextureListener caller);
-
-        void destroy(long nativeSurfaceTextureListener, SurfaceTextureListener caller);
-    }
-}

ui/android/java/src/org/chromium/ui/gl/SurfaceTexturePlatformWrapper.java

@@ -1,72 +0,0 @@
-// Copyright 2013 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-package org.chromium.ui.gl;
-
-import android.graphics.SurfaceTexture;
-import android.util.Log;
-
-import org.jni_zero.CalledByNative;
-import org.jni_zero.JNINamespace;
-
-/**
- * Wrapper class for the underlying platform's SurfaceTexture in order to
- * provide a stable JNI API.
- */
-@JNINamespace("gl")
-class SurfaceTexturePlatformWrapper {
-    private static final String TAG = "SurfaceTexturePlatformWrapper";
-
-    @CalledByNative
-    private static SurfaceTexture create(int textureId) {
-        return new SurfaceTexture(textureId);
-    }
-
-    @CalledByNative
-    private static void destroy(SurfaceTexture surfaceTexture) {
-        surfaceTexture.setOnFrameAvailableListener(null);
-        surfaceTexture.release();
-    }
-
-    @CalledByNative
-    private static void setFrameAvailableCallback(
-            SurfaceTexture surfaceTexture, long nativeSurfaceTextureListener) {
-        surfaceTexture.setOnFrameAvailableListener(
-                new SurfaceTextureListener(nativeSurfaceTextureListener));
-    }
-
-    @CalledByNative
-    private static void updateTexImage(SurfaceTexture surfaceTexture) {
-        try {
-            surfaceTexture.updateTexImage();
-        } catch (RuntimeException e) {
-            Log.e(TAG, "Error calling updateTexImage", e);
-        }
-    }
-
-    @CalledByNative
-    private static void getTransformMatrix(SurfaceTexture surfaceTexture, float[] matrix) {
-        surfaceTexture.getTransformMatrix(matrix);
-    }
-
-    @CalledByNative
-    private static void attachToGLContext(SurfaceTexture surfaceTexture, int texName) {
-        surfaceTexture.attachToGLContext(texName);
-    }
-
-    @CalledByNative
-    private static void detachFromGLContext(SurfaceTexture surfaceTexture) {
-        surfaceTexture.detachFromGLContext();
-    }
-
-    @CalledByNative
-    private static void release(SurfaceTexture surfaceTexture) {
-        surfaceTexture.release();
-    }
-
-    @CalledByNative
-    private static void setDefaultBufferSize(SurfaceTexture surfaceTexture, int width, int height) {
-        surfaceTexture.setDefaultBufferSize(width, height);
-    }
-}

ui/gl/BUILD.gn

@@ -570,9 +570,8 @@ if (is_android) {
 
   generate_jni("gl_jni_headers") {
     sources = [
+      "../android/java/src/org/chromium/ui/gl/ChromeSurfaceTexture.java",
       "../android/java/src/org/chromium/ui/gl/ScopedJavaSurfaceControl.java",
-      "../android/java/src/org/chromium/ui/gl/SurfaceTextureListener.java",
-      "../android/java/src/org/chromium/ui/gl/SurfaceTexturePlatformWrapper.java",
     ]
     public_deps = [ ":surface_jni_headers" ]
   }

ui/gl/android/surface_texture.cc

@@ -16,7 +16,7 @@
 #include "ui/gl/gl_bindings.h"
 
 // Must come after all headers that specialize FromJniType() / ToJniType().
-#include "ui/gl/gl_jni_headers/SurfaceTexturePlatformWrapper_jni.h"
+#include "ui/gl/gl_jni_headers/ChromeSurfaceTexture_jni.h"
 
 #ifndef GL_ANGLE_texture_storage_external
 #define GL_ANGLE_texture_storage_external 1
@@ -41,7 +41,7 @@ scoped_refptr<SurfaceTexture> SurfaceTexture::Create(int texture_id) {
 
   JNIEnv* env = base::android::AttachCurrentThread();
   return new SurfaceTexture(
-      Java_SurfaceTexturePlatformWrapper_create(env, native_id));
+      Java_ChromeSurfaceTexture_Constructor(env, native_id));
 }
 
 SurfaceTexture::SurfaceTexture(
@@ -50,14 +50,13 @@ SurfaceTexture::SurfaceTexture(
 }
 
 SurfaceTexture::~SurfaceTexture() {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SurfaceTexturePlatformWrapper_destroy(env, j_surface_texture_);
+  ReleaseBackBuffers();
 }
 
 void SurfaceTexture::SetFrameAvailableCallback(
     base::RepeatingClosure callback) {
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SurfaceTexturePlatformWrapper_setFrameAvailableCallback(
+  Java_ChromeSurfaceTexture_setNativeListener(
       env, j_surface_texture_,
       reinterpret_cast<intptr_t>(
           new SurfaceTextureListener(std::move(callback), false)));
@@ -66,7 +65,7 @@ void SurfaceTexture::SetFrameAvailableCallback(
 void SurfaceTexture::SetFrameAvailableCallbackOnAnyThread(
     base::RepeatingClosure callback) {
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SurfaceTexturePlatformWrapper_setFrameAvailableCallback(
+  Java_ChromeSurfaceTexture_setNativeListener(
       env, j_surface_texture_,
       reinterpret_cast<intptr_t>(
           new SurfaceTextureListener(std::move(callback), true)));
@@ -78,7 +77,7 @@ void SurfaceTexture::UpdateTexImage() {
       base::debug::CrashKeySize::Size256);
   base::debug::ScopedCrashKeyString scoped_crash_key(kCrashKey, "1");
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SurfaceTexturePlatformWrapper_updateTexImage(env, j_surface_texture_);
+  Java_ChromeSurfaceTexture_updateTexImage(env, j_surface_texture_);
 
   // Notify ANGLE that the External texture binding has changed
   if (gl::g_current_gl_driver->ext.b_GL_ANGLE_texture_external_update)
@@ -90,8 +89,8 @@ void SurfaceTexture::GetTransformMatrix(base::span<float, 16> mtx) {
 
   base::android::ScopedJavaLocalRef<jfloatArray> jmatrix(
       env, env->NewFloatArray(16));
-  Java_SurfaceTexturePlatformWrapper_getTransformMatrix(env, j_surface_texture_,
-                                                        jmatrix);
+  Java_ChromeSurfaceTexture_getTransformMatrix(env, j_surface_texture_,
+                                               jmatrix);
 
   jfloat* elements = env->GetFloatArrayElements(jmatrix.obj(), nullptr);
   for (int i = 0; i < 16; ++i) {
@@ -113,8 +112,8 @@ void SurfaceTexture::AttachToGLContext() {
   DCHECK(texture_id);
 
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SurfaceTexturePlatformWrapper_attachToGLContext(env, j_surface_texture_,
-                                                       texture_id);
+  Java_ChromeSurfaceTexture_attachToGLContext(env, j_surface_texture_,
+                                              texture_id);
 
   // Notify ANGLE that the External texture binding has changed
   if (gl::g_current_gl_driver->ext.b_GL_ANGLE_texture_external_update) {
@@ -124,8 +123,7 @@ void SurfaceTexture::AttachToGLContext() {
 
 void SurfaceTexture::DetachFromGLContext() {
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SurfaceTexturePlatformWrapper_detachFromGLContext(env,
-                                                         j_surface_texture_);
+  Java_ChromeSurfaceTexture_detachFromGLContext(env, j_surface_texture_);
 }
 
 ScopedANativeWindow SurfaceTexture::CreateSurface() {
@@ -135,13 +133,13 @@ ScopedANativeWindow SurfaceTexture::CreateSurface() {
 
 void SurfaceTexture::ReleaseBackBuffers() {
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SurfaceTexturePlatformWrapper_release(env, j_surface_texture_);
+  Java_ChromeSurfaceTexture_destroy(env, j_surface_texture_);
 }
 
 void SurfaceTexture::SetDefaultBufferSize(int width, int height) {
   JNIEnv* env = base::android::AttachCurrentThread();
-  Java_SurfaceTexturePlatformWrapper_setDefaultBufferSize(
-      env, j_surface_texture_, width, height);
+  Java_ChromeSurfaceTexture_setDefaultBufferSize(env, j_surface_texture_, width,
+                                                 height);
 }
 
 }  // namespace gl

ui/gl/android/surface_texture_listener.cc

@@ -9,11 +9,6 @@
 #include "base/location.h"
 #include "base/task/single_thread_task_runner.h"
 
-// Must come after all headers that specialize FromJniType() / ToJniType().
-#include "ui/gl/gl_jni_headers/SurfaceTextureListener_jni.h"
-
-using base::android::JavaParamRef;
-
 namespace gl {
 
 SurfaceTextureListener::SurfaceTextureListener(base::RepeatingClosure callback,
@@ -25,15 +20,13 @@ SurfaceTextureListener::SurfaceTextureListener(base::RepeatingClosure callback,
 SurfaceTextureListener::~SurfaceTextureListener() {
 }
 
-void SurfaceTextureListener::Destroy(JNIEnv* env,
-                                     const JavaParamRef<jobject>& obj) {
+void SurfaceTextureListener::Destroy(JNIEnv* env) {
   if (!browser_loop_->DeleteSoon(FROM_HERE, this)) {
     delete this;
   }
 }
 
-void SurfaceTextureListener::FrameAvailable(JNIEnv* env,
-                                            const JavaParamRef<jobject>& obj) {
+void SurfaceTextureListener::FrameAvailable(JNIEnv* env) {
   if (!use_any_thread_ && !browser_loop_->BelongsToCurrentThread()) {
     browser_loop_->PostTask(FROM_HERE, callback_);
   } else {

ui/gl/android/surface_texture_listener.h

@@ -11,7 +11,6 @@
 #include "base/functional/callback.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/task/sequenced_task_runner_helpers.h"
-#include "ui/gl/gl_export.h"
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -20,18 +19,17 @@ class SingleThreadTaskRunner;
 namespace gl {
 
 // Listener class for all the callbacks from android SurfaceTexture.
-class GL_EXPORT SurfaceTextureListener {
+class SurfaceTextureListener {
  public:
   SurfaceTextureListener() = delete;
   SurfaceTextureListener(const SurfaceTextureListener&) = delete;
   SurfaceTextureListener& operator=(const SurfaceTextureListener&) = delete;
 
   // Destroy this listener.
-  void Destroy(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
+  void Destroy(JNIEnv* env);
 
   // A new frame is available to consume.
-  void FrameAvailable(JNIEnv* env,
-                      const base::android::JavaParamRef<jobject>& obj);
+  void FrameAvailable(JNIEnv* env);
 
  private:
   friend class base::DeleteHelper<SurfaceTextureListener>;