Use signal/slot for passing messages through QQmlDebugServer

This results in much cleaner code than the previous implementation
using QMetaObject::invokeMethod().

We have to use read locks now for adding and removing engines, as we
should have done already before. If a condition is waiting on a write
lock you cannot acquire a read lock from another thread. So, if we kept
the write locks we wouldn't be able to receive messages while the engines
are waiting.

Change-Id: Icfe641601dec2f8d7181ae579146ed603d57a4c2
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

15 files changed, 59 insertions, 74 deletions

diff --git a/src/plugins/qmltooling/shared/abstractviewinspector.cpp b/src/plugins/qmltooling/shared/abstractviewinspector.cpp
index b23cff9a78..c32a297d6a 100644
--- a/src/plugins/qmltooling/shared/abstractviewinspector.cpp
+++ b/src/plugins/qmltooling/shared/abstractviewinspector.cpp
@@ -266,7 +266,7 @@ void AbstractViewInspector::onQmlObjectDestroyed(QObject *object)
     QQmlDebugStream rs(&response, QIODevice::WriteOnly);
     rs << QByteArray(RESPONSE) << ids.first << true << ids.second;
 
-    m_debugService->sendMessage(response);
+    emit m_debugService->messageToClient(m_debugService->name(), response);
 }
 
 void AbstractViewInspector::handleMessage(const QByteArray &message)
@@ -360,7 +360,7 @@ void AbstractViewInspector::handleMessage(const QByteArray &message)
     QByteArray response;
     QQmlDebugStream rs(&response, QIODevice::WriteOnly);
     rs << QByteArray(RESPONSE) << requestId << success;
-    m_debugService->sendMessage(response);
+    emit m_debugService->messageToClient(m_debugService->name(), response);
 }
 
 void AbstractViewInspector::sendCurrentObjects(const QList<QObject*> &objects)
@@ -376,7 +376,7 @@ void AbstractViewInspector::sendCurrentObjects(const QList<QObject*> &objects)
         debugIds << QQmlDebugService::idForObject(object);
     ds << debugIds;
 
-    m_debugService->sendMessage(message);
+    emit m_debugService->messageToClient(m_debugService->name(), message);
 }
 
 void AbstractViewInspector::sendQmlFileReloaded(bool success)
@@ -389,7 +389,7 @@ void AbstractViewInspector::sendQmlFileReloaded(bool success)
     QQmlDebugStream rs(&response, QIODevice::WriteOnly);
     rs << QByteArray(RESPONSE) << m_reloadEventId << success;
 
-    m_debugService->sendMessage(response);
+    emit m_debugService->messageToClient(m_debugService->name(), response);
 }
 
 QString AbstractViewInspector::idStringForObject(QObject *obj) const
diff --git a/src/qml/debugger/qdebugmessageservice.cpp b/src/qml/debugger/qdebugmessageservice.cpp
index 4483cf1573..88ef9eb96c 100644
--- a/src/qml/debugger/qdebugmessageservice.cpp
+++ b/src/qml/debugger/qdebugmessageservice.cpp
@@ -93,7 +93,7 @@ void QDebugMessageService::sendDebugMessage(QtMsgType type,
     ws << QString::fromLatin1(ctxt.file).toUtf8();
     ws << ctxt.line << QString::fromLatin1(ctxt.function).toUtf8();
 
-    sendMessage(message);
+    emit messageToClient(name(), message);
     if (d->oldMsgHandler)
         (*d->oldMsgHandler)(type, ctxt, buf);
 }
diff --git a/src/qml/debugger/qqmldebugconnector_p.h b/src/qml/debugger/qqmldebugconnector_p.h
index e7b8b98c66..30283eae67 100644
--- a/src/qml/debugger/qqmldebugconnector_p.h
+++ b/src/qml/debugger/qqmldebugconnector_p.h
@@ -70,7 +70,6 @@ public:
     virtual bool addService(QQmlDebugService *service) = 0;
     virtual bool removeService(QQmlDebugService *service) = 0;
 
