QmlDebug: Don't use QmlDebugConnection unconditionally

It might be null. We wrap the retrieval of the data stream version in
QmlDebugClient and return the minimum if connection is null. The extra
copy of the connection QmlEngine is dropped as QmlDebugClient already
has one and we don't want to hit a dangling pointer.

Change-Id: Ida8c45d357d46b4942eea99b77065d3c51c7edb9
Reviewed-by: hjk <hjk@qt.io>

src/libs/qmldebug/baseenginedebugclient.cpp

@@ -178,7 +178,7 @@ void BaseEngineDebugClient::stateChanged(State state)
 
 void BaseEngineDebugClient::messageReceived(const QByteArray &data)
 {
-    QPacket ds(connection()->currentDataStreamVersion(), data);
+    QPacket ds(dataStreamVersion(), data);
     int queryId;
     QByteArray type;
     ds >> type >> queryId;
@@ -251,7 +251,7 @@ quint32 BaseEngineDebugClient::addWatch(const PropertyReference &property)
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("WATCH_PROPERTY") << id << property.m_objectDebugId
            << property.m_name.toUtf8();
         sendMessage(ds.data());
@@ -272,7 +272,7 @@ quint32 BaseEngineDebugClient::addWatch(const ObjectReference &object,
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("WATCH_EXPR_OBJECT") << id << object.m_debugId << expr;
         sendMessage(ds.data());
     }
@@ -284,7 +284,7 @@ quint32 BaseEngineDebugClient::addWatch(int objectDebugId)
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("WATCH_OBJECT") << id << objectDebugId;
         sendMessage(ds.data());
     }
@@ -300,7 +300,7 @@ quint32 BaseEngineDebugClient::addWatch(const FileReference &/*file*/)
 void BaseEngineDebugClient::removeWatch(quint32 id)
 {
     if (state() == Enabled) {
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("NO_WATCH") << id;
         sendMessage(ds.data());
     }
@@ -311,7 +311,7 @@ quint32 BaseEngineDebugClient::queryAvailableEngines()
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("LIST_ENGINES") << id;
         sendMessage(ds.data());
     }
@@ -323,7 +323,7 @@ quint32 BaseEngineDebugClient::queryRootContexts(const EngineReference &engine)
     quint32 id = 0;
     if (state() == Enabled && engine.m_debugId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("LIST_OBJECTS") << id << engine.m_debugId;
         sendMessage(ds.data());
     }
