QmlProfiler: Move unrelated code out of QmlProfilerClientManager

The client manager should not be bothered with details of QML events,
but rather just connect the client, the model manager, and the state
manager.

Change-Id: Iec4499f8441a06d4ef5cbcf7bfe23da6f5e7f239
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/plugins/qmlprofiler/qmlprofilerclientmanager.cpp

@@ -59,16 +59,6 @@ void QmlProfilerClientManager::setFlushInterval(quint32 flushInterval)
     m_flushInterval = flushInterval;
 }
 
-bool QmlProfilerClientManager::aggregateTraces() const
-{
-    return m_aggregateTraces;
-}
-
-void QmlProfilerClientManager::setAggregateTraces(bool aggregateTraces)
-{
-    m_aggregateTraces = aggregateTraces;
-}
-
 void QmlProfilerClientManager::setRetryParams(int interval, int maxAttempts)
 {
     m_retryInterval = interval;
@@ -183,6 +173,12 @@ void QmlProfilerClientManager::startLocalServer()
     m_connectionTimer.start(m_retryInterval);
 }
 
+void QmlProfilerClientManager::stopRecording()
+{
+    QTC_ASSERT(m_qmlclientplugin, return);
+    m_qmlclientplugin->setRecording(false);
+}
+
 void QmlProfilerClientManager::retryConnect()
 {
     if (!m_localSocket.isEmpty()) {
@@ -230,24 +226,31 @@ void QmlProfilerClientManager::connectClientSignals()
 
 
     QTC_ASSERT(m_qmlclientplugin, return);
-    QObject::connect(m_qmlclientplugin.data(), &QmlProfilerTraceClient::complete,
-                     this, &QmlProfilerClientManager::qmlComplete);
-    QObject::connect(m_qmlclientplugin.data(), &QmlProfilerTraceClient::newEngine,
-                     this, &QmlProfilerClientManager::qmlNewEngine);
-
     QTC_ASSERT(m_modelManager, return);
     QObject::connect(m_qmlclientplugin.data(), &QmlProfilerTraceClient::traceFinished,
                      m_modelManager->traceTime(), &QmlProfilerTraceTime::increaseEndTime);
-    QObject::connect(m_qmlclientplugin.data(), &QmlProfilerTraceClient::traceStarted,
-                     m_modelManager->traceTime(), &QmlProfilerTraceTime::decreaseStartTime);
 
     QTC_ASSERT(m_profilerState, return);
-    QObject::connect(m_qmlclientplugin.data(), &QmlProfilerTraceClient::recordingChanged,
-                     m_profilerState.data(), &QmlProfilerStateManager::setServerRecording);
     QObject::connect(m_profilerState.data(), &QmlProfilerStateManager::requestedFeaturesChanged,
                      m_qmlclientplugin.data(), &QmlProfilerTraceClient::setRequestedFeatures);
     QObject::connect(m_qmlclientplugin.data(), &QmlProfilerTraceClient::recordedFeaturesChanged,
                      m_profilerState.data(), &QmlProfilerStateManager::setRecordedFeatures);
+
+    QObject::connect(m_qmlclientplugin.data(), &QmlProfilerTraceClient::traceStarted,
+                     this, [this](qint64 time) {
+        m_profilerState->setServerRecording(true);
+        m_modelManager->traceTime()->decreaseStartTime(time);
+    });
+
+    QObject::connect(m_qmlclientplugin.data(), &QmlProfilerTraceClient::complete,
+                     this, [this](qint64 time) {
+        m_modelManager->traceTime()->increaseEndTime(time);
+        m_profilerState->setServerRecording(false);
+    });
+
+    QObject::connect(m_profilerState.data(), &QmlProfilerStateManager::clientRecordingChanged,
+                     m_qmlclientplugin.data(), &QmlProfilerTraceClient::setRecording);
+
 }
 
 void QmlProfilerClientManager::disconnectClientSignals()
@@ -261,6 +264,8 @@ void QmlProfilerClientManager::disconnectClientSignals()
     QTC_ASSERT(m_profilerState, return);
     QObject::disconnect(m_profilerState.data(), &QmlProfilerStateManager::requestedFeaturesChanged,
                         m_qmlclientplugin.data(), &QmlProfilerTraceClient::setRequestedFeatures);
+    QObject::disconnect(m_profilerState.data(), &QmlProfilerStateManager::clientRecordingChanged,
+                        m_qmlclientplugin.data(), &QmlProfilerTraceClient::setRecording);
 }
 
 bool QmlProfilerClientManager::isConnected() const
