Analyzer: Base ISettingsAspect on Utils::AspectContainer

Change-Id: Ib4c19d0cb167911dc50d989771dd53f3569db087
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/libs/utils/aspects.cpp

@@ -2183,6 +2183,9 @@ void AspectContainer::fromMap(const QVariantMap &map)
 {
     for (BaseAspect *aspect : qAsConst(d->m_items))
         aspect->fromMap(map);
+
+    emit fromMapFinished();
+
 }
 
 void AspectContainer::toMap(QVariantMap &map) const

src/libs/utils/aspects.h

@@ -513,6 +513,7 @@ public:
         registerAspect(aspect);
         return aspect;
     }
+
     void fromMap(const QVariantMap &map);
     void toMap(QVariantMap &map) const;
 
@@ -560,6 +561,7 @@ public:
 
 signals:
     void applied();
+    void fromMapFinished();
 
 private:
     std::unique_ptr<Internal::AspectContainerPrivate> d;

src/plugins/perfprofiler/perfsettings.cpp

@@ -46,19 +46,19 @@ PerfSettings::PerfSettings(ProjectExplorer::Target *target)
         return widget;
     });
 
-    group.registerAspect(&period);
+    registerAspect(&period);
     period.setSettingsKey("Analyzer.Perf.Frequency");
     period.setRange(250, 2147483647);
     period.setDefaultValue(250);
     period.setLabelText(tr("Sample period:"));
 
-    group.registerAspect(&stackSize);
+    registerAspect(&stackSize);
     stackSize.setSettingsKey("Analyzer.Perf.StackSize");
     stackSize.setRange(4096, 65536);
     stackSize.setDefaultValue(4096);
     stackSize.setLabelText(tr("Stack snapshot size (kB):"));
 
-    group.registerAspect(&sampleMode);
+    registerAspect(&sampleMode);
     sampleMode.setSettingsKey("Analyzer.Perf.SampleMode");
     sampleMode.setDisplayStyle(SelectionAspect::DisplayStyle::ComboBox);
     sampleMode.setLabelText(tr("Sample mode:"));
@@ -66,7 +66,7 @@ PerfSettings::PerfSettings(ProjectExplorer::Target *target)
     sampleMode.addOption({tr("event count"), {}, QString("-c")});
     sampleMode.setDefaultValue(0);
 
-    group.registerAspect(&callgraphMode);
+    registerAspect(&callgraphMode);
     callgraphMode.setSettingsKey("Analyzer.Perf.CallgraphMode");
     callgraphMode.setDisplayStyle(SelectionAspect::DisplayStyle::ComboBox);
     callgraphMode.setLabelText(tr("Call graph mode:"));
@@ -75,11 +75,11 @@ PerfSettings::PerfSettings(ProjectExplorer::Target *target)
     callgraphMode.addOption({tr("last branch record"), {}, QString("lbr")});
     callgraphMode.setDefaultValue(0);
 
-    group.registerAspect(&events);
+    registerAspect(&events);
     events.setSettingsKey("Analyzer.Perf.Events");
     events.setDefaultValue({"cpu-cycles"});
 
-    group.registerAspect(&extraArguments);
+    registerAspect(&extraArguments);
     extraArguments.setSettingsKey("Analyzer.Perf.ExtraArguments");
     extraArguments.setDisplayStyle(StringAspect::DisplayStyle::LineEditDisplay);
     extraArguments.setLabelText(tr("Additional arguments:"));
@@ -89,6 +89,8 @@ PerfSettings::PerfSettings(ProjectExplorer::Target *target)
         stackSize.setEnabled(index == 0);
     });
 
+    connect(this, &AspectContainer::fromMapFinished, this, &PerfSettings::changed);
+
     readGlobalSettings();
 }
 
@@ -122,17 +124,6 @@ void PerfSettings::writeGlobalSettings() const
     settings->endGroup();
 }
 
-void PerfSettings::toMap(QVariantMap &map) const
-{
-    group.toMap(map);
-}
-
-void PerfSettings::fromMap(const QVariantMap &map)
-{
-    group.fromMap(map);
-    emit changed();
-}
-
 QStringList PerfSettings::perfRecordArguments() const
 {
     QString callgraphArg = callgraphMode.itemValue().toString();

src/plugins/perfprofiler/perfsettings.h

@@ -56,14 +56,8 @@ public:
     Utils::StringListAspect events;
     Utils::StringAspect extraArguments;
 
-    Utils::AspectContainer group;
-
 signals:
     void changed();
-
-protected:
-    void toMap(QVariantMap &map) const final;
-    void fromMap(const QVariantMap &map) final;
 };
 
 } // namespace PerfProfiler

src/plugins/projectexplorer/runconfiguration.h

@@ -61,7 +61,7 @@ class Target;
  *
  */
 
-class PROJECTEXPLORER_EXPORT ISettingsAspect : public QObject
+class PROJECTEXPLORER_EXPORT ISettingsAspect : public Utils::AspectContainer
 {
     Q_OBJECT
 
@@ -76,10 +76,6 @@ protected:
     void setConfigWidgetCreator(const ConfigWidgetCreator &configWidgetCreator);
 
     friend class GlobalOrProjectAspect;
-    /// Converts current object into map for storage.
-    virtual void toMap(QVariantMap &map) const = 0;
-    /// Read object state from @p map.
-    virtual void fromMap(const QVariantMap &map) = 0;
 
     ConfigWidgetCreator m_configWidgetCreator;
 };

src/plugins/qmlprofiler/qmlprofilersettings.cpp

@@ -57,9 +57,9 @@ QmlProfilerSettings::QmlProfilerSettings()
 {
     setConfigWidgetCreator([this] { return createQmlConfigWidget(this); });
 
-    group.setSettingsGroup(Constants::ANALYZER);
+    setSettingsGroup(Constants::ANALYZER);
 
-    group.registerAspect(&flushEnabled);
+    registerAspect(&flushEnabled);
     flushEnabled.setSettingsKey("Analyzer.QmlProfiler.FlushEnabled");
     flushEnabled.setLabelPlacement(BoolAspect::LabelPlacement::InExtraLabel);
     flushEnabled.setLabelText(tr("Flush data while profiling:"));
@@ -68,17 +68,17 @@ QmlProfilerSettings::QmlProfilerSettings()
         "data and the memory usage in the application. It distorts the profile as the flushing\n"
         "itself takes time."));
 
-    group.registerAspect(&flushInterval);
+    registerAspect(&flushInterval);
     flushInterval.setSettingsKey("Analyzer.QmlProfiler.FlushInterval");
     flushInterval.setRange(1, 10000000);
     flushInterval.setDefaultValue(1000);
     flushInterval.setLabelText(tr("Flush interval (ms):", nullptr));
     flushInterval.setEnabled(false); // Controled by flushEnabled.
 
-    group.registerAspect(&lastTraceFile);
+    registerAspect(&lastTraceFile);
     lastTraceFile.setSettingsKey("Analyzer.QmlProfiler.LastTraceFile");
 
-    group.registerAspect(&aggregateTraces);
+    registerAspect(&aggregateTraces);
     aggregateTraces.setSettingsKey("Analyzer.QmlProfiler.AggregateTraces");
     aggregateTraces.setLabelPlacement(BoolAspect::LabelPlacement::InExtraLabel);
     aggregateTraces.setLabelText(tr("Process data only when process ends:"));
@@ -94,25 +94,14 @@ QmlProfilerSettings::QmlProfilerSettings()
             &flushInterval, &BaseAspect::setEnabled);
 
     // Read stored values
