ProjectExplorer: Make kit aspects self-register

Also, construct the KitManager implicitly when the first
KitAspect is created. Ramp-down is still explicit and
somewhat odd.

Change-Id: If1506e1d0789ecabbaad2d8008851d0b42c5218b
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/android/androidplugin.cpp

@@ -136,6 +136,7 @@ public:
     AndroidManifestEditorFactory manifestEditorFactory;
     AndroidRunConfigurationFactory runConfigFactory;
     AndroidBuildApkStepFactory buildApkStepFactory;
+    AndroidGdbServerKitAspect gdbServerKitAspect;
 };
 
 AndroidPlugin::~AndroidPlugin()
@@ -160,8 +161,6 @@ bool AndroidPlugin::initialize(const QStringList &arguments, QString *errorMessa
 
     d = new AndroidPluginPrivate;
 
-    KitManager::registerKitAspect<Internal::AndroidGdbServerKitAspect>();
-
     connect(KitManager::instance(), &KitManager::kitsLoaded,
             this, &AndroidPlugin::kitsRestored);
 

src/plugins/cmakeprojectmanager/cmakeprojectplugin.cpp

@@ -70,6 +70,10 @@ public:
     CMakeBuildConfigurationFactory buildConfigFactory;
     CMakeEditorFactory editorFactor;
     CMakeLocatorFilter locatorFiler;
+
+    CMakeKitAspect cmakeKitAspect;
+    CMakeGeneratorKitAspect cmakeGeneratorKitAspect;
+    CMakeConfigurationKitAspect cmakeConfigurationKitAspect;
 };
 
 const std::unique_ptr<CMakeSpecificSettings>
@@ -106,10 +110,6 @@ bool CMakeProjectPlugin::initialize(const QStringList & /*arguments*/, QString *
 
     new CMakeToolManager(this);
 
-    KitManager::registerKitAspect<CMakeKitAspect>();
-    KitManager::registerKitAspect<CMakeGeneratorKitAspect>();
-    KitManager::registerKitAspect<CMakeConfigurationKitAspect>();
-
     //menus
     ActionContainer *msubproject =
             ActionManager::actionContainer(ProjectExplorer::Constants::M_SUBPROJECTCONTEXT);

src/plugins/debugger/debuggerplugin.cpp

@@ -775,6 +775,8 @@ public:
 
     DebugInfoTaskHandler m_debugInfoTaskHandler;
     Perspective m_perspective{Constants::PRESET_PERSPECTIVE_ID, tr("Debugger")};
+
+    DebuggerKitAspect debuggerKitAspect;
 };
 
 DebuggerPluginPrivate::DebuggerPluginPrivate(DebuggerPlugin *plugin)