@@ -285,10 +290,11 @@ void QmlProfilerClientManager::disconnectClient()
 void QmlProfilerClientManager::qmlDebugConnectionOpened()
 {
     logState(tr("Debug connection opened"));
+    QTC_ASSERT(m_profilerState, return);
     QTC_ASSERT(m_connection && m_qmlclientplugin, return);
     QTC_ASSERT(m_connection->isConnected(), return);
     stopConnectionTimer();
-    clientRecordingChanged();
+    m_qmlclientplugin->setRecording(m_profilerState->clientRecording());
     emit connectionOpened();
 }
 
@@ -318,70 +324,12 @@ void QmlProfilerClientManager::logState(const QString &msg)
     QmlProfilerTool::logState(QLatin1String("QML Profiler: ") + msg);
 }
 
-void QmlProfilerClientManager::qmlComplete(qint64 maximumTime)
-{
-    QTC_ASSERT(m_profilerState && m_modelManager, return);
-    if (m_profilerState->currentState() == QmlProfilerStateManager::AppStopRequested)
-        m_profilerState->setCurrentState(QmlProfilerStateManager::Idle);
-    m_modelManager->traceTime()->increaseEndTime(maximumTime);
-    if (m_modelManager && !m_aggregateTraces)
-        m_modelManager->acquiringDone();
-}
-
-void QmlProfilerClientManager::qmlNewEngine(int engineId)
-{
-    QTC_ASSERT(m_connection && m_qmlclientplugin, return);
-    if (m_qmlclientplugin->isRecording() != m_profilerState->clientRecording())
-        m_qmlclientplugin->setRecording(m_profilerState->clientRecording());
-    else
-        m_qmlclientplugin->sendRecordingStatus(engineId);
-}
-
 void QmlProfilerClientManager::setProfilerStateManager(QmlProfilerStateManager *profilerState)
 {
     // Don't do this while connecting
     QTC_ASSERT(m_connection.isNull() && m_qmlclientplugin.isNull(), disconnectClient());
 
-    if (m_profilerState) {
-        disconnect(m_profilerState.data(), &QmlProfilerStateManager::stateChanged,
-                   this, &QmlProfilerClientManager::profilerStateChanged);
-        disconnect(m_profilerState.data(), &QmlProfilerStateManager::clientRecordingChanged,
-                   this, &QmlProfilerClientManager::clientRecordingChanged);
-    }
-
     m_profilerState = profilerState;
-
-    // connect
-    if (m_profilerState) {
-        connect(m_profilerState.data(), &QmlProfilerStateManager::stateChanged,
-                this, &QmlProfilerClientManager::profilerStateChanged);
-        connect(m_profilerState.data(), &QmlProfilerStateManager::clientRecordingChanged,
-                this, &QmlProfilerClientManager::clientRecordingChanged);
-    }
-}
-
-void QmlProfilerClientManager::profilerStateChanged()
-{
-    QTC_ASSERT(m_profilerState, return);
-    switch (m_profilerState->currentState()) {
-    case QmlProfilerStateManager::AppStopRequested :
-        if (m_profilerState->serverRecording()) {
-            if (m_qmlclientplugin)
-                m_qmlclientplugin->setRecording(false);
-        } else {
-            m_profilerState->setCurrentState(QmlProfilerStateManager::Idle);
-        }
-        break;
-    default:
-        break;
-    }
-}
-
-void QmlProfilerClientManager::clientRecordingChanged()
-{
-    QTC_ASSERT(m_profilerState, return);
-    if (m_qmlclientplugin)
-        m_qmlclientplugin->setRecording(m_profilerState->clientRecording());
 }
 
 void QmlProfilerClientManager::stopConnectionTimer()