-    group.readSettings(Core::ICore::settings());
+    readSettings(Core::ICore::settings());
 }
 
 void QmlProfilerSettings::writeGlobalSettings() const
 {
-    group.writeSettings(Core::ICore::settings());
+    writeSettings(Core::ICore::settings());
 }
 
-void QmlProfilerSettings::toMap(QVariantMap &map) const
-{
-    group.toMap(map);
-}
-
-void QmlProfilerSettings::fromMap(const QVariantMap &map)
-{
-    group.fromMap(map);
-}
-
-
 // QmlProfilerOptionsPage
 
 QmlProfilerOptionsPage::QmlProfilerOptionsPage()

src/plugins/qmlprofiler/qmlprofilersettings.h

@@ -46,12 +46,6 @@ public:
     Utils::IntegerAspect flushInterval;
     Utils::StringAspect lastTraceFile;
     Utils::BoolAspect aggregateTraces;
-
-    Utils::AspectContainer group;
-
-protected:
-    void toMap(QVariantMap &map) const override;
-    void fromMap(const QVariantMap &map) override;
 };
 
 class QmlProfilerOptionsPage final : public Core::IOptionsPage

src/plugins/valgrind/valgrindconfigwidget.cpp

@@ -50,13 +50,13 @@ public:
 
     void apply() final
     {
-        ValgrindGlobalSettings::instance()->group.apply();
+        ValgrindGlobalSettings::instance()->apply();
         ValgrindGlobalSettings::instance()->writeSettings();
     }
 
     void finish() final
     {
-        ValgrindGlobalSettings::instance()->group.finish();
+        ValgrindGlobalSettings::instance()->finish();
     }
 };
 

src/plugins/valgrind/valgrindsettings.cpp

@@ -297,9 +297,9 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
     // the same key to facilitate copying using fromMap/toMap.
     QString base = "Analyzer.Valgrind.";
 
-    group.registerAspect(&suppressions);
+    registerAspect(&suppressions);
 
-    group.registerAspect(&valgrindExecutable);
+    registerAspect(&valgrindExecutable);
     valgrindExecutable.setSettingsKey(base + "ValgrindExecutable");
     valgrindExecutable.setDefaultValue("valgrind");
     valgrindExecutable.setDisplayStyle(StringAspect::PathChooserDisplay);
@@ -315,12 +315,12 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
         //valgrindExecutable. ... buttonAtIndex(0)->hide();
     }
 
-    group.registerAspect(&valgrindArguments);
+    registerAspect(&valgrindArguments);
     valgrindArguments.setSettingsKey(base + "ValgrindArguments");
     valgrindArguments.setDisplayStyle(StringAspect::LineEditDisplay);
     valgrindArguments.setLabelText(tr("Valgrind arguments:"));
 
-    group.registerAspect(&selfModifyingCodeDetection);
+    registerAspect(&selfModifyingCodeDetection);
     selfModifyingCodeDetection.setSettingsKey(base + "SelfModifyingCodeDetection");
     selfModifyingCodeDetection.setDefaultValue(DetectSmcStackOnly);
     selfModifyingCodeDetection.setDisplayStyle(SelectionAspect::DisplayStyle::ComboBox);
@@ -331,12 +331,12 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
     selfModifyingCodeDetection.setLabelText(tr("Detect self-modifying code:"));
 
     // Memcheck
-    group.registerAspect(&memcheckArguments);
+    registerAspect(&memcheckArguments);
     memcheckArguments.setSettingsKey(base + "Memcheck.Arguments");
     memcheckArguments.setDisplayStyle(StringAspect::LineEditDisplay);
     memcheckArguments.setLabelText(tr("Extra MemCheck arguments:"));
 
-    group.registerAspect(&filterExternalIssues);
+    registerAspect(&filterExternalIssues);
     filterExternalIssues.setSettingsKey(base + "FilterExternalIssues");
     filterExternalIssues.setDefaultValue(true);
     filterExternalIssues.setIcon(Icons::FILTER.icon());