-    virtual void sendMessages(QQmlDebugService *service, const QList<QByteArray> &messages) = 0;
     virtual bool open(const QVariantHash &configuration = QVariantHash()) = 0;
 };
 
diff --git a/src/qml/debugger/qqmldebugserver.cpp b/src/qml/debugger/qqmldebugserver.cpp
index c48424f0fd..a2cb8fd5a1 100644
--- a/src/qml/debugger/qqmldebugserver.cpp
+++ b/src/qml/debugger/qqmldebugserver.cpp
@@ -102,7 +102,6 @@ public:
     bool open(const QVariantHash &configuration);
 
     void receiveMessage(const QByteArray &message);
-    void sendMessages(QQmlDebugService *service, const QList<QByteArray> &messages);
 
     template<class Action>
     bool enable(Action action);
@@ -110,7 +109,8 @@ public:
 
 private slots:
     void wakeEngine(QQmlEngine *engine);
-    void sendMessages(const QList<QByteArray> &messages);
+    void sendMessage(const QString &name, const QByteArray &message);
+    void sendMessages(const QString &name, const QList<QByteArray> &messages);
     void changeServiceState(const QString &serviceName, QQmlDebugService::State state);
     void removeThread();
 
@@ -623,15 +623,6 @@ void QQmlDebugServerImpl::changeServiceState(const QString &serviceName,
     m_changeServiceStateCalls.deref();
 }
 
-void QQmlDebugServerImpl::sendMessages(const QList<QByteArray> &messages)
-{
-    // to be executed in debugger thread
-    Q_ASSERT(QThread::currentThread() == thread());
-
-    if (m_connection)
-        m_connection->send(messages);
-}
-
 void QQmlDebugServerImpl::removeThread()
 {
     Q_ASSERT(m_thread->isFinished());
@@ -690,6 +681,11 @@ bool QQmlDebugServerImpl::addService(QQmlDebugService *service)
     // to be executed before thread starts
     Q_ASSERT(!m_thread);
 
+    connect(service, SIGNAL(messageToClient(QString,QByteArray)),
+            this, SLOT(sendMessage(QString,QByteArray)));
+    connect(service, SIGNAL(messagesToClient(QString,QList<QByteArray>)),
+            this, SLOT(sendMessages(QString,QList<QByteArray>)));
+
     connect(service, SIGNAL(attachedToEngine(QQmlEngine*)),
             this, SLOT(wakeEngine(QQmlEngine*)), Qt::QueuedConnection);
     connect(service, SIGNAL(detachedFromEngine(QQmlEngine*)),
@@ -720,24 +716,46 @@ bool QQmlDebugServerImpl::removeService(QQmlDebugService *service)
 
     if (!service || !m_plugins.contains(service->name()))
         return false;
+
+    disconnect(service, SIGNAL(messagesToClient(QString,QList<QByteArray>)),
+               this, SLOT(sendMessages(QString,QList<QByteArray>)));
+    disconnect(service, SIGNAL(messageToClient(QString,QByteArray)),
+               this, SLOT(sendMessage(QString,QByteArray)));
+
     m_plugins.remove(service->name());
 
     return true;
 }
 
-void QQmlDebugServerImpl::sendMessages(QQmlDebugService *service, const QList<QByteArray> &messages)
+void QQmlDebugServerImpl::sendMessage(const QString &name, const QByteArray &message)
 {
-    QList<QByteArray> prefixedMessages;
-    prefixedMessages.reserve(messages.count());
-    foreach (const QByteArray &message, messages) {
-        QByteArray prefixed;
-        QQmlDebugStream out(&prefixed, QIODevice::WriteOnly);
-        out << service->name() << message;
-        prefixedMessages << prefixed;
-    }
+    sendMessages(name, QList<QByteArray>() << message);
+}
 
-    QMetaObject::invokeMethod(this, "sendMessages", Qt::QueuedConnection,
-                              Q_ARG(QList<QByteArray>, prefixedMessages));
+void QQmlDebugServerImpl::sendMessages(const QString &name, const QList<QByteArray> &messages)
+{
+    // to be executed in debugger thread
+    Q_ASSERT(QThread::currentThread() == thread());
+
+    if (!m_connection)
+        return;
+
+    if (!name.isEmpty()) {
+        if (!m_clientPlugins.contains(name))
+            return;
+        QList<QByteArray> prefixedMessages;
+        prefixedMessages.reserve(messages.count());
+        foreach (const QByteArray &message, messages) {
+            QByteArray prefixed;
+            QQmlDebugStream out(&prefixed, QIODevice::WriteOnly);
+            out << name << message;
+            prefixedMessages << prefixed;
+        }
+
+        m_connection->send(prefixedMessages);
+    } else {
+        m_connection->send(messages);
+    }
 }
 
 template<class Action>
diff --git a/src/qml/debugger/qqmldebugservice.cpp b/src/qml/debugger/qqmldebugservice.cpp
index 44f42af5e4..edebf7a8cb 100644
--- a/src/qml/debugger/qqmldebugservice.cpp
+++ b/src/qml/debugger/qqmldebugservice.cpp
@@ -174,20 +174,6 @@ const QHash<int, QObject *> &QQmlDebugService::objectsForIds()
     return objectReferenceHash()->ids;
 }
 
