AutoTest: Move TestCodeParser and TestTreeModel ownership to plugin pimpl

Change-Id: I01b32aae894a4b419c8a067f604d5f04a2b14dfe
Reviewed-by: Christian Stenger <[email protected]>

9 files changed, 63 insertions, 61 deletions

diff --git a/src/plugins/autotest/autotestplugin.cpp b/src/plugins/autotest/autotestplugin.cpp
index 5b8c796ac3c..9e37f7bb0f4 100644
--- a/src/plugins/autotest/autotestplugin.cpp
+++ b/src/plugins/autotest/autotestplugin.cpp
@@ -102,6 +102,9 @@ public:
 
     TestSettings m_settings;
     TestSettingsPage m_testSettingPage{&m_settings};
+
+    TestCodeParser m_testCodeParser;
+    TestTreeModel m_testTreeModel{&m_testCodeParser};
 };
 
 static AutotestPluginPrivate *dd = nullptr;
@@ -118,6 +121,7 @@ AutotestPlugin::AutotestPlugin()
 AutotestPlugin::~AutotestPlugin()
 {
     delete dd;
+    dd = nullptr;
 }
 
 AutotestPluginPrivate::AutotestPluginPrivate()
@@ -144,7 +148,7 @@ AutotestPluginPrivate::AutotestPluginPrivate()
     ProjectExplorer::ProjectPanelFactory::registerFactory(panelFactory);
 
     m_frameworkManager->activateFrameworksFromSettings(&m_settings);
-    TestTreeModel::instance()->synchronizeTestFrameworks();
+    m_testTreeModel.synchronizeTestFrameworks();
 
     auto sessionManager = ProjectExplorer::SessionManager::instance();
     connect(sessionManager, &ProjectExplorer::SessionManager::startupProjectChanged,
@@ -232,9 +236,8 @@ void AutotestPluginPrivate::initializeMenuEntries()
     command = ActionManager::registerAction(action, Constants::ACTION_SCAN_ID);
     command->setDefaultKeySequence(
         QKeySequence(useMacShortcuts ? tr("Ctrl+Meta+T, Ctrl+Meta+S") : tr("Alt+Shift+T,Alt+S")));
-    connect(action, &QAction::triggered, this, []() {
-        TestTreeModel::instance()->parser()->updateTestTree();
-    });
+
+    connect(action, &QAction::triggered, this, [] { dd->m_testCodeParser.updateTestTree(); });
     menu->addAction(command);
 
     ActionContainer *toolsMenu = ActionManager::actionContainer(Core::Constants::M_TOOLS);
@@ -246,7 +249,7 @@ void AutotestPluginPrivate::initializeMenuEntries()
             this, &AutotestPlugin::updateMenuItemsEnabledState);
     connect(ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::runActionsUpdated,
             this, &AutotestPlugin::updateMenuItemsEnabledState);
-    connect(TestTreeModel::instance(), &TestTreeModel::testTreeModelChanged,
+    connect(&dd->m_testTreeModel, &TestTreeModel::testTreeModelChanged,
             this, &AutotestPlugin::updateMenuItemsEnabledState);
 }
 
@@ -288,23 +291,22 @@ void AutotestPlugin::extensionsInitialized()
 
 ExtensionSystem::IPlugin::ShutdownFlag AutotestPlugin::aboutToShutdown()
 {
-    TestTreeModel::instance()->parser()->aboutToShutdown();
+    dd->m_testCodeParser.aboutToShutdown();
+    dd->m_testTreeModel.disconnect();
     return SynchronousShutdown;
 }
 
 void AutotestPluginPrivate::onRunAllTriggered()
 {
     TestRunner *runner = TestRunner::instance();
-    TestTreeModel *model = TestTreeModel::instance();
-    runner->setSelectedTests(model->getAllTestCases());
+    runner->setSelectedTests(m_testTreeModel.getAllTestCases());
     runner->prepareToRunTests(TestRunMode::Run);
 }
 
 void AutotestPluginPrivate::onRunSelectedTriggered()
 {
     TestRunner *runner = TestRunner::instance();
-    TestTreeModel *model = TestTreeModel::instance();
-    runner->setSelectedTests(model->getSelectedTests());
+    runner->setSelectedTests(m_testTreeModel.getSelectedTests());
     runner->prepareToRunTests(TestRunMode::Run);
 }
 