@@ -344,18 +344,18 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
     filterExternalIssues.setLabelText(tr("Show Project Costs Only"));
     filterExternalIssues.setToolTip(tr("Show only profiling info that originated from this project source."));
 
-    group.registerAspect(&trackOrigins);
+    registerAspect(&trackOrigins);
     trackOrigins.setSettingsKey(base + "TrackOrigins");
     trackOrigins.setDefaultValue(true);
     trackOrigins.setLabelPlacement(BoolAspect::LabelPlacement::AtCheckBoxWithoutDummyLabel);
     trackOrigins.setLabelText(tr("Track origins of uninitialized memory"));
 
-    group.registerAspect(&showReachable);
+    registerAspect(&showReachable);
     showReachable.setSettingsKey(base + "ShowReachable");
     showReachable.setLabelPlacement(BoolAspect::LabelPlacement::AtCheckBoxWithoutDummyLabel);
     showReachable.setLabelText(tr("Show reachable and indirectly lost blocks"));
 
-    group.registerAspect(&leakCheckOnFinish);
+    registerAspect(&leakCheckOnFinish);
     leakCheckOnFinish.setSettingsKey(base + "LeakCheckOnFinish");
     leakCheckOnFinish.setDefaultValue(LeakCheckOnFinishSummaryOnly);
     leakCheckOnFinish.setDisplayStyle(SelectionAspect::DisplayStyle::ComboBox);
@@ -364,14 +364,14 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
     leakCheckOnFinish.addOption(tr("Full"));
     leakCheckOnFinish.setLabelText(tr("Check for leaks on finish:"));
 
-    group.registerAspect(&numCallers);
+    registerAspect(&numCallers);
     numCallers.setSettingsKey(base + "NumCallers");
     numCallers.setDefaultValue(25);
     numCallers.setLabelText(tr("Backtrace frame count:"));
 
     // Callgrind
 
-    group.registerAspect(&kcachegrindExecutable);
+    registerAspect(&kcachegrindExecutable);
     kcachegrindExecutable.setSettingsKey(base + "KCachegrindExecutable");
     kcachegrindExecutable.setDefaultValue("kcachegrind");
     kcachegrindExecutable.setDisplayStyle(StringAspect::PathChooserDisplay);
@@ -379,18 +379,18 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
     kcachegrindExecutable.setExpectedKind(Utils::PathChooser::Command);
     kcachegrindExecutable.setDisplayName(tr("KCachegrind Command"));
 
-    group.registerAspect(&callgrindArguments);
+    registerAspect(&callgrindArguments);
     callgrindArguments.setSettingsKey(base + "Callgrind.Arguments");
     callgrindArguments.setDisplayStyle(StringAspect::LineEditDisplay);
     callgrindArguments.setLabelText(tr("Extra CallGrind arguments:"));
 
-    group.registerAspect(&enableEventToolTips);
+    registerAspect(&enableEventToolTips);
     enableEventToolTips.setDefaultValue(true);
     enableEventToolTips.setSettingsKey(base + "Callgrind.EnableEventToolTips");
     enableEventToolTips.setLabelPlacement(BoolAspect::LabelPlacement::AtCheckBoxWithoutDummyLabel);
     enableEventToolTips.setLabelText(tr("Show additional information for events in tooltips"));
 
-    group.registerAspect(&enableCacheSim);
+    registerAspect(&enableCacheSim);
     enableCacheSim.setSettingsKey(base + "Callgrind.EnableCacheSim");
     enableCacheSim.setLabelPlacement(BoolAspect::LabelPlacement::AtCheckBoxWithoutDummyLabel);
     enableCacheSim.setLabelText(tr("Enable cache simulation"));
@@ -404,7 +404,7 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
         "<li>Data write accesses (\"Dw\") and related cache misses (\"D1mw\"/\"D2mw\").</li></ul>\n"
         "</p>") + "</body></html>");
 
