VPN-3511 Remove: GetTransportType Code (#8835)

* Remove transport type * fixup! Remove transport type * Remove wrong qualifier and fix build issue * fixup! Remove wrong qualifier and fix build issue * Fix Windows build * Update telemetry documentation * Apply suggestions from code review Co-authored-by: Beatriz Rizental <[email protected]> --------- Co-authored-by: Beatriz Rizental <[email protected]>
mozilla-mobile · Dec 19, 2023 · ad73766 · ad73766
1 parent ceade05
commit ad73766
Show file tree

Hide file tree

Showing 14 changed files with 47 additions and 62 deletions.
diff --git a/src/networkwatcher.cpp b/src/networkwatcher.cpp
@@ -5,6 +5,7 @@
 #include "networkwatcher.h"
 
 #include <QMetaEnum>
+#include <QNetworkInformation>
 
 #include "controller.h"
 #include "leakdetector.h"
@@ -168,9 +169,6 @@ void NetworkWatcher::notificationClicked(NotificationHandler::Message message) {
   }
 }
 
-QString NetworkWatcher::getCurrentTransport() {
-  auto type = m_impl->getTransportType();
-  QMetaEnum metaEnum = QMetaEnum::fromType<NetworkWatcherImpl::TransportType>();
-  return QString(metaEnum.valueToKey(type))
-      .remove("TransportType_", Qt::CaseSensitive);
+QNetworkInformation::Reachability NetworkWatcher::getReachability() {
+  return QNetworkInformation::instance()->reachability();
 }
diff --git a/src/networkwatcher.h b/src/networkwatcher.h
@@ -7,6 +7,7 @@
 
 #include <QElapsedTimer>
 #include <QMap>
+#include <QNetworkInformation>
 
 #include "notificationhandler.h"
 
@@ -26,7 +27,7 @@ class NetworkWatcher final : public QObject {
   // public for the inspector.
   void unsecuredNetwork(const QString& networkName, const QString& networkId);
 
-  QString getCurrentTransport();
+  QNetworkInformation::Reachability getReachability();
 
  signals:
   void networkChange();

diff --git a/src/networkwatcherimpl.h b/src/networkwatcherimpl.h
@@ -5,6 +5,7 @@
 #ifndef NETWORKWATCHERIMPL_H
 #define NETWORKWATCHERIMPL_H
 
+#include <QNetworkInformation>
 #include <QObject>
 
 class NetworkWatcherImpl : public QObject {
@@ -34,7 +35,7 @@ class NetworkWatcherImpl : public QObject {
   Q_ENUM(TransportType);
 
   // Returns the current type of Network Connection
-  virtual TransportType getTransportType() = 0;
+  virtual QNetworkInformation::Reachability getReachability() = 0;
 
  signals:
   // Fires when the Device Connects to an unsecured Network
@@ -44,9 +45,6 @@ class NetworkWatcherImpl : public QObject {
   // too.
   void networkChanged(QString newBSSID);
 
-  // Fired when the Device changed the Type of Transport
-  void transportChanged(NetworkWatcherImpl::TransportType transportType);
-
  private:
   bool m_active = false;
 };

diff --git a/src/platforms/android/androidnetworkwatcher.cpp b/src/platforms/android/androidnetworkwatcher.cpp
@@ -7,6 +7,7 @@
 #include <QApplication>
 #include <QJniEnvironment>
 #include <QJniObject>
+#include <QNetworkInformation>
 
 #include "leakdetector.h"
 #include "logger.h"
@@ -15,9 +16,6 @@
 
 namespace {
 Logger logger("AndroidNetworkWatcher");
-
-constexpr auto VPNNetworkWatcher_CLASS =
-    "org/mozilla/firefox/vpn/qt/VPNNetworkWatcher";
 }  // namespace
 
 AndroidNetworkWatcher::AndroidNetworkWatcher(QObject* parent)
@@ -31,11 +29,6 @@ AndroidNetworkWatcher::~AndroidNetworkWatcher() {
 
 void AndroidNetworkWatcher::initialize() {}
 
-NetworkWatcherImpl::TransportType AndroidNetworkWatcher::getTransportType() {
-  QJniEnvironment env;
-  QJniObject activity = AndroidCommons::getActivity();
-  int type = QJniObject::callStaticMethod<int>(
-      VPNNetworkWatcher_CLASS, "getTransportType",
-      "(Landroid/content/Context;)I", activity.object());
-  return (NetworkWatcherImpl::TransportType)type;
-};
+QNetworkInformation::Reachability AndroidNetworkWatcher::getReachability() {
+  return QNetworkInformation::instance()->reachability();
+}
diff --git a/src/platforms/android/androidnetworkwatcher.h b/src/platforms/android/androidnetworkwatcher.h
@@ -14,7 +14,7 @@ class AndroidNetworkWatcher final : public NetworkWatcherImpl {
 
   void initialize() override;
 
-  NetworkWatcherImpl::TransportType getTransportType() override;
+  QNetworkInformation::Reachability getReachability() override;
 };
 
 #endif  // ANDROIDNETWORKWATCHER_H
diff --git a/src/platforms/dummy/dummynetworkwatcher.h b/src/platforms/dummy/dummynetworkwatcher.h
@@ -14,9 +14,9 @@ class DummyNetworkWatcher final : public NetworkWatcherImpl {
 
   void initialize() override;
 
-  NetworkWatcherImpl::TransportType getTransportType() override {
-    return TransportType_Other;
-  };
+  QNetworkInformation::Reachability getReachability() override {
+    return QNetworkInformation::instance()->reachability();
+  }
 };
 
 #endif  // DUMMYNETWORKWATCHER_H
diff --git a/src/platforms/ios/iosnetworkwatcher.h b/src/platforms/ios/iosnetworkwatcher.h
@@ -15,7 +15,7 @@ class IOSNetworkWatcher : public NetworkWatcherImpl {
   ~IOSNetworkWatcher();
 
   void initialize() override;
-  NetworkWatcherImpl::TransportType getTransportType() override;
+  QNetworkInformation::Reachability getReachability() override;
 
  private:
   NetworkWatcherImpl::TransportType toTransportType(nw_path_t path);

diff --git a/src/platforms/ios/iosnetworkwatcher.mm b/src/platforms/ios/iosnetworkwatcher.mm
@@ -40,18 +40,9 @@
           &IOSNetworkWatcher::controllerStateChanged);
 }
 
-NetworkWatcherImpl::TransportType IOSNetworkWatcher::getTransportType() {
-  if (MozillaVPN::instance()->controller()->state() != Controller::StateOn) {
-    // If we're not in stateON, the result from the Observer is fine, as
-    // the default-route-transport  is not the vpn-tunnel
-    return m_currentDefaultTransport;
-  }
-  if (m_observableConnection != nil) {
-    return m_currentVPNTransport;
+  QNetworkInformation::Reachability IOSNetworkWatcher::getReachability() {
+    return QNetworkInformation::instance()->reachability();
   }
-  // If we don't have an open tunnel-observer, m_currentVPNTransport is probably wrong.
-  return NetworkWatcherImpl::TransportType_Unknown;
-}
 
 NetworkWatcherImpl::TransportType IOSNetworkWatcher::toTransportType(nw_path_t path) {
   if (path == nil) {

diff --git a/src/platforms/linux/linuxnetworkwatcher.h b/src/platforms/linux/linuxnetworkwatcher.h
@@ -22,10 +22,9 @@ class LinuxNetworkWatcher final : public NetworkWatcherImpl {
 
   void start() override;
 
-  NetworkWatcherImpl::TransportType getTransportType() {
-    // TODO: Find out how to do that on linux generally. (VPN-2382)
-    return NetworkWatcherImpl::TransportType_Unknown;
-  };
+  QNetworkInformation::Reachability getReachability() {
+    return QNetworkInformation::instance()->reachability();
+  }
 
  signals:
   void checkDevicesInThread();

diff --git a/src/platforms/wasm/wasmnetworkwatcher.h b/src/platforms/wasm/wasmnetworkwatcher.h
@@ -5,6 +5,8 @@
 #ifndef WASMNETWORKWATCHER_H
 #define WASMNETWORKWATCHER_H
 
+#include <QNetworkInformation>
+
 #include "networkwatcherimpl.h"
 
 class WasmNetworkWatcher final : public NetworkWatcherImpl {
@@ -18,9 +20,9 @@ class WasmNetworkWatcher final : public NetworkWatcherImpl {
 
   void start() override;
 
-  NetworkWatcherImpl::TransportType getTransportType() override {
-    return TransportType_Other;
-  };
+  QNetworkInformation::Reachability getReachability() override {
+    return QNetworkInformation::instance()->reachability();
+  }
 };
 
 #endif  // WASMNETWORKWATCHER_H
diff --git a/src/platforms/windows/windowsnetworkwatcher.cpp b/src/platforms/windows/windowsnetworkwatcher.cpp
@@ -4,6 +4,7 @@
 
 #include "windowsnetworkwatcher.h"
 
+#include <QNetworkInformation>
 #include <QScopeGuard>
 
 #include "leakdetector.h"
@@ -138,7 +139,6 @@ void WindowsNetworkWatcher::processWlan(PWLAN_NOTIFICATION_DATA data) {
   emit unsecuredNetwork(ssid, bssid);
 }
 
-NetworkWatcherImpl::TransportType WindowsNetworkWatcher::getTransportType() {
-  // TODO: Implement this once we update to Qt6.3 (VPN-3511)
-  return TransportType_Other;
+QNetworkInformation::Reachability WindowsNetworkWatcher::getReachability() {
+  return QNetworkInformation::instance()->reachability();
 }
diff --git a/src/platforms/windows/windowsnetworkwatcher.h b/src/platforms/windows/windowsnetworkwatcher.h
@@ -17,7 +17,7 @@ class WindowsNetworkWatcher final : public NetworkWatcherImpl {
 
   void initialize() override;
 
-  NetworkWatcherImpl::TransportType getTransportType() override;
+  QNetworkInformation::Reachability getReachability() override;
 
  private:
   static void wlanCallback(PWLAN_NOTIFICATION_DATA data, PVOID context);

diff --git a/src/telemetry.cpp b/src/telemetry.cpp
@@ -120,17 +120,17 @@ void Telemetry::initialize() {
           this, &Telemetry::onDaemonStatus);
 #endif
 
-  connect(controller, &Controller::handshakeFailed, this,
-          [](const QString& publicKey) {
-            logger.info() << "Send a handshake failure event";
-
-            mozilla::glean::sample::connectivity_handshake_timeout.record(
-                mozilla::glean::sample::ConnectivityHandshakeTimeoutExtra{
-                    ._server = publicKey,
-                    ._transport = MozillaVPN::instance()
-                                      ->networkWatcher()
-                                      ->getCurrentTransport()});
-          });
+  connect(
+      controller, &Controller::handshakeFailed, this,
+      [](const QString& publicKey) {
+        logger.info() << "Send a handshake failure event";
+        mozilla::glean::sample::connectivity_handshake_timeout.record(
+            mozilla::glean::sample::ConnectivityHandshakeTimeoutExtra{
+                ._server = publicKey,
+                ._transport = QVariant::fromValue(MozillaVPN::instance()
+                                                      ->networkWatcher()
+                                                      ->getReachability())});
+      });
 
   connect(controller, &Controller::stateChanged, this, [this]() {
     MozillaVPN* vpn = MozillaVPN::instance();
@@ -258,7 +258,8 @@ void Telemetry::connectionStabilityEvent() {
           ._loss = QString::number(vpn->connectionHealth()->loss()),
           ._server = vpn->controller()->currentServer().exitServerPublicKey(),
           ._stddev = QString::number(vpn->connectionHealth()->stddev()),
-          ._transport = vpn->networkWatcher()->getCurrentTransport()});
+          ._transport =
+              QVariant::fromValue(vpn->networkWatcher()->getReachability())});
 }
 
 void Telemetry::vpnSessionPingTimeout() {

diff --git a/src/telemetry/metrics.yaml b/src/telemetry/metrics.yaml
@@ -1111,7 +1111,7 @@ sample:
           loss, stddev.
         type: string
       transport:
-        description: The Host network type (i.e wifi or 4g)
+        description: The Host network type (i.e wifi or 4g). Possible values are Unknown, Disconnected, Local, Site, Online
         type: string
 
   connectivity_stable:
@@ -1148,7 +1148,9 @@ sample:
         description: Standard deviation of the ping responses
         type: string
       transport:
-        description: The Host network type (i.e wifi or 4g)
+        description: |
+             The Host network type (i.e wifi or 4g). 
+             Possible values are Unknown, Disconnected, Local, Site, Online
         type: string
 
   server_unavailable_error: