JNI API review: use has-a-QJniObject relationship for QtJniTypes types

Don't subclass QJniObject. It's not necessary, and gets us into UB
territory due to QJniObject not having a virtual destructor. Also,
rename the helper class to JObject, as Object is a valid Java class
that one might want to be able to declare explicitly.

Instead, give the declared QtJniTypes types a QJniObject member that
they forward the calls to. That requires some duplication of APIs, but
at the same time makes it unnecessary to explicitly remove the old
QJniObject APIs that we want to ultimately deprecate.

We need to specialize a few more of the conversion routines to handle
such types now, as QJniObject is no longer a base class. To be able to
do that we need to add a base class that we can test for, and that has
the APIs that don't depend on the template parameter.

Since we now need to know about QtJniTypes::JObject(Base) in the
conversion routines that are implemented in qjniobject.h, we have to
move these base types into that header as well. This reduces the
qjnitypes.h header to the macros for declaring types and a few helpers
for native methods, which is perhaps how it should be anyway.

Pick-to: 6.7
Change-Id: If2052a79a108fdb62ca71c88f4fa04d9f5ea2c4b
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

3 files changed, 148 insertions, 175 deletions

diff --git a/src/corelib/kernel/qjniarray.h b/src/corelib/kernel/qjniarray.h
index 4541804725..743314f12b 100644
--- a/src/corelib/kernel/qjniarray.h
+++ b/src/corelib/kernel/qjniarray.h
@@ -204,6 +204,8 @@ public:
             jobject element = env->GetObjectArrayElement(object<jobjectArray>(), i);
             if constexpr (std::is_base_of_v<QJniObject, T>)
                 return QJniObject::fromLocalRef(element);
+            else if constexpr (std::is_base_of_v<QtJniTypes::JObjectBase, T>)
+                return T::fromLocalRef(element);
             else
                 return T{element};
         } else {
diff --git a/src/corelib/kernel/qjniobject.h b/src/corelib/kernel/qjniobject.h
index 681d2bcec0..ee111b49cd 100644
--- a/src/corelib/kernel/qjniobject.h
+++ b/src/corelib/kernel/qjniobject.h
@@ -662,6 +662,144 @@ inline bool operator!=(const QJniObject &obj1, const QJniObject &obj2)
     return !obj1.isSameObject(obj2);
 }
 
