From a730c426084f18d3890e5fc4462e535e72dbadc6 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Wed, 31 Jan 2024 10:30:55 +0100 Subject: [PATCH] JNI: Support QStringList as a parameter of native functions Since we support QString and QList specializations, we should also support QStringList directly. That's a bit more involved as we need to specialize the code path for QString, which is not convertible to or from jobject. But once we have mapped the type to jstring it follows the implementation for lists of objects. We now need to generate temporary local references when converting a QString to a jstring, so manage a local frame. We do so explicitly in chunks of 100 local references. Change-Id: I7ae5cf7d0ba0099992c36f3677980c346526804b Reviewed-by: Assam Boudjelthia --- src/corelib/kernel/qjniarray.h | 44 +++++++++++++++---- src/corelib/kernel/qjniobject.h | 2 + .../kernel/qjniarray/tst_qjniarray.cpp | 12 +++++ .../testdata/QtJniObjectTestClass.java | 5 +++ .../kernel/qjniobject/tst_qjniobject.cpp | 20 +++++++++ 5 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/corelib/kernel/qjniarray.h b/src/corelib/kernel/qjniarray.h index f2f4dba92e7..862bd71bafc 100644 --- a/src/corelib/kernel/qjniarray.h +++ b/src/corelib/kernel/qjniarray.h @@ -127,6 +127,7 @@ public: using ElementType = typename std::remove_reference_t::value_type; if constexpr (std::disjunction_v, std::is_same, + std::is_same, std::is_base_of >) { return makeObjectArray(std::forward(container)); @@ -275,6 +276,9 @@ public: return T::fromLocalRef(element); else return T{element}; + } else if constexpr (std::is_base_of_v, std::remove_pointer_t>) { + // jstring, jclass etc + return static_cast(env->GetObjectArrayElement(object(), i)); } else { T res = {}; if constexpr (std::is_same_v) @@ -305,6 +309,12 @@ public: for (auto element : *this) res.append(element); return res; + } else if constexpr (std::is_same_v) { + QStringList res; + res.reserve(size()); + for (auto element : *this) + res.append(QJniObject(element).toString()); + return res; } else if constexpr (std::is_same_v) { const qsizetype bytecount = size(); QByteArray res(bytecount, Qt::Initialization::Uninitialized); @@ -364,8 +374,12 @@ template auto QJniArrayBase::makeObjectArray(List &&list) { using ElementType = typename q20::remove_cvref_t::value_type; + using ResultType = QJniArray>().convertToJni( + std::declval())) + >; + if (list.isEmpty()) - return QJniArray(); + return ResultType(); JNIEnv *env = QJniEnvironment::getJniEnv(); const size_type length = size_type(list.size()); @@ -375,21 +389,33 @@ auto QJniArrayBase::makeObjectArray(List &&list) if constexpr (std::disjunction_v, std::is_base_of>) { elementClass = list.first().objectClass(); + } else if constexpr (std::is_same_v) { + elementClass = env->FindClass("java/lang/String"); } else { elementClass = env->GetObjectClass(list.first()); } auto localArray = env->NewObjectArray(length, elementClass, nullptr); if (QJniEnvironment::checkAndClearExceptions(env)) - return QJniArray(); - for (size_type i = 0; i < length; ++i) { - jobject object; - if constexpr (std::is_same_v) - object = list.at(i).object(); - else - object = list.at(i); + return ResultType(); + + // explicitly manage the frame for local references in chunks of 100 + QJniObject::LocalFrame frame(env); + constexpr jint frameCapacity = 100; + qsizetype i = 0; + for (const auto &element : std::as_const(list)) { + if (i % frameCapacity == 0) { + if (i) + env->PopLocalFrame(nullptr); + if (env->PushLocalFrame(frameCapacity) != 0) + return ResultType{}; + } + jobject object = frame.convertToJni(element); env->SetObjectArrayElement(localArray, i, object); + ++i; } - return QJniArray(localArray); + if (i) + env->PopLocalFrame(nullptr); + return ResultType(localArray); } diff --git a/src/corelib/kernel/qjniobject.h b/src/corelib/kernel/qjniobject.h index 178a4b7faac..7c6eefc0825 100644 --- a/src/corelib/kernel/qjniobject.h +++ b/src/corelib/kernel/qjniobject.h @@ -16,6 +16,8 @@ class QJniObjectPrivate; class Q_CORE_EXPORT QJniObject { + friend class QJniArrayBase; + template struct LocalFrame { mutable JNIEnv *env; diff --git a/tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp b/tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp index 55c98711576..10c8b016fc6 100644 --- a/tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp +++ b/tests/auto/corelib/kernel/qjniarray/tst_qjniarray.cpp @@ -16,6 +16,7 @@ public: tst_QJniArray() = default; private slots: + void construct(); void size(); void operators(); }; @@ -74,6 +75,17 @@ VERIFY_RETURN_FOR_TYPE(QList, QList); VERIFY_RETURN_FOR_TYPE(QString, QString); +void tst_QJniArray::construct() +{ + { + QStringList strings; + for (int i = 0; i < 10000; ++i) + strings << QString::number(i); + QJniArray list(strings); + QCOMPARE(list.size(), 10000); + } +} + void tst_QJniArray::size() { QJniArray array; 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 386c6aa5c8c..48b456a4a31 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 @@ -283,6 +283,7 @@ public class QtJniObjectTestClass native public int callbackWithDouble(double value); native public int callbackWithJniArray(double[] value); native public int callbackWithQList(double[] value); + native public int callbackWithStringList(String[] value); public int callMeBackWithObject(QtJniObjectTestClass that) { @@ -326,4 +327,8 @@ public class QtJniObjectTestClass { return callbackWithQList(value); } + public int callMeBackWithStringList(String[] value) + { + return callbackWithStringList(value); + } } diff --git a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp index b462a69befa..29f6b04f09a 100644 --- a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp +++ b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp @@ -1874,6 +1874,7 @@ enum class CallbackParameterType Double, JniArray, QList, + QStringList, }; static std::optional calledWithObject; @@ -1946,6 +1947,14 @@ static int callbackWithQList(JNIEnv *, jobject, const QList &value) } Q_DECLARE_JNI_NATIVE_METHOD(callbackWithQList) +static std::optional calledWithStringList; +static int callbackWithStringList(JNIEnv *, jobject, const QStringList &value) +{ + calledWithStringList.emplace(value); + return int(CallbackParameterType::QStringList); +} +Q_DECLARE_JNI_NATIVE_METHOD(callbackWithStringList) + void tst_QJniObject::callback_data() { QTest::addColumn("parameterType"); @@ -1959,6 +1968,7 @@ void tst_QJniObject::callback_data() QTest::addRow("Double") << CallbackParameterType::Double; QTest::addRow("JniArray") << CallbackParameterType::JniArray; QTest::addRow("QList") << CallbackParameterType::QList; + QTest::addRow("QStringList") << CallbackParameterType::QStringList; } void tst_QJniObject::callback() @@ -2045,6 +2055,16 @@ void tst_QJniObject::callback() QCOMPARE(calledWithQList.value(), doubles); break; } + case CallbackParameterType::QStringList: { + QVERIFY(TestClass::registerNativeMethods({ + Q_JNI_NATIVE_METHOD(callbackWithStringList) + })); + const QStringList strings = { "one", "two" }; + result = testObject.callMethod("callMeBackWithStringList", strings); + QVERIFY(calledWithStringList); + QCOMPARE(calledWithStringList.value(), strings); + break; + } } QCOMPARE(result, int(parameterType)); }