QmlDebugClient: hide all the socket details from clients

Like this no one will get the idea that the socket state represents
the connection state and we can safely replace the underlying
implementation with something not derived from QAbstractSocket.
All the logging is retained but the connection creates the messages
now.

Change-Id: If84ff42f1fa9785254fbd49c75be867b9f663c83
Reviewed-by: Kai Koehne <kai.koehne@digia.com>

src/libs/qmldebug/qmldebugclient.cpp

@@ -71,13 +71,15 @@ public:
 
 public Q_SLOTS:
     void connected();
+    void disconnected();
+    void error(QAbstractSocket::SocketError error);
     void readyRead();
+    void stateChanged(QAbstractSocket::SocketState state);
 };
 
 QmlDebugConnectionPrivate::QmlDebugConnectionPrivate(QmlDebugConnection *c)
     : QObject(c), q(c), protocol(0), device(0), gotHello(false)
 {
-    QObject::connect(c, SIGNAL(connected()), this, SLOT(connected()));
 }
 
 void QmlDebugConnectionPrivate::advertisePlugins()
@@ -100,6 +102,35 @@ void QmlDebugConnectionPrivate::connected()
     flush();
 }
 
+void QmlDebugConnectionPrivate::disconnected()
+{
+    if (gotHello) {
+        gotHello = false;
+        QHash<QString, QmlDebugClient*>::iterator iter = plugins.begin();
+        for (; iter != plugins.end(); ++iter)
+            iter.value()->stateChanged(QmlDebugClient::NotConnected);
+        emit q->closed();
+    }
+    delete protocol;
+    protocol = 0;
+    if (device) {
+        // Don't immediately delete it as it may do some cleanup on returning from a signal.
+        device->deleteLater();
+        device = 0;
+    }
+}
+
+void QmlDebugConnectionPrivate::error(QAbstractSocket::SocketError socketError)
+{
+    //: %1=error code, %2=error message
+    emit q->errorMessage(tr("Error: (%1) %2").arg(socketError)
+             .arg(device ? device->errorString() : tr("<device is gone>")));
+    if (socketError == QAbstractSocket::RemoteHostClosedError)
+        emit q->error(QDebugSupport::RemoteClosedConnectionError);
+    else
+        emit q->error(QDebugSupport::UnknownError);
+}
+
 void QmlDebugConnectionPrivate::readyRead()
 {
     if (!gotHello) {
@@ -156,6 +187,7 @@ void QmlDebugConnectionPrivate::readyRead()
                 newState = QmlDebugClient::Enabled;
             iter.value()->stateChanged(newState);
         }
+        emit q->opened();
     }
 
     while (protocol->packetsAvailable()) {
@@ -216,6 +248,33 @@ void QmlDebugConnectionPrivate::readyRead()
     }
 }
 