+namespace QtJniTypes {
+struct JObjectBase
+{
+    operator QJniObject() const { return m_object; }
+
+    bool isValid() const { return m_object.isValid(); }
+    jclass objectClass() const { return m_object.objectClass(); }
+    QString toString() const { return m_object.toString(); }
+
+    template <typename T = jobject>
+    T object() const {
+        return m_object.object<T>();
+    }
+
+protected:
+    JObjectBase() = default;
+    ~JObjectBase() = default;
+
+    Q_IMPLICIT JObjectBase(jobject object) : m_object(object) {}
+    Q_IMPLICIT JObjectBase(const QJniObject &object) : m_object(object) {}
+    Q_IMPLICIT JObjectBase(QJniObject &&object) noexcept : m_object(std::move(object)) {}
+
+    QJniObject m_object;
+};
+
+template<typename Type>
+class JObject : public JObjectBase
+{
+public:
+    using Class = Type;
+
+    JObject()
+        : JObjectBase{QJniObject(QtJniTypes::Traits<Class>::className())}
+    {}
+    Q_IMPLICIT JObject(jobject object) : JObjectBase(object) {}
+    Q_IMPLICIT JObject(const QJniObject &object) : JObjectBase(object) {}
+    Q_IMPLICIT JObject(QJniObject &&object) noexcept : JObjectBase(std::move(object)) {}
+
+    // base class destructor is protected, so need to provide all SMFs
+    JObject(const JObject &other) = default;
+    JObject(JObject &&other) noexcept = default;
+    JObject &operator=(const JObject &other) = default;
+    JObject &operator=(JObject &&other) noexcept = default;
+
+    ~JObject() = default;
+
+    template<typename Arg, typename ...Args
+            , std::enable_if_t<!std::is_same_v<Arg, JObject>, bool> = true
+            , IfValidSignatureTypes<Arg, Args...> = true
+    >
+    explicit JObject(Arg && arg, Args &&...args)
+        : JObjectBase{QJniObject(QtJniTypes::Traits<Class>::className(),
+                                 std::forward<Arg>(arg), std::forward<Args>(args)...)}
+    {}
+
+    // named constructors avoid ambiguities
+    static Type fromJObject(jobject object) { return Type(object); }
+    template <typename ...Args>
+    static Type construct(Args &&...args) { return Type(std::forward<Args>(args)...); }
+    static Type fromLocalRef(jobject lref) { return Type(QJniObject::fromLocalRef(lref)); }
+
+    static bool registerNativeMethods(std::initializer_list<JNINativeMethod> methods)
+    {
+        QJniEnvironment env;
+        return env.registerNativeMethods<Class>(methods);
+    }
+
+    // public API forwarding to QJniObject, with the implicit Class template parameter
+    template <typename Ret, typename ...Args
+#ifndef Q_QDOC
+        , QtJniTypes::IfValidSignatureTypes<Ret, Args...> = true
+#endif
+    >
+    static auto callStaticMethod(const char *name, Args &&...args)
+    {
+        return QJniObject::callStaticMethod<Class, Ret, Args...>(name,
+                                                                 std::forward<Args>(args)...);
+    }
+    template <typename T
+#ifndef Q_QDOC
+        , QtJniTypes::IfValidFieldType<T> = true
+#endif
+    >
+    static auto getStaticField(const char *field)
+    {
+        return QJniObject::getStaticField<Class, T>(field);
+    }
+    template <typename T
+#ifndef Q_QDOC
+        , QtJniTypes::IfValidFieldType<T> = true
+#endif
+    >
+    static void setStaticField(const char *field, T &&value)
+    {
+        QJniObject::setStaticField<Class, T>(field, std::forward<T>(value));
+    }
+
+    // keep only these overloads, the rest is made private
+    template <typename Ret, typename ...Args
+#ifndef Q_QDOC
+        , QtJniTypes::IfValidSignatureTypes<Ret, Args...> = true
+#endif
+    >
+    auto callMethod(const char *method, Args &&...args) const
+    {
+        return m_object.callMethod<Ret>(method, std::forward<Args>(args)...);
+    }
+    template <typename T
+#ifndef Q_QDOC
+        , QtJniTypes::IfValidFieldType<T> = true
+#endif
+    >
+    auto getField(const char *fieldName) const
+    {
+        return m_object.getField<T>(fieldName);
+    }
+
+    template <typename T
+#ifndef Q_QDOC
+        , QtJniTypes::IfValidFieldType<T> = true
+#endif
+    >
+    void setField(const char *fieldName, T &&value)
+    {
+        m_object.setField(fieldName, std::forward<T>(value));
+    }
+
+    QByteArray className() const {
+        return QtJniTypes::Traits<Class>::className().data();
+    }
+
+private:
+    friend bool comparesEqual(const JObject &lhs, const JObject &rhs) noexcept
+    { return lhs.m_object == rhs.m_object; }
+    Q_DECLARE_EQUALITY_COMPARABLE_LITERAL_TYPE(JObject);
+};
+}
+
 // This cannot be included earlier as QJniArray is a QJniObject subclass, but it
 // must be included so that we can implement QJniObject::LocalFrame conversion.
 QT_END_NAMESPACE
@@ -681,7 +819,8 @@ auto QJniObject::LocalFrame<Args...>::convertToJni(T &&value)
         using QJniArrayType = decltype(QJniArrayBase::fromContainer(std::forward<T>(value)));
         using ArrayType = decltype(std::declval<QJniArrayType>().arrayObject());
         return newLocalRef<ArrayType>(QJniArrayBase::fromContainer(std::forward<T>(value)).template object<jobject>());
-    } else if constexpr (std::is_base_of_v<QJniObject, Type>) {
+    } else if constexpr (std::is_base_of_v<QJniObject, Type>
+                      || std::is_base_of_v<QtJniTypes::JObjectBase, Type>) {
         return value.object();
     } else {
         return std::forward<T>(value);
@@ -710,6 +849,8 @@ auto QJniObject::LocalFrame<Args...>::convertFromJni(QJniObject &&object)
     } else if constexpr (std::is_base_of_v<QJniObject, Type>
                         && !std::is_same_v<QJniObject, Type>) {
         return T{std::move(object)};
+    } else if constexpr (std::is_base_of_v<QtJniTypes::JObjectBase, Type>) {
+        return T{std::move(object)};
     } else {
         return std::move(object);
     }