src/plugins/qmlprofiler/qmlprofilerclientmanager.h

@@ -59,14 +59,13 @@ public:
     void setModelManager(QmlProfilerModelManager *m);
     void setFlushInterval(quint32 flushInterval);
 
-    bool aggregateTraces() const;
-    void setAggregateTraces(bool aggregateTraces);
-
     void setRetryParams(int interval, int maxAttempts);
     void retryConnect();
     void connectToTcpServer();
     void startLocalServer();
 
+    void stopRecording();
+
 signals:
     void connectionOpened();
     void connectionFailed();
@@ -89,8 +88,6 @@ private:
     int m_maximumRetries = 50;
     int m_numRetries = 0;
 
-    bool m_aggregateTraces = true;
-
     void disconnectClient();
     void stopConnectionTimer();
 
@@ -100,12 +97,6 @@ private:
 
     void logState(const QString &);
 
-    void qmlComplete(qint64 maximumTime);
-    void qmlNewEngine(int engineId);
-
-    void profilerStateChanged();
-    void clientRecordingChanged();
-
     void createConnection();
     void connectClientSignals();
     void disconnectClientSignals();

src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp

@@ -149,6 +149,7 @@ public:
     quint64 availableFeatures;
     quint64 visibleFeatures;
     quint64 recordedFeatures;
+    bool aggregateTraces;
 
     QHash<ProfileFeature, QVector<EventLoader> > eventLoaders;
     QVector<Finalizer> finalizers;
@@ -164,6 +165,7 @@ QmlProfilerModelManager::QmlProfilerModelManager(Utils::FileInProjectFinder *fin
     d->availableFeatures = 0;
     d->visibleFeatures = 0;
     d->recordedFeatures = 0;
+    d->aggregateTraces = false;
     d->model = new QmlProfilerDataModel(finder, this);
     d->state = Empty;
     d->traceTime = new QmlProfilerTraceTime(this);
@@ -275,6 +277,17 @@ void QmlProfilerModelManager::setRecordedFeatures(quint64 features)
     }
 }
 
+bool QmlProfilerModelManager::aggregateTraces() const
+{
+    return d->aggregateTraces;
+}
+
+void QmlProfilerModelManager::setAggregateTraces(bool aggregateTraces)
+{
+    d->aggregateTraces = aggregateTraces;
+}
+
+
 const char *QmlProfilerModelManager::featureName(ProfileFeature feature)
 {
     return ProfileFeatureNames[feature];

src/plugins/qmlprofiler/qmlprofilermodelmanager.h

@@ -114,6 +114,8 @@ public:
     void setVisibleFeatures(quint64 features);
     quint64 recordedFeatures() const;
     void setRecordedFeatures(quint64 features);
+    bool aggregateTraces() const;
+    void setAggregateTraces(bool aggregateTraces);
 
     void acquiringDone();
     void processingDone();

src/plugins/qmlprofiler/qmlprofilerstatemanager.cpp

@@ -145,7 +145,7 @@ void QmlProfilerStateManager::setClientRecording(bool recording)
 #endif
     if (d->m_clientRecording != recording) {
         d->m_clientRecording = recording;
-        emit clientRecordingChanged();
+        emit clientRecordingChanged(recording);
     }
 }
 
@@ -156,7 +156,7 @@ void QmlProfilerStateManager::setServerRecording(bool recording)
 #endif
     if (d->m_serverRecording != recording) {
         d->m_serverRecording = recording;
-        emit serverRecordingChanged();
+        emit serverRecordingChanged(recording);
     }
 }
 

src/plugins/qmlprofiler/qmlprofilerstatemanager.h

