JNI: Fix native functions that take a declared QtJniTypes class

Now that QtJniTypes::Objects are no longer primitive types that are the
same as a jobject, using those types in registered native functions
breaks. JNI will call those function with a jobject on the function
pointer, and lacking any type safety, the call to the registered
function will proceed with a wrong type of object on the stack.

To fix that, register the native function via a proxy that is a variadic
argument function, and unpack the variadic arguments into a list of
typed arguments, using the types we know the user-code function wants.
Then call the function with a tuple of those types using std::apply,
which gives us type safety and implicit conversion for free.

Add a test that exercises this.

Change-Id: I9f980e55d3d13f8fc16c410dc0d17dbdc200cb47
Reviewed-by: Juha Vuolle <juha.vuolle@qt.io>

4 files changed, 129 insertions, 8 deletions

diff --git a/src/corelib/kernel/qjnitypes.h b/src/corelib/kernel/qjnitypes.h
index a3346ec3f4..cfba9d0ac0 100644
--- a/src/corelib/kernel/qjnitypes.h
+++ b/src/corelib/kernel/qjnitypes.h
@@ -218,16 +218,79 @@ struct QtJniTypes::Traits<QtJniTypes::Type> {                   \
     }                                                           \
 };                                                              \
 
+// Macros for native methods
+
+namespace QtJniMethods {
+// 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
+// can only call functions that take jobjects.
+
+// In Var-arg functions, any argument narrower than (unsigned) int or double
+// is promoted to (unsigned) int or double.
+template <typename Arg> struct PromotedType { using Type = Arg; };
+template <> struct PromotedType<bool> { using Type = int; };
+template <> struct PromotedType<char> { using Type = int; };
+template <> struct PromotedType<unsigned char> { using Type = unsigned int; };
+template <> struct PromotedType<short> { using Type = int; };
+template <> struct PromotedType<unsigned short> { using Type = unsigned int; };
+
+// 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>;
+
+// Turn a va_list into a tuple of typed arguments
+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>)... };
+}
+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>)... };
+}
+
+// Get the return type of a function point
+template <typename Ret, typename ...Args>
+auto nativeFunctionReturnType(Ret(*function)(Args...))
+{
+    return function(std::declval<Args>()...);
+}
+
+} // 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(...)                        \
     QT_OVERLOADED_MACRO(QT_DECLARE_JNI_NATIVE_METHOD, __VA_ARGS__) \
 
 #define QT_DECLARE_JNI_NATIVE_METHOD_2(Method, Name)            \
 namespace QtJniMethods {                                        \
+Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method)                      \
 static constexpr auto Method##_signature =                      \
     QtJniTypes::nativeMethodSignature(Method);                  \
 static const JNINativeMethod Method##_method = {                \
     #Name, Method##_signature.data(),                           \
-    reinterpret_cast<void *>(Method)                            \
+    reinterpret_cast<void *>(va_##Method)                       \
 };                                                              \
 }                                                               \
 
@@ -240,9 +303,10 @@ static const JNINativeMethod Method##_method = {                \
     QT_OVERLOADED_MACRO(QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE, __VA_ARGS__)              \
 
 #define QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE_2(Method, Name)                            \
+    Q_DECLARE_JNI_NATIVE_METHOD_HELPER(Method)                                                   \
     static inline constexpr auto Method##_signature = QtJniTypes::nativeMethodSignature(Method); \
     static inline const JNINativeMethod Method##_method = {                                      \
-        #Name, Method##_signature.data(), reinterpret_cast<void *>(Method)                       \
+        #Name, Method##_signature.data(), reinterpret_cast<void *>(va_##Method)                  \
     };
 
 #define QT_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE_1(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 dc55ad0f89..813aaf2bbf 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
@@ -170,5 +170,12 @@ public class QtJniObjectTestClass
     public static double[] staticDoubleArrayMethod()
     { double[] array = { 3.0, 2.0, 1.0 }; return array; }
     public double[] doubleArrayMethod() { return staticDoubleArrayMethod(); }
-}
 
+    // --------------------------------------------------------------------------------------------
+    native public int callbackWithObject(QtJniObjectTestClass that);
+
+    public int callMeBackWithObject(QtJniObjectTestClass that)
+    {
+        return callbackWithObject(that);
+    }
+}
diff --git a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp
index 49a814f122..65c9680d4e 100644
--- a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp
+++ b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp
@@ -10,7 +10,7 @@
 
 using namespace Qt::StringLiterals;
 
