Merge branch 'cancel-pending-notifications'

3 files changed, 62 insertions, 16 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7c5c3ddf23..6409a27013 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -40,6 +40,7 @@ Line wrap the file at 100 chars.                                              Th
 ### Fixed
 - Pick new random relay for each reconnect attempt instead of just retrying with the same one.
 - Make the `problem-report` tool fall back to the bundled API IP if DNS resolution fails.
+- Cancel pending system notifications when the app becomes visible.
 
 #### macOS
 - Correctly backup and restore search domains and other DNS settings.
diff --git a/gui/packages/desktop/src/renderer/app.js b/gui/packages/desktop/src/renderer/app.js
index a8b0ed8556..8e4328f66a 100644
--- a/gui/packages/desktop/src/renderer/app.js
+++ b/gui/packages/desktop/src/renderer/app.js
@@ -124,7 +124,10 @@ export default class AppRenderer {
       }
     });
 
-    ipcRenderer.on('window-shown', () => this.updateAccountExpiry());
+    ipcRenderer.on('window-shown', () => {
+      this.updateAccountExpiry();
+      this._notificationController.cancelPendingNotifications();
+    });
 
     // disable pinch to zoom
     webFrame.setVisualZoomLevelLimits(1, 1);
diff --git a/gui/packages/desktop/src/renderer/lib/notification-controller.js b/gui/packages/desktop/src/renderer/lib/notification-controller.js
index 9cb2f925dc..cd829de90e 100644
--- a/gui/packages/desktop/src/renderer/lib/notification-controller.js
+++ b/gui/packages/desktop/src/renderer/lib/notification-controller.js
@@ -7,9 +7,10 @@ import config from '../../config';
 import type { TunnelStateTransition } from './daemon-rpc';
 
 export default class NotificationController {
-  _activeNotification: ?Notification;
+  _lastTunnelStateNotification: ?Notification;
   _reconnecting = false;
   _presentedNotifications = {};
+  _pendingNotifications: Array<Notification> = [];
 
   notifyTunnelState(tunnelState: TunnelStateTransition) {
     switch (tunnelState.state) {
@@ -47,15 +48,24 @@ export default class NotificationController {
   }
 
   notifyInconsistentVersion() {
+    if (remote.getCurrentWindow().isVisible()) {
+      return;
+    }
+
     this._presentNotificationOnce('inconsistent-version', () => {
-      new Notification(remote.app.getName(), {
+      const notification = new Notification(remote.app.getName(), {
         body: 'Inconsistent internal version information, please restart the app',
         silent: true,
       });
+      this._addPendingNotification(notification);
     });
   }
 
   notifyUnsupportedVersion(upgradeVersion: string) {
+    if (remote.getCurrentWindow().isVisible()) {
+      return;
+    }
+
     this._presentNotificationOnce('unsupported-version', () => {
       const notification = new Notification(remote.app.getName(), {
         body: `You are running an unsupported app version. Please upgrade to ${upgradeVersion} now to ensure your security`,
@@ -65,33 +75,36 @@ export default class NotificationController {
       notification.addEventListener('click', () => {
         remote.shell.openExternal(config.links.download);
       });
+
+      this._addPendingNotification(notification);
     });
   }
 
+  cancelPendingNotifications() {
+    for (const notification of this._pendingNotifications) {
+      this._closeNotification(notification);
+    }
+  }
+
   _showTunnelStateNotification(message: string) {
-    const lastNotification = this._activeNotification;
+    const lastNotification = this._lastTunnelStateNotification;
     const sameAsLastNotification = lastNotification && lastNotification.body === message;
 
     if (sameAsLastNotification || remote.getCurrentWindow().isVisible()) {
       return;
     }
 
-    const newNotification = new Notification(remote.app.getName(), { body: message, silent: true });
-
-    this._activeNotification = newNotification;
-
-    newNotification.addEventListener('show', () => {
-      // If the notification is closed too soon, it might still get shown. If that happens, close()
-      // should be called again so that it is closed immediately.
-      // Tracking issue: https://github.com/electron/electron/issues/12887
-      if (this._activeNotification !== newNotification) {
-        newNotification.close();
-      }
+    const newNotification = new Notification(remote.app.getName(), {
+      body: message,
+      silent: true,
     });
 
     if (lastNotification) {
-      lastNotification.close();
+      this._closeNotification(lastNotification);
     }
+
+    this._lastTunnelStateNotification = newNotification;
+    this._addPendingNotification(newNotification);
   }
 
   _presentNotificationOnce(notificationName: string, presentNotification: () => void) {
@@ -101,4 +114,33 @@ export default class NotificationController {
       presentNotification();
     }
   }
+
+  _closeNotification(notification: Notification) {
+    // If the notification is closed too soon, it might still get shown. If that happens, close()
+    // should be called again so that it is closed immediately.
+    // Tracking issue: https://github.com/electron/electron/issues/12887
+    notification.addEventListener('show', () => {
+      notification.close();
+    });
+
+    notification.close();
+  }
+
+  _addPendingNotification(notification: Notification) {
+    // Quirk: chromium postpones the 'close' event until new notifications pump the queue or window
+    // becomes visible. It's possible that there is going to be one stale notification in
+    // `_pendingNotifications` array but that shouldn't be a big deal.
+    notification.addEventListener('close', () => {
+      this._removePendingNotification(notification);
+    });
+
+    this._pendingNotifications.push(notification);
+  }
+
+  _removePendingNotification(notification: Notification) {
+    const index = this._pendingNotifications.indexOf(notification);
+    if (index !== -1) {
+      this._pendingNotifications.splice(index, 1);
+    }
+  }
 }