@@ -318,8 +320,7 @@ void AutotestPluginPrivate::onRunFileTriggered()
     if (fileName.isEmpty())
         return;
 
-    TestTreeModel *model = TestTreeModel::instance();
-    const QList<TestConfiguration *> tests = model->getTestsForFile(fileName);
+    const QList<TestConfiguration *> tests = m_testTreeModel.getTestsForFile(fileName);
     if (tests.isEmpty())
         return;
 
@@ -348,7 +349,7 @@ void AutotestPluginPrivate::onRunUnderCursorTriggered(TestRunMode mode)
     if (text.isEmpty())
         return; // Do not trigger when no name under cursor
 
-    const QList<TestTreeItem *> testsItems = TestTreeModel::instance()->testItemsByName(text);
+    const QList<TestTreeItem *> testsItems = m_testTreeModel.testItemsByName(text);
     if (testsItems.isEmpty())
         return; // Wrong location triggered
 
@@ -377,8 +378,8 @@ void AutotestPlugin::updateMenuItemsEnabledState()
     const ProjectExplorer::Project *project = ProjectExplorer::SessionManager::startupProject();
     const ProjectExplorer::Target *target = project ? project->activeTarget() : nullptr;
     const bool canScan = !TestRunner::instance()->isTestRunning()
-            && TestTreeModel::instance()->parser()->state() == TestCodeParser::Idle;
-    const bool hasTests = TestTreeModel::instance()->hasTests();
+            && dd->m_testCodeParser.state() == TestCodeParser::Idle;
+    const bool hasTests = dd->m_testTreeModel.hasTests();
     // avoid expensive call to PE::canRunStartupProject() - limit to minimum necessary checks
     const bool canRun = hasTests && canScan
             && project && !project->needsConfiguration()
@@ -425,7 +426,7 @@ QVector<QObject *> AutotestPlugin::createTestObjects() const
 {
     QVector<QObject *> tests;
 #ifdef WITH_TESTS
-    tests << new AutoTestUnitTests(TestTreeModel::instance());
+    tests << new AutoTestUnitTests(&dd->m_testTreeModel);
 #endif
     return tests;
 }
diff --git a/src/plugins/autotest/itestframework.h b/src/plugins/autotest/itestframework.h
index bc3e5cb3862..8d81b28289a 100644
--- a/src/plugins/autotest/itestframework.h
+++ b/src/plugins/autotest/itestframework.h
@@ -38,7 +38,6 @@ public:
     explicit ITestFramework(bool activeByDefault) : m_active(activeByDefault) {}
     virtual ~ITestFramework()
     {
-        delete m_rootNode;
         delete m_testParser;
     }
 
@@ -50,6 +49,7 @@ public:
     TestTreeItem *rootNode()
     {   if (!m_rootNode)
             m_rootNode = createRootNode();
+        // These are stored in the TestTreeModel and destroyed on shutdown there.
         return m_rootNode;
     }
 
@@ -70,6 +70,17 @@ public:
     void setGrouping(bool group) { m_grouping = group; }
     // framework specific tool tip to be displayed on the general settings page
     virtual QString groupingToolTip() const { return QString(); }
+
+    void resetRootNode()
+    {
+        if (!m_rootNode)
+            return;
+        if (m_rootNode->model())
+            static_cast<TestTreeModel *>(m_rootNode->model())->takeItem(m_rootNode);
+        delete m_rootNode;
+        m_rootNode = nullptr;
+    }
+
 protected:
     virtual ITestParser *createTestParser() = 0;
     virtual TestTreeItem *createRootNode() const = 0;
