JNI: allow QString as a parameter to native methods

Expect a jstring on the va_list, and implicitly construct a QString from
that.
As a drive-by, allow native methods to take parameters by reference, and
move implementation details into a Detail namespace.

Add test coverage.

Change-Id: I31214938ccaea3f4d539b432e29d12434dd98377
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Petri Virkkunen <petri.virkkunen@qt.io>
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>

4 files changed, 127 insertions, 44 deletions

diff --git a/src/corelib/kernel/qjnitypes.h b/src/corelib/kernel/qjnitypes.h
index c03e92314e..a3539e3233 100644
--- a/src/corelib/kernel/qjnitypes.h
+++ b/src/corelib/kernel/qjnitypes.h
@@ -214,6 +214,7 @@ struct QtJniTypes::Traits<QtJniTypes::Type> {                   \
 // Macros for native methods
 
 namespace QtJniMethods {
+namespace Detail {
 // Various helpers to forward a call from a variadic argument function to
 // the real function with proper type conversion. This is needed because we
 // want to write functions that take QJniObjects (subclasses), while Java
@@ -232,19 +233,53 @@ template <> struct PromotedType<float> { using Type = double; };
 
 // Map any QJniObject type to jobject; that's what's on the va_list
 template <typename Arg>
-using JNITypeForArg = std::conditional_t<std::is_base_of_v<QJniObject, Arg>, jobject,
-                                                                             typename PromotedType<Arg>::Type>;
+struct JNITypeForArgImpl
+{
+    using Type = std::conditional_t<std::is_base_of_v<QJniObject, Arg>,
+                                    jobject, typename PromotedType<Arg>::Type>;
+    static Arg fromVarArg(Type &&t)
+    {
+        return static_cast<Arg>(t);
+    }
+};
+
+template <>
+struct JNITypeForArgImpl<QString>
+{
+    using Type = jstring;
+
+    static QString fromVarArg(Type &&t)
+    {
+        return QJniObject(t).toString();
+    }
+};
+
+template <typename Arg>
+using JNITypeForArg = typename JNITypeForArgImpl<q20::remove_cvref_t<Arg>>::Type;
+template <typename Arg, typename Type>
+static inline auto methodArgFromVarArg(Type &&t)
+{
+    return JNITypeForArgImpl<q20::remove_cvref_t<Arg>>::fromVarArg(std::move(t));
+}
 
 // Turn a va_list into a tuple of typed arguments
+template <typename ...Args>
+static constexpr auto makeTupleFromArgsHelper(va_list args)
+{
+    return std::tuple<q20::remove_cvref_t<Args>...>{
+        methodArgFromVarArg<q20::remove_cvref_t<Args>>(va_arg(args, JNITypeForArg<Args>))...
+    };
+}
+
 template <typename Ret, typename ...Args>
 static constexpr auto makeTupleFromArgs(Ret (*)(JNIEnv *, jobject, Args...), va_list args)
 {
-    return std::tuple<Args...>{ va_arg(args, JNITypeForArg<Args>)... };
+    return makeTupleFromArgsHelper<Args...>(args);
 }
 template <typename Ret, typename ...Args>
 static constexpr auto makeTupleFromArgs(Ret (*)(JNIEnv *, jclass, Args...), va_list args)
 {
-    return std::tuple<Args...>{ va_arg(args, JNITypeForArg<Args>)... };
+    return makeTupleFromArgsHelper<Args...>(args);
 }
 
 // Get the return type of a function point
@@ -254,25 +289,26 @@ auto nativeFunctionReturnType(Ret(*function)(Args...))
     return function(std::declval<Args>()...);
 }
 
-} // QtJniMethods
+} // namespace Detail
+} // namespace QtJniMethods
 
 // A va_ variadic arguments function that we register with JNI as a proxy
 // for the function we have. This function uses the helpers to unpack the
 // variadic arguments into a tuple of typed arguments, which we then call
 // the actual function with. This then takes care of implicit conversions,
 // e.g. a jobject becomes a QJniObject.