-    group.registerAspect(&enableBranchSim);
+    registerAspect(&enableBranchSim);
     enableBranchSim.setSettingsKey(base + "Callgrind.EnableBranchSim");
     enableBranchSim.setLabelPlacement(BoolAspect::LabelPlacement::AtCheckBoxWithoutDummyLabel);
     enableBranchSim.setLabelText(tr("Enable branch prediction simulation"));
@@ -416,20 +416,20 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
         "<li>Executed indirect jumps and related misses of the jump address predictor (\n"
         "\"Bi\"/\"Bim\").)</li></ul>") + "</body></html>");
 
-    group.registerAspect(&collectSystime);
+    registerAspect(&collectSystime);
     collectSystime.setSettingsKey(base + "Callgrind.CollectSystime");
     collectSystime.setLabelPlacement(BoolAspect::LabelPlacement::AtCheckBoxWithoutDummyLabel);
     collectSystime.setLabelText(tr("Collect system call time"));
     collectSystime.setToolTip(tr("Collects information for system call times."));
 
-    group.registerAspect(&collectBusEvents);
+    registerAspect(&collectBusEvents);
     collectBusEvents.setLabelPlacement(BoolAspect::LabelPlacement::AtCheckBoxWithoutDummyLabel);
     collectBusEvents.setSettingsKey(base + "Callgrind.CollectBusEvents");
     collectBusEvents.setLabelText(tr("Collect global bus events"));
     collectBusEvents.setToolTip(tr("Collect the number of global bus events that are executed. "
         "The event type \"Ge\" is used for these events."));
 
-    group.registerAspect(&minimumInclusiveCostRatio);
+    registerAspect(&minimumInclusiveCostRatio);
     minimumInclusiveCostRatio.setSettingsKey(base + "Callgrind.MinimumCostRatio");
     minimumInclusiveCostRatio.setDefaultValue(0.01);
     minimumInclusiveCostRatio.setSuffix(tr("%"));
@@ -437,13 +437,13 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
     minimumInclusiveCostRatio.setToolTip(tr("Limits the amount of results the profiler gives you. "
          "A lower limit will likely increase performance."));
 
-    group.registerAspect(&visualizationMinimumInclusiveCostRatio);
+    registerAspect(&visualizationMinimumInclusiveCostRatio);
     visualizationMinimumInclusiveCostRatio.setSettingsKey(base + "Callgrind.VisualisationMinimumCostRatio");
     visualizationMinimumInclusiveCostRatio.setDefaultValue(10.0);
     visualizationMinimumInclusiveCostRatio.setLabelText(tr("Visualization: Minimum event cost:"));
     visualizationMinimumInclusiveCostRatio.setSuffix(tr("%"));
 
-    group.registerAspect(&visibleErrorKinds);
+    registerAspect(&visibleErrorKinds);
     visibleErrorKinds.setSettingsKey(base + "VisibleErrorKinds");
     QList<int> defaultErrorKinds;
     for (int i = 0; i < Valgrind::XmlProtocol::MemcheckErrorKindCount; ++i)
@@ -451,24 +451,6 @@ ValgrindBaseSettings::ValgrindBaseSettings(bool global)
     visibleErrorKinds.setDefaultValue(defaultErrorKinds);
 }
 
-void ValgrindBaseSettings::fromMap(const QVariantMap &map)
-{
-    group.fromMap(map);
-    if (ValgrindGlobalSettings::instance() != this) {
-        // FIXME: Update project page e.g. on "Restore Global", aspects
-        // there are 'autoapply', and Aspect::cancel() is normally part of
-        // the 'manual apply' machinery.
-        group.setAutoApply(false);
-        group.cancel();
-        group.setAutoApply(true);
-    }
-}
-
-void ValgrindBaseSettings::toMap(QVariantMap &map) const
-{
-    group.toMap(map);
-}
-
 
 //////////////////////////////////////////////////////////////////
 //
