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 <assam.boudjelthia@qt.io>

5 files changed, 74 insertions, 9 deletions

diff --git a/src/corelib/kernel/qjniarray.h b/src/corelib/kernel/qjniarray.h
index f2f4dba92e..862bd71baf 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<Container>::value_type;
         if constexpr (std::disjunction_v<std::is_same<ElementType, jobject>,
                                          std::is_same<ElementType, QJniObject>,
+                                         std::is_same<ElementType, QString>,
                                          std::is_base_of<QtJniTypes::JObjectBase, ElementType>
                                         >) {
             return makeObjectArray(std::forward<Container>(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<jobject>, std::remove_pointer_t<T>>) {
+            // jstring, jclass etc
+            return static_cast<T>(env->GetObjectArrayElement(object<jobjectArray>(), i));
         } else {
             T res = {};
             if constexpr (std::is_same_v<T, jbyte>)
@@ -305,6 +309,12 @@ public:
             for (auto element : *this)
                 res.append(element);
             return res;
+        } else if constexpr (std::is_same_v<T, jstring>) {
+            QStringList res;
+            res.reserve(size());
+            for (auto element : *this)
+                res.append(QJniObject(element).toString());
+            return res;
         } else if constexpr (std::is_same_v<T, jbyte>) {
             const qsizetype bytecount = size();
             QByteArray res(bytecount, Qt::Initialization::Uninitialized);
@@ -364,8 +374,12 @@ template <typename List>
 auto QJniArrayBase::makeObjectArray(List &&list)
 {
     using ElementType = typename q20::remove_cvref_t<List>::value_type;
+    using ResultType = QJniArray<decltype(std::declval<QJniObject::LocalFrame<>>().convertToJni(
+                                    std::declval<ElementType>()))
+                                >;
+
     if (list.isEmpty())
-        return QJniArray<jobject>();
+        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_same<ElementType, QJniObject>,
                                      std::is_base_of<QtJniTypes::JObjectBase, ElementType>>) {
         elementClass = list.first().objectClass();
+    } else if constexpr (std::is_same_v<ElementType, QString>) {
+        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<jobject>();
-    for (size_type i = 0; i < length; ++i) {
-        jobject object;
-        if constexpr (std::is_same_v<ElementType, QJniObject>)
-            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<jobject>(localArray);
+    if (i)
+        env->PopLocalFrame(nullptr);
+    return ResultType(localArray);
 }
 
 
diff --git a/src/corelib/kernel/qjniobject.h b/src/corelib/kernel/qjniobject.h
index 178a4b7faa..7c6eefc082 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 <typename ...Args>
     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 55c9871157..10c8b016fc 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<double>, QList<double>);
 
 VERIFY_RETURN_FOR_TYPE(QString, QString);
 
+void tst_QJniArray::construct()
+{
+    {
+        QStringList strings;
+        for (int i = 0; i < 10000; ++i)
+            strings << QString::number(i);
+        QJniArray<QString> list(strings);
+        QCOMPARE(list.size(), 10000);
+    }
+}
+
 void tst_QJniArray::size()
 {
     QJniArray<jint> 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 386c6aa5c8..48b456a4a3 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 b462a69bef..29f6b04f09 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<TestClass> calledWithObject;
@@ -1946,6 +1947,14 @@ static int callbackWithQList(JNIEnv *, jobject, const QList<double> &value)
 }
 Q_DECLARE_JNI_NATIVE_METHOD(callbackWithQList)
 
+static std::optional<QStringList> 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<CallbackParameterType>("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<int>("callMeBackWithStringList", strings);
+        QVERIFY(calledWithStringList);
+        QCOMPARE(calledWithStringList.value(), strings);
+        break;
+    }
     }
     QCOMPARE(result, int(parameterType));
 }