-#define Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method)                      \
-static decltype(QtJniMethods::nativeFunctionReturnType(Method))         \
-va_##Method(JNIEnv *env, jclass thiz, ...)                              \
-{                                                                       \
-    va_list args;                                                       \
-    va_start(args, thiz);                                               \
-    auto va_cleanup = qScopeGuard([&args]{ va_end(args); });            \
-    auto argTuple = QtJniMethods::makeTupleFromArgs(Method, args);      \
-    return std::apply([env, thiz](auto &&... args) {                    \
-        return Method(env, thiz, args...);                              \
-    }, argTuple);                                                       \
-}                                                                       \
+#define Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method)                              \
+static decltype(QtJniMethods::Detail::nativeFunctionReturnType(Method))         \
+va_##Method(JNIEnv *env, jclass thiz, ...)                                      \
+{                                                                               \
+    va_list args;                                                               \
+    va_start(args, thiz);                                                       \
+    auto va_cleanup = qScopeGuard([&args]{ va_end(args); });                    \
+    auto argTuple = QtJniMethods::Detail::makeTupleFromArgs(Method, args);      \
+    return std::apply([env, thiz](auto &&... args) {                            \
+        return Method(env, thiz, args...);                                      \
+    }, argTuple);                                                               \
+}                                                                               \
 
 
 #define Q_DECLARE_JNI_NATIVE_METHOD(...)                        \
diff --git a/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java b/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java
index b56447198f..ef1c6e2f88 100644
--- a/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java
+++ b/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java
@@ -275,6 +275,8 @@ public class QtJniObjectTestClass
 
     // --------------------------------------------------------------------------------------------
     native public int callbackWithObject(QtJniObjectTestClass that);
+    native public int callbackWithObjectRef(QtJniObjectTestClass that);
+    native public int callbackWithString(String string);
     native public int callbackWithByte(byte value);
     native public int callbackWithBoolean(boolean value);
     native public int callbackWithInt(int value);
@@ -285,6 +287,16 @@ public class QtJniObjectTestClass
         return callbackWithObject(that);
     }
 
+    public int callMeBackWithObjectRef(QtJniObjectTestClass that)
+    {
+        return callbackWithObjectRef(that);
+    }
+
+    public int callMeBackWithString(String string)
+    {
+        return callbackWithString(string);
+    }
+
     public int callMeBackWithByte(byte value)
     {
         return callbackWithByte(value);
diff --git a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp
index c658c35bb0..95707c2ae9 100644
--- a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp
+++ b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp
@@ -1863,20 +1863,44 @@ void tst_QJniObject::largeObjectArray()
     }
 }
 
+enum class CallbackParameterType
+{
+    Object,
+    ObjectRef,
+    String,
+    Byte,
+    Boolean,
+    Int,
+    Double
+};
 
 static std::optional<TestClass> calledWithObject;
 static int callbackWithObject(JNIEnv *, jobject, TestClass that)
 {
     calledWithObject.emplace(that);
-    return 42;
+    return int(CallbackParameterType::Object);
 }
 Q_DECLARE_JNI_NATIVE_METHOD(callbackWithObject)
+static int callbackWithObjectRef(JNIEnv *, jobject, const TestClass &that)
+{
+    calledWithObject.emplace(that);
+    return int(CallbackParameterType::ObjectRef);
+}
+Q_DECLARE_JNI_NATIVE_METHOD(callbackWithObjectRef)
+
+static std::optional<QString> calledWithString;
+static int callbackWithString(JNIEnv *, jobject, const QString &string)
+{
+    calledWithString.emplace(string);
+    return int(CallbackParameterType::String);
+}
+Q_DECLARE_JNI_NATIVE_METHOD(callbackWithString)
 
 static std::optional<jbyte> calledWithByte;
 static int callbackWithByte(JNIEnv *, jobject, jbyte value)
 {
     calledWithByte.emplace(value);
-    return 43;
+    return int(CallbackParameterType::Byte);
 }
 Q_DECLARE_JNI_NATIVE_METHOD(callbackWithByte)
 
@@ -1884,7 +1908,7 @@ static std::optional<jbyte> calledWithBoolean;
 static int callbackWithBoolean(JNIEnv *, jobject, bool value)
 {
     calledWithBoolean.emplace(value);
-    return 44;
+    return int(CallbackParameterType::Boolean);
 }
 Q_DECLARE_JNI_NATIVE_METHOD(callbackWithBoolean)
 
@@ -1892,7 +1916,7 @@ static std::optional<int> calledWithInt;
 static int callbackWithInt(JNIEnv *, jobject, int value)
 {
     calledWithInt.emplace(value);
-    return 45;
+    return int(CallbackParameterType::Int);
 }
 Q_DECLARE_JNI_NATIVE_METHOD(callbackWithInt)
 
