From ad73766593ff8e48d218fcac9848b47d15bcd10b Mon Sep 17 00:00:00 2001 From: Gela Date: Tue, 19 Dec 2023 10:32:57 -0800 Subject: [PATCH] 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 --------- Co-authored-by: Beatriz Rizental --- src/networkwatcher.cpp | 8 +++--- src/networkwatcher.h | 3 ++- src/networkwatcherimpl.h | 6 ++--- .../android/androidnetworkwatcher.cpp | 15 +++-------- src/platforms/android/androidnetworkwatcher.h | 2 +- src/platforms/dummy/dummynetworkwatcher.h | 6 ++--- src/platforms/ios/iosnetworkwatcher.h | 2 +- src/platforms/ios/iosnetworkwatcher.mm | 13 ++-------- src/platforms/linux/linuxnetworkwatcher.h | 7 +++--- src/platforms/wasm/wasmnetworkwatcher.h | 8 +++--- .../windows/windowsnetworkwatcher.cpp | 6 ++--- src/platforms/windows/windowsnetworkwatcher.h | 2 +- src/telemetry.cpp | 25 ++++++++++--------- src/telemetry/metrics.yaml | 6 +++-- 14 files changed, 47 insertions(+), 62 deletions(-) diff --git a/src/networkwatcher.cpp b/src/networkwatcher.cpp index 8e13c31b80..7cf1802816 100644 --- a/src/networkwatcher.cpp +++ b/src/networkwatcher.cpp @@ -5,6 +5,7 @@ #include "networkwatcher.h" #include +#include #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(); - 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 index ce07c49ad2..41406eeae1 100644 --- a/src/networkwatcher.h +++ b/src/networkwatcher.h @@ -7,6 +7,7 @@ #include #include +#include #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 index d156c0db55..07ec36acf2 100644 --- a/src/networkwatcherimpl.h +++ b/src/networkwatcherimpl.h @@ -5,6 +5,7 @@ #ifndef NETWORKWATCHERIMPL_H #define NETWORKWATCHERIMPL_H +#include #include 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 index 9ff1ce004a..25609b9f4c 100644 --- a/src/platforms/android/androidnetworkwatcher.cpp +++ b/src/platforms/android/androidnetworkwatcher.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #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( - 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 index 8194a7f786..0f7150e653 100644 --- 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 index 4e6d2a6626..2047c38326 100644 --- 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 index 5b779a879a..68355b459c 100644 --- 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 index a77acb1c81..b98f778120 100644 --- 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 index ed8c76babb..144c919d05 100644 --- 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 index 08d07bdda6..adb94646f2 100644 --- a/src/platforms/wasm/wasmnetworkwatcher.h +++ b/src/platforms/wasm/wasmnetworkwatcher.h @@ -5,6 +5,8 @@ #ifndef WASMNETWORKWATCHER_H #define WASMNETWORKWATCHER_H +#include + #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 index 2de5a72618..e201338c6b 100644 --- a/src/platforms/windows/windowsnetworkwatcher.cpp +++ b/src/platforms/windows/windowsnetworkwatcher.cpp @@ -4,6 +4,7 @@ #include "windowsnetworkwatcher.h" +#include #include #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 index 29b998080f..5f8efb1445 100644 --- 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 index d928977b61..5cd8d420a6 100644 --- 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 index c7cc3e5560..96afcf2ecf 100644 --- 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: