Fixed crash when using -noload to disable multiple plugins

The noload behavior is also changed: it now always shows the plugins in
About Plugins... menu, even though -noload was used. When using -noload,
the enabled state of the plugin is not saved, so if the command line
arg is removed, the plugin will be loaded normally.

Reviewed-by: con

src/app/main.cpp

@@ -296,7 +296,7 @@ int main(int argc, char **argv)
         QStringList errors;
         foreach (ExtensionSystem::PluginSpec *p, pluginManager.plugins())
             // only show errors on startup if plugin is enabled.
-            if (p->hasError() && p->isEnabled() && !p->isDisabledByDependency())
+            if (p->hasError() && p->isEnabled() && !p->isDisabledIndirectly())
                 errors.append(p->name() + "\n" + p->errorString());
         if (!errors.isEmpty())
             QMessageBox::warning(0,

src/libs/extensionsystem/optionsparser.cpp

@@ -128,7 +128,7 @@ bool OptionsParser::checkForNoLoadOption()
                                                              "The plugin '%1' does not exist.").arg(m_currentArg);
             m_hasError = true;
         } else {
-            m_pmPrivate->removePluginSpec(spec);
+            m_pmPrivate->disablePluginIndirectly(spec);
             m_isDependencyRefreshNeeded = true;
         }
     }

src/libs/extensionsystem/pluginmanager.cpp