-static const char testClassName[] = "org/qtproject/qt/android/testdatapackage/QtJniObjectTestClass";
+static constexpr const char testClassName[] = "org/qtproject/qt/android/testdatapackage/QtJniObjectTestClass";
 Q_DECLARE_JNI_CLASS(QtJniObjectTestClass, testClassName)
 using TestClass = QtJniTypes::QtJniObjectTestClass;
 
@@ -113,6 +113,8 @@ private slots:
     void isClassAvailable();
     void fromLocalRef();
 
+    void callback();
+
     void cleanupTestCase();
 };
 
@@ -1664,6 +1666,31 @@ void tst_QJniObject::fromLocalRef()
         QJniObject o = QJniObject::fromLocalRef(env->FindClass("java/lang/String"));
 }
 
+
+static std::optional<TestClass> calledWithObject;
+static int callbackWithObject(JNIEnv *env, jobject thiz, TestClass that)
+{
+    Q_UNUSED(env);
+    Q_UNUSED(thiz);
+    calledWithObject.emplace(that);
+    return 42;
+}
+
+Q_DECLARE_JNI_NATIVE_METHOD(callbackWithObject)
+
+void tst_QJniObject::callback()
+{
+    TestClass testObject;
+    QJniEnvironment env;
+    QVERIFY(env.registerNativeMethods(testObject.objectClass(), {
+        Q_JNI_NATIVE_METHOD(callbackWithObject)
+    }));
+    int result = testObject.callMethod<int>("callMeBackWithObject", testObject);
+    QVERIFY(calledWithObject);
+    QVERIFY(calledWithObject.value() == testObject);
+    QCOMPARE(result, 42);
+}
+
 QTEST_MAIN(tst_QJniObject)
 
 #include "tst_qjniobject.moc"
diff --git a/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp b/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp
index 0ce2295092..a499b6f2d2 100644
--- a/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp
+++ b/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp
@@ -14,6 +14,9 @@ class tst_QJniTypes : public QObject
 public:
     tst_QJniTypes() = default;
 
+    static void nativeClassMethod(JNIEnv *, jclass, int);
+    Q_DECLARE_JNI_NATIVE_METHOD_IN_CURRENT_SCOPE(nativeClassMethod);
+
 private slots:
     void initTestCase();
     void nativeMethod();
@@ -154,12 +157,32 @@ Q_DECLARE_JNI_NATIVE_METHOD(nativeFunction)
 
 static_assert(QtJniTypes::nativeMethodSignature(nativeFunction) == "(ILjava/lang/String;J)Z");
 
+static int forwardDeclaredNativeFunction(JNIEnv *, jobject, bool);
+Q_DECLARE_JNI_NATIVE_METHOD(forwardDeclaredNativeFunction)
+static int forwardDeclaredNativeFunction(JNIEnv *, jobject, bool) { return 0; }
+static_assert(QtJniTypes::nativeMethodSignature(forwardDeclaredNativeFunction) == "(Z)I");
+
+static_assert(QtJniTypes::nativeMethodSignature(tst_QJniTypes::nativeClassMethod) == "(I)V");
+void tst_QJniTypes::nativeClassMethod(JNIEnv *, jclass, int) {}
+
 void tst_QJniTypes::nativeMethod()
 {
-    const auto method = Q_JNI_NATIVE_METHOD(nativeFunction);
-    QVERIFY(method.fnPtr == nativeFunction);
-    QCOMPARE(method.name, "nativeFunction");
-    QCOMPARE(method.signature, "(ILjava/lang/String;J)Z");
+    {
+        const auto method = Q_JNI_NATIVE_METHOD(nativeFunction);
+        QVERIFY(method.fnPtr == QtJniMethods::va_nativeFunction);
+        QCOMPARE(method.name, "nativeFunction");
+        QCOMPARE(method.signature, "(ILjava/lang/String;J)Z");
+    }
+
+    {
+        const auto method = Q_JNI_NATIVE_METHOD(forwardDeclaredNativeFunction);
+        QVERIFY(method.fnPtr == QtJniMethods::va_forwardDeclaredNativeFunction);
+    }
+
+    {
+        const auto method = Q_JNI_NATIVE_SCOPED_METHOD(nativeClassMethod, tst_QJniTypes);
+        QVERIFY(method.fnPtr == va_nativeClassMethod);
+    }
 }
 
 void tst_QJniTypes::construct()