Part 2 - Update Callers

Author: strseb

src/addons/addon.cpp

@@ -24,7 +24,8 @@
 #include "conditionwatchers/addonconditionwatchertranslationthreshold.h"
 #include "conditionwatchers/addonconditionwatchertriggertimesecs.h"
 #include "env.h"
-#include "feature/feature.h"
+#include "feature/featuremodel.h"
+#include "feature/features.h"
 #include "leakdetector.h"
 #include "localizer.h"
 #include "logger.h"
@@ -59,8 +60,7 @@ QList<ConditionCallback> s_conditionCallbacks{
        for (const QJsonValue& enabledFeature : enabledFeatures) {
          QString featureName = enabledFeature.toString();
 
-         const Feature* feature = Feature::getOrNull(featureName);
-         if (!feature) {
+         if (!FeatureModel::instance()->get(featureName)) {
            logger.info() << "Feature not found" << featureName;
            return false;
          }
@@ -369,7 +369,7 @@ Addon* Addon::create(QObject* parent, const QString& manifestFileName) {
   }
 
   else if (type == "replacer") {
-    if (!Feature::get(Feature::Feature_replacerAddon)->isSupported()) {
+    if (!Feature::isEnabled(Feature::replacerAddon)) {
       // This addon type is blocked pending
       // https://mozilla-hub.atlassian.net/browse/VPN-4858
       logger.error() << "The replacer type addon is unsupported.";
@@ -461,7 +461,12 @@ Addon::Addon(QObject* parent, const QString& manifestFileName,
   }
 }
 
-Addon::~Addon() { MZ_COUNT_DTOR(Addon); }
+Addon::~Addon() {
+  MZ_COUNT_DTOR(Addon);
+  if (!m_translators.isEmpty()) {
+    unloadTranslators();
+  }
+}
 
 void Addon::updateAddonStatus(Status newStatus) {
   Q_ASSERT(newStatus != Unknown);

src/addons/addonreplacer.cpp

@@ -114,7 +114,12 @@ AddonReplacer::AddonReplacer(QObject* parent, const QString& manifestFileName,
   MZ_COUNT_CTOR(AddonReplacer);
 }
 
-AddonReplacer::~AddonReplacer() { MZ_COUNT_DTOR(AddonReplacer); }
+AddonReplacer::~AddonReplacer() {
+  MZ_COUNT_DTOR(AddonReplacer);
+  if (enabled()) {
+    AddonReplacer::disable();
+  }
+}
 
 void AddonReplacer::enable() {
   Addon::enable();

src/addons/conditionwatchers/addonconditionwatcherfeaturesenabled.cpp

@@ -4,7 +4,8 @@
 
 #include "addonconditionwatcherfeaturesenabled.h"
 
-#include "feature/feature.h"
+#include "feature/featuremodel.h"
+#include "feature/featureproxy.h"
 #include "leakdetector.h"
 
 // static
@@ -15,8 +16,7 @@ AddonConditionWatcher* AddonConditionWatcherFeaturesEnabled::maybeCreate(
   }
 
   for (const QString& featureName : features) {
-    const Feature* feature = Feature::getOrNull(featureName);
-    if (!feature) {
+    if (!FeatureModel::instance()->get(featureName)) {
       return nullptr;
     }
   }
@@ -32,10 +32,11 @@ AddonConditionWatcherFeaturesEnabled::AddonConditionWatcherFeaturesEnabled(
   m_currentStatus = conditionApplied();
 
   for (const QString& featureName : m_features) {
-    const Feature* feature = Feature::get(featureName);
-    Q_ASSERT(feature);
+    auto* proxy =
+        qobject_cast<FeatureProxy*>(FeatureModel::instance()->get(featureName));
+    Q_ASSERT(proxy);
 
-    connect(feature, &Feature::supportedChanged, this, [this]() {
+    connect(proxy, &FeatureProxy::supportedChanged, this, [this]() {
       bool newStatus = conditionApplied();
       if (m_currentStatus != newStatus) {
         m_currentStatus = newStatus;
@@ -51,11 +52,7 @@ AddonConditionWatcherFeaturesEnabled::~AddonConditionWatcherFeaturesEnabled() {
 
 bool AddonConditionWatcherFeaturesEnabled::conditionApplied() const {
   for (const QString& featureName : m_features) {
-    // If the feature doesn't exist, we crash.
-    const Feature* feature = Feature::get(featureName);
-    Q_ASSERT(feature);
-
-    if (!feature->isSupported()) {
+    if (!FeatureModel::instance()->isEnabledById(featureName)) {
       return false;
     }
   }

src/addons/manager/addonmanager.cpp

@@ -20,7 +20,7 @@
 #include "addonindex.h"
 #include "addons/addonmessage.h"
 #include "constants.h"
-#include "feature/feature.h"
+#include "feature/features.h"
 #include "leakdetector.h"
 #include "logger.h"
 #include "qmlengineholder.h"
@@ -474,7 +474,7 @@ bool AddonManager::signatureVerificationNeeded() {
     return false;
   }
 
-  return Feature::get(Feature::Feature_addonSignature)->isSupported();
+  return Feature::isEnabled(Feature::addonSignature);
 }
 
 void AddonManager::loadCompleted() {

src/authenticationinapp/authenticationinappsession.cpp

@@ -13,7 +13,7 @@
 #include "authenticationinapp.h"
 #include "authenticationlistener.h"
 #include "constants.h"
-#include "feature/feature.h"
+#include "feature/features.h"
 #include "hawkauth.h"
 #include "hkdf.h"
 #include "leakdetector.h"
@@ -222,7 +222,7 @@ void AuthenticationInAppSession::accountChecked(bool exists, bool hasPassword,
     return;
   }
 
-  if (Feature::get(Feature::Feature_inAppAccountCreate)->isSupported()) {
+  if (Feature::isEnabled(Feature::inAppAccountCreate)) {
     AuthenticationInApp::instance()->requestState(
         AuthenticationInApp::StateSignUp, this);
     return;

src/commands/commandexcludeip.cpp

@@ -4,13 +4,15 @@
 
 #include "commandexcludeip.h"
 
+#include <QCoreApplication>
 #include <QHostAddress>
 #include <QTextStream>
 
 #include "commandlineparser.h"
 #include "constants.h"
 #include "ipaddress.h"
 #include "leakdetector.h"
+#include "loghandler.h"
 #include "mozillavpn.h"
 #include "settingsholder.h"
 

src/controller.cpp

@@ -12,7 +12,7 @@
 #include "controller_p.h"
 #include "controllerimpl.h"
 #include "dnshelper.h"
-#include "feature/feature.h"
+#include "feature/features.h"
 #include "frontend/navigator.h"
 #include "ipaddress.h"
 #include "leakdetector.h"
@@ -408,16 +408,15 @@ void Controller::activateInternal(
   logger.debug() << "DNS Set" << exitConfig.m_dnsServer;
 
   // Splittunnel-feature could have been disabled due to a driver conflict.
-  if (Feature::get(Feature::Feature_splitTunnel)->isSupported()) {
+  if (Feature::isEnabled(Feature::splitTunnel)) {
     exitConfig.m_vpnDisabledApps = settingsHolder->vpnDisabledApps();
   }
-  if (Feature::get(Feature::Feature_alwaysPort53)->isSupported()) {
+  if (Feature::isEnabled(Feature::alwaysPort53)) {
     dnsPort = ForceDNSPort;
   }
 
   // For single-hop connections, exclude the entry server
-  if (!Feature::get(Feature::Feature_multiHop)->isSupported() ||
-      !m_serverData.multihop()) {
+  if (!Feature::multiHop.supported || !m_serverData.multihop()) {
     logger.info() << "Activating single hop";
     exitConfig.m_hopType = InterfaceConfig::SingleHop;
 
@@ -943,8 +942,7 @@ bool Controller::activate(const ServerData& serverData,
       return true;
     }
 
-    if (Feature::get(Feature::Feature_checkConnectivityOnActivation)
-            ->isSupported()) {
+    if (Feature::isEnabled(Feature::checkConnectivityOnActivation)) {
       // Ensure that the device is connected to the Internet.
       if (MozillaVPN::instance()->networkWatcher()->getReachability() ==
           QNetworkInformation::Reachability::Disconnected) {

src/dnshelper.cpp

@@ -6,7 +6,7 @@
 
 #include <QHostAddress>
 
-#include "feature/feature.h"
+#include "feature/features.h"
 #include "logger.h"
 #include "settingsholder.h"
 
@@ -37,7 +37,7 @@ QString DNSHelper::getDNSType() {
 
 // static
 dnsData DNSHelper::getDNSDetails(const QString& fallback) {
-  if (!Feature::get(Feature::Feature_customDNS)->isSupported()) {
+  if (!Feature::customDNS.supported) {
     return dnsData{fallback, "NoCustomDNSAvailable"};
   }
 

src/feature/featuremodel.cpp

@@ -30,6 +30,13 @@ FeatureModel* FeatureModel::instance() {
   return s_instance;
 }
 
+#ifdef UNIT_TEST
+void FeatureModel::testCleanup() {
+  delete s_instance;
+  s_instance = nullptr;
+}
+#endif
+
 void FeatureModel::initialize() {
   if (m_initialized) return;
   m_initialized = true;
@@ -117,7 +124,7 @@ void FeatureModel::toggle(const QString& featureId) {
 
 // static
 QHash<QString, bool> FeatureModel::parseFeatures(const QByteArray& data,
-                                                  bool acceptExperiments) {
+                                                 bool acceptExperiments) {
   QHash<QString, bool> overrides;
   QJsonObject json = QJsonDocument::fromJson(data).object();
 

src/feature/featuremodel.h

@@ -22,6 +22,7 @@ inline const AnyFeature s_exposedFeatures[] = {
     ref(addonSignature),
     ref(alwaysPort53),
     ref(annualUpgrade),
+    ref(appReview),
     ref(captivePortal),
     ref(checkConnectivityOnActivation),
     ref(customDNS),
@@ -67,14 +68,18 @@ class FeatureModel final : public QAbstractListModel {
 
   bool isEnabledById(const QString& id) const;
 
+#ifdef UNIT_TEST
+  static void testCleanup();
+#endif
+
  private:
   FeatureModel();
   void initialize();
 
   FeatureProxy* proxyForId(const QString& id);
 
   static QHash<QString, bool> parseFeatures(const QByteArray& data,
-                                             bool acceptExperiments);
+                                            bool acceptExperiments);
 
   bool m_initialized = false;
   std::array<FeatureProxy, std::size(s_exposedFeatures)> m_proxies;