Make sure class enums work dynamically

This includes making sure typed class enums get the right size.  This also
fixes other enums that were incorrectly tested as variants.

Change-Id: Ie330ceee91fe4192a11405a4e231f1750247cd5e
Reviewed-by: Michael Brasser <michael.brasser@live.com>
 

7 files changed, 87 insertions, 25 deletions

diff --git a/src/remoteobjects/qremoteobjectdynamicreplica.cpp b/src/remoteobjects/qremoteobjectdynamicreplica.cpp
index f778c3a..a004593 100644
--- a/src/remoteobjects/qremoteobjectdynamicreplica.cpp
+++ b/src/remoteobjects/qremoteobjectdynamicreplica.cpp
@@ -184,10 +184,21 @@ int QRemoteObjectDynamicReplica::qt_metacall(QMetaObject::Call call, int id, voi
             QVariantList args;
             args.reserve(typeSize);
             for (int i = 0; i < typeSize; ++i) {
-                if (impl->m_metaObject->indexOfEnumerator(types[i].constData()) != -1)
-                    args.push_back(QVariant(QMetaType::Int, argv[i + 1]));
-                else
-                    args.push_back(QVariant(QMetaType::type(types[i].constData()), argv[i + 1]));
+                const int type = QMetaType::type(types[i].constData());
+                if (impl->m_metaObject->indexOfEnumerator(types[i].constData()) != -1) {
+                    const auto size = QMetaType(type).sizeOf();
+                    switch (size) {
+                    case 1: args.push_back(QVariant(QMetaType::Char, argv[i + 1])); break;
+                    case 2: args.push_back(QVariant(QMetaType::Short, argv[i + 1])); break;
+                    case 4: args.push_back(QVariant(QMetaType::Int, argv[i + 1])); break;
+                    // Qt currently only supports enum values of 4 or less bytes (QMetaEnum value(index) returns int)
+//                    case 8: args.push_back(QVariant(QMetaType::Int, argv[i + 1])); break;
+                    default:
+                        qWarning() << "Invalid enum detected (Dynamic Replica)" << QMetaType::typeName(type) << "with size" << size;
+                        args.push_back(QVariant(QMetaType::Int, argv[i + 1])); break;
+                    }
+                } else
+                    args.push_back(QVariant(type, argv[i + 1]));
             }
 
             if (debugArgs) {
diff --git a/src/remoteobjects/qremoteobjectnode.cpp b/src/remoteobjects/qremoteobjectnode.cpp
index 3353de1..127ed9d 100644
--- a/src/remoteobjects/qremoteobjectnode.cpp
+++ b/src/remoteobjects/qremoteobjectnode.cpp
@@ -703,7 +703,7 @@ struct EnumPair {
 struct EnumData {
     QByteArray name;
     bool isFlag, isScoped;
-    quint32 keyCount;
+    quint32 keyCount, size;
     QVector<EnumPair> values;
 };
 
@@ -727,9 +727,30 @@ static void registerEnum(const QByteArray &name, const QMetaObject *meta, int si
     if (QMetaType::isRegistered(QMetaType::type(name)))
         return;
     static const auto flags = QMetaType::IsEnumeration | QMetaType::NeedsConstruction | QMetaType::NeedsDestruction;
-    int id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor<qint32>,
-                                     &EnumConstructor<qint32>, size, flags, meta);
-    qCDebug(QT_REMOTEOBJECT) << "Registering new enum with id" << id << name;
+    int id;
+    switch (size) {
+    case 1: id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor<qint8>,
+                                                 &EnumConstructor<qint8>, size, flags, meta);
+        break;
+    case 2: id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor<qint16>,
+                                                 &EnumConstructor<qint16>, size, flags, meta);
+        break;
+    case 4: id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor<qint32>,
+                                                 &EnumConstructor<qint32>, size, flags, meta);
+        break;
+    // Qt currently only supports enum values of 4 or less bytes (QMetaEnum value(index) returns int)
+//    case 8: id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor<qint64>,
+//                                                 &EnumConstructor<qint64>, size, flags, meta);
+//        break;
+    default:
+        qWarning() << "Invalid enum detected" << name << "with size" << size << ".  Defaulting to register as int.";
+        id = QMetaType::registerType(name.constData(), nullptr, nullptr, &EnumDestructor<qint32>,
+                                                 &EnumConstructor<qint32>, size, flags, meta);
+    }
+#ifdef QTRO_VERBOSE_PROTOCOL
+    qDebug() << "Registering new enum with id" << id << name << "size:" << size;
+#endif
+    qCDebug(QT_REMOTEOBJECT) << "Registering new enum with id" << id << name << "size:" << size;
 }
 
 static int registerGadgets(IoDeviceBase *connection, Gadgets &gadgets, QByteArray typeName)
