FXVPN-222: Improve page reload times when client is in StateOnPartial (…

…#141)
mozilla-extensions · Dec 23, 2024 · 78bf491 · 78bf491
1 parent ff67f1c
commit 78bf491
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 12 deletions.
diff --git a/src/background/tabReloader.js b/src/background/tabReloader.js
@@ -44,15 +44,32 @@ export class TabReloader extends Component {
 
   currentExtState;
 
-  static async onExtensionStateChanged(extState) {
+  static async onExtensionStateChanged(newState) {
     if (
-      this.currentExtState == extState.state ||
-      !["Enabled", "Disabled"].includes(extState.state)
+      this.currentExtState == newState.state ||
+      !["Enabled", "Disabled", "Connecting"].includes(newState.state)
     ) {
       return;
     }
-    this.currentExtState = extState.state;
-    TabReloader.onOriginChanged();
+
+    const cachedCurrentState = this.currentExtState;
+    this.currentExtState = newState.state;
+
+    // We don't need to reload tabs when we move to "Connecting"
+    // so return
+    if (newState.state === "Connecting") {
+      return;
+    }
+
+    // Hack to mitigate FXVPN-217 and FXVPN-222
+    // See Utils.delayToStateEnabledNeeded() for details
+    if (Utils.delayToStateEnabledNeeded(cachedCurrentState, newState.state)) {
+      setTimeout(() => {
+        TabReloader.onOriginChanged();
+      }, Utils.connectingDelay);
+    } else {
+      TabReloader.onOriginChanged();
+    }
   }
 
   static async onOriginChanged(origin = null) {

diff --git a/src/components/vpncard.js b/src/components/vpncard.js
@@ -138,7 +138,9 @@ export class VPNCard extends LitElement {
             ${VPNCard.shield(
               this.enabled,
               this.connecting,
-              this.#shieldElement
+              this.#shieldElement,
+              this.stability,
+              this.clientConnected
             )}
             <div class="infobox">
               <h1>${vpnHeader()}</h1>
@@ -177,16 +179,16 @@ export class VPNCard extends LitElement {
     `;
   }
 
-  static subline(enabled, stability, clientIsConnected) {
+  static subline(enabled, stability, connecting, clientConnected) {
     const onLinkClick = () => {
       browser.tabs.create({
         url: "https://support.mozilla.org/kb/get-started-mozilla-vpn-extension?utm_medium=mozilla-vpn&utm_source=vpn-extension",
       });
       window.close();
     };
 
-    if (!enabled) {
-      return clientIsConnected
+    if (!enabled && !connecting) {
+      return clientConnected
         ? html`<p class="subline ext-is-off">
             ${tr("extensionVpnIsOff")}
             <a class="in-copy-link" @click=${onLinkClick} href=""

diff --git a/src/shared/utils.js b/src/shared/utils.js
@@ -23,6 +23,21 @@ export const Utils = {
     return ["win"].includes(os);
   },
 
+  connectingDelay: 2500,
+
+  // Hack to mitigate FXVPN-217 and FXVPN-222.
+  // When transitioning from StateFirefoxVPNConnecting to StateFirefoxVPNEnabled,
+  // the client enters StateOnPartial wherein routing table updates can cause
+  // long page reloads or even timeouts.
+  // To offset this, we introduce a delay in the UI and before reloading tabs,
+  // allowing the OS a buffer in which to update routing tables.
+  delayToStateEnabledNeeded(currentState, newState) {
+    if (!currentState) {
+      return false;
+    }
+    return currentState === "Connecting" && newState === "Enabled";
+  },
+
   isViableClientVersion: (clientVersion) => {
     // TODO we should do something better here
     // We'll likely want to update the minimumViableClient

diff --git a/src/ui/browserAction/popupPage.js b/src/ui/browserAction/popupPage.js
@@ -84,8 +84,20 @@ export class BrowserActionPopup extends LitElement {
       this._siteContexts = s;
     });
     extController.state.subscribe((s) => {
-      this.extState = s;
-      this.updatePage();
+      const currentState = this.extState;
+
+      // Hack to mitigate FXVPN-217 and FXVPN-222
+      // See Utils.delayToStateEnabledNeeded() for details
+      const timer = Utils.delayToStateEnabledNeeded(
+        currentState?.state,
+        s.state
+      )
+        ? Utils.connectingDelay
+        : 0;
+      setTimeout(() => {
+        this.extState = s;
+        this.updatePage();
+      }, timer);
     });
     butterBarService.butterBarList.subscribe((s) => {
       this.alerts = s;
@@ -237,7 +249,7 @@ export class BrowserActionPopup extends LitElement {
   }
 
   locationSettings() {
-    if (!this.pageURL || !this.extState.enabled) {
+    if (!this.pageURL || !this.extState?.enabled) {
       return null;
     }
     const resetSitePreferences = async () => {