Properly convert JSON data to target type of function parameter.

This used to work but broke in one of the last few commits apparently.
Now we add a proper unit test to ensure it stays working.

The issue was that JSON only knows e.g. numeric types stored as double.
When we then try to call a method taking an int with the VariantArgument
that tries to convert the double to int, we failed and produced an
invalid QVariant which then converts to 0.

Now we use the appropriate API to convert the JSON data to the correct
target type before calling the method. Furthermore, it became clear
that we can greatly cleanup the VariantArgument thanks to that. It now
is reduced to just a QVariant wrapper class with an implicit cast
operator to QGenericArgument.

Change-Id: Ieaf60f548ea9584e7d760f9cd935da455787f376
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

4 files changed, 64 insertions, 44 deletions

diff --git a/src/webchannel/qmetaobjectpublisher.cpp b/src/webchannel/qmetaobjectpublisher.cpp
index 2f0ef74..1d0124c 100644
--- a/src/webchannel/qmetaobjectpublisher.cpp
+++ b/src/webchannel/qmetaobjectpublisher.cpp
@@ -343,7 +343,11 @@ bool QMetaObjectPublisher::invokeMethod(QObject *const object, const int methodI
     // construct converter objects of QVariant to QGenericArgument
     VariantArgument arguments[10];
     for (int i = 0; i < qMin(args.size(), method.parameterCount()); ++i) {
-        arguments[i].setValue(args.at(i).toVariant(), method.parameterType(i));
+        QVariant arg = args.at(i).toVariant();
+        if (method.parameterType(i) != QMetaType::QVariant && !arg.convert(method.parameterType(i))) {
+            qWarning() << "Could not convert argument" << args.at(i) << "to target type" << method.parameterTypes().at(i) << '.';
+        }
+        arguments[i].value = arg;
     }
 
     // construct QGenericReturnArgument
diff --git a/src/webchannel/variantargument_p.h b/src/webchannel/variantargument_p.h
index ee625fc..5f17953 100644
--- a/src/webchannel/variantargument_p.h
+++ b/src/webchannel/variantargument_p.h
@@ -44,60 +44,21 @@
 #define VARIANTARGUMENT_H
 
 #include <QVariant>
-#include <QMetaType>
 
 /**
  * RAII QVariant to Q[Generic]Argument conversion
  */
-class VariantArgument
+struct VariantArgument
 {
-public:
-    explicit VariantArgument()
-    : m_data(0)
-    , m_paramType(0)
-    {
-    }
-
-    /// TODO: test with C++ methods that don't take a QVariant as arg
-    ///       also test conversions
-    void setValue(const QVariant &value, int paramType)
-    {
-        if (m_data) {
-            QMetaType::destroy(m_paramType, m_data);
-            m_name.clear();
-            m_data = 0;
-        }
-
-        m_paramType = paramType;
-
-        if (value.isValid()) {
-            m_name = value.typeName();
-            m_data = QMetaType::create(m_paramType, value.constData());
-        }
-    }
-
-    ~VariantArgument()
-    {
-        if (m_data) {
-            QMetaType::destroy(m_paramType, m_data);
-            m_data = 0;
-        }
-    }
-
     operator QGenericArgument() const
     {
-        if (!m_data) {
+        if (!value.isValid()) {
             return QGenericArgument();
         }
-        return QGenericArgument(m_name.constData(), m_data);
+        return QGenericArgument(value.typeName(), value.constData());
     }
 
-private:
-    Q_DISABLE_COPY(VariantArgument)
-
-    QByteArray m_name;
-    void* m_data;
-    int m_paramType;
+    QVariant value;
 };
 
 #endif // VARIANTARGUMENT_H
diff --git a/tests/webchannel/tst_webchannel.cpp b/tests/webchannel/tst_webchannel.cpp
index 7fbe5f9..c8f212c 100644
--- a/tests/webchannel/tst_webchannel.cpp
+++ b/tests/webchannel/tst_webchannel.cpp
@@ -50,6 +50,8 @@
 
 TestWebChannel::TestWebChannel(QObject *parent)
     : QObject(parent)
+    , m_lastInt(0)
+    , m_lastDouble(0)
 {
 }
 
@@ -58,6 +60,21 @@ TestWebChannel::~TestWebChannel()
 
 }
 
+void TestWebChannel::setInt(int i)
+{
+    m_lastInt = i;
+}
+
+void TestWebChannel::setDouble(double d)
+{
+    m_lastDouble = d;
+}
+
+void TestWebChannel::setVariant(const QVariant &v)
+{
+    m_lastVariant = v;
+}
+
 void TestWebChannel::testInitChannel()
 {
     QWebChannel channel;
@@ -214,6 +231,33 @@ void TestWebChannel::testInfoForObject()
     }
 }
 
+void TestWebChannel::testInvokeMethodConversion()
+{
+    QWebChannel channel;
+
+    QJsonArray args;
+    args.append(QJsonValue(1000));
+
+    {
+        int method = metaObject()->indexOfMethod("setInt(int)");
+        QVERIFY(method != -1);
+        QVERIFY(channel.d->publisher->invokeMethod(this, method, args, QJsonValue()));
+        QCOMPARE(m_lastInt, args.at(0).toInt());
+    }
+    {
+        int method = metaObject()->indexOfMethod("setDouble(double)");
+        QVERIFY(method != -1);
+        QVERIFY(channel.d->publisher->invokeMethod(this, method, args, QJsonValue()));
+        QCOMPARE(m_lastDouble, args.at(0).toDouble());
+    }
+    {
+        int method = metaObject()->indexOfMethod("setVariant(QVariant)");
+        QVERIFY(method != -1);
+        QVERIFY(channel.d->publisher->invokeMethod(this, method, args, QJsonValue()));
+        QCOMPARE(m_lastVariant, args.at(0).toVariant());
+    }
+}
+
 static QHash<QString, QObject*> createObjects(QObject *parent)
 {
     const int num = 100;
diff --git a/tests/webchannel/tst_webchannel.h b/tests/webchannel/tst_webchannel.h
index 173921b..003f921 100644
--- a/tests/webchannel/tst_webchannel.h
+++ b/tests/webchannel/tst_webchannel.h
@@ -44,6 +44,7 @@
 #define TST_WEBCHANNEL_H
 
 #include <QObject>
+#include <QVariant>
 
 class TestObject : public QObject
 {
@@ -192,15 +193,25 @@ public:
     explicit TestWebChannel(QObject *parent = 0);
     virtual ~TestWebChannel();
 
+    Q_INVOKABLE void setInt(int i);
+    Q_INVOKABLE void setDouble(double d);
+    Q_INVOKABLE void setVariant(const QVariant &v);
+
 private slots:
     void testInitChannel();
     void testRegisterObjects();
     void testInfoForObject();
+    void testInvokeMethodConversion();
 
     void benchClassInfo();
     void benchInitializeClients();
     void benchPropertyUpdates();
     void benchRegisterObjects();
+
+private:
+    int m_lastInt;
+    double m_lastDouble;
+    QVariant m_lastVariant;
 };
 
 #endif // TST_WEBCHANNEL_H