diff --git a/src/corelib/kernel/qjnitypes.h b/src/corelib/kernel/qjnitypes.h
index b129afb50b..56af657fa3 100644
--- a/src/corelib/kernel/qjnitypes.h
+++ b/src/corelib/kernel/qjnitypes.h
@@ -11,182 +11,11 @@
 
 QT_BEGIN_NAMESPACE
 
-namespace QtJniTypes
-{
-// A generic base class for specialized QJniObject types, to be used by
-// subclasses via CRTP. It's implicitly convertible to and from jobject, which
-// allows the QJniObject implementation to implicitly pass instances of this
-// type through the variadic argument JNI APIs.
-template<typename Type>
-struct Object : QJniObject
-{
-    using Class = Type;
-    Q_IMPLICIT Object(jobject object) : QJniObject(object) {}
-    Q_IMPLICIT Object(const QJniObject &object) : QJniObject(object) {}
-    Q_IMPLICIT Object(QJniObject &&object) : QJniObject(std::move(object)) {}
-
-    // Compiler-generated copy/move semantics based on QJniObject's shared d-pointer are fine!
-    Object(const Object &other) = default;
-    Object(Object &&other) = default;
-    Object &operator=(const Object &other) = default;
-    Object &operator=(Object &&other) = default;
-
-    // avoid ambiguities with deleted const char * constructor
-    Q_IMPLICIT Object(std::nullptr_t) : QJniObject() {}
-
-    Object()
-        : QJniObject(QtJniTypes::Traits<Class>::className())
-    {}
-
-    template<typename Arg, typename ...Args
-            , std::enable_if_t<!std::is_same_v<Arg, Object>, bool> = true
-            , IfValidSignatureTypes<Arg, Args...> = true
-    >
-    explicit Object(Arg && arg, Args &&...args)
-        : QJniObject(QtJniTypes::Traits<Class>::className(), std::forward<Arg>(arg),
-                                                             std::forward<Args>(args)...)
-    {}
-
-    // named constructors avoid ambiguities
-    static Object fromJObject(jobject object) { return Object(object); }
-    template <typename ...Args>
-    static Object construct(Args &&...args) { return Object{std::forward<Args>(args)...}; }
-
-    static bool registerNativeMethods(std::initializer_list<JNINativeMethod> methods)
-    {
-        QJniEnvironment env;
-        return env.registerNativeMethods<Class>(methods);
-    }
-
-    // public API forwarding to QJniObject, with the implicit Class template parameter
-    template <typename Ret, typename ...Args
-#ifndef Q_QDOC
-        , QtJniTypes::IfValidSignatureTypes<Ret, Args...> = true
-#endif
-    >
-    static auto callStaticMethod(const char *name, Args &&...args)
-    {
-        return QJniObject::callStaticMethod<Class, Ret, Args...>(name,
-                                                                 std::forward<Args>(args)...);
-    }
-    template <typename T
-#ifndef Q_QDOC
-        , QtJniTypes::IfValidFieldType<T> = true
-#endif
-    >
-    static auto getStaticField(const char *field)
-    {
-        return QJniObject::getStaticField<Class, T>(field);
-    }
-    template <typename T
-#ifndef Q_QDOC
-        , QtJniTypes::IfValidFieldType<T> = true
-#endif
-    >
-    static void setStaticField(const char *field, T &&value)
-    {
-        QJniObject::setStaticField<Class, T>(field, std::forward<T>(value));
-    }
-
-    // keep only these overloads, the rest is made private
-    template <typename Ret, typename ...Args
-#ifndef Q_QDOC
-        , QtJniTypes::IfValidSignatureTypes<Ret, Args...> = true
-#endif
-    >
-    auto callMethod(const char *method, Args &&...args) const
-    {
-        return QJniObject::callMethod<Ret>(method, std::forward<Args>(args)...);
-    }
-    template <typename T
-#ifndef Q_QDOC
-        , QtJniTypes::IfValidFieldType<T> = true
-#endif
-    >
-    auto getField(const char *fieldName) const
-    {
-        return QJniObject::getField<T>(fieldName);
-    }
-
-    template <typename T
-#ifndef Q_QDOC
-        , QtJniTypes::IfValidFieldType<T> = true
-#endif
-    >
-    void setField(const char *fieldName, T &&value)
-    {
-        QJniObject::setField(fieldName, std::forward<T>(value));
-    }
-
-private:
-    // The following declutters the API of these types compared to the QJniObject API.
-    // 1) 'Object' methods; the functions we have have return type auto and will return
-    //     the type specified by the template parameter.
-    using QJniObject::callObjectMethod;
-    using QJniObject::callStaticObjectMethod;
-    using QJniObject::getObjectField;
-    using QJniObject::getStaticObjectField;
-
-    // 2) Constructors that take a class name, signature string, or class template argument
-    explicit Object(const char *className) = delete;
-    explicit Object(const char *className, const char *signature, ...) = delete;
-    template<typename ...Args>
-    explicit Object(const char *className, Args &&...args) = delete;
-    explicit Object(jclass clazz, const char *signature, ...) = delete;
-    template<typename Class, typename ...Args>
-    static QJniObject construct(Args &&...args) = delete;
-
-    // 3) Overloads that take a class name/jclass, methodID, signature string, or an
-    //    explicit class template argument
-    template <typename Ret, typename ...Args>
-    auto callMethod(const char *methodName, const char *signature, Args &&...args) const = delete;
-    template <typename Ret, typename ...Args>
-
-    static auto callStaticMethod(const char *className, const char *methodName,
-                                 const char *signature, Args &&...args) = delete;
-    template <typename Ret, typename ...Args>
-    static auto callStaticMethod(jclass clazz, const char *methodName,
-                                 const char *signature, Args &&...args) = delete;
-    template <typename Ret, typename ...Args>
-    static auto callStaticMethod(jclass clazz, jmethodID methodId, Args &&...args) = delete;
-    template <typename Ret, typename ...Args>
-    static auto callStaticMethod(const char *className, const char *methodName,
-                                 Args &&...args) = delete;
-    template <typename Ret, typename ...Args>
-    static auto callStaticMethod(jclass clazz, const char *methodName, Args &&...args) = delete;
-    template <typename Klass, typename Ret, typename ...Args>
-    static auto callStaticMethod(const char *methodName, Args &&...args) = delete;
-
-    template <typename T>
-    static auto getStaticField(const char *className, const char *fieldName) = delete;
-    template <typename T>
-    static auto getStaticField(jclass clazz, const char *fieldName) = delete;
-    template <typename Klass, typename T>
-    static auto getStaticField(const char *fieldName) = delete;
-
-    template <typename T>
-    void setField(const char *fieldName, const char *signature, T value) = delete;
-    template <typename T>
-    static void setStaticField(const char *className, const char *fieldName, T value) = delete;
-    template <typename T>
-    static void setStaticField(const char *className, const char *fieldName,
-                               const char *signature, T value) = delete;
-    template <typename T>
-    static void setStaticField(jclass clazz, const char *fieldName,
-                               const char *signature, T value) = delete;
-    template <typename T>
-    static void setStaticField(jclass clazz, const char *fieldName, T value) = delete;
-    template <typename Klass, typename T>
-    static void setStaticField(const char *fieldName, T value) = delete;
-};
-
-} // namespace QtJniTypes
-
 #define Q_DECLARE_JNI_TYPE_HELPER(Type)                         \
 namespace QtJniTypes {                                          \
-struct Type : Object<Type>                                      \
+struct Type : JObject<Type>                                     \
 {                                                               \
-    using Object::Object;                                       \
+    using JObject::JObject;                                     \
 };                                                              \
 }                                                               \
 
@@ -246,7 +75,8 @@ template <> struct PromotedType<float> { using Type = double; };
 template <typename Arg>
 struct JNITypeForArgImpl
 {
-    using Type = std::conditional_t<std::is_base_of_v<QJniObject, Arg>,
+    using Type = std::conditional_t<std::disjunction_v<std::is_base_of<QJniObject, Arg>,
+                                                       std::is_base_of<QtJniTypes::JObjectBase, Arg>>,
                                     jobject, typename PromotedType<Arg>::Type>;
     static Arg fromVarArg(Type &&t)
     {