@@ -768,7 +789,7 @@ static int registerGadgets(IoDeviceBase *connection, Gadgets &gadgets, QByteArra
    const auto enumCount = meta->enumeratorCount();
    for (int i = 0; i < enumCount; i++) {
        const QByteArray registeredName = QByteArray(typeName).append("::").append(meta->enumerator(i).name());
-       registerEnum(registeredName, meta);
+       registerEnum(registeredName, meta, gadget.enums.at(i).size);
    }
    QMetaType::TypeFlags flags = QMetaType::IsGadget;
    int gadgetTypeId;
@@ -806,6 +827,7 @@ static void deserializeEnum(QDataStream &ds, EnumData &enumData)
     ds >> enumData.name;
     ds >> enumData.isFlag;
     ds >> enumData.isScoped;
+    ds >> enumData.size;
     ds >> enumData.keyCount;
     for (quint32 i = 0; i < enumData.keyCount; i++) {
         EnumPair pair;
@@ -871,12 +893,14 @@ QMetaObject *QRemoteObjectMetaObjectManager::addDynamicType(IoDeviceBase *connec
     builder.setClassName(type);
 
     in >> numEnums;
+    QVector<quint32> enumSizes(numEnums);
     for (quint32 i = 0; i < numEnums; ++i) {
         EnumData enumData;
         deserializeEnum(in, enumData);
         auto enumBuilder = builder.addEnumerator(enumData.name);
         enumBuilder.setIsFlag(enumData.isFlag);
         enumBuilder.setIsScoped(enumData.isScoped);
+        enumSizes[i] = enumData.size;
 
         for (quint32 k = 0; k < enumData.keyCount; ++k) {
             const auto pair = enumData.values.at(k);
@@ -935,10 +959,11 @@ QMetaObject *QRemoteObjectMetaObjectManager::addDynamicType(IoDeviceBase *connec
     // We only want to register the new enumerations, and since we just added them, we know they
     // are the last indices.  Thus a backwards count seems most efficient.
     const int totalEnumCount = meta->enumeratorCount();
+    int incrementingIndex = 0;
     for (int i = numEnums; i > 0; i--) {
         auto const enumMeta = meta->enumerator(totalEnumCount - i);
         const QByteArray registeredName = QByteArray(type).append("::").append(enumMeta.name());
-        registerEnum(registeredName, meta);
+        registerEnum(registeredName, meta, enumSizes.at(incrementingIndex++));
     }
     dynamicTypes.insert(typeString, meta);
     return meta;
diff --git a/src/remoteobjects/qremoteobjectpacket.cpp b/src/remoteobjects/qremoteobjectpacket.cpp
index 631c362..4b8f77c 100644
--- a/src/remoteobjects/qremoteobjectpacket.cpp
+++ b/src/remoteobjects/qremoteobjectpacket.cpp
@@ -81,11 +81,21 @@ namespace QRemoteObjectPackets {
 const QVariant encodeVariant(const QVariant &value)
 {
     if (QMetaType::typeFlags(value.userType()).testFlag(QMetaType::IsEnumeration)) {
+        auto converted = QVariant(value);
+        const auto size = QMetaType(value.userType()).sizeOf();
+        switch (size) {
+        case 1: converted.convert(QMetaType::Char); break;
+        case 2: converted.convert(QMetaType::Short); break;
+        case 4: converted.convert(QMetaType::Int); break;
+        // Qt currently only supports enum values of 4 or less bytes (QMetaEnum value(index) returns int)
+//        case 8: converted.convert(QMetaType::Long); break; // typeId for long from qmetatype.h
+        default:
+            qWarning() << "Invalid enum detected" << QMetaType::typeName(value.userType()) << "with size" << size;
+            converted.convert(QMetaType::Int);
+        }
 #ifdef QTRO_VERBOSE_PROTOCOL
-        qDebug() << "Converting from enum to integer type" << value << value.value<qint32>();
+        qDebug() << "Converting from enum to integer type" << size << converted << value;
 #endif
-        auto converted = QVariant(value);
-        converted.convert(2); // typeId for int from qmetatype.h
         return converted;
     }
     return value;
@@ -95,11 +105,11 @@ QVariant &decodeVariant(QVariant &value, int type)
 {
     if (QMetaType::typeFlags(type).testFlag(QMetaType::IsEnumeration)) {
 #ifdef QTRO_VERBOSE_PROTOCOL
-        int asInt = value.value<qint32>();
+        QVariant encoded(value);
 #endif
         value.convert(type);
 #ifdef QTRO_VERBOSE_PROTOCOL
-        qDebug() << "Converting to enum from integer type" << value << asInt;
+        qDebug() << "Converting to enum from integer type" << value << encoded;
 #endif
     }
     return value;
@@ -363,8 +373,11 @@ static void serializeEnum(QDataStream &ds, const QMetaEnum &enumerator)
     ds << QByteArray::fromRawData(enumerator.name(), qstrlen(enumerator.name()));
     ds << enumerator.isFlag();
     ds << enumerator.isScoped();
+    const auto typeName = QByteArray(enumerator.scope()).append("::").append(enumerator.name());
+    quint32 size = QMetaType(QMetaType::type(typeName.constData())).sizeOf();
+    ds << size;
 #ifdef QTRO_VERBOSE_PROTOCOL
-    qDebug("  Enum (name = %s, isFlag = %s, isScoped = %s):", enumerator.name(), enumerator.isFlag() ? "true" : "false", enumerator.isScoped() ? "true" : "false");
+    qDebug("  Enum (name = %s, size = %d, isFlag = %s, isScoped = %s):", enumerator.name(), size, enumerator.isFlag() ? "true" : "false", enumerator.isScoped() ? "true" : "false");
 #endif
     const int keyCount = enumerator.keyCount();
     ds << keyCount;
diff --git a/tests/auto/proxy_multiprocess/client/main.cpp b/tests/auto/proxy_multiprocess/client/main.cpp
index 5f11402..19ba46f 100644
--- a/tests/auto/proxy_multiprocess/client/main.cpp
+++ b/tests/auto/proxy_multiprocess/client/main.cpp
@@ -60,16 +60,18 @@ private Q_SLOTS:
             QCOMPARE(m_rep->subClass()->i(), initialI);
             QVERIFY(m_rep->tracks() != nullptr);
             QVERIFY(tracksSpy.count() || tracksSpy.wait());
-            QCOMPARE(m_rep->myEnum(), QVariant::fromValue(ParentClassReplica::bar));
-            QCOMPARE(m_rep->date(), QVariant::fromValue(Qt::SystemLocaleShortDate));
-            QCOMPARE(m_rep->nsEnum(), QVariant::fromValue(NS::Bravo));
+            QCOMPARE(m_rep->myEnum(), ParentClassReplica::bar);
+            QCOMPARE(m_rep->date(), Qt::SystemLocaleShortDate);
+            QCOMPARE(m_rep->nsEnum(), NS::Bravo);
+            QCOMPARE(m_rep->ns2Enum(), NS2::NamespaceEnum::Bravo);
             QCOMPARE(m_rep->variant(), QVariant::fromValue(42.0f));
         } else {
             QVERIFY(m_rep->subClass() == nullptr);
             QVERIFY(m_rep->tracks() == nullptr);
-            QCOMPARE(m_rep->myEnum(), QVariant::fromValue(ParentClassReplica::foo));
-            QCOMPARE(m_rep->date(), QVariant::fromValue(Qt::ISODate));
-            QCOMPARE(m_rep->nsEnum(), QVariant::fromValue(NS::Alpha));
+            QCOMPARE(m_rep->myEnum(), ParentClassReplica::foo);
+            QCOMPARE(m_rep->date(), Qt::ISODate);
+            QCOMPARE(m_rep->nsEnum(), NS::Alpha);
+            QCOMPARE(m_rep->ns2Enum(), NS2::NamespaceEnum::Alpha);
             QCOMPARE(m_rep->variant(), QVariant());
         }
 
@@ -97,9 +99,10 @@ private Q_SLOTS:
         QCOMPARE(m_rep->subClass()->myPOD(), updatedValue);
         QCOMPARE(m_rep->subClass()->i(), updatedI);
         QVERIFY(m_rep->tracks() != nullptr);
-        QCOMPARE(m_rep->myEnum(), QVariant::fromValue(ParentClassReplica::foobar));
-        QCOMPARE(m_rep->date(), QVariant::fromValue(Qt::SystemLocaleLongDate));
-        QCOMPARE(m_rep->nsEnum(), QVariant::fromValue(NS::Charlie));
+        QCOMPARE(m_rep->myEnum(), ParentClassReplica::foobar);
+        QCOMPARE(m_rep->date(), Qt::SystemLocaleLongDate);
+        QCOMPARE(m_rep->nsEnum(), NS::Charlie);
+        QCOMPARE(m_rep->ns2Enum(), NS2::NamespaceEnum::Charlie);
         QCOMPARE(m_rep->variant(), QVariant::fromValue(podValue));
         qDebug() << "Verified expected final states, cleaning up.";
     }
diff --git a/tests/auto/proxy_multiprocess/namespace.h b/tests/auto/proxy_multiprocess/namespace.h
index 0b370ba..2d06e67 100644
--- a/tests/auto/proxy_multiprocess/namespace.h
+++ b/tests/auto/proxy_multiprocess/namespace.h
@@ -7,3 +7,9 @@ namespace NS
     Q_ENUM_NS(NamespaceEnum)
 }
 
+namespace NS2
+{
+    Q_NAMESPACE
+    enum class NamespaceEnum : quint8 { Alpha=1, Bravo, Charlie };
+    Q_ENUM_NS(NamespaceEnum)
+}
diff --git a/tests/auto/proxy_multiprocess/server/main.cpp b/tests/auto/proxy_multiprocess/server/main.cpp
index 9e806b9..1cd005d 100644
--- a/tests/auto/proxy_multiprocess/server/main.cpp
+++ b/tests/auto/proxy_multiprocess/server/main.cpp
@@ -58,6 +58,7 @@ private Q_SLOTS:
             parent.setMyEnum(ParentClassSource::bar);
             parent.setDate(Qt::SystemLocaleShortDate);
             parent.setNsEnum(NS::Bravo);
+            parent.setNs2Enum(NS2::NamespaceEnum::Bravo);
             parent.setVariant(QVariant::fromValue(42.0f));
         }
 
@@ -86,6 +87,7 @@ private Q_SLOTS:
         parent.setMyEnum(ParentClassSource::foobar);
         parent.setDate(Qt::SystemLocaleLongDate);
         parent.setNsEnum(NS::Charlie);
+        parent.setNs2Enum(NS2::NamespaceEnum::Charlie);
         parent.setVariant(QVariant::fromValue(podValue));
         emit parent.enum2(ParentClassSource::foo, ParentClassSource::bar);
 
diff --git a/tests/auto/proxy_multiprocess/subclass.rep b/tests/auto/proxy_multiprocess/subclass.rep
index b1fe352..2538b88 100644
--- a/tests/auto/proxy_multiprocess/subclass.rep
+++ b/tests/auto/proxy_multiprocess/subclass.rep
@@ -3,6 +3,7 @@
 
 USE_ENUM(Qt::DateFormat)
 USE_ENUM(NS::NamespaceEnum)
+USE_ENUM(NS2::NamespaceEnum)
 
 POD MyPOD(int i, float f, QString s)
 POD VariantPOD(int i, int j)
@@ -21,6 +22,7 @@ class ParentClass
     PROP(QVariant variant)
     PROP(Qt::DateFormat date = Qt::ISODate)
     PROP(NS::NamespaceEnum nsEnum = NS::Alpha)
+    PROP(NS2::NamespaceEnum ns2Enum = NS2::NamespaceEnum::Alpha)
 
     SLOT(bool start())
     SLOT(bool quit())