@@ -1900,40 +1924,26 @@ static std::optional<double> calledWithDouble;
 static int callbackWithDouble(JNIEnv *, jobject, double value)
 {
     calledWithDouble.emplace(value);
-    return 46;
+    return int(CallbackParameterType::Double);
 }
 Q_DECLARE_JNI_NATIVE_METHOD(callbackWithDouble)
 
-enum class CallbackParameterType
-{
-    Object,
-    Byte,
-    Boolean,
-    Int,
-    Double,
-};
-
 void tst_QJniObject::callback_data()
 {
     QTest::addColumn<CallbackParameterType>("parameterType");
-    QTest::addColumn<int>("expectedResult");
 
-    QTest::addRow("Object")     << CallbackParameterType::Object
-                                << callbackWithObject(nullptr, nullptr, {});
-    QTest::addRow("Byte")       << CallbackParameterType::Byte
-                                << callbackWithByte(nullptr, nullptr, {});
-    QTest::addRow("Boolean")    << CallbackParameterType::Boolean
-                                << callbackWithBoolean(nullptr, nullptr, {});
-    QTest::addRow("Int")        << CallbackParameterType::Int
-                                << callbackWithInt(nullptr, nullptr, {});
-    QTest::addRow("Double")     << CallbackParameterType::Double
-                                << callbackWithDouble(nullptr, nullptr, {});
+    QTest::addRow("Object")     << CallbackParameterType::Object;
+    QTest::addRow("ObjectRef")  << CallbackParameterType::ObjectRef;
+    QTest::addRow("String")     << CallbackParameterType::String;
+    QTest::addRow("Byte")       << CallbackParameterType::Byte;
+    QTest::addRow("Boolean")    << CallbackParameterType::Boolean;
+    QTest::addRow("Int")        << CallbackParameterType::Int;
+    QTest::addRow("Double")     << CallbackParameterType::Double;
 }
 
 void tst_QJniObject::callback()
 {
     QFETCH(const CallbackParameterType, parameterType);
-    QFETCH(const int, expectedResult);
 
     TestClass testObject;
     QJniEnvironment env;
@@ -1948,6 +1958,22 @@ void tst_QJniObject::callback()
         QVERIFY(calledWithObject);
         QCOMPARE(calledWithObject.value(), testObject);
         break;
+    case CallbackParameterType::ObjectRef:
+        QVERIFY(env.registerNativeMethods(testObject.objectClass(), {
+            Q_JNI_NATIVE_METHOD(callbackWithObjectRef)
+        }));
+        result = testObject.callMethod<int>("callMeBackWithObjectRef", testObject);
+        QVERIFY(calledWithObject);
+        QCOMPARE(calledWithObject.value(), testObject);
+        break;
+    case CallbackParameterType::String:
+        QVERIFY(env.registerNativeMethods(testObject.objectClass(), {
+            Q_JNI_NATIVE_METHOD(callbackWithString)
+        }));
+        result = testObject.callMethod<int>("callMeBackWithString", QString::number(123));
+        QVERIFY(calledWithString);
+        QCOMPARE(calledWithString.value(), "123");
+        break;
     case CallbackParameterType::Byte:
         QVERIFY(env.registerNativeMethods(testObject.objectClass(), {
             Q_JNI_NATIVE_METHOD(callbackWithByte)
@@ -1981,7 +2007,7 @@ void tst_QJniObject::callback()
         QCOMPARE(calledWithDouble.value(), 1.2345);
         break;
     }
-    QCOMPARE(result, expectedResult);
+    QCOMPARE(result, int(parameterType));
 }
 
 QTEST_MAIN(tst_QJniObject)
diff --git a/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp b/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp
index 58fd6790c1..c332d6634a 100644
--- a/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp
+++ b/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp
@@ -161,6 +161,15 @@ Q_DECLARE_JNI_NATIVE_METHOD(nativeFunction)
 
 static_assert(QtJniTypes::nativeMethodSignature(nativeFunction) == "(ILjava/lang/String;J)Z");
 
+static QString nativeFunctionStrings(JNIEnv *, jclass, const QString &, const QtJniTypes::String &)
+{
+    return QString();
+}
+Q_DECLARE_JNI_NATIVE_METHOD(nativeFunctionStrings)
+
+static_assert(QtJniTypes::nativeMethodSignature(nativeFunctionStrings)
+                == "(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;");
+
 static int forwardDeclaredNativeFunction(JNIEnv *, jobject, bool);
 Q_DECLARE_JNI_NATIVE_METHOD(forwardDeclaredNativeFunction)
 static int forwardDeclaredNativeFunction(JNIEnv *, jobject, bool) { return 0; }