diff --git a/src/plugins/autotest/testcodeparser.cpp b/src/plugins/autotest/testcodeparser.cpp
index e5336a9bdaf..c8c74075aed 100644
--- a/src/plugins/autotest/testcodeparser.cpp
+++ b/src/plugins/autotest/testcodeparser.cpp
@@ -55,10 +55,8 @@ namespace Internal {
 
 using namespace ProjectExplorer;
 
-TestCodeParser::TestCodeParser(TestTreeModel *parent)
-    : QObject(parent),
-      m_model(parent),
-      m_threadPool(new QThreadPool(this))
+TestCodeParser::TestCodeParser()
+    :  m_threadPool(new QThreadPool(this))
 {
     // connect to ProgressManager to postpone test parsing when CppModelManager is parsing
     auto progressManager = qobject_cast<Core::ProgressManager *>(Core::ProgressManager::instance());
@@ -206,7 +204,6 @@ void TestCodeParser::onQmlDocumentUpdated(const QmlJS::Document::Ptr &document)
 
 void TestCodeParser::onStartupProjectChanged(Project *project)
 {
-    m_model->synchronizeTestFrameworks(); // we might have project settings
     if (m_parserState == FullParse || m_parserState == PartialParse) {
         qCDebug(LOG) << "Canceling scanForTest (startup project changed)";
         Core::ProgressManager::instance()->cancelTasks(Constants::TASK_PARSE);
@@ -346,7 +343,7 @@ void TestCodeParser::scanForTests(const QStringList &fileList, const QList<ITest
             for (ITestFramework *framework : parsers)
                 framework->rootNode()->markForRemovalRecursively(true);
         } else {
-            m_model->markAllForRemoval();
+            emit requestRemoveAll();
         }
     } else if (!parsers.isEmpty()) {
         for (ITestFramework *framework : parsers) {
@@ -355,7 +352,7 @@ void TestCodeParser::scanForTests(const QStringList &fileList, const QList<ITest
         }
     } else {
         for (const QString &filePath : list)
-            m_model->markForRemoval(filePath);
+            emit requestRemoval(filePath);
     }
 
     QTC_ASSERT(!(isFullParse && list.isEmpty()), onFinished(); return);
diff --git a/src/plugins/autotest/testcodeparser.h b/src/plugins/autotest/testcodeparser.h
index 57b65f06336..a480c78f819 100644
--- a/src/plugins/autotest/testcodeparser.h
+++ b/src/plugins/autotest/testcodeparser.h
@@ -59,7 +59,8 @@ public:
         Shutdown
     };
 
-    explicit TestCodeParser(TestTreeModel *parent = nullptr);
+    TestCodeParser();
+
     void setState(State state);
     State state() const { return m_parserState; }
     bool isParsing() const { return m_parserState == PartialParse || m_parserState == FullParse; }
@@ -76,6 +77,8 @@ signals:
     void parsingStarted();
     void parsingFinished();
     void parsingFailed();
+    void requestRemoval(const QString &filePath);
+    void requestRemoveAll();
 
 public:
     void emitUpdateTestTree(ITestParser *parser = nullptr);
@@ -100,8 +103,6 @@ private:
     void parsePostponedFiles();
     void releaseParserInternals();
 
-    TestTreeModel *m_model;
-
     bool m_codeModelParsing = false;
     bool m_fullUpdatePostponed = false;
     bool m_partialUpdatePostponed = false;
diff --git a/src/plugins/autotest/testframeworkmanager.cpp b/src/plugins/autotest/testframeworkmanager.cpp
index 87893c3bcae..e64845e5324 100644
--- a/src/plugins/autotest/testframeworkmanager.cpp
+++ b/src/plugins/autotest/testframeworkmanager.cpp
@@ -51,7 +51,6 @@ static TestFrameworkManager *s_instance = nullptr;
 
 TestFrameworkManager::TestFrameworkManager()
 {
-    m_testTreeModel = TestTreeModel::instance();
     m_testRunner = Internal::TestRunner::instance();
     s_instance = this;
 }
@@ -66,7 +65,6 @@ TestFrameworkManager *TestFrameworkManager::instance()
 TestFrameworkManager::~TestFrameworkManager()
 {
     delete m_testRunner;
-    delete m_testTreeModel;
     for (ITestFramework *framework : m_registeredFrameworks.values())
         delete framework;
 }
diff --git a/src/plugins/autotest/testframeworkmanager.h b/src/plugins/autotest/testframeworkmanager.h
index e572c58a0f0..553484dcb96 100644
--- a/src/plugins/autotest/testframeworkmanager.h
+++ b/src/plugins/autotest/testframeworkmanager.h
@@ -36,18 +36,13 @@ QT_END_NAMESPACE
 namespace Core { class Id; }
 
 namespace Autotest {
-
-class TestTreeItem;
-
 namespace Internal {
-
 class TestRunner;
 struct TestSettings;
 }
 
 class IFrameworkSettings;
 class ITestParser;
-class TestTreeModel;
 
 class TestFrameworkManager
 {
@@ -73,7 +68,6 @@ private:
     explicit TestFrameworkManager();
     QHash<Core::Id, ITestFramework *> m_registeredFrameworks;
     QHash<Core::Id, IFrameworkSettings *> m_frameworkSettings;
-    TestTreeModel *m_testTreeModel;
     Internal::TestRunner *m_testRunner;
 
     typedef QHash<Core::Id, ITestFramework *>::ConstIterator FrameworkIterator;
diff --git a/src/plugins/autotest/testprojectsettings.cpp b/src/plugins/autotest/testprojectsettings.cpp
index 6aab384aafe..d20b70bfc97 100644
--- a/src/plugins/autotest/testprojectsettings.cpp
+++ b/src/plugins/autotest/testprojectsettings.cpp
@@ -60,7 +60,10 @@ void TestProjectSettings::setUseGlobalSettings(bool useGlobal)
 
 void TestProjectSettings::activateFramework(const Core::Id &id, bool activate)
 {
-    m_activeTestFrameworks[TestFrameworkManager::instance()->frameworkForId(id)] = activate;
+    ITestFramework *framework = TestFrameworkManager::instance()->frameworkForId(id);
+    m_activeTestFrameworks[framework] = activate;
+    if (!activate)
+        framework->resetRootNode();
 }
 
 void TestProjectSettings::load()
diff --git a/src/plugins/autotest/testtreemodel.cpp b/src/plugins/autotest/testtreemodel.cpp
index 9e0b3745142..2b792774933 100644
--- a/src/plugins/autotest/testtreemodel.cpp
+++ b/src/plugins/autotest/testtreemodel.cpp
@@ -39,14 +39,18 @@
 #include <texteditor/texteditor.h>
 #include <utils/qtcassert.h>
 
+using namespace ProjectExplorer;
+using namespace Autotest::Internal;
+
 namespace Autotest {
 
-using namespace Internal;
+static TestTreeModel *s_instance = nullptr;
 
-TestTreeModel::TestTreeModel(QObject *parent) :
-    TreeModel<>(parent),
-    m_parser(new TestCodeParser(this))
+TestTreeModel::TestTreeModel(TestCodeParser *parser) :
+    m_parser(parser)
 {
+    s_instance = this;
+
     connect(m_parser, &TestCodeParser::aboutToPerformFullParse, this,
             &TestTreeModel::removeAllTestItems, Qt::QueuedConnection);
     connect(m_parser, &TestCodeParser::testParseResultReady,
@@ -55,22 +59,21 @@ TestTreeModel::TestTreeModel(QObject *parent) :
             this, &TestTreeModel::sweep, Qt::QueuedConnection);
     connect(m_parser, &TestCodeParser::parsingFailed,
             this, &TestTreeModel::sweep, Qt::QueuedConnection);
+    connect(m_parser, &TestCodeParser::requestRemoveAll,
+            this, &TestTreeModel::markAllForRemoval);
+    connect(m_parser, &TestCodeParser::requestRemoval,
+            this, &TestTreeModel::markForRemoval);
 
     setupParsingConnections();
 }
 
-static TestTreeModel *s_instance = nullptr;
-
 TestTreeModel *TestTreeModel::instance()
 {
-    if (!s_instance)
-        s_instance = new TestTreeModel;
     return s_instance;
 }
 
 TestTreeModel::~TestTreeModel()
 {
-    removeTestRootNodes();
     s_instance = nullptr;
 }
 
@@ -82,9 +85,11 @@ void TestTreeModel::setupParsingConnections()
     m_parser->setDirty();
     m_parser->setState(TestCodeParser::Idle);
 
-    ProjectExplorer::SessionManager *sm = ProjectExplorer::SessionManager::instance();
-    connect(sm, &ProjectExplorer::SessionManager::startupProjectChanged,
-            m_parser, &TestCodeParser::onStartupProjectChanged);
+    SessionManager *sm = SessionManager::instance();
+    connect(sm, &SessionManager::startupProjectChanged, [this](Project *project) {
+        synchronizeTestFrameworks(); // we might have project settings
+        m_parser->onStartupProjectChanged(project);
+    });
 
     CppTools::CppModelManager *cppMM = CppTools::CppModelManager::instance();
     connect(cppMM, &CppTools::CppModelManager::documentUpdated,
@@ -498,17 +503,6 @@ void TestTreeModel::removeAllTestItems()
     emit testTreeModelChanged();
 }
 
-void TestTreeModel::removeTestRootNodes()
-{
-    const Utils::TreeItem *invisibleRoot = rootItem();
-    const int frameworkRootCount = invisibleRoot ? invisibleRoot->childCount() : 0;
-    for (int row = frameworkRootCount - 1; row >= 0; --row) {
-        Utils::TreeItem *item = invisibleRoot->childAt(row);
-        item->removeChildren();
-        takeItem(item); // do NOT delete the item as it's still a ptr held by TestFrameworkManager
-    }
-}
-
 #ifdef WITH_TESTS
 // we're inside tests - so use some internal knowledge to make testing easier
 static TestTreeItem *qtRootNode()
diff --git a/src/plugins/autotest/testtreemodel.h b/src/plugins/autotest/testtreemodel.h
index c22a8f8d493..ec84b43b0f7 100644
--- a/src/plugins/autotest/testtreemodel.h
+++ b/src/plugins/autotest/testtreemodel.h
@@ -36,6 +36,7 @@
 
 namespace Autotest {
 namespace Internal {
+class AutotestPluginPrivate;
 class TestCodeParser;
 } // namespace Internal
 
@@ -45,6 +46,10 @@ using TestParseResultPtr = QSharedPointer<TestParseResult>;
 class AUTOTESTSHARED_EXPORT TestTreeModel : public Utils::TreeModel<>
 {
     Q_OBJECT
+
+    friend class Internal::AutotestPluginPrivate; // For ctor.
+    explicit TestTreeModel(Internal::TestCodeParser *parser);
+
 public:
     static TestTreeModel* instance();
     ~TestTreeModel() override;
@@ -89,12 +94,10 @@ private:
     void onParseResultReady(const TestParseResultPtr result);
     void handleParseResult(const TestParseResult *result, TestTreeItem *rootNode);
     void removeAllTestItems();
-    void removeTestRootNodes();
     void removeFiles(const QStringList &files);
     bool sweepChildren(TestTreeItem *item);
     void insertItemInParent(TestTreeItem *item, TestTreeItem *root, bool groupingEnabled);
     void revalidateCheckState(TestTreeItem *item);
-    explicit TestTreeModel(QObject *parent = nullptr);
     void setupParsingConnections();
     void filterAndInsert(TestTreeItem *item, TestTreeItem *root, bool groupingEnabled);
     QList<TestTreeItem *> testItemsByName(TestTreeItem *root, const QString &testName);