Skip to content

Commit 6132804

Browse files
agrievecopybara-github
agrieve
authored and
copybara-github
committed
Android: Make JniOnceCallback implement Runnable
So that you can use a base::OnceCallback that takes no args. Also: * Simplifies by using a single Java impl class with an mIsRepeating field (rather than two different classes). * Makes the class implement Destroyable * Adds 4 interfaces to make the type of the callback clear in Java, and to ensure that Runnable vs Callback are never confused. * Although, it still is possible to use a no-parameter OnceCallback and type the Java side as "JniOnceCallback" instead of "JniOnceRunnable", and it would still work as long as you passed "null" as the argument. Bug: 40113306 Change-Id: I16dcd6ef8973729d00e4295723c80c423eeb955c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6520019 Commit-Queue: Andrew Grieve <[email protected]> Reviewed-by: Sam Maier <[email protected]> Cr-Commit-Position: refs/heads/main@{#1458304} NOKEYCHECK=True GitOrigin-RevId: fcac15060e0d21dcc54a394c8a6841a8fbd8aa30
1 parent 23be56e commit 6132804

File tree

9 files changed

+180
-130
lines changed

9 files changed

+180
-130
lines changed

‎BUILD.gn‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4430,9 +4430,7 @@ if (is_android) {
44304430
"android/java/src/org/chromium/base/IntStringCallback.java",
44314431
"android/java/src/org/chromium/base/JNIUtils.java",
44324432
"android/java/src/org/chromium/base/JavaExceptionReporter.java",
4433-
"android/java/src/org/chromium/base/JniCallbackUtils.java",
4434-
"android/java/src/org/chromium/base/JniOnceCallback.java",
4435-
"android/java/src/org/chromium/base/JniRepeatingCallback.java",
4433+
"android/java/src/org/chromium/base/JniCallbackImpl.java",
44364434
"android/java/src/org/chromium/base/Token.java",
44374435
"android/java/src/org/chromium/base/TokenBase.java",
44384436
"android/java/src/org/chromium/base/UnguessableToken.java",
@@ -4612,9 +4610,11 @@ if (is_android) {
46124610
"android/java/src/org/chromium/base/IntentUtils.java",
46134611
"android/java/src/org/chromium/base/JavaExceptionReporter.java",
46144612
"android/java/src/org/chromium/base/JavaHandlerThread.java",
4615-
"android/java/src/org/chromium/base/JniCallbackUtils.java",
4613+
"android/java/src/org/chromium/base/JniCallbackImpl.java",
46164614
"android/java/src/org/chromium/base/JniOnceCallback.java",
4615+
"android/java/src/org/chromium/base/JniOnceRunnable.java",
46174616
"android/java/src/org/chromium/base/JniRepeatingCallback.java",
4617+
"android/java/src/org/chromium/base/JniRepeatingRunnable.java",
46184618
"android/java/src/org/chromium/base/LocaleUtils.java",
46194619
"android/java/src/org/chromium/base/MathUtils.java",
46204620
"android/java/src/org/chromium/base/PackageManagerUtils.java",

‎android/java/src/org/chromium/base/JniCallbackImpl.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright 2025 The Chromium Authors
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
package org.chromium.base;
6+
7+
import org.jni_zero.CalledByNative;
8+
import org.jni_zero.JNINamespace;
9+
import org.jni_zero.NativeMethods;
10+
11+
import org.chromium.build.annotations.NullMarked;
12+
import org.chromium.build.annotations.Nullable;
13+
14+
/*
15+
* A wrapper that owns a native side base::OnceCallback.
16+
*
17+
* You must call JniOnceCallback#destroy() if you never end up calling onResult
18+
* so as to not leak the native callback.
19+
*
20+
* This class has no additional thread safety measures compared to
21+
* base::RepeatingCallback.
22+
*
23+
* Class is package-private in order to force clients to use one of the implemented
24+
* interfaces (to reduce confusion of a Callback that is also a Runnable), and to
25+
* better document Once vs Repeated semantics.
26+
*/
27+
@JNINamespace("base::android")
28+
@NullMarked
29+
final class JniCallbackImpl<T extends @Nullable Object>
30+
implements JniOnceCallback<T>,
31+
JniOnceRunnable,
32+
JniRepeatingCallback<T>,
33+
JniRepeatingRunnable {
34+
private final @Nullable LifetimeAssert mLifetimeAssert = LifetimeAssert.create(this);
35+
private long mNativePointer;
36+
private final boolean mIsRepeating;
37+
38+
@CalledByNative
39+
private JniCallbackImpl(boolean isRepeating, long nativePointer) {
40+
mIsRepeating = isRepeating;
41+
mNativePointer = nativePointer;
42+
}
43+
44+
@Override
45+
@SuppressWarnings("NullAway")
46+
public void run() {
47+
// When used as a Runnable instead of a Callback<T>, the JNI call still expects a
48+
// parameter, but discards it and calls the no-arg OnceCallback. This was simpler
49+
// than introducing a second JNI call that does not take a parameter.
50+
onResult(null);
51+
}
52+
53+
@Override
54+
public void onResult(T result) {
55+
// TODO(mheikal): maybe store destroy callstack to output here?
56+
assert mNativePointer != 0 : "Called destroyed callback";
57+
JniCallbackImplJni.get().onResult(mIsRepeating, mNativePointer, result);
58+
if (!mIsRepeating) {
59+
mNativePointer = 0;
60+
LifetimeAssert.setSafeToGc(mLifetimeAssert, true);
61+
}
62+
}
63+
64+
/** Frees the owned base::OnceCallback's memory */
65+
@Override
66+
public void destroy() {
67+
if (mNativePointer != 0) {
68+
JniCallbackImplJni.get().destroy(mIsRepeating, mNativePointer);
69+
mNativePointer = 0;
70+
LifetimeAssert.setSafeToGc(mLifetimeAssert, true);
71+
}
72+
}
73+
74+
@NativeMethods
75+
interface Natives {
76+
void onResult(boolean isRepeating, long callbackPtr, Object result);
77+
78+
void destroy(boolean isRepeating, long callbackPtr);
79+
}
80+
}

‎android/java/src/org/chromium/base/JniCallbackUtils.java‎

Lines changed: 0 additions & 45 deletions
This file was deleted.

‎android/java/src/org/chromium/base/JniOnceCallback.java‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44

55
package org.chromium.base;
66

7-
import org.jni_zero.CalledByNative;
8-
import org.jni_zero.JNINamespace;
9-
7+
import org.chromium.base.lifetime.Destroyable;
108
import org.chromium.build.annotations.NullMarked;
119
import org.chromium.build.annotations.Nullable;
1210

@@ -19,35 +17,5 @@
1917
* This class has no additional thread safety measures compared to
2018
* base::RepeatingCallback.
2119
*/
22-
@JNINamespace("base::android")
2320
@NullMarked
24-
public final class JniOnceCallback<T extends @Nullable Object> implements Callback<T> {
25-
private final @Nullable LifetimeAssert mLifetimeAssert = LifetimeAssert.create(this);
26-
long mNativePointer;
27-
28-
@CalledByNative
29-
private JniOnceCallback(long nativePointer) {
30-
mNativePointer = nativePointer;
31-
}
32-
33-
@Override
34-
public void onResult(T result) {
35-
if (mNativePointer != 0) {
36-
JniCallbackUtils.runNativeCallback(this, result);
37-
mNativePointer = 0;
38-
LifetimeAssert.setSafeToGc(mLifetimeAssert, true);
39-
} else {
40-
// TODO(mheikal): maybe store destroy callstack to output here?
41-
assert false : "Called destroyed callback";
42-
}
43-
}
44-
45-
/** Frees the owned base::OnceCallback's memory */
46-
public void destroy() {
47-
if (mNativePointer != 0) {
48-
JniCallbackUtils.destroyNativeCallback(this);
49-
mNativePointer = 0;
50-
}
51-
LifetimeAssert.setSafeToGc(mLifetimeAssert, true);
52-
}
53-
}
21+
public interface JniOnceCallback<T extends @Nullable Object> extends Callback<T>, Destroyable {}

‎android/java/src/org/chromium/base/JniOnceRunnable.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2025 The Chromium Authors
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
package org.chromium.base;
6+
7+
import org.chromium.base.lifetime.Destroyable;
8+
import org.chromium.build.annotations.NullMarked;
9+
10+
/*
11+
* A wrapper that owns a native side base::OnceCallback.
12+
*
13+
* You must call JniOnceCallback#destroy() if you never end up calling onResult
14+
* so as to not leak the native callback.
15+
*
16+
* This class has no additional thread safety measures compared to
17+
* base::RepeatingCallback.
18+
*/
19+
@NullMarked
20+
public interface JniOnceRunnable extends Runnable, Destroyable {}

‎android/java/src/org/chromium/base/JniRepeatingCallback.java‎

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,7 @@
44

55
package org.chromium.base;
66

7-
import org.jni_zero.CalledByNative;
8-
import org.jni_zero.JNINamespace;
9-
7+
import org.chromium.base.lifetime.Destroyable;
108
import org.chromium.build.annotations.NullMarked;
119
import org.chromium.build.annotations.Nullable;
1210

@@ -19,33 +17,6 @@
1917
* This class has no additional thread safety measures compared to
2018
* base::RepeatingCallback.
2119
*/
22-
@JNINamespace("base::android")
2320
@NullMarked
24-
public class JniRepeatingCallback<T extends @Nullable Object> implements Callback<T> {
25-
private final @Nullable LifetimeAssert mLifetimeAssert = LifetimeAssert.create(this);
26-
long mNativePointer;
27-
28-
@CalledByNative
29-
private JniRepeatingCallback(long nativePointer) {
30-
mNativePointer = nativePointer;
31-
}
32-
33-
@Override
34-
public void onResult(T result) {
35-
if (mNativePointer != 0) {
36-
JniCallbackUtils.runNativeCallback(this, result);
37-
} else {
38-
// TODO(mheikal): maybe store destroy callstack to output here?
39-
assert false : "Called destroyed callback";
40-
}
41-
}
42-
43-
/** Frees the owned base::RepeatingCallback's memory */
44-
public void destroy() {
45-
if (mNativePointer != 0) {
46-
JniCallbackUtils.destroyNativeCallback(this);
47-
mNativePointer = 0;
48-
}
49-
LifetimeAssert.setSafeToGc(mLifetimeAssert, true);
50-
}
51-
}
21+
public interface JniRepeatingCallback<T extends @Nullable Object>
22+
extends Callback<T>, Destroyable {}

‎android/java/src/org/chromium/base/JniRepeatingRunnable.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2025 The Chromium Authors
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
package org.chromium.base;
6+
7+
import org.chromium.base.lifetime.Destroyable;
8+
import org.chromium.build.annotations.NullMarked;
9+
10+
/*
11+
* A wrapper that owns a native side base::RepeatingCallback.
12+
*
13+
* You must call JniRepeatingCallback#destroy() when you are done with it to
14+
* not leak the native callback.
15+
*
16+
* This class has no additional thread safety measures compared to
17+
* base::RepeatingCallback.
18+
*/
19+
@NullMarked
20+
public interface JniRepeatingRunnable extends Runnable, Destroyable {}

‎android/jni_callback.cc‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@
55
#include "base/android/jni_callback.h"
66

77
// Must come after all headers that specialize FromJniType() / ToJniType().
8-
#include "base/base_minimal_jni/JniCallbackUtils_jni.h"
9-
#include "base/base_minimal_jni/JniOnceCallback_jni.h"
10-
#include "base/base_minimal_jni/JniRepeatingCallback_jni.h"
8+
#include "base/base_minimal_jni/JniCallbackImpl_jni.h"
119

1210
namespace base::android {
1311

@@ -25,8 +23,10 @@ class JniOnceCallback {
2523
jni_zero::ScopedJavaLocalRef<jobject> TransferToJava(JNIEnv* env) && {
2624
CHECK(wrapped_callback_);
2725
CHECK(!wrapped_callback_->is_null());
28-
return Java_JniOnceCallback_Constructor(
29-
env, reinterpret_cast<jlong>(wrapped_callback_.release()));
26+
bool is_repeating = false;
27+
return Java_JniCallbackImpl_Constructor(
28+
env, is_repeating,
29+
reinterpret_cast<jlong>(wrapped_callback_.release()));
3030
}
3131

3232
private:
@@ -47,8 +47,10 @@ class JniRepeatingCallback {
4747
jni_zero::ScopedJavaLocalRef<jobject> TransferToJava(JNIEnv* env) && {
4848
CHECK(wrapped_callback_);
4949
CHECK(!wrapped_callback_->is_null());
50-
return Java_JniRepeatingCallback_Constructor(
51-
env, reinterpret_cast<jlong>(wrapped_callback_.release()));
50+
bool is_repeating = true;
51+
return Java_JniCallbackImpl_Constructor(
52+
env, is_repeating,
53+
reinterpret_cast<jlong>(wrapped_callback_.release()));
5254
}
5355
JniRepeatingCallback(const JniRepeatingCallback&) = delete;
5456
const JniRepeatingCallback& operator=(const JniRepeatingCallback&) = delete;
@@ -76,10 +78,39 @@ ScopedJavaLocalRef<jobject> ToJniCallback(
7678
return JniRepeatingCallback(callback).TransferToJava(env);
7779
}
7880

79-
void JNI_JniCallbackUtils_OnResult(
81+
ScopedJavaLocalRef<jobject> ToJniCallback(
82+
JNIEnv* env,
83+
base::OnceCallback<void()>&& callback) {
84+
return ToJniCallback(env, base::BindOnce(
85+
[](base::OnceCallback<void()> captured_callback,
86+
const jni_zero::JavaRef<jobject>& j_null) {
87+
// For callbacks with no parameters, the
88+
// parameter from Java should be null.
89+
CHECK(!j_null);
90+
std::move(captured_callback).Run();
91+
},
92+
std::move(callback)));
93+
}
94+
95+
ScopedJavaLocalRef<jobject> ToJniCallback(
96+
JNIEnv* env,
97+
const base::RepeatingCallback<void()>& callback) {
98+
return ToJniCallback(
99+
env, base::BindOnce(
100+
[](const base::RepeatingCallback<void()>& captured_callback,
101+
const jni_zero::JavaRef<jobject>& j_null) {
102+
// For callbacks with no parameters, the parameter from Java
103+
// should be null.
104+
CHECK(!j_null);
105+
captured_callback.Run();
106+
},
107+
std::move(callback)));
108+
}
109+
110+
void JNI_JniCallbackImpl_OnResult(
80111
JNIEnv* env,
81-
jlong callbackPtr,
82112
jboolean isRepeating,
113+
jlong callbackPtr,
83114
const jni_zero::JavaParamRef<jobject>& j_result) {
84115
if (isRepeating) {
85116
auto* callback =
@@ -92,9 +123,9 @@ void JNI_JniCallbackUtils_OnResult(
92123
}
93124
}
94125

95-
void JNI_JniCallbackUtils_Destroy(JNIEnv* env,
96-
jlong callbackPtr,
97-
jboolean isRepeating) {
126+
void JNI_JniCallbackImpl_Destroy(JNIEnv* env,
127+
jboolean isRepeating,
128+
jlong callbackPtr) {
98129
if (isRepeating) {
99130
auto* callback =
100131
reinterpret_cast<JniRepeatingWrappedCallbackType*>(callbackPtr);
@@ -111,6 +142,4 @@ void JNI_JniCallbackUtils_Destroy(JNIEnv* env,
111142

112143
} // namespace base::android
113144

114-
DEFINE_JNI_FOR_JniCallbackUtils()
115-
DEFINE_JNI_FOR_JniOnceCallback()
116-
DEFINE_JNI_FOR_JniRepeatingCallback()
145+
DEFINE_JNI_FOR_JniCallbackImpl()

0 commit comments

Comments
 (0)