-void QQmlDebugService::sendMessage(const QByteArray &message)
-{
-    sendMessages(QList<QByteArray>() << message);
-}
-
-void QQmlDebugService::sendMessages(const QList<QByteArray> &messages)
-{
-    if (state() != Enabled)
-        return;
-
-    if (QQmlDebugConnector *inst = QQmlDebugConnector::instance())
-        inst->sendMessages(this, messages);
-}
-
 void QQmlDebugService::stateAboutToBeChanged(State)
 {
 }
diff --git a/src/qml/debugger/qqmldebugservice_p.h b/src/qml/debugger/qqmldebugservice_p.h
index e17c02b477..311ce3e355 100644
--- a/src/qml/debugger/qqmldebugservice_p.h
+++ b/src/qml/debugger/qqmldebugservice_p.h
@@ -81,9 +81,6 @@ public:
     virtual void engineAdded(QQmlEngine *);
     virtual void engineRemoved(QQmlEngine *);
 
-    void sendMessage(const QByteArray &);
-    void sendMessages(const QList<QByteArray> &);
-
     static const QHash<int, QObject *> &objectsForIds();
     static int idForObject(QObject *);
     static QObject *objectForId(int id) { return objectsForIds().value(id); }
@@ -97,6 +94,9 @@ protected:
 signals:
     void attachedToEngine(QQmlEngine *);
     void detachedFromEngine(QQmlEngine *);
+
+    void messageToClient(const QString &name, const QByteArray &message);
+    void messagesToClient(const QString &name, const QList<QByteArray> &messages);
 };
 
 class Q_QML_PRIVATE_EXPORT QQmlDebugStream : public QDataStream
diff --git a/src/qml/debugger/qqmlenginecontrolservice.cpp b/src/qml/debugger/qqmlenginecontrolservice.cpp
index ed3fc9e1de..eaf948426d 100644
--- a/src/qml/debugger/qqmlenginecontrolservice.cpp
+++ b/src/qml/debugger/qqmlenginecontrolservice.cpp
@@ -117,7 +117,7 @@ void QQmlEngineControlService::sendMessage(QQmlEngineControlService::MessageType
     QByteArray message;
     QQmlDebugStream d(&message, QIODevice::WriteOnly);
     d << type << idForObject(engine);
-    QQmlDebugService::sendMessage(message);
+    emit messageToClient(name(), message);
 }
 
 void QQmlEngineControlService::stateChanged(State)
diff --git a/src/qml/debugger/qqmlenginedebugservice.cpp b/src/qml/debugger/qqmlenginedebugservice.cpp
index ad8f4fb15a..1d481d9eda 100644
--- a/src/qml/debugger/qqmlenginedebugservice.cpp
+++ b/src/qml/debugger/qqmlenginedebugservice.cpp
@@ -626,7 +626,7 @@ void QQmlEngineDebugService::processMessage(const QByteArray &message)
         rs << QByteArray("SET_METHOD_BODY_R") << queryId << ok;
 
     }