@@ -485,25 +467,25 @@ ValgrindGlobalSettings::ValgrindGlobalSettings()
 
     const QString base = "Analyzer.Valgrind";
 
-    group.registerAspect(&lastSuppressionDirectory);
+    registerAspect(&lastSuppressionDirectory);
     lastSuppressionDirectory.setSettingsKey(base + "LastSuppressionDirectory");
 
-    group.registerAspect(&lastSuppressionHistory);
+    registerAspect(&lastSuppressionHistory);
     lastSuppressionHistory.setSettingsKey(base + "LastSuppressionHistory");
 
-    group.registerAspect(&detectCycles);
+    registerAspect(&detectCycles);
     detectCycles.setSettingsKey(base + "Callgrind.CycleDetection");
     detectCycles.setDefaultValue(true);
     detectCycles.setLabelText("O"); // FIXME: Create a real icon
     detectCycles.setToolTip(tr("Enable cycle detection to properly handle recursive "
         "or circular function calls."));
 
-    group.registerAspect(&costFormat);
+    registerAspect(&costFormat);
     costFormat.setSettingsKey(base + "Callgrind.CostFormat");
     costFormat.setDefaultValue(CostDelegate::FormatRelative);
     costFormat.setDisplayStyle(SelectionAspect::DisplayStyle::ComboBox);
 
-    group.registerAspect(&shortenTemplates);
+    registerAspect(&shortenTemplates);
     shortenTemplates.setSettingsKey(base + "Callgrind.ShortenTemplates");
     shortenTemplates.setDefaultValue(true);
     shortenTemplates.setLabelText("<>"); // FIXME: Create a real icon
@@ -512,7 +494,7 @@ ValgrindGlobalSettings::ValgrindGlobalSettings()
     setConfigWidgetCreator([this] { return ValgrindOptionsPage::createSettingsWidget(this); });
     readSettings();
 
-    group.forEachAspect([](BaseAspect *aspect) { aspect->setAutoApply(false); });
+    setAutoApply(false);
 }
 
 ValgrindGlobalSettings *ValgrindGlobalSettings::instance()
@@ -527,7 +509,7 @@ ValgrindGlobalSettings *ValgrindGlobalSettings::instance()
 QVariantMap ValgrindBaseSettings::defaultSettings() const
 {
     QVariantMap defaults;
-    group.forEachAspect([&defaults](BaseAspect *aspect) {
+    forEachAspect([&defaults](BaseAspect *aspect) {
         defaults.insert(aspect->settingsKey(), aspect->defaultValue());
     });
     return defaults;
@@ -572,6 +554,15 @@ ValgrindProjectSettings::ValgrindProjectSettings()
     : ValgrindBaseSettings(false)
 {
     setConfigWidgetCreator([this] { return ValgrindOptionsPage::createSettingsWidget(this); });
+
+    connect(this, &AspectContainer::fromMapFinished, [this] {
+        // FIXME: Update project page e.g. on "Restore Global", aspects
+        // there are 'autoapply', and Aspect::cancel() is normally part of
+        // the 'manual apply' machinery.
+        setAutoApply(false);
+        cancel();
+        setAutoApply(true);
+    });
 }
 
 } // namespace Internal

src/plugins/valgrind/valgrindsettings.h

@@ -89,9 +89,6 @@ public:
         LeakCheckOnFinishYes
     };
 
-    void toMap(QVariantMap &map) const override;
-    void fromMap(const QVariantMap &map) override;
-
 signals:
     void changed(); // sent when multiple values have changed simulatenously (e.g. fromMap)
 
@@ -134,7 +131,6 @@ public:
     Utils::DoubleAspect minimumInclusiveCostRatio;
     Utils::DoubleAspect visualizationMinimumInclusiveCostRatio;
 
-    Utils::AspectContainer group;
     QVariantMap defaultSettings() const;
 };