From efa81f8354f70d99e6341ffa5428495b28e68641 Mon Sep 17 00:00:00 2001
From: Kai Koehne <kai.koehne@digia.com>
Date: Thu, 6 Nov 2014 11:43:19 +0100
Subject: [PATCH] Inspector: Do not assert when trying to stream QModelIndex

Some QVariant's like QModelIndex cannot be streamed in a meaningful way:
QDataType::save() will return false for them. However, this leads to a
qWarning and Q_ASSERT in QVariant::operator<<().

To prevent this we're calling QDataType::save() manually beforehand. We
however throw away the result if it succeeds, and still call
QVariant::operator<<() to get the benefits of the QDataStream version
handling.

The alternatives would be to make QVariant::operator<<() not assert,
or blacklist all known types with problems manually. Both seem to
be difficult though ...

Change-Id: I4f5fe6d5a3a076c24fbc73371a4d12d720de53da
Task-number: QTBUG-42438
Reviewed-by: Ulf Hermann <ulf.hermann@theqtcompany.com>
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>
---
 src/qml/debugger/qqmlenginedebugservice.cpp   | 12 +++++-
 .../tst_qqmlenginedebugservice.cpp            | 40 ++++++++++++++++++-
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/src/qml/debugger/qqmlenginedebugservice.cpp b/src/qml/debugger/qqmlenginedebugservice.cpp
index cb533a0459..088f3203d6 100644
--- a/src/qml/debugger/qqmlenginedebugservice.cpp
+++ b/src/qml/debugger/qqmlenginedebugservice.cpp
@@ -96,8 +96,16 @@ QDataStream &operator>>(QDataStream &ds,
 QDataStream &operator<<(QDataStream &ds,
                         const QQmlEngineDebugService::QQmlObjectProperty &data)
 {
-    ds << (int)data.type << data.name << data.value << data.valueTypeName
-       << data.binding << data.hasNotifySignal;
+    ds << (int)data.type << data.name;
+    // check first whether the data can be saved
+    // (otherwise we assert in QVariant::operator<<)
+    QByteArray buffer;
+    QDataStream fakeStream(&buffer, QIODevice::WriteOnly);
+    if (QMetaType::save(fakeStream, data.value.type(), data.value.constData()))
+        ds << data.value;
+    else
+        ds << QVariant();
+    ds << data.valueTypeName << data.binding << data.hasNotifySignal;
     return ds;
 }
 
diff --git a/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp b/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
index 5bbbe8e1e9..7c931928d4 100644
--- a/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
+++ b/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
@@ -36,6 +36,7 @@
 #include <QHostAddress>
 #include <QDebug>
 #include <QThread>
+#include <QModelIndex>
 
 #include <QtQml/qqmlengine.h>
 #include <QtQml/qqmlcontext.h>
@@ -73,6 +74,16 @@ signals:
 };
 QML_DECLARE_TYPE(NonScriptProperty)
 
+class CustomTypes : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(QModelIndex modelIndex READ modelIndex)
+public:
+    CustomTypes(QObject *parent = 0) : QObject(parent) {}
+
+    QModelIndex modelIndex() { return QModelIndex(); }
+};
+
 class tst_QQmlEngineDebugService : public QObject
 {
     Q_OBJECT
@@ -125,6 +136,7 @@ private slots:
     void setBindingInStates();
 
     void regression_QTCREATORBUG_7451();
+    void queryObjectWithNonStreamableTypes();
 };
 
 QmlDebugObjectReference tst_QQmlEngineDebugService::findRootObject(
@@ -314,6 +326,12 @@ void tst_QQmlEngineDebugService::initTestCase()
            "}\n"
            ;
 
+    // test non-streamable properties
+    qmlRegisterType<CustomTypes>("Backend", 1, 0, "CustomTypes");
+    qml << "import Backend 1.0\n"
+           "CustomTypes {}"
+           ;
+
     for (int i=0; i<qml.count(); i++) {
         QQmlComponent component(m_engine);
         component.setData(qml[i], QUrl::fromLocalFile(""));
@@ -620,7 +638,7 @@ void tst_QQmlEngineDebugService::queryRootContexts()
     // root context query sends only root object data - it doesn't fill in
     // the children or property info
     QCOMPARE(context.objects.count(), 0);
-    QCOMPARE(context.contexts.count(), 5);
+    QCOMPARE(context.contexts.count(), 6);
     QVERIFY(context.contexts[0].debugId >= 0);
     QCOMPARE(context.contexts[0].name, QString("tst_QQmlDebug_childContext"));
 }
@@ -819,6 +837,26 @@ void tst_QQmlEngineDebugService::regression_QTCREATORBUG_7451()
     }
 }
 
+void tst_QQmlEngineDebugService::queryObjectWithNonStreamableTypes()
+{
+    bool success;
+
+    QmlDebugObjectReference rootObject = findRootObject(4, true);
+
+    QQmlEngineDebugClient *unconnected = new QQmlEngineDebugClient(0);
+    unconnected->queryObject(rootObject, &success);
+    QVERIFY(!success);
+    delete unconnected;
+
+    m_dbg->queryObject(rootObject, &success);
+    QVERIFY(success);
+    QVERIFY(QQmlDebugTest::waitForSignal(m_dbg, SIGNAL(result())));
+
+    QmlDebugObjectReference obj = m_dbg->object();
+
+    QCOMPARE(findProperty(obj.properties, "modelIndex").value, QVariant());
+}
+
 
 void tst_QQmlEngineDebugService::queryExpressionResult()
 {
-- 
GitLab