-    sendMessage(reply);
+    emit messageToClient(name(), reply);
 }
 
 bool QQmlEngineDebugService::setBinding(int objectId,
@@ -783,7 +783,7 @@ void QQmlEngineDebugService::propertyChanged(int id, int objectId, const QMetaPr
 
     rs << QByteArray("UPDATE_WATCH") << id << objectId << QByteArray(property.name()) << valueContents(value);
 
-    sendMessage(reply);
+    emit messageToClient(name(), reply);
 }
 
 void QQmlEngineDebugService::engineAboutToBeAdded(QQmlEngine *engine)
@@ -818,7 +818,7 @@ void QQmlEngineDebugService::objectCreated(QQmlEngine *engine, QObject *object)
 
     //unique queryId -1
     rs << QByteArray("OBJECT_CREATED") << -1 << engineId << objectId << parentId;
-    sendMessage(reply);
+    emit messageToClient(name(), reply);
 }
 
 void QQmlEngineDebugService::setStatesDelegate(QQmlDebugStatesDelegate *delegate)
diff --git a/src/qml/debugger/qqmlinspectorservice.cpp b/src/qml/debugger/qqmlinspectorservice.cpp
index e3c7ad7d55..c6ee8e4d52 100644
--- a/src/qml/debugger/qqmlinspectorservice.cpp
+++ b/src/qml/debugger/qqmlinspectorservice.cpp
@@ -73,14 +73,6 @@ void QQmlInspectorService::removeView(QObject *view)
     updateState();
 }
 
-void QQmlInspectorService::sendMessage(const QByteArray &message)
-{
-    if (state() != Enabled)
-        return;
-
-    QQmlDebugService::sendMessage(message);
-}
-
 void QQmlInspectorService::stateChanged(State /*state*/)
 {
     QMetaObject::invokeMethod(this, "updateState", Qt::QueuedConnection);
diff --git a/src/qml/debugger/qqmlinspectorservice_p.h b/src/qml/debugger/qqmlinspectorservice_p.h
index f49c36aacb..e69575788a 100644
--- a/src/qml/debugger/qqmlinspectorservice_p.h
+++ b/src/qml/debugger/qqmlinspectorservice_p.h
@@ -66,8 +66,6 @@ public:
     void addView(QObject *);
     void removeView(QObject *);
 
-    void sendMessage(const QByteArray &message);
-
 protected:
     virtual void stateChanged(State state);
     virtual void messageReceived(const QByteArray &);
diff --git a/src/qml/debugger/qqmlprofilerservice.cpp b/src/qml/debugger/qqmlprofilerservice.cpp
index e48771090e..62eae0ddd8 100644
--- a/src/qml/debugger/qqmlprofilerservice.cpp
+++ b/src/qml/debugger/qqmlprofilerservice.cpp
@@ -245,7 +245,7 @@ void QQmlProfilerService::startProfiling(QQmlEngine *engine, quint64 features)
         }
     }
 
