Fix handling of QVariant properties

This required adding additional checks for QVariants that contain custom
types like PODs.

Change-Id: I5d77a07741000975ce8c5936688eab1af068dd6f
Fixes: QTBUG-75056
Fixes: QTBUG-74084
Reviewed-by: Michael Brasser <michael.brasser@live.com>

12 files changed, 110 insertions, 35 deletions

diff --git a/src/remoteobjects/qremoteobjectdynamicreplica.cpp b/src/remoteobjects/qremoteobjectdynamicreplica.cpp
index de22ff3..72f0c69 100644
--- a/src/remoteobjects/qremoteobjectdynamicreplica.cpp
+++ b/src/remoteobjects/qremoteobjectdynamicreplica.cpp
@@ -121,7 +121,7 @@ int QRemoteObjectDynamicReplica::qt_metacall(QMetaObject::Call call, int id, voi
 {
     static const bool debugArgs = qEnvironmentVariableIsSet("QT_REMOTEOBJECT_DEBUG_ARGUMENTS");
 
-    auto impl = qSharedPointerCast<QRemoteObjectReplicaImplementation>(d_impl);
+    auto impl = qSharedPointerCast<QConnectedReplicaImplementation>(d_impl);
 
     int saved_id = id;
     id = QRemoteObjectReplica::qt_metacall(call, id, argv);
@@ -134,12 +134,19 @@ int QRemoteObjectDynamicReplica::qt_metacall(QMetaObject::Call call, int id, voi
 
         if (call == QMetaObject::WriteProperty) {
             QVariantList args;
-            args << QVariant(mp.userType(), argv[0]);
+            if (mp.userType() == QMetaType::QVariant)
+                args << *reinterpret_cast<QVariant*>(argv[0]);
+            else
+                args << QVariant(mp.userType(), argv[0]);
             QRemoteObjectReplica::send(QMetaObject::WriteProperty, saved_id, args);
         } else {
-            const QVariant value = propAsVariant(id);
-            QMetaType::destruct(mp.userType(), argv[0]);
-            QMetaType::construct(mp.userType(), argv[0], value.data());
+            if (mp.userType() == QMetaType::QVariant)
+                *reinterpret_cast<QVariant*>(argv[0]) = impl->m_propertyStorage[id];
+            else {
+                const QVariant value = propAsVariant(id);
+                QMetaType::destruct(mp.userType(), argv[0]);
+                QMetaType::construct(mp.userType(), argv[0], value.data());
+            }
             const bool readStatus = true;
             // Caller supports QVariant returns? Then we can also report errors
             // by storing an invalid variant.
diff --git a/src/remoteobjects/qremoteobjectnode.cpp b/src/remoteobjects/qremoteobjectnode.cpp
index 8e7b2c9..977d06f 100644
--- a/src/remoteobjects/qremoteobjectnode.cpp
+++ b/src/remoteobjects/qremoteobjectnode.cpp
@@ -703,6 +703,25 @@ static void registerAllGadgets(IoDeviceBase *connection, QRemoteObjectPackets::G
         registerGadget(connection, gadgets, gadgets.constBegin().key());
 }
 
+static void parseGadgets(IoDeviceBase *connection, QDataStream &in, quint32 numGadgets = 1)
+{
+    QRemoteObjectPackets::GadgetsData gadgets;
+    for (quint32 i = 0; i < numGadgets; ++i) {
+        QByteArray type;
+        in >> type;
+        quint32 numProperties;
+        in >> numProperties;
+        auto &properties = gadgets[type];
+        for (quint32 p = 0; p < numProperties; ++p) {
+            QRemoteObjectPackets::GadgetProperty prop;
+            in >> prop.name;
+            in >> prop.type;
+            properties.push_back(prop);
+        }
+    }
+    registerAllGadgets(connection, gadgets);
+}
+
 QMetaObject *QRemoteObjectMetaObjectManager::addDynamicType(IoDeviceBase *connection, QDataStream &in)
 {
     QMetaObjectBuilder builder;
@@ -745,21 +764,7 @@ QMetaObject *QRemoteObjectMetaObjectManager::addDynamicType(IoDeviceBase *connec
         }
     }
     in >> numGadgets;
-    QRemoteObjectPackets::GadgetsData gadgets;
-    for (quint32 i = 0; i < numGadgets; ++i) {
-        QByteArray type;
-        in >> type;
-        quint32 numProperties;
-        in >> numProperties;
-        auto &properties = gadgets[type];
-        for (quint32 p = 0; p < numProperties; ++p) {
-            QRemoteObjectPackets::GadgetProperty prop;
-            in >> prop.name;
-            in >> prop.type;
-            properties.push_back(prop);
-        }
-    }
-    registerAllGadgets(connection, gadgets);
+    parseGadgets(connection, in, numGadgets);
 
     int curIndex = 0;
 
@@ -1184,6 +1189,14 @@ void QRemoteObjectNodePrivate::onClientRead(QObject *obj)
                     rep->setProperty(propertyIndex, handlePointerToQObjectProperty(connectedRep, propertyIndex, rxValue));
                 else {
                     const QMetaProperty property = rep->m_metaObject->property(propertyIndex + rep->m_metaObject->propertyOffset());
+                    if (property.userType() == QMetaType::QVariant && rxValue.canConvert<QRO_>()) {
+                        // This is a type that requires registration
+                        QRO_ typeInfo = rxValue.value<QRO_>();
+                        QDataStream in(typeInfo.classDefinition);
+                        parseGadgets(connection, in);
+                        QDataStream ds(typeInfo.parameters);
+                        ds >> rxValue;
+                    }
                     rep->setProperty(propertyIndex, deserializedProperty(rxValue, property));
                 }
             } else { //replica has been deleted, remove from list
@@ -1203,8 +1216,12 @@ void QRemoteObjectNodePrivate::onClientRead(QObject *obj)
                 QVarLengthArray<void*, 10> param(rxArgs.size() + 1);
                 param[0] = null.data(); //Never a return value
                 if (rxArgs.size()) {
+                    auto signal = rep->m_metaObject->method(index+rep->m_signalOffset);
                     for (int i = 0; i < rxArgs.size(); i++) {
-                        param[i + 1] = const_cast<void *>(rxArgs[i].data());
+                        if (signal.parameterType(i) == QMetaType::QVariant)
+                            param[i + 1] = const_cast<void*>(reinterpret_cast<const void*>(&rxArgs.at(i)));
+                        else
+                            param[i + 1] = const_cast<void *>(rxArgs.at(i).data());
                     }
                 } else if (propertyIndex != -1) {
                     param.resize(2);
diff --git a/src/remoteobjects/qremoteobjectpacket.cpp b/src/remoteobjects/qremoteobjectpacket.cpp
index 58c8a2d..bb793d1 100644
--- a/src/remoteobjects/qremoteobjectpacket.cpp
+++ b/src/remoteobjects/qremoteobjectpacket.cpp
@@ -60,7 +60,9 @@ void serializeProperty(QDataStream &ds, const QRemoteObjectSourceBase *source, i
     const QVariant value = property.read(target);
     if (property.isEnumType()) {
         ds << QVariant::fromValue<qint32>(value.toInt());
-    } else if (QMetaType::typeFlags(property.userType()).testFlag(QMetaType::PointerToQObject)) {
+        return;
+    }
+    if (QMetaType::typeFlags(property.userType()).testFlag(QMetaType::PointerToQObject)) {
         auto const childSource = source->m_children.value(internalIndex);
         auto valueAsPointerToQObject = qvariant_cast<QObject *>(value);
         if (childSource->m_object != valueAsPointerToQObject)
@@ -82,9 +84,20 @@ void serializeProperty(QDataStream &ds, const QRemoteObjectSourceBase *source, i
         for (int internalIndex = 0; internalIndex < propertyCount; ++internalIndex)
             serializeProperty(params, childSource, internalIndex);
         ds << qro.parameters;
-    } else {
-        ds << value; // return original
+        return;
+    }
+    if (source->d->isDynamic && property.userType() == QMetaType::QVariant &&
+        QMetaType::typeFlags(value.userType()).testFlag(QMetaType::IsGadget)) {
+        const auto typeName = QString::fromLatin1(QMetaType::typeName(value.userType()));
+        if (!source->d->sentTypes.contains(typeName)) {
+            QRO_ qro(value);
+            ds << QVariant::fromValue<QRO_>(qro);
+            ds << qro.parameters;
+            source->d->sentTypes.insert(typeName);
+            return;
+        }
     }
+    ds << value; // return original
 }
 
 QVariant deserializedProperty(const QVariant &in, const QMetaProperty &property)
@@ -473,10 +486,29 @@ QRO_::QRO_(QRemoteObjectSourceBase *source)
     , parameters()
 {}
 
+QRO_::QRO_(const QVariant &value)
+    : type(ObjectType::GADGET)
+    , isNull(false)
+{
+    auto meta = QMetaType::metaObjectForType(value.userType());
+    QDataStream out(&classDefinition, QIODevice::WriteOnly);
+    const int numProperties = meta->propertyCount();
+    const auto typeName = QByteArray(QMetaType::typeName(value.userType()));
+    out << typeName;
+    out << numProperties;
+    for (int i = 0; i < numProperties; ++i) {
+        const auto property = meta->property(i);
+        out << property.name();
+        out << property.typeName();
+    }
+    QDataStream ds(&parameters, QIODevice::WriteOnly);
+    ds << value;
+}
+
 QDataStream &operator<<(QDataStream &stream, const QRO_ &info)
 {
     stream << info.name << info.typeName << (quint8)(info.type) << info.classDefinition << info.isNull;
-    qCDebug(QT_REMOTEOBJECT) << "Serializing QRO_" << info.name << info.typeName << (info.type == ObjectType::CLASS ? "Class" : "Model")
+    qCDebug(QT_REMOTEOBJECT) << "Serializing QRO_" << info.name << info.typeName << (info.type == ObjectType::CLASS ? "Class" : info.type == ObjectType::MODEL ? "Model" : "Gadget")
                              << (info.isNull ? "nullptr" : "valid pointer") << (info.classDefinition.isEmpty() ? "no definitions" : "with definitions");
     // info.parameters will be filled in by serializeProperty
     return stream;
diff --git a/src/remoteobjects/qremoteobjectpacket_p.h b/src/remoteobjects/qremoteobjectpacket_p.h
index 8082e2b..542f998 100644
--- a/src/remoteobjects/qremoteobjectpacket_p.h
+++ b/src/remoteobjects/qremoteobjectpacket_p.h
@@ -99,7 +99,7 @@ inline QDataStream& operator>>(QDataStream &stream, ObjectInfo &info)
 
 typedef QVector<ObjectInfo> ObjectInfoList;
 
-enum class ObjectType : quint8 { CLASS, MODEL };
+enum class ObjectType : quint8 { CLASS, MODEL, GADGET };
 
 // Use a short name, as QVariant::save writes the name every time a qvariant of
 // this type is serialized
@@ -108,6 +108,7 @@ class QRO_
 public:
     QRO_() : type(ObjectType::CLASS), isNull(true) {}
     explicit QRO_(QRemoteObjectSourceBase *source);
+    explicit QRO_(const QVariant &value);
     QString name, typeName;
     ObjectType type;
     bool isNull;
diff --git a/src/remoteobjects/qremoteobjectsource.cpp b/src/remoteobjects/qremoteobjectsource.cpp
index 015cda7..8b8b280 100644
--- a/src/remoteobjects/qremoteobjectsource.cpp
+++ b/src/remoteobjects/qremoteobjectsource.cpp
@@ -365,7 +365,7 @@ void QRemoteObjectSourceBase::handleMetaCall(int index, QMetaObject::Call call,
     }
 
     qCDebug(QT_REMOTEOBJECT) << "# Listeners" << d->m_listeners.length();
-    qCDebug(QT_REMOTEOBJECT) << "Invoke args:" << m_object << call << index << marshalArgs(index, a);
+    qCDebug(QT_REMOTEOBJECT) << "Invoke args:" << m_object << call << index << *marshalArgs(index, a);
 
     serializeInvokePacket(d->m_packet, name(), call, index, *marshalArgs(index, a), -1, propertyIndex);
     d->m_packet.baseAddress = 0;
diff --git a/tests/auto/proxy_multiprocess/client/client.pro b/tests/auto/proxy_multiprocess/client/client.pro
index ce435c2..d6c31db 100644
--- a/tests/auto/proxy_multiprocess/client/client.pro
+++ b/tests/auto/proxy_multiprocess/client/client.pro
@@ -12,5 +12,6 @@ REPC_REPLICA = ../subclass.rep
 SOURCES += main.cpp \
 
 HEADERS += \
+    ../shared.h
 
 INCLUDEPATH += $$PWD
diff --git a/tests/auto/proxy_multiprocess/client/main.cpp b/tests/auto/proxy_multiprocess/client/main.cpp
index 05f31ea..986d6d6 100644
--- a/tests/auto/proxy_multiprocess/client/main.cpp
+++ b/tests/auto/proxy_multiprocess/client/main.cpp
@@ -60,10 +60,13 @@ private Q_SLOTS:
             QCOMPARE(m_rep->subClass()->i(), initialI);
             QVERIFY(m_rep->tracks() != nullptr);
             QVERIFY(tracksSpy.count() || tracksSpy.wait());
+            QCOMPARE(m_rep->variant(), QVariant::fromValue(42.0f));
         } else {
             QVERIFY(m_rep->subClass() == nullptr);
             QVERIFY(m_rep->tracks() == nullptr);
+            QCOMPARE(m_rep->variant(), QVariant());
         }
+
         qDebug() << "Verified expected initial states, sending start.";
         auto reply = m_rep->start();
         QVERIFY(reply.waitForFinished());
@@ -74,6 +77,7 @@ private Q_SLOTS:
         QCOMPARE(m_rep->subClass()->myPOD(), updatedValue);
         QCOMPARE(m_rep->subClass()->i(), updatedI);
         QVERIFY(m_rep->tracks() != nullptr);
+        QCOMPARE(m_rep->variant(), QVariant::fromValue(podValue));
         qDebug() << "Verified expected final states, cleaning up.";
     }
 
diff --git a/tests/auto/proxy_multiprocess/server/main.cpp b/tests/auto/proxy_multiprocess/server/main.cpp
index af94879..72f062c 100644
--- a/tests/auto/proxy_multiprocess/server/main.cpp
+++ b/tests/auto/proxy_multiprocess/server/main.cpp
@@ -55,6 +55,7 @@ private Q_SLOTS:
         if (objectMode == QLatin1Literal("ObjectPointer")) {
             parent.setSubClass(&subclass);
             parent.setTracks(&model);
+            parent.setVariant(QVariant::fromValue(42.0f));
         }
 
         if (templated)
@@ -79,6 +80,7 @@ private Q_SLOTS:
         parent.setSubClass(&updatedSubclass);
         if (objectMode == QLatin1Literal("NullPointer"))
             parent.setTracks(&model);
+        parent.setVariant(QVariant::fromValue(podValue));
 
         emit parent.advance();
 
diff --git a/tests/auto/proxy_multiprocess/server/server.pro b/tests/auto/proxy_multiprocess/server/server.pro
index 7167fda..24c9a13 100644
--- a/tests/auto/proxy_multiprocess/server/server.pro
+++ b/tests/auto/proxy_multiprocess/server/server.pro
@@ -13,6 +13,7 @@ SOURCES += main.cpp \
     mytestserver.cpp
 
 HEADERS += \
+    ../shared.h \
     mytestserver.h \
     $$OUT_PWD/rep_subclass_source.h
 
diff --git a/tests/auto/proxy_multiprocess/shared.h b/tests/auto/proxy_multiprocess/shared.h
index 8aa2dfe..c3bd763 100644
--- a/tests/auto/proxy_multiprocess/shared.h
+++ b/tests/auto/proxy_multiprocess/shared.h
@@ -1,5 +1,6 @@
 const MyPOD initialValue(42, 3.14, QStringLiteral("SubClass"));
 const MyPOD updatedValue(-1, 123.456, QStringLiteral("Updated"));
+const VariantPOD podValue(10, 11);
 const int initialI = 100;
 const int updatedI = 200;
 
diff --git a/tests/auto/proxy_multiprocess/subclass.rep b/tests/auto/proxy_multiprocess/subclass.rep
index 5d3f9e9..e578b8f 100644
--- a/tests/auto/proxy_multiprocess/subclass.rep
+++ b/tests/auto/proxy_multiprocess/subclass.rep
@@ -1,4 +1,5 @@
 POD MyPOD(int i, float f, QString s)
+POD VariantPOD(int i, int j)
 
 class SubClass
 {
@@ -9,6 +10,7 @@ class SubClass
 class ParentClass
 {
     PROP(bool started = false)
+    PROP(QVariant variant)
 
     SLOT(bool start())
     SLOT(bool quit())
diff --git a/tools/repc/repcodegenerator.cpp b/tools/repc/repcodegenerator.cpp
index 0c3ba11..3d55fa9 100644
--- a/tools/repc/repcodegenerator.cpp
+++ b/tools/repc/repcodegenerator.cpp
@@ -799,14 +799,21 @@ void RepCodeGenerator::generateClass(Mode mode, QTextStream &out, const ASTClass
         int i = 0;
         Q_FOREACH (const ASTProperty &property, astClass.properties) {
             auto type = typeForMode(property, mode);
-            out << "    " << type << " " << property.name << "() const" << endl;
-            out << "    {" << endl;
-            out << "        const QVariant variant = propAsVariant(" << i << ");" << endl;
-            out << "        if (!variant.canConvert<" << type << ">()) {" << endl;
-            out << "            qWarning() << \"QtRO cannot convert the property " << property.name << " to type " << type << "\";" << endl;
-            out << "        }" << endl;
-            out << "        return variant.value<" << type << " >();" << endl;
-            out << "    }" << endl;
+            if (type == QLatin1String("QVariant")) {
+                out << "    " << type << " " << property.name << "() const" << endl;
+                out << "    {" << endl;
+                out << "        return propAsVariant(" << i << ");" << endl;
+                out << "    }" << endl;
+            } else {
+                out << "    " << type << " " << property.name << "() const" << endl;
+                out << "    {" << endl;
+                out << "        const QVariant variant = propAsVariant(" << i << ");" << endl;
+                out << "        if (!variant.canConvert<" << type << ">()) {" << endl;
+                out << "            qWarning() << \"QtRO cannot convert the property " << property.name << " to type " << type << "\";" << endl;
+                out << "        }" << endl;
+                out << "        return variant.value<" << type << " >();" << endl;
+                out << "    }" << endl;
+            }
             i++;
             if (property.modifier == ASTProperty::ReadWrite) {
                 out << "" << endl;