+void QmlDebugConnectionPrivate::stateChanged(QAbstractSocket::SocketState state)
+{
+    switch (state) {
+    case QAbstractSocket::UnconnectedState:
+        emit q->stateMessage(tr("Network connection dropped"));
+        break;
+    case QAbstractSocket::HostLookupState:
+        emit q->stateMessage(tr("Resolving host"));
+        break;
+    case QAbstractSocket::ConnectingState:
+        emit q->stateMessage(tr("Establishing network connection ..."));
+        break;
+    case QAbstractSocket::ConnectedState:
+        emit q->stateMessage(tr("Network connection established"));
+        break;
+    case QAbstractSocket::ClosingState:
+        emit q->stateMessage(tr("Network connection closing"));
+        break;
+    case QAbstractSocket::BoundState:
+        emit q->errorMessage(tr("Socket state changed to BoundState. This should not happen!"));
+        break;
+    case QAbstractSocket::ListeningState:
+        emit q->errorMessage(tr("Socket state changed to ListeningState. This should not happen!"));
+        break;
+    }
+}
+
 QmlDebugConnection::QmlDebugConnection(QObject *parent)
     : QObject(parent), d(new QmlDebugConnectionPrivate(this))
 {
@@ -223,45 +282,22 @@ QmlDebugConnection::QmlDebugConnection(QObject *parent)
 
 QmlDebugConnection::~QmlDebugConnection()
 {
+    d->disconnected();
     QHash<QString, QmlDebugClient*>::iterator iter = d->plugins.begin();
-    for (; iter != d->plugins.end(); ++iter) {
+    for (; iter != d->plugins.end(); ++iter)
         iter.value()->d_func()->connection = 0;
-        iter.value()->stateChanged(QmlDebugClient::NotConnected);
-    }
 }
 
 bool QmlDebugConnection::isOpen() const
 {
-    return socketState() == QAbstractSocket::ConnectedState && d->gotHello;
+    // gotHello can only be set if the connection is open.
+    return d->gotHello;
 }
 
 void QmlDebugConnection::close()
 {
-    if (d->device->isOpen()) {
+    if (d->device && d->device->isOpen())
-        d->device->close();
+        d->device->close(); // will trigger disconnected() at some point.
-        emit socketStateChanged(QAbstractSocket::UnconnectedState);
-
-        QHash<QString, QmlDebugClient*>::iterator iter = d->plugins.begin();
-        for (; iter != d->plugins.end(); ++iter) {
-            iter.value()->stateChanged(QmlDebugClient::NotConnected);
-        }
-    }
-}
-
-QString QmlDebugConnection::errorString() const
-{
-    return d->device->errorString();
-}
-
-// For ease of refactoring we use QAbstractSocket's states even if we're actually using a OstChannel underneath
-// since serial ports have a subset of the socket states afaics
-QAbstractSocket::SocketState QmlDebugConnection::socketState() const
-{
-    QAbstractSocket *socket = qobject_cast<QAbstractSocket*>(d->device);
-    if (socket)
-        return socket->state();
-
-    return QAbstractSocket::UnconnectedState;
 }
 
 void QmlDebugConnectionPrivate::flush()
@@ -275,15 +311,20 @@ void QmlDebugConnectionPrivate::flush()
 
 void QmlDebugConnection::connectToHost(const QString &hostName, quint16 port)
 {
+    d->disconnected();
+    emit stateMessage(tr("Connecting to debug server at %1:%2 ...")
+             .arg(hostName).arg(QString::number(port)));
     QTcpSocket *socket = new QTcpSocket(d);
     socket->setProxy(QNetworkProxy::NoProxy);
     d->device = socket;
     d->protocol = new QPacketProtocol(d->device, this);
     connect(d->protocol, SIGNAL(readyRead()), d, SLOT(readyRead()));
-    d->gotHello = false;
+    connect(socket, SIGNAL(stateChanged(QAbstractSocket::SocketState)),
-    connect(socket, SIGNAL(stateChanged(QAbstractSocket::SocketState)), this, SIGNAL(socketStateChanged(QAbstractSocket::SocketState)));
+            d, SLOT(stateChanged(QAbstractSocket::SocketState)));
-    connect(socket, SIGNAL(error(QAbstractSocket::SocketError)), this, SIGNAL(error(QAbstractSocket::SocketError)));
+    connect(socket, SIGNAL(error(QAbstractSocket::SocketError)),
-    connect(socket, SIGNAL(connected()), this, SIGNAL(connected()));
+            d, SLOT(error(QAbstractSocket::SocketError)));
+    connect(socket, SIGNAL(connected()), d, SLOT(connected()));
+    connect(socket, SIGNAL(disconnected()), d, SLOT(disconnected()));
     socket->connectToHost(hostName, port);
 }
 

src/libs/qmldebug/qmldebugclient.h

@@ -35,6 +35,13 @@
 
 #include <QDataStream>
 
+namespace QDebugSupport {
+enum Error {
+    RemoteClosedConnectionError,
+    UnknownError
+};
+}
+
 namespace QmlDebug {
 
 class QmlDebugConnectionPrivate;
@@ -49,14 +56,14 @@ public:
     void connectToHost(const QString &hostName, quint16 port);
 
     bool isOpen() const;
-    QAbstractSocket::SocketState socketState() const;
     void close();
-    QString errorString() const;
 
 signals:
-    void connected();
+    void opened();
-    void socketStateChanged(QAbstractSocket::SocketState state);
+    void error(QDebugSupport::Error);
-    void error(QAbstractSocket::SocketError socketError);
+    void closed();
+    void stateMessage(const QString &message);
+    void errorMessage(const QString &message);
 
 private:
     QmlDebugConnectionPrivate *d;

src/plugins/debugger/qml/qmladapter.cpp

@@ -59,10 +59,13 @@ QmlAdapter::QmlAdapter(DebuggerEngine *engine, QObject *parent)
     connect(&m_connectionTimer, SIGNAL(timeout()), SLOT(checkConnectionState()));
 
     m_conn = new QmlDebugConnection(this);
-    connect(m_conn, SIGNAL(socketStateChanged(QAbstractSocket::SocketState)),
+    connect(m_conn, SIGNAL(stateMessage(QString)), SLOT(showConnectionStateMessage(QString)));
-            SLOT(connectionStateChanged()));
+    connect(m_conn, SIGNAL(errorMessage(QString)), SLOT(showConnectionErrorMessage(QString)));
-    connect(m_conn, SIGNAL(error(QAbstractSocket::SocketError)),
+    connect(m_conn, SIGNAL(error(QDebugSupport::Error)),
-            SLOT(connectionErrorOccurred(QAbstractSocket::SocketError)));
+            SLOT(connectionErrorOccurred(QDebugSupport::Error)));
+    connect(m_conn, SIGNAL(opened()), &m_connectionTimer, SLOT(stop()));
+    connect(m_conn, SIGNAL(opened()), SIGNAL(connected()));
+    connect(m_conn, SIGNAL(closed()), SIGNAL(disconnected()));
 
     createDebuggerClients();
     m_msgClient = new QDebugMessageClient(m_conn);
@@ -77,12 +80,9 @@ QmlAdapter::~QmlAdapter()
 
 void QmlAdapter::beginConnectionTcp(const QString &address, quint16 port)
 {
-    if (m_engine.isNull()
+    if (m_engine.isNull() || (m_conn && m_conn->isOpen()))
-            || (m_conn && m_conn->socketState() != QAbstractSocket::UnconnectedState))
         return;
 
-    showConnectionStateMessage(tr("Connecting to debug server %1:%2").arg(address).arg(
-                                    QString::number(port)));
     m_conn->connectToHost(address, port);
 
     //A timeout to check the connection state
@@ -99,14 +99,11 @@ void QmlAdapter::closeConnection()
     }
 }
 
-void QmlAdapter::connectionErrorOccurred(QAbstractSocket::SocketError socketError)
+void QmlAdapter::connectionErrorOccurred(QDebugSupport::Error error)
 {
-    showConnectionStateMessage(tr("Error: (%1) %2", "%1=error code, %2=error message")
-                                .arg(socketError).arg(m_conn->errorString()));
-
     // this is only an error if we are already connected and something goes wrong.
     if (isConnected()) {
-        emit connectionError(socketError);
+        emit connectionError(error);
     } else {
         m_connectionTimer.stop();
         emit connectionStartupFailed();
@@ -136,41 +133,6 @@ void QmlAdapter::debugClientStateChanged(QmlDebugClient::State state)
     m_qmlClient->startSession();
 }
 
-void QmlAdapter::connectionStateChanged()
-{
-    switch (m_conn->socketState()) {
-    case QAbstractSocket::UnconnectedState:
-    {
-        showConnectionStateMessage(tr("Disconnected.") + QLatin1String("\n\n"));
-        emit disconnected();
-
-        break;
-    }
-    case QAbstractSocket::HostLookupState:
-        showConnectionStateMessage(tr("Resolving host."));
-        break;
-    case QAbstractSocket::ConnectingState:
-        showConnectionStateMessage(tr("Connecting to debug server."));
-        break;
-    case QAbstractSocket::ConnectedState:
-    {
-        showConnectionStateMessage(tr("Connected.") + QLatin1Char('\n'));
-
-        m_connectionTimer.stop();
-
-        //reloadEngines();
-        emit connected();
-        break;
-    }
-    case QAbstractSocket::ClosingState:
-        showConnectionStateMessage(tr("Closing."));
-        break;
-    case QAbstractSocket::BoundState:
-    case QAbstractSocket::ListeningState:
-        break;
-    }
-}
-
 void QmlAdapter::checkConnectionState()
 {
     if (!isConnected()) {
@@ -181,7 +143,7 @@ void QmlAdapter::checkConnectionState()
 
 bool QmlAdapter::isConnected() const
 {
-    return m_conn && m_qmlClient && m_conn->socketState() == QAbstractSocket::ConnectedState;
+    return m_conn && m_qmlClient && m_conn->isOpen();
 }
 
 void QmlAdapter::createDebuggerClients()

src/plugins/debugger/qml/qmladapter.h

@@ -77,21 +77,20 @@ signals:
     void connected();
     void disconnected();
     void connectionStartupFailed();
-    void connectionError(QAbstractSocket::SocketError socketError);
+    void connectionError(QDebugSupport::Error error);
     void serviceConnectionError(const QString serviceName);
 
 private slots:
-    void connectionErrorOccurred(QAbstractSocket::SocketError socketError);
+    void connectionErrorOccurred(QDebugSupport::Error socketError);
     void clientStateChanged(QmlDebug::QmlDebugClient::State state);
     void debugClientStateChanged(QmlDebug::QmlDebugClient::State state);
-    void connectionStateChanged();
     void checkConnectionState();
+    void showConnectionStateMessage(const QString &message);
+    void showConnectionErrorMessage(const QString &message);
 
 private:
     bool isConnected() const;
     void createDebuggerClients();
-    void showConnectionStateMessage(const QString &message);
-    void showConnectionErrorMessage(const QString &message);
 
 private:
     QPointer<DebuggerEngine> m_engine;

src/plugins/debugger/qml/qmlengine.cpp

@@ -266,8 +266,8 @@ QmlEngine::QmlEngine(const DebuggerStartParameters &startParameters, DebuggerEng
     if (masterEngine)
         setMasterEngine(masterEngine);
 
-    connect(&m_adapter, SIGNAL(connectionError(QAbstractSocket::SocketError)),
+    connect(&m_adapter, SIGNAL(connectionError(QDebugSupport::Error)),
-        SLOT(connectionError(QAbstractSocket::SocketError)));
+        SLOT(connectionError(QDebugSupport::Error)));
     connect(&m_adapter, SIGNAL(serviceConnectionError(QString)),
         SLOT(serviceConnectionError(QString)));
     connect(&m_adapter, SIGNAL(connected()),
@@ -499,9 +499,9 @@ void QmlEngine::errorMessageBoxFinished(int result)
     }
 }
 
-void QmlEngine::connectionError(QAbstractSocket::SocketError socketError)
+void QmlEngine::connectionError(QDebugSupport::Error error)
 {
-    if (socketError == QAbstractSocket::RemoteHostClosedError)
+    if (error == QDebugSupport::RemoteClosedConnectionError)
         showMessage(tr("QML Debugger: Remote host closed connection."), StatusBar);
 
     if (!isSlaveEngine()) { // normal flow for slave engine when gdb exits

src/plugins/debugger/qml/qmlengine.h

@@ -107,7 +107,7 @@ private slots:
     void connectionEstablished();
     void connectionStartupFailed();
     void appStartupFailed(const QString &errorMessage);
-    void connectionError(QAbstractSocket::SocketError error);
+    void connectionError(QDebugSupport::Error error);
     void serviceConnectionError(const QString &service);
     void appendMessage(const QString &msg, Utils::OutputFormat);
 

src/plugins/qmlprofiler/qmlprofilerclientmanager.cpp

@@ -147,8 +147,10 @@ void QmlProfilerClientManager::connectClient(quint16 port)
         delete d->connection;
     d->connection = new QmlDebugConnection;
     enableServices();
-    connect(d->connection, SIGNAL(socketStateChanged(QAbstractSocket::SocketState)),
+    connect(d->connection, SIGNAL(stateMessage(QString)), this, SLOT(logState(QString)));
-            this, SLOT(connectionStateChanged()));
+    connect(d->connection, SIGNAL(errorMessage(QString)), this, SLOT(logState(QString)));
+    connect(d->connection, SIGNAL(opened()), this, SLOT(qmlDebugConnectionOpened()));
+    connect(d->connection, SIGNAL(closed()), this, SLOT(qmlDebugConnectionClosed()));
     d->connectionTimer.start();
     d->tcpPort = port;
 }
@@ -235,7 +237,7 @@ void QmlProfilerClientManager::disconnectClientSignals()
 
 void QmlProfilerClientManager::connectToClient()
 {
-    if (!d->connection || d->connection->socketState() != QAbstractSocket::UnconnectedState)
+    if (!d->connection || d->connection->isOpen())
         return;
 
     d->connection->connectToHost(d->tcpHost, d->tcpPort);
@@ -287,45 +289,25 @@ void QmlProfilerClientManager::tryToConnect()
     }
 }
 
-void QmlProfilerClientManager::connectionStateChanged()
+void QmlProfilerClientManager::qmlDebugConnectionOpened()
 {
-    if (!d->connection)
+    logState(tr("Debug connection opened"));
-        return;
+    clientRecordingChanged();
-    switch (d->connection->socketState()) {
+}
-    case QAbstractSocket::UnconnectedState:
+
-    {
+void QmlProfilerClientManager::qmlDebugConnectionClosed()
-        if (QmlProfilerPlugin::debugOutput)
+{
-            qWarning("QML Profiler: disconnected");
+    logState(tr("Debug connection closed"));
     disconnectClient();
     emit connectionClosed();
-        break;
+}
-    }
+
-    case QAbstractSocket::HostLookupState:
+void QmlProfilerClientManager::logState(const QString &msg)
-        break;
+{
-    case QAbstractSocket::ConnectingState: {
+    QString state = QLatin1String("QML Profiler: ") + msg;
     if (QmlProfilerPlugin::debugOutput)
-            qWarning("QML Profiler: Connecting to debug server ...");
+        qWarning() << state;
-        QmlProfilerTool::logState(tr("QML Profiler: Connecting to %1:%2 ...")
+    QmlProfilerTool::logState(state);
-            .arg(d->tcpHost, QString::number(d->tcpPort)));
-        break;
-    }
-    case QAbstractSocket::ConnectedState:
-    {
-        if (QmlProfilerPlugin::debugOutput)
-            qWarning("QML Profiler: connected and running");
-        // notify the client recording status
-        clientRecordingChanged();
-        QmlProfilerTool::logState(tr("QML Profiler: connected and running"));
-        break;
-    }
-    case QAbstractSocket::ClosingState:
-        if (QmlProfilerPlugin::debugOutput)
-            qWarning("QML Profiler: closing ...");
-        break;
-    case QAbstractSocket::BoundState:
-    case QAbstractSocket::ListeningState:
-        break;
-    }
 }
 
 void QmlProfilerClientManager::retryMessageBoxFinished(int result)
@@ -341,11 +323,8 @@ void QmlProfilerClientManager::retryMessageBoxFinished(int result)
         // fall through
     }
     default: {
-        if (d->connection)
+        // The actual error message has already been logged.
-            QmlProfilerTool::logState(QLatin1String("QML Profiler: Failed to connect! ") + d->connection->errorString());
+        logState(tr("Failed to connect!"));
-        else
-            QmlProfilerTool::logState(QLatin1String("QML Profiler: Failed to connect!"));
-
         emit connectionFailed();
         break;
     }

src/plugins/qmlprofiler/qmlprofilerclientmanager.h

@@ -69,7 +69,10 @@ public slots:
 
 private slots:
     void tryToConnect();
-    void connectionStateChanged();
+    void qmlDebugConnectionOpened();
+    void qmlDebugConnectionClosed();
+    void logState(const QString &);
+
     void retryMessageBoxFinished(int result);
 
     void qmlComplete(qint64 maximumTime);