-    QQmlDebugService::sendMessage(message);
+    emit messageToClient(name(), message);
 }
 
 /*!
@@ -334,7 +334,7 @@ void QQmlProfilerService::sendMessages()
     ds << (qint64)-1 << (int)Complete;
     messages << data;
 
-    QQmlDebugService::sendMessages(messages);
+    emit messagesToClient(name(), messages);
 }
 
 void QQmlProfilerService::stateAboutToBeChanged(QQmlDebugService::State newState)
diff --git a/src/qml/debugger/qv4debugservice.cpp b/src/qml/debugger/qv4debugservice.cpp
index 0677273982..81d4c7b70a 100644
--- a/src/qml/debugger/qv4debugservice.cpp
+++ b/src/qml/debugger/qv4debugservice.cpp
@@ -373,7 +373,7 @@ public:
 
         TRACE_PROTOCOL(qDebug() << "sending response for:" << responseData.constData() << endl);
 
-        q_func()->sendMessage(packMessage("v8message", responseData));
+        emit q_func()->messageToClient(name, packMessage("v8message", responseData));
     }
 
     void processCommand(const QByteArray &command, const QByteArray &data);
@@ -1136,7 +1136,7 @@ void QV4DebugService::messageReceived(const QByteArray &message)
         TRACE_PROTOCOL(qDebug() << "... type:" << type);
 
         if (type == V4_CONNECT) {
-            sendMessage(d->packMessage(type));
+            emit messageToClient(name(), d->packMessage(type));
             stopWaiting();
         } else if (type == V4_PAUSE) {
             d->debuggerAgent.pauseAll();
@@ -1170,7 +1170,7 @@ void QV4DebugService::sendSomethingToSomebody(const char *type, int magicNumber)
     QQmlDebugStream rs(&response, QIODevice::WriteOnly);
     rs << QByteArray(type)
        << QByteArray::number(d->version) << QByteArray::number(magicNumber);
-    sendMessage(d->packMessage(type, response));
+    emit messageToClient(name(), d->packMessage(type, response));
 }
 
 void QV4DebuggerAgent::debuggerPaused(QV4::Debugging::Debugger *debugger, QV4::Debugging::PauseReason reason)
diff --git a/tests/auto/qml/debugger/qqmldebugservice/tst_qqmldebugservice.cpp b/tests/auto/qml/debugger/qqmldebugservice/tst_qqmldebugservice.cpp
index 6ae59078f6..b63c5c0a6d 100644
--- a/tests/auto/qml/debugger/qqmldebugservice/tst_qqmldebugservice.cpp
+++ b/tests/auto/qml/debugger/qqmldebugservice/tst_qqmldebugservice.cpp
@@ -178,7 +178,7 @@ void tst_QQmlDebugService::sendMessage()
     QTest::ignoreMessage(QtWarningMsg,
                          "QQmlDebugService: Conflicting plugin name \"tst_QQmlDebugService\"");
     QQmlDebugTestService duplicate("tst_QQmlDebugService");
-    duplicate.sendMessage("msg");
+    emit duplicate.messageToClient(duplicate.name(), "msg");
     QTest::ignoreMessage(QtWarningMsg,
                          "QQmlDebugService: Plugin \"tst_QQmlDebugService\" is not registered.");
 }
diff --git a/tests/auto/qml/debugger/shared/qqmldebugtestservice.cpp b/tests/auto/qml/debugger/shared/qqmldebugtestservice.cpp
index 94eaa2c083..e62aa2ce61 100644
--- a/tests/auto/qml/debugger/shared/qqmldebugtestservice.cpp
+++ b/tests/auto/qml/debugger/shared/qqmldebugtestservice.cpp
@@ -42,7 +42,7 @@ QQmlDebugTestService::QQmlDebugTestService(const QString &s, float version, QObj
 void QQmlDebugTestService::messageReceived(const QByteArray &ba)
 {
     Q_ASSERT(QThread::currentThread() != thread());
-    QMetaObject::invokeMethod(this, "_sendMessage", Qt::QueuedConnection, Q_ARG(QByteArray, ba));
+    emit messageToClient(name(), ba);
 }
 
 void QQmlDebugTestService::stateAboutToBeChanged(QQmlDebugService::State)
@@ -55,8 +55,3 @@ void QQmlDebugTestService::stateChanged(State)
     Q_ASSERT(QThread::currentThread() != thread());
     emit stateHasChanged();
 }
-
-void QQmlDebugTestService::_sendMessage(const QByteArray &msg)
-{
-    QQmlDebugService::sendMessage(msg);
-}
diff --git a/tests/auto/qml/debugger/shared/qqmldebugtestservice.h b/tests/auto/qml/debugger/shared/qqmldebugtestservice.h
index 7cb09798de..cc24f3c119 100644
--- a/tests/auto/qml/debugger/shared/qqmldebugtestservice.h
+++ b/tests/auto/qml/debugger/shared/qqmldebugtestservice.h
@@ -46,9 +46,6 @@ public:
 signals:
     void stateHasChanged();
 
-private slots:
-    void _sendMessage(const QByteArray &msg);
-
 protected:
     virtual void messageReceived(const QByteArray &ba);
     virtual void stateAboutToBeChanged(State state);