@@ -335,7 +335,7 @@ quint32 BaseEngineDebugClient::queryObject(int objectId)
     quint32 id = 0;
     if (state() == Enabled && objectId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("FETCH_OBJECT") << id << objectId << false <<
               true;
         sendMessage(ds.data());
@@ -348,7 +348,7 @@ quint32 BaseEngineDebugClient::queryObjectRecursive(int objectId)
     quint32 id = 0;
     if (state() == Enabled && objectId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("FETCH_OBJECT") << id << objectId << true <<
               true;
         sendMessage(ds.data());
@@ -363,7 +363,7 @@ quint32 BaseEngineDebugClient::queryExpressionResult(int objectDebugId,
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("EVAL_EXPRESSION") << id << objectDebugId << expr
            << engineId;
         sendMessage(ds.data());
@@ -381,7 +381,7 @@ quint32 BaseEngineDebugClient::setBindingForObject(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("SET_BINDING") << id << objectDebugId << propertyName
            << bindingExpression << isLiteralValue << source << line;
         sendMessage(ds.data());
@@ -396,7 +396,7 @@ quint32 BaseEngineDebugClient::resetBindingForObject(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("RESET_BINDING") << id << objectDebugId << propertyName;
         sendMessage(ds.data());
     }
@@ -410,7 +410,7 @@ quint32 BaseEngineDebugClient::setMethodBody(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("SET_METHOD_BODY") << id << objectDebugId
            << methodName << methodBody;
         sendMessage(ds.data());
@@ -424,7 +424,7 @@ quint32 BaseEngineDebugClient::queryObjectsForLocation(
     quint32 id = 0;
     if (state() == Enabled) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("FETCH_OBJECTS_FOR_LOCATION") << id <<
               fileName << lineNumber << columnNumber << false <<
               true;

src/libs/qmldebug/declarativeenginedebugclient.cpp

@@ -46,7 +46,7 @@ quint32 DeclarativeEngineDebugClient::setBindingForObject(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("SET_BINDING") << objectDebugId << propertyName
            << bindingExpression << isLiteralValue << source << line;
         sendMessage(ds.data());
@@ -61,7 +61,7 @@ quint32 DeclarativeEngineDebugClient::resetBindingForObject(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("RESET_BINDING") << objectDebugId << propertyName;
         sendMessage(ds.data());
     }
@@ -75,7 +75,7 @@ quint32 DeclarativeEngineDebugClient::setMethodBody(
     quint32 id = 0;
     if (state() == Enabled && objectDebugId != -1) {
         id = getId();
-        QPacket ds(connection()->currentDataStreamVersion());
+        QPacket ds(dataStreamVersion());
         ds << QByteArray("SET_METHOD_BODY") << objectDebugId
            << methodName << methodBody;
         sendMessage(ds.data());
@@ -85,7 +85,7 @@ quint32 DeclarativeEngineDebugClient::setMethodBody(
 
 void DeclarativeEngineDebugClient::messageReceived(const QByteArray &data)
 {
-    QPacket ds(connection()->currentDataStreamVersion(), data);
+    QPacket ds(dataStreamVersion(), data);
     QByteArray type;
     ds >> type;
 

src/libs/qmldebug/qmldebugclient.cpp

@@ -38,6 +38,7 @@
 namespace QmlDebug {
 
 const int protocolVersion = 1;
+const int minimumDataStreamVersion = QDataStream::Qt_4_7;
 
 const QString serverId = QLatin1String("QDeclarativeDebugServer");
 const QString clientId = QLatin1String("QDeclarativeDebugClient");
@@ -86,7 +87,7 @@ static QString socketErrorToString(QAbstractSocket::SocketError error)
 
 QmlDebugConnectionPrivate::QmlDebugConnectionPrivate() :
     protocol(0), server(0), device(0), gotHello(false),
-    currentDataStreamVersion(QDataStream::Qt_4_7),
+    currentDataStreamVersion(minimumDataStreamVersion),
     maximumDataStreamVersion(QDataStream::Qt_DefaultCompiledVersion)
 {
 }
@@ -493,6 +494,12 @@ QmlDebugConnection *QmlDebugClient::connection() const
     return d->connection;
 }
 
+int QmlDebugClient::dataStreamVersion() const
+{
+    Q_D(const QmlDebugClient);
+    return (d->connection ? d->connection->currentDataStreamVersion() : minimumDataStreamVersion);
+}
+
 void QmlDebugClient::sendMessage(const QByteArray &message)
 {
     Q_D(QmlDebugClient);

src/libs/qmldebug/qmldebugclient.h

@@ -95,6 +95,7 @@ public:
     float serviceVersion() const;
     State state() const;
     QmlDebugConnection *connection() const;
+    int dataStreamVersion() const;
 
     virtual void sendMessage(const QByteArray &);
     virtual void stateChanged(State);

src/libs/qmldebug/qmltoolsclient.cpp

@@ -55,7 +55,7 @@ QmlToolsClient::QmlToolsClient(QmlDebugConnection *client)
 
 void QmlToolsClient::messageReceived(const QByteArray &message)
 {
-    QPacket ds(connection()->currentDataStreamVersion(), message);
+    QPacket ds(dataStreamVersion(), message);
 
     QByteArray type;
     int requestId;
@@ -98,7 +98,7 @@ void QmlToolsClient::setObjectIdList(const QList<ObjectReference> &objectRoots)
     foreach (const ObjectReference &object, objectRoots)
         debugIds << object.debugId();
 
-    QPacket ds(connection()->currentDataStreamVersion());
+    QPacket ds(dataStreamVersion());
     ds << QByteArray(REQUEST) << m_requestId++ << QByteArray(SELECT) << debugIds;
     sendMessage(ds.data());
 }
@@ -108,7 +108,7 @@ void QmlToolsClient::setDesignModeBehavior(bool inDesignMode)
     if (!m_connection || !m_connection->isConnected())
         return;
 
-    QPacket ds(connection()->currentDataStreamVersion());
+    QPacket ds(dataStreamVersion());
     ds << QByteArray(REQUEST) << m_requestId++;
     if (inDesignMode)
         ds << QByteArray(ENABLE);
@@ -140,7 +140,7 @@ void QmlToolsClient::showAppOnTop(bool showOnTop)
     if (!m_connection || !m_connection->isConnected())
         return;
 
-    QPacket ds(connection()->currentDataStreamVersion());
+    QPacket ds(dataStreamVersion());
     ds << QByteArray(REQUEST) << m_requestId++
        << QByteArray(SHOW_APP_ON_TOP) << showOnTop;
 

src/plugins/debugger/qml/qmlengine.cpp

@@ -140,11 +140,10 @@ typedef QHash<int, LookupData> LookupItems; // id -> (iname, exp)
 class QmlEnginePrivate : public QmlDebugClient
 {
 public:
-    QmlEnginePrivate(QmlEngine *engine_, QmlDebugConnection *connection_)
+    QmlEnginePrivate(QmlEngine *engine_, QmlDebugConnection *connection)
-        : QmlDebugClient("V8Debugger", connection_),
+        : QmlDebugClient("V8Debugger", connection),
           engine(engine_),
-          inspectorAgent(engine, connection_),
+          inspectorAgent(engine, connection)
-          connection(connection_)
     {}
 
     void messageReceived(const QByteArray &data);
@@ -221,7 +220,6 @@ public:
     bool contextEvaluate = false;
 
     QTimer connectionTimer;
-    QmlDebug::QmlDebugConnection *connection;
     QmlDebug::QDebugMessageClient *msgClient = 0;
 
     QHash<int, QmlCallback> callbackForToken;
@@ -250,6 +248,7 @@ QmlEngine::QmlEngine(bool useTerminal)
   :  d(new QmlEnginePrivate(this, new QmlDebugConnection(this)))
 {
     setObjectName("QmlEngine");
+    QmlDebugConnection *connection = d->connection();
 
     connect(stackHandler(), &StackHandler::stackChanged,
             this, &QmlEngine::updateCurrentContext);
@@ -280,21 +279,21 @@ QmlEngine::QmlEngine(bool useTerminal)
     connect(&d->connectionTimer, &QTimer::timeout,
             this, &QmlEngine::checkConnectionState);
 
-    connect(d->connection, &QmlDebugConnection::logStateChange,
+    connect(connection, &QmlDebugConnection::logStateChange,
             this, &QmlEngine::showConnectionStateMessage);
-    connect(d->connection, &QmlDebugConnection::logError, this,
+    connect(connection, &QmlDebugConnection::logError, this,
             [this](const QString &error) { showMessage("QML Debugger: " + error, LogWarning); });
 
-    connect(d->connection, &QmlDebugConnection::connectionFailed,
+    connect(connection, &QmlDebugConnection::connectionFailed,
             this, &QmlEngine::connectionFailed);
-    connect(d->connection, &QmlDebugConnection::connected,
+    connect(connection, &QmlDebugConnection::connected,
             &d->connectionTimer, &QTimer::stop);
-    connect(d->connection, &QmlDebugConnection::connected,
+    connect(connection, &QmlDebugConnection::connected,
             this, &QmlEngine::connectionEstablished);
-    connect(d->connection, &QmlDebugConnection::disconnected,
+    connect(connection, &QmlDebugConnection::disconnected,
             this, &QmlEngine::disconnected);
 
-    d->msgClient = new QDebugMessageClient(d->connection);
+    d->msgClient = new QDebugMessageClient(connection);
     connect(d->msgClient, &QDebugMessageClient::newState,
             this, [this](QmlDebugClient::State state) {
         logServiceStateChange(d->msgClient->name(), d->msgClient->serviceVersion(), state);
@@ -404,10 +403,11 @@ void QmlEngine::beginConnection()
      */
     int port = runParameters().qmlServer.port();
 
-    if (!d->connection || d->connection->isConnected())
+    QmlDebugConnection *connection = d->connection();
+    if (!connection || connection->isConnected())
         return;
 
-    d->connection->connectToHost(host, port);
+    connection->connectToHost(host, port);
 
     //A timeout to check the connection state
     d->connectionTimer.start();
@@ -513,8 +513,8 @@ void QmlEngine::closeConnection()
     if (d->connectionTimer.isActive()) {
         d->connectionTimer.stop();
     } else {
-        if (d->connection)
+        if (QmlDebugConnection *connection = d->connection())
-            d->connection->close();
+            connection->close();
     }
 }
 
@@ -1194,7 +1194,10 @@ void QmlEngine::checkConnectionState()
 
 bool QmlEngine::isConnected() const
 {
-    return d->connection->isConnected();
+    if (QmlDebugConnection *connection = d->connection())
+        return connection->isConnected();
+    else
+        return false;
 }
 
 void QmlEngine::showConnectionStateMessage(const QString &message)
@@ -1452,7 +1455,7 @@ void QmlEnginePrivate::setBreakpoint(const QString type, const QString target,
     //                    }
     //    }
     if (type == EVENT) {
-        QPacket rs(connection->currentDataStreamVersion());
+        QPacket rs(dataStreamVersion());
         rs <<  target.toUtf8() << enabled;
         engine->showMessage(QString("%1 %2 %3")
                             .arg(BREAKONSIGNAL, target, QLatin1String(enabled ? "enabled" : "disabled")), LogInput);
@@ -1672,7 +1675,7 @@ void QmlEnginePrivate::runDirectCommand(const QString &type, const QByteArray &m
 
     engine->showMessage(QString("%1 %2").arg(type, QString::fromLatin1(msg)), LogInput);
 
-    QPacket rs(connection->currentDataStreamVersion());
+    QPacket rs(dataStreamVersion());
     rs << cmd << type.toLatin1() << msg;
 
     if (state() == Enabled)
@@ -1694,7 +1697,7 @@ void QmlEnginePrivate::memorizeRefs(const QVariant &refs)
 
 void QmlEnginePrivate::messageReceived(const QByteArray &data)
 {
-    QPacket ds(connection->currentDataStreamVersion(), data);
+    QPacket ds(dataStreamVersion(), data);
     QByteArray command;
     ds >> command;
 

src/plugins/qmlprofiler/qmlprofilertraceclient.cpp

@@ -166,7 +166,7 @@ void QmlProfilerTraceClientPrivate::processCurrentEvent()
 
 void QmlProfilerTraceClientPrivate::sendRecordingStatus(int engineId)
 {
-    QmlDebug::QPacket stream(q->connection()->currentDataStreamVersion());
+    QmlDebug::QPacket stream(q->dataStreamVersion());
     stream << recording << engineId; // engineId -1 is OK. It means "all of them"
     if (recording) {
         stream << requestedFeatures << flushInterval;
@@ -282,7 +282,7 @@ void QmlProfilerTraceClient::stateChanged(State status)
 
 void QmlProfilerTraceClient::messageReceived(const QByteArray &data)
 {
-    QmlDebug::QPacket stream(connection()->currentDataStreamVersion(), data);
+    QmlDebug::QPacket stream(dataStreamVersion(), data);
 
     stream >> d->currentEvent;