@@ -53,8 +53,8 @@ public:
 
 signals:
     void stateChanged();
-    void clientRecordingChanged();
-    void serverRecordingChanged();
+    void clientRecordingChanged(bool);
+    void serverRecordingChanged(bool);
     void requestedFeaturesChanged(quint64);
     void recordedFeaturesChanged(quint64);
 

src/plugins/qmlprofiler/qmlprofilertool.cpp

@@ -327,7 +327,7 @@ AnalyzerRunControl *QmlProfilerTool::createRunControl(RunConfiguration *runConfi
             if (QmlProfilerSettings *settings = static_cast<QmlProfilerSettings *>(aspect->currentSettings())) {
                 d->m_profilerConnections->setFlushInterval(settings->flushEnabled() ?
                                                                settings->flushInterval() : 0);
-                d->m_profilerConnections->setAggregateTraces(settings->aggregateTraces());
+                d->m_profilerModelManager->setAggregateTraces(settings->aggregateTraces());
             }
         }
     }
@@ -448,7 +448,7 @@ void QmlProfilerTool::recordingButtonChanged(bool recording)
 
     if (recording && d->m_profilerState->currentState() == QmlProfilerStateManager::AppRunning) {
         if (checkForUnsavedNotes()) {
-            if (!d->m_profilerConnections->aggregateTraces() ||
+            if (!d->m_profilerModelManager->aggregateTraces() ||
                     d->m_profilerModelManager->state() == QmlProfilerModelManager::Done)
                 clearData(); // clear before the recording starts, unless we aggregate recordings
             if (d->m_profilerState->clientRecording())
@@ -731,7 +731,7 @@ void QmlProfilerTool::restoreFeatureVisibility()
 void QmlProfilerTool::clientsDisconnected()
 {
     if (d->m_profilerModelManager->state() == QmlProfilerModelManager::AcquiringData) {
-        if (d->m_profilerConnections->aggregateTraces()) {
+        if (d->m_profilerModelManager->aggregateTraces()) {
             d->m_profilerModelManager->acquiringDone();
         } else {
             // If the application stopped by itself, check if we have all the data
@@ -868,8 +868,14 @@ void QmlProfilerTool::profilerStateChanged()
         break;
     case QmlProfilerStateManager::AppStopRequested:
         // Don't allow toggling the recording while data is loaded when application quits
-        if (d->m_profilerState->serverRecording())
+        if (d->m_profilerState->serverRecording()) {
             d->m_recordButton->setEnabled(false);
+            // Turn off recording and wait for remaining data
+            d->m_profilerConnections->stopRecording();
+        } else {
+            // Directly transition to idle
+            d->m_profilerState->setCurrentState(QmlProfilerStateManager::Idle);
+        }
         break;
     default:
         // no special action needed for other states
@@ -903,7 +909,7 @@ void QmlProfilerTool::serverRecordingChanged()
                 showSaveDialog();
 
             setRecording(true);
-            if (!d->m_profilerConnections->aggregateTraces() ||
+            if (!d->m_profilerModelManager->aggregateTraces() ||
                     d->m_profilerModelManager->state() == QmlProfilerModelManager::Done)
                 clearData();
             d->m_profilerModelManager->startAcquiring();
@@ -911,9 +917,14 @@ void QmlProfilerTool::serverRecordingChanged()
             setRecording(false);
 
             // changes back once loading is finished, see profilerDataModelStateChanged()
-            if (!d->m_profilerConnections->aggregateTraces())
+            if (!d->m_profilerModelManager->aggregateTraces()) {
                 d->m_recordButton->setEnabled(false);
+                d->m_profilerModelManager->acquiringDone();
+            }
         }
+    } else if (d->m_profilerState->currentState() == QmlProfilerStateManager::AppStopRequested) {
+        d->m_profilerModelManager->acquiringDone();
+        d->m_profilerState->setCurrentState(QmlProfilerStateManager::Idle);
     }
 }
 

src/plugins/qmlprofiler/qmlprofilertraceclient.cpp

@@ -180,7 +180,7 @@ QmlProfilerTraceClient::QmlProfilerTraceClient(QmlDebug::QmlDebugConnection *cli
 {
     setRequestedFeatures(features);
     connect(&d->engineControl, &QmlDebug::QmlEngineControlClient::engineAboutToBeAdded,
-            this, &QmlProfilerTraceClient::newEngine);
+            this, &QmlProfilerTraceClient::sendRecordingStatus);
 }
 
 QmlProfilerTraceClient::~QmlProfilerTraceClient()
@@ -259,14 +259,6 @@ void QmlProfilerTraceClient::setFlushInterval(quint32 flushInterval)
     d->flushInterval = flushInterval;
 }
 
-void QmlProfilerTraceClient::setRecordingFromServer(bool v)
-{
-    if (v == d->recording)
-        return;
-    d->recording = v;
-    emit recordingChanged(v);
-}
-
 bool QmlProfilerTraceClientPrivate::updateFeatures(ProfileFeature feature)
 {
     quint64 flag = 1ULL << feature;
@@ -300,11 +292,8 @@ void QmlProfilerTraceClient::messageReceived(const QByteArray &data)
             d->processCurrentEvent();
         }
         emit complete(d->maximumTime);
-        setRecordingFromServer(false);
     } else if (d->currentEvent.type.message() == Event
                && d->currentEvent.type.detailType() == StartTrace) {
-        if (!d->recording)
-            setRecordingFromServer(true);
         emit traceStarted(d->currentEvent.event.timestamp(),
                           d->currentEvent.event.numbers<QList<int>, qint32>());
     } else if (d->currentEvent.type.message() == Event

src/plugins/qmlprofiler/qmlprofilertraceclient.h

@@ -65,7 +65,6 @@ signals:
 
     void recordingChanged(bool arg);
     void recordedFeaturesChanged(quint64 features);
-    void newEngine(int engineId);
 
     void cleared();
 
@@ -73,9 +72,6 @@ protected:
     virtual void stateChanged(State status);
     virtual void messageReceived(const QByteArray &);
 
-private:
-    void setRecordingFromServer(bool);
-
 private:
     class QmlProfilerTraceClientPrivate *d;
 };

src/plugins/qmlprofiler/tests/qmlprofilerclientmanager_test.cpp

@@ -223,12 +223,9 @@ void QmlProfilerClientManagerTest::testUnresponsiveLocal()
 void responsiveTestData()
 {
     QTest::addColumn<quint32>("flushInterval");
-    QTest::addColumn<bool>("aggregateTraces");
 
-    QTest::newRow("no flush, no aggregate") << 0u << false;
-    QTest::newRow("no flush, aggregate")    << 0u << true;
-    QTest::newRow("flush, no aggregate")    << 1u << false;
-    QTest::newRow("flush, aggregate")       << 1u << true;
+    QTest::newRow("no flush") << 0u;
+    QTest::newRow("flush")    << 1u;
 }
 
 void QmlProfilerClientManagerTest::testResponsiveTcp_data()
@@ -257,7 +254,6 @@ void fakeDebugServer(QIODevice *socket)
 void QmlProfilerClientManagerTest::testResponsiveTcp()
 {
     QFETCH(quint32, flushInterval);
-    QFETCH(bool, aggregateTraces);
 
     QString hostName;
     Utils::Port port = LocalQmlProfilerRunner::findFreePort(hostName);
@@ -280,8 +276,6 @@ void QmlProfilerClientManagerTest::testResponsiveTcp()
         clientManager.setProfilerStateManager(&stateManager);
         clientManager.setModelManager(&modelManager);
         clientManager.setFlushInterval(flushInterval);
-        clientManager.setAggregateTraces(aggregateTraces);
-        QCOMPARE(clientManager.aggregateTraces(), aggregateTraces);
 
         connect(&clientManager, &QmlProfilerClientManager::connectionFailed,