VPN-6570/VPN-6583: Fix logout race condition when VPN is active (#9859)

* Fix logout race condition when VPN is active * Fix build fail in unit tests
mozilla-mobile · Sep 11, 2024 · 1611944 · 1611944
1 parent fad1e36
commit 1611944
Show file tree

Hide file tree

Showing 10 changed files with 7 additions and 27 deletions.
diff --git a/nebula/ui/components/MZSignOut.qml b/nebula/ui/components/MZSignOut.qml
@@ -31,6 +31,6 @@ MZFooterLink {
 
     onClicked: () => {
         preLogoutCallback();
-        VPNController.logout();
+        VPN.logout();
     }
 }
diff --git a/src/controller.cpp b/src/controller.cpp
@@ -372,23 +372,6 @@ void Controller::updateRequired() {
   }
 }
 
-void Controller::logout() {
-  logger.debug() << "Logout";
-
-  MozillaVPN::instance()->logout();
-
-  if (m_state == StateOff) {
-    return;
-  }
-
-  m_nextStep = Disconnect;
-
-  if (m_state == StateOn || m_state == StateOnPartial) {
-    deactivate();
-    return;
-  }
-}
-
 void Controller::activateInternal(
     DNSPortPolicy dnsPort,
     ServerSelectionPolicy serverSelectionPolicy = RandomizeServerSelection,

diff --git a/src/controller.h b/src/controller.h
@@ -84,7 +84,6 @@ class Controller : public QObject, public LogSerializer {
 
   int connectionRetry() const { return m_connectionRetry; }
   State state() const;
-  Q_INVOKABLE void logout();
   bool silentServerSwitchingSupported() const;
   void cleanupBackendLogs();
 

diff --git a/src/mozillavpn.cpp b/src/mozillavpn.cpp
@@ -767,6 +767,8 @@ void MozillaVPN::logout() {
 
   ErrorHandler::instance()->requestAlert(ErrorHandler::LogoutAlert);
 
+  deactivate();
+
   TaskScheduler::deleteTasks();
 
   // Schedule the removal of our device and let it run in the background.

diff --git a/src/mozillavpn.h b/src/mozillavpn.h
@@ -167,7 +167,7 @@ class MozillaVPN final : public App {
 
   Q_INVOKABLE void silentSwitch();
 
-  void logout();
+  Q_INVOKABLE void logout();
 
   bool startMinimized() const { return m_startMinimized; }
 

diff --git a/src/ui/main.qml b/src/ui/main.qml
@@ -155,7 +155,7 @@ Window {
     Connections {
         target: VPN
         function onAccountDeleted() {
-            VPNController.logout();
+            VPN.logout();
         }
     }
 

diff --git a/src/ui/screens/ScreenSubscriptionInUseError.qml b/src/ui/screens/ScreenSubscriptionInUseError.qml
@@ -27,7 +27,7 @@ MZStackView {
            primaryButtonText: MZI18n.GlobalSignOut,
            primaryButtonObjectName: "errorSignOutButton",
            primaryButtonOnClick: () => {
-               VPNController.logout();
+               VPN.logout();
                stackView.pop();
            },
            secondaryButtonIsSignOff: false,

diff --git a/src/ui/screens/devices/ViewDeviceLimit.qml b/src/ui/screens/devices/ViewDeviceLimit.qml
@@ -86,7 +86,7 @@ MZViewBase {
             labelText: MZI18n.GlobalSignOut
             fontName: MZTheme.theme.fontBoldFamily
             linkColor: MZTheme.theme.redLinkButton
-            onClicked: VPNController.logout();
+            onClicked: VPN.logout();
         }
     }
 

diff --git a/tests/qml/moccontroller.cpp b/tests/qml/moccontroller.cpp
@@ -39,8 +39,6 @@ void Controller::disconnected() {}
 
 void Controller::timerTimeout() {}
 
-void Controller::logout() {}
-
 bool Controller::processNextStep() { return false; }
 
 void Controller::setState(State) {}

diff --git a/tests/unit/moccontroller.cpp b/tests/unit/moccontroller.cpp
@@ -43,8 +43,6 @@ void Controller::disconnected() {}
 
 void Controller::timerTimeout() {}
 
-void Controller::logout() {}
-
 bool Controller::processNextStep() { return false; }
 
 void Controller::setState(State) {}