From 5d6f5ff2c969046cd19c13a5b7bb0775c9925ea3 Mon Sep 17 00:00:00 2001
From: Ulf Hermann <ulf.hermann@qt.io>
Date: Thu, 4 Aug 2016 17:29:05 +0200
Subject: [PATCH] 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>
---
 .../qmlprofiler/qmlprofilerclientmanager.cpp  | 104 +++++-------------
 .../qmlprofiler/qmlprofilerclientmanager.h    |  13 +--
 .../qmlprofiler/qmlprofilermodelmanager.cpp   |  13 +++
 .../qmlprofiler/qmlprofilermodelmanager.h     |   2 +
 .../qmlprofiler/qmlprofilerstatemanager.cpp   |   4 +-
 .../qmlprofiler/qmlprofilerstatemanager.h     |   4 +-
 src/plugins/qmlprofiler/qmlprofilertool.cpp   |  23 +++-
 .../qmlprofiler/qmlprofilertraceclient.cpp    |  13 +--
 .../qmlprofiler/qmlprofilertraceclient.h      |   4 -
 .../tests/qmlprofilerclientmanager_test.cpp   |  13 +--
 10 files changed, 67 insertions(+), 126 deletions(-)

diff --git a/src/plugins/qmlprofiler/qmlprofilerclientmanager.cpp b/src/plugins/qmlprofiler/qmlprofilerclientmanager.cpp
index 56c39c06973..779a999ee5f 100644
--- a/src/plugins/qmlprofiler/qmlprofilerclientmanager.cpp
+++ b/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()
diff --git a/src/plugins/qmlprofiler/qmlprofilerclientmanager.h b/src/plugins/qmlprofiler/qmlprofilerclientmanager.h
index 8357acd8c7c..2c4a78f6b09 100644
--- a/src/plugins/qmlprofiler/qmlprofilerclientmanager.h
+++ b/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();
diff --git a/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp b/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp
index e19df2893ea..e004d229a26 100644
--- a/src/plugins/qmlprofiler/qmlprofilermodelmanager.cpp
+++ b/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];
diff --git a/src/plugins/qmlprofiler/qmlprofilermodelmanager.h b/src/plugins/qmlprofiler/qmlprofilermodelmanager.h
index c66a5e7948a..d26cfa4e22c 100644
--- a/src/plugins/qmlprofiler/qmlprofilermodelmanager.h
+++ b/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();
diff --git a/src/plugins/qmlprofiler/qmlprofilerstatemanager.cpp b/src/plugins/qmlprofiler/qmlprofilerstatemanager.cpp
index 772b2b2b38d..4ad395df59a 100644
--- a/src/plugins/qmlprofiler/qmlprofilerstatemanager.cpp
+++ b/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);
     }
 }
 
diff --git a/src/plugins/qmlprofiler/qmlprofilerstatemanager.h b/src/plugins/qmlprofiler/qmlprofilerstatemanager.h
index 6483d3a0701..35b9bca98a6 100644
--- a/src/plugins/qmlprofiler/qmlprofilerstatemanager.h
+++ b/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);
 
diff --git a/src/plugins/qmlprofiler/qmlprofilertool.cpp b/src/plugins/qmlprofiler/qmlprofilertool.cpp
index 4407a984209..dd4d84d91d0 100644
--- a/src/plugins/qmlprofiler/qmlprofilertool.cpp
+++ b/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);
     }
 }
 
diff --git a/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp b/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp
index 0e8b16d444d..ec15dcac9bd 100644
--- a/src/plugins/qmlprofiler/qmlprofilertraceclient.cpp
+++ b/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
diff --git a/src/plugins/qmlprofiler/qmlprofilertraceclient.h b/src/plugins/qmlprofiler/qmlprofilertraceclient.h
index b22d2fd52e3..ea6977d88cf 100644
--- a/src/plugins/qmlprofiler/qmlprofilertraceclient.h
+++ b/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;
 };
diff --git a/src/plugins/qmlprofiler/tests/qmlprofilerclientmanager_test.cpp b/src/plugins/qmlprofiler/tests/qmlprofilerclientmanager_test.cpp
index 192c857236a..fed9b53d325 100644
--- a/src/plugins/qmlprofiler/tests/qmlprofilerclientmanager_test.cpp
+++ b/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,
                 &clientManager, &QmlProfilerClientManager::retryConnect);
@@ -320,7 +314,6 @@ void QmlProfilerClientManagerTest::testResponsiveLocal_data()
 void QmlProfilerClientManagerTest::testResponsiveLocal()
 {
     QFETCH(quint32, flushInterval);
-    QFETCH(bool, aggregateTraces);
 
     QString socketFile = LocalQmlProfilerRunner::findFreeSocket();
 
@@ -332,8 +325,6 @@ void QmlProfilerClientManagerTest::testResponsiveLocal()
     clientManager.setProfilerStateManager(&stateManager);
     clientManager.setModelManager(&modelManager);
     clientManager.setFlushInterval(flushInterval);
-    clientManager.setAggregateTraces(aggregateTraces);
-    QCOMPARE(clientManager.aggregateTraces(), aggregateTraces);
 
     connect(&clientManager, &QmlProfilerClientManager::connectionFailed,
             &clientManager, &QmlProfilerClientManager::retryConnect);
-- 
GitLab