@@ -2256,8 +2258,6 @@ bool DebuggerPlugin::initialize(const QStringList &arguments, QString *errorMess
     mstart->addSeparator(MENU_GROUP_GENERAL);
     mstart->addSeparator(MENU_GROUP_SPECIAL);
 
-    KitManager::registerKitAspect<DebuggerKitAspect>();
-
     // Task integration.
     //: Category under which Analyzer tasks are listed in Issues view
     ProjectExplorer::TaskHub::addCategory(Debugger::Constants::ANALYZERTASK_ID, tr("Debugger"));

src/plugins/projectexplorer/kitmanager.cpp

@@ -48,6 +48,16 @@ using namespace Utils;
 using namespace ProjectExplorer::Internal;
 
 namespace ProjectExplorer {
+
+class KitList
+{
+public:
+    Core::Id defaultKit;
+    std::vector<std::unique_ptr<Kit>> kits;
+};
+
+static KitList restoreKitsHelper(const Utils::FileName &fileName);
+
 namespace Internal {
 
 const char KIT_DATA_KEY[] = "Profile.";
@@ -75,8 +85,35 @@ public:
     std::unique_ptr<PersistentSettingsWriter> m_writer;
     QSet<Id> m_irrelevantAspects;
 
-    // Sorted by priority, in descending order.
-    std::vector<std::unique_ptr<KitAspect>> m_informationList;
+    void addKitAspect(KitAspect *ki)
+    {
+        QTC_ASSERT(!m_aspectList.contains(ki), return);
+        m_aspectList.append(ki);
+        m_aspectListIsSorted = false;
+    }
+
+    void removeKitAspect(KitAspect *ki)
+    {
+        int removed = m_aspectList.removeAll(ki);
+        QTC_CHECK(removed == 1);
+    }
+
+    const QList<KitAspect *> kitAspects()
+    {
+        if (!m_aspectListIsSorted) {
+            Utils::sort(m_aspectList, [](const KitAspect *a, const KitAspect *b) {
+                return a->priority() > b->priority();
+            });
+            m_aspectListIsSorted = true;
+        }
+        return m_aspectList;
+    }
+
+private:
+    // Sorted by priority, in descending order...
+    QList<KitAspect *> m_aspectList;
+    // ... if this here is set:
+    bool m_aspectListIsSorted = true;
 };
 
 } // namespace Internal
@@ -90,11 +127,12 @@ static KitManager *m_instance = nullptr;
 
 KitManager *KitManager::instance()
 {
+    if (!m_instance)
+        m_instance = new KitManager;
     return m_instance;
 }
 
-KitManager::KitManager(QObject *parent)
-    : QObject(parent)
+KitManager::KitManager()
 {
     d = new KitManagerPrivate;
     QTC_CHECK(!m_instance);
@@ -107,6 +145,14 @@ KitManager::KitManager(QObject *parent)
     connect(this, &KitManager::kitUpdated, this, &KitManager::kitsChanged);
 }
 
+void KitManager::destroy()
+{
+    delete d;
+    d = nullptr;
+    delete m_instance;
+    m_instance = nullptr;
+}
+
 void KitManager::restoreKits()
 {
     QTC_ASSERT(!d->m_initialized, return );
@@ -117,7 +163,7 @@ void KitManager::restoreKits()
     Core::Id defaultUserKit;
     std::vector<std::unique_ptr<Kit>> kitsToCheck;
     {
-        KitList userKits = restoreKits(settingsFileName());
+        KitList userKits = restoreKitsHelper(settingsFileName());
         defaultUserKit = userKits.defaultKit;
 
         for (auto &k : userKits.kits) {
@@ -132,8 +178,8 @@ void KitManager::restoreKits()
 
     // read all kits from SDK
     {
-        KitList system
-                = restoreKits(FileName::fromString(ICore::installerResourcePath() + KIT_FILENAME));
+        KitList system = restoreKitsHelper
+                (FileName::fromString(ICore::installerResourcePath() + KIT_FILENAME));
 
         // SDK kits need to get updated with the user-provided extra settings:
         for (auto &current : system.kits) {
@@ -194,19 +240,17 @@ void KitManager::restoreKits()
 
     d->m_writer = std::make_unique<PersistentSettingsWriter>(settingsFileName(), "QtCreatorProfiles");
     d->m_initialized = true;
-    emit kitsLoaded();
-    emit kitsChanged();
+    emit m_instance->kitsLoaded();
+    emit m_instance->kitsChanged();
 }
 
 KitManager::~KitManager()
 {
-    delete d;
-    d = nullptr;
-    m_instance = nullptr;
 }
 
 void KitManager::saveKits()
 {
+    QTC_ASSERT(d, return);
     if (!d->m_writer) // ignore save requests while we are not initialized.
         return;
 
@@ -234,28 +278,27 @@ bool KitManager::isLoaded()
     return d->m_initialized;
 }
 
-void KitManager::registerKitAspect(std::unique_ptr<KitAspect> &&ki)
+void KitManager::registerKitAspect(KitAspect *ki)
 {
-    QTC_ASSERT(ki->id().isValid(), return );
-    QTC_ASSERT(!Utils::contains(d->m_informationList, ki.get()), return );
+    instance();
+    QTC_ASSERT(d, return);
+    d->addKitAspect(ki);
 
-    auto it = std::lower_bound(std::begin(d->m_informationList),
-                               std::end(d->m_informationList),
-                               ki,
-                               [](const std::unique_ptr<KitAspect> &a,
-                                  const std::unique_ptr<KitAspect> &b) {
-                                   return a->priority() > b->priority();
-                               });
-    d->m_informationList.insert(it, std::move(ki));
-
-    foreach (Kit *k, kits()) {
-        if (!k->hasValue(ki->id()))
-            ki->setup(k);
-        else
-            ki->fix(k);
+    // Adding this aspect to possibly already existing kits is currently not
+    // needed here as kits are only created after all aspects are created
+    // in *Plugin::initialize().
+    // Make sure we notice when this assumption breaks:
+    QTC_CHECK(d->m_kitList.empty());
 }
 
-    return;
+void KitManager::deregisterKitAspect(KitAspect *ki)
+{
+    // Happens regularly for the aspects from the ProjectExplorerPlugin as these
+    // are destroyed after the manual call to KitManager::destroy() there, but as
+    // this here is just for sanity reasons that the KitManager does not access
+    // a destroyed aspect, a destroyed KitManager is not a problem.
+    if (d)
+        d->removeKitAspect(ki);
 }
 
 QSet<Id> KitManager::supportedPlatforms()
@@ -296,7 +339,7 @@ QList<Kit *> KitManager::sortKits(const QList<Kit *> &kits)
     return Utils::transform(sortList, &QPair<QString, Kit *>::second);
 }
 
-KitManager::KitList KitManager::restoreKits(const FileName &fileName)
+static KitList restoreKitsHelper(const FileName &fileName)
 {
     KitList result;
 
@@ -376,7 +419,7 @@ Kit *KitManager::defaultKit()
 
 const QList<KitAspect *> KitManager::kitAspects()
 {
-    return Utils::toRawPointer<QList>(d->m_informationList);
+    return d->kitAspects();
 }
 
 const QSet<Id> KitManager::irrelevantAspects()
@@ -449,7 +492,7 @@ void KitManager::completeKit(Kit *k)
 {
     QTC_ASSERT(k, return);
     KitGuard g(k);
-    for (const std::unique_ptr<KitAspect> &ki : d->m_informationList) {
+    for (KitAspect *ki : d->kitAspects()) {
         ki->upgrade(k);
         if (!k->hasValue(ki->id()))
             ki->setup(k);
@@ -462,6 +505,16 @@ void KitManager::completeKit(Kit *k)
 // KitAspect:
 // --------------------------------------------------------------------
 
+KitAspect::KitAspect()
+{
+    KitManager::registerKitAspect(this);
+}
+
+KitAspect::~KitAspect()
+{
+    KitManager::deregisterKitAspect(this);
+}
+
 void KitAspect::addToEnvironment(const Kit *k, Environment &env) const
 {
     Q_UNUSED(k);

src/plugins/projectexplorer/kitmanager.h

@@ -63,7 +63,7 @@ class KitModel;
  *
  * One piece of information stored in the kit.
  *
- * This needs to get registered with the \a KitManager.
+ * They auto-register with the \a KitManager for their life time
  */
 class PROJECTEXPLORER_EXPORT KitAspect : public QObject
 {
@@ -105,6 +105,9 @@ public:
     virtual bool isApplicableToKit(const Kit *) const { return true; }
 
 protected:
+    KitAspect();
+    ~KitAspect();
+
     void setId(Core::Id id) { m_id = id; }
     void setDisplayName(const QString &name) { m_displayName = name; }
     void setDescription(const QString &desc) { m_description = desc; }
@@ -175,11 +178,6 @@ public:
     static void deregisterKit(Kit *k);
     static void setDefaultKit(Kit *k);
 
-    template<typename KI, typename... Args>
-    static void registerKitAspect(Args&&... args) {
-        registerKitAspect(std::make_unique<KI>(std::forward<Args>(args)...));
-    }
-
     static QSet<Core::Id> supportedPlatforms();
     static QSet<Core::Id> availableFeatures(Core::Id platformId);
 
@@ -204,20 +202,16 @@ signals:
     void kitsLoaded();
 
 private:
-    explicit KitManager(QObject *parent = nullptr);
+    KitManager();
 
-    static void registerKitAspect(std::unique_ptr<KitAspect> &&ki);
+    static void destroy();
+
+    static void registerKitAspect(KitAspect *ki);
+    static void deregisterKitAspect(KitAspect *ki);
 
     // Make sure the this is only called after all
     // KitAspects are registered!
-    void restoreKits();
-    class KitList
-    {
-    public:
-        Core::Id defaultKit;
-        std::vector<std::unique_ptr<Kit>> kits;
-    };
-    KitList restoreKits(const Utils::FileName &fileName);
+    static void restoreKits();
 
     static void notifyAboutUpdate(Kit *k);
     static void completeKit(Kit *k);
@@ -225,7 +219,7 @@ private:
     friend class ProjectExplorerPlugin; // for constructor
     friend class Kit;
     friend class Internal::KitModel;
-    friend class KitAspect; // for notifyAbutUpdate
+    friend class KitAspect; // for notifyAboutUpdate and self-registration
 };
 
 } // namespace ProjectExplorer

src/plugins/projectexplorer/projectexplorer.cpp

@@ -484,7 +484,6 @@ public:
     bool m_shuttingDown = false;
     Core::Id m_runMode = Constants::NO_RUN_MODE;
 
-    KitManager *m_kitManager = nullptr;
     ToolChainManager *m_toolChainManager = nullptr;
     QStringList m_arguments;
 
@@ -560,6 +559,12 @@ public:
     DefaultDeployConfigurationFactory m_defaultDeployConfigFactory;
 
     IDocumentFactory m_documentFactory;
+
+    DeviceTypeKitAspect deviceTypeKitAspect;
+    DeviceKitAspect deviceeKitAspect;
+    ToolChainKitAspect toolChainKitAspect;
+    SysRootKitAspect sysRootKitAspect;
+    EnvironmentKitAspect environmentKitAspect;
 };
 
 static ProjectExplorerPlugin *m_instance = nullptr;
@@ -576,7 +581,7 @@ ProjectExplorerPlugin::~ProjectExplorerPlugin()
     JsonWizardFactory::destroyAllFactories();
 
     // Force sequence of deletion:
-    delete dd->m_kitManager; // remove all the profile information
+    KitManager::destroy(); // remove all the profile information
     delete dd->m_toolChainManager;
     ProjectPanelFactory::destroyFactories();
     delete dd;
@@ -603,7 +608,6 @@ bool ProjectExplorerPlugin::initialize(const QStringList &arguments, QString *er
     CustomWizard::setVerbose(arguments.count(QLatin1String("-customwizard-verbose")));
     JsonWizardFactory::setVerbose(arguments.count(QLatin1String("-customwizard-verbose")));
 
-    dd->m_kitManager = new KitManager; // register before ToolChainManager
     dd->m_toolChainManager = new ToolChainManager;
 
     // Register languages
@@ -612,13 +616,6 @@ bool ProjectExplorerPlugin::initialize(const QStringList &arguments, QString *er
 
     IWizardFactory::registerFeatureProvider(new KitFeatureProvider);
 
-    // Register KitAspects:
-    KitManager::registerKitAspect<DeviceTypeKitAspect>();
-    KitManager::registerKitAspect<DeviceKitAspect>();
-    KitManager::registerKitAspect<ToolChainKitAspect>();
-    KitManager::registerKitAspect<SysRootKitAspect>();
-    KitManager::registerKitAspect<EnvironmentKitAspect>();
-
     IWizardFactory::registerFactoryCreator([]() -> QList<IWizardFactory *> {
         QList<IWizardFactory *> result;
         result << CustomWizard::createWizards();
@@ -1796,7 +1793,7 @@ void ProjectExplorerPlugin::restoreKits()
     ExtraAbi::load(); // Load this before Toolchains!
     DeviceManager::instance()->load();
     ToolChainManager::restoreToolChains();
-    dd->m_kitManager->restoreKits();
+    KitManager::restoreKits();
     QTimer::singleShot(0, dd, &ProjectExplorerPluginPrivate::restoreSession); // delay a bit...
 }
 

src/plugins/qbsprojectmanager/qbsprojectmanagerplugin.cpp

@@ -92,6 +92,7 @@ public:
     QbsInstallStepFactory installStepFactory;
     QbsRunConfigurationFactory runConfigFactory;
     QbsProfilesSettingsPage profilesSetttingsPage;
+    QbsKitAspect qbsKitAspect;
 };
 
 QbsProjectManagerPlugin::~QbsProjectManagerPlugin()
@@ -112,7 +113,6 @@ bool QbsProjectManagerPlugin::initialize(const QStringList &arguments, QString *
     Core::HelpManager::registerDocumentation({Core::HelpManager::documentationPath() + "/qbs.qch"});
 
     ProjectManager::registerProjectType<QbsProject>(QmlJSTools::Constants::QBS_MIMETYPE);
-    KitManager::registerKitAspect<QbsKitAspect>();
 
     //menus
     // Build Menu:

src/plugins/qmakeprojectmanager/qmakeprojectmanagerplugin.cpp

@@ -124,6 +124,8 @@ public:
     Utils::ParameterAction *m_buildFileAction = nullptr;
     QAction *m_addLibraryAction = nullptr;
     QAction *m_addLibraryActionContextMenu = nullptr;
+
+    QmakeKitAspect qmakeKitAspect;
 };
 
 QmakeProjectManagerPlugin::~QmakeProjectManagerPlugin()
@@ -143,8 +145,6 @@ bool QmakeProjectManagerPlugin::initialize(const QStringList &arguments, QString
     //create and register objects
     ProjectManager::registerProjectType<QmakeProject>(QmakeProjectManager::Constants::PROFILE_MIMETYPE);
 
-    ProjectExplorer::KitManager::registerKitAspect<QmakeKitAspect>();
-
     IWizardFactory::registerFactoryCreator([] {
         return QList<IWizardFactory *> {
             new SubdirsProjectWizard,

src/plugins/qtsupport/qtsupportplugin.cpp

@@ -68,6 +68,8 @@ public:
 
     ExamplesWelcomePage examplesPage{true};
     ExamplesWelcomePage tutorialPage{false};
+
+    QtKitAspect qtKiAspect;
 };
 
 QtSupportPlugin::~QtSupportPlugin()
@@ -87,8 +89,6 @@ bool QtSupportPlugin::initialize(const QStringList &arguments, QString *errorMes
 
     d = new QtSupportPluginPrivate;
 
-    ProjectExplorer::KitManager::registerKitAspect<QtKitAspect>();
-
     (void) new UicGeneratorFactory(this);
     (void) new QScxmlcGeneratorFactory(this);