@@ -767,7 +767,7 @@ bool PluginManagerPrivate::loadQueue(PluginSpec *spec, QList<PluginSpec *> &queu
     circularityCheckQueue.append(spec);
     // check if we have the dependencies
     if (spec->state() == PluginSpec::Invalid || spec->state() == PluginSpec::Read) {
-        if (!spec->isDisabledByDependency() && spec->isEnabled()) {
+        if (!spec->isDisabledIndirectly() && spec->isEnabled()) {
             spec->d->hasError = true;
             spec->d->errorString += "\n";
             spec->d->errorString += PluginManager::tr("Cannot load plugin because dependencies are not resolved");
@@ -795,7 +795,7 @@ bool PluginManagerPrivate::loadQueue(PluginSpec *spec, QList<PluginSpec *> &queu
 */
 void PluginManagerPrivate::loadPlugin(PluginSpec *spec, PluginSpec::State destState)
 {
-    if (spec->hasError() || spec->isDisabledByDependency())
+    if (spec->hasError() || spec->isDisabledIndirectly())
         return;
 
     switch (destState) {
@@ -934,19 +934,9 @@ PluginSpec *PluginManagerPrivate::pluginForOption(const QString &option, bool *r
     return 0;
 }
 
-void PluginManagerPrivate::removePluginSpec(PluginSpec *spec)
+void PluginManagerPrivate::disablePluginIndirectly(PluginSpec *spec)
 {
-    pluginSpecs.removeAll(spec);
-
-    if (pluginCategories.contains(spec->category()))
-        pluginCategories.value(spec->category())->removePlugin(spec);
-
-    foreach(PluginSpec *dep, spec->dependencySpecs()) {
-        dep->removeDependentPlugin(spec);
-    }
-
-    delete spec;
-    spec = 0;
+    spec->d->disabledIndirectly = true;
 }
 
 PluginSpec *PluginManagerPrivate::pluginByName(const QString &name) const

src/libs/extensionsystem/pluginmanager_p.h

@@ -71,7 +71,7 @@ public:
     void profilingReport(const char *what, const PluginSpec *spec = 0);
     void loadSettings();
     void writeSettings();
-    void removePluginSpec(PluginSpec *spec);
+    void disablePluginIndirectly(PluginSpec *spec);
 
     QHash<QString, PluginCollection *> pluginCategories;
     QList<PluginSpec *> pluginSpecs;

src/libs/extensionsystem/pluginspec.cpp

@@ -254,9 +254,14 @@ bool PluginSpec::isEnabled() const
     return d->enabled;
 }
 
-bool PluginSpec::isDisabledByDependency() const
+/*!
+    \fn bool PluginSpec::isDisabledIndirectly() const
+    Returns true if loading was not done due to user unselecting this plugin or its dependencies,
+    or if command-line parameter -noload was used.
+*/
+bool PluginSpec::isDisabledIndirectly() const
 {
-    return d->disabledByDependency;
+    return d->disabledIndirectly;
 }
 
 /*!
@@ -397,27 +402,11 @@ QList<PluginSpec *> PluginSpec::dependencySpecs() const
 
     \sa PluginSpec::dependencySpecs()
 */
-QList<PluginSpec *> PluginSpec::providesSpecs() const
+QList<PluginSpec *> PluginSpec::providesForSpecs() const
 {
     return d->providesSpecs;
 }
 
-/*!
-    \fn void PluginSpec::addDependentPlugin(PluginSpec *dependent)
-    Adds a dependent the list of plugins that depend on this one.
-
-    \sa PluginSpec::providesSpecs()
-*/
-void PluginSpec::addDependentPlugin(PluginSpec *dependent)
-{
-    d->providesSpecs.append(dependent);
-}
-
-void PluginSpec::removeDependentPlugin(PluginSpec *dependent)
-{
-    d->providesSpecs.removeOne(dependent);
-}
-
 //==========PluginSpecPrivate==================
 
 namespace {
@@ -448,7 +437,7 @@ namespace {
 PluginSpecPrivate::PluginSpecPrivate(PluginSpec *spec)
     :
     enabled(true),
-    disabledByDependency(false),
+    disabledIndirectly(false),
     plugin(0),
     state(PluginSpec::Invalid),
     hasError(false),
@@ -810,10 +799,10 @@ bool PluginSpecPrivate::resolveDependencies(const QList<PluginSpec *> &specs)
         foreach (PluginSpec *spec, specs) {
             if (spec->provides(dependency.name, dependency.version)) {
                 found = spec;
-                if (!spec->isEnabled() || spec->isDisabledByDependency())
-                    disabledByDependency = true;
+                if (!spec->isEnabled() || spec->isDisabledIndirectly())
+                    disabledIndirectly = true;
 
-                spec->addDependentPlugin(q);
+                spec->d->addProvidesForPlugin(q);
 
                 break;
             }
@@ -833,7 +822,7 @@ bool PluginSpecPrivate::resolveDependencies(const QList<PluginSpec *> &specs)
 
     dependencySpecs = resolvedDependencies;
 
-    if (enabled && !disabledByDependency)
+    if (enabled && !disabledIndirectly)
         state = PluginSpec::Resolved;
 
     return true;
@@ -976,3 +965,18 @@ void PluginSpecPrivate::kill()
     state = PluginSpec::Deleted;
 }
 
+/*!
+    \fn void PluginSpec::addDependentPlugin(PluginSpec *dependent)
+    Adds a dependent the list of plugins that depend on this one.
+
+    \sa PluginSpec::providesSpecs()
+*/
+void PluginSpecPrivate::addProvidesForPlugin(PluginSpec *dependent)
+{
+    providesSpecs.append(dependent);
+}
+
+void PluginSpecPrivate::removeProvidesForPlugin(PluginSpec *dependent)
+{
+    providesSpecs.removeOne(dependent);
+}

src/libs/extensionsystem/pluginspec.h

@@ -81,8 +81,7 @@ public:
     QString category() const;
     bool isExperimental() const;
     bool isEnabled() const;
-    // true if loading was not done due to user unselecting this plugin or its dependencies
-    bool isDisabledByDependency() const;
+    bool isDisabledIndirectly() const;
     QList<PluginDependency> dependencies() const;
 
     typedef QList<PluginArgumentDescription> PluginArgumentDescriptions;
@@ -104,11 +103,7 @@ public:
     QList<PluginSpec *> dependencySpecs() const;
 
     // list of plugins that depend on this - e.g. this plugins provides for them
-    QList<PluginSpec *> providesSpecs() const;
-
-    // add/remove from providesSpecs
-    void addDependentPlugin(PluginSpec *dependent);
-    void removeDependentPlugin(PluginSpec *dependent);
+    QList<PluginSpec *> providesForSpecs() const;
 
     // linked plugin instance, valid after 'Loaded' state is reached
     IPlugin *plugin() const;
@@ -123,6 +118,7 @@ private:
 
     Internal::PluginSpecPrivate *d;
     friend class Internal::PluginManagerPrivate;
+    friend class Internal::PluginSpecPrivate;
 };
 
 } // namespace ExtensionSystem

src/libs/extensionsystem/pluginspec_p.h

@@ -71,7 +71,7 @@ public:
     QString category;
     QList<PluginDependency> dependencies;
     bool enabled;
-    bool disabledByDependency;
+    bool disabledIndirectly;
 
     QString location;
     QString filePath;
@@ -89,6 +89,10 @@ public:
     static bool isValidVersion(const QString &version);
     static int versionCompare(const QString &version1, const QString &version2);
 
+    // add/remove from providesSpecs
+    void addProvidesForPlugin(PluginSpec *dependent);
+    void removeProvidesForPlugin(PluginSpec *dependent);
+
 private:
     PluginSpec *q;
 

src/libs/extensionsystem/pluginview.cpp

@@ -178,7 +178,7 @@ void PluginView::updateList()
     defaultCollectionItem->setData(0, Qt::UserRole, qVariantFromValue(defaultCollection));
 
     foreach (PluginSpec *spec, m_specToItem.keys())
-        toggleRelatedPlugins(spec, spec->isEnabled() && !spec->isDisabledByDependency());
+        toggleRelatedPlugins(spec, spec->isEnabled() && !spec->isDisabledIndirectly());
 
     m_ui->categoryWidget->clear();
     if (!m_items.isEmpty()) {
@@ -347,8 +347,8 @@ void PluginView::updatePluginSettings(QTreeWidgetItem *item, int column)
 void PluginView::toggleRelatedPlugins(PluginSpec *modifiedPlugin, bool isPluginEnabled)
 {
 
-    for(int i = 0; i < modifiedPlugin->providesSpecs().length(); ++i) {
-        PluginSpec *spec = modifiedPlugin->providesSpecs().at(i);
+    for(int i = 0; i < modifiedPlugin->providesForSpecs().length(); ++i) {
+        PluginSpec *spec = modifiedPlugin->providesForSpecs().at(i);
         QTreeWidgetItem *childItem = m_specToItem.value(spec);
 
         if (childItem->isDisabled() != !isPluginEnabled) {

src/plugins/coreplugin/plugindialog.cpp

@@ -111,7 +111,7 @@ void PluginDialog::updateButtons()
     if (selectedSpec) {
         m_detailsButton->setEnabled(true);
         m_errorDetailsButton->setEnabled(selectedSpec->hasError()
-                                         || selectedSpec->isDisabledByDependency()
+                                         || selectedSpec->isDisabledIndirectly()
                                          || !selectedSpec->isEnabled());
     } else {
         m_detailsButton->setEnabled(false);