Fix detection of popup UI close for Chrome MV3 (#1485)

The extension's popup UI allows the user to do several things,
including disabling/enabling protections for the current website.

When the user disables protections for a website, the website is added
to the allowlist. After a delay (or when the user clicks away) the
popup UI is closed and the website is reloaded automatically. Until
now, that worked by listening for the window.unload event, and then
using the chrome.extension.getBackgroundPage() API to fetch the
background page before triggering the unload function in the
background.

With Chrome Manifest v3, the getBackgroundPage() API is no longer
available and so a different approach must be used. We're now opening
a messaging connection to the background, sending through user actions
as they happen, then finally having the background detect when the
connection is closed (popup is closed) to trigger any corresponding
actions like reloading the active website.

1 - https://developer.chrome.com/docs/extensions/mv3/mv3-migration-checklist/#api-background-context

shared/js/background/devtools.es6.js

@@ -19,6 +19,10 @@ export function init () {
 }
 
 function connected (port) {
+    if (port.name !== 'devtools') {
+        return
+    }
+
     let tabId = -1
     port.onMessage.addListener((m) => {
         if (m.action === 'setTab') {

shared/js/background/events.es6.js

@@ -381,6 +381,46 @@ browser.runtime.onMessage.addListener((req, sender) => {
     return false
 })
 
+/**
+ * Messaging connections
+ */
+
+// List of actions that the user is taking in the currently open popup UI.
+// Note: This is lost when the background ServiceWorker is restarted. At that
+//       point, the popup window reopens the connection - restarting the
+//       background ServiceWorker in the process - and then resends its user
+//       actions. It is not usually safe to store state from events in this way.
+// See https://developer.chrome.com/docs/extensions/mv3/migrating_to_service_workers/#state
+const popupUserActions = []
+
+chrome.runtime.onConnect.addListener(port => {
+    // Popup UI is opened.
+    if (port.name === 'popup') {
+        // Take note of new user actions as they happen.
+        port.onMessage.addListener(userAction => {
+            popupUserActions.push(userAction)
+        })
+
+        // Popup UI closed again, refresh page etc based on the actions user
+        // took in the popup while it was open.
+        port.onDisconnect.addListener(async () => {
+            // Reload the website after a short delay if the user toggled
+            // allowlisting for it.
+            if (popupUserActions.includes('toggleAllowlist')) {
+                const currentTab = await utils.getCurrentTab()
+                if (currentTab) {
+                    setTimeout(() => {
+                        browser.tabs.reload(currentTab.id)
+                    }, 500)
+                }
+            }
+
+            // Clear the list of user actions.
+            popupUserActions.length = 0
+        })
+    }
+})
+
 /*
  * Referrer Trimming
  */

shared/js/background/startup.js

@@ -1,6 +1,4 @@
 import browser from 'webextension-polyfill'
-const { getCurrentTab } = require('./utils.es6')
-const browserUIWrapper = require('../ui/base/ui-wrapper.es6')
 const browserWrapper = require('./wrapper.es6')
 const Companies = require('./companies.es6')
 const experiment = require('./experiments.es6')
@@ -23,7 +21,6 @@ export async function onStartup () {
         await dnrSessionId.setSessionRuleOffsetFromStorage()
     }
 
-    registerUnloadHandler()
     await settings.ready()
     experiment.setActiveExperiment()
 
@@ -65,26 +62,3 @@ export async function onStartup () {
 export function ready () {
     return readyPromise
 }
-
-/**
- * Register a global function that the popup can call when it's closed.
- *
- * NOTE: This has to be a global method because messages via `chrome.runtime.sendMessage` don't make it in time
- * when the popup is closed.
- */
-function registerUnloadHandler () {
-    let timeout
-    // @ts-ignore - popupUnloaded is not a standard property of self.
-    self.popupUnloaded = (userActions) => {
-        clearTimeout(timeout)
-        if (userActions.includes('toggleAllowlist')) {
-            timeout = setTimeout(() => {
-                getCurrentTab().then(tab => {
-                    if (tab) {
-                        browserUIWrapper.reloadTab(tab.id)
-                    }
-                })
-            }, 500)
-        }
-    }
-}

shared/js/devtools/panel.es6.js

@@ -16,7 +16,7 @@ let tabId = chrome.devtools?.inspectedWindow?.tabId || parseInt(0 + new URL(docu
 // disconnect for MV3 builds when the background ServiceWorker becomes inactive.
 let port
 function openPort () {
-    port = chrome.runtime.connect()
+    port = chrome.runtime.connect({ name: 'devtools' })
     port.onDisconnect.addListener(openPort)
 }
 openPort()

shared/js/ui/base/ui-wrapper.es6.js

@@ -74,16 +74,6 @@ export const closePopup = () => {
     w.close()
 }
 
-/**
- * Notify the background script that the popup was closed.
- * @param {string[]} userActions - a list of string indicating what actions a user may have taken
- */
-export const popupUnloaded = (userActions) => {
-    const bg = chrome.extension.getBackgroundPage()
-    // @ts-ignore - popupUnloaded is not a standard property of self.
-    bg?.popupUnloaded?.(userActions)
-}
-
 module.exports = {
     sendMessage,
     reloadTab,
@@ -93,6 +83,5 @@ module.exports = {
     search,
     openOptionsPage,
     openExtensionPage,
-    getExtensionURL,
-    popupUnloaded
+    getExtensionURL
 }

shared/js/ui/views/mixins/unload-listener.es6.js

@@ -1,27 +1,41 @@
-const browserUIWrapper = require('./../../base/ui-wrapper.es6.js')
+import browser from 'webextension-polyfill'
 
 module.exports = {
     /**
-     * Collect any 'user-action' messages and pass them along
-     * when we call `browserUIWrapper.popupUnloaded` - this allows the background script
-     * to make a decision about whether to reload the current page.
+     * Passes any 'user-action' messages through a messaging connection to the
+     * extension's background, so that they can be used at the point this window
+     * is closed. This allows the background script to make a decision about
+     * whether to reload the current page.
      *
      * For example: If a user has manually disabled protections in the popup,
-     * we don't immediately reload the page behind - but instead we want to wait until the
-     * popup is closed.
+     * we don't immediately reload the page behind - but instead we want to wait
+     * until the popup is closed.
      *
      * @param store
      */
     registerUnloadListener: function (store) {
         /** @type {string[]} */
         const userActions = []
+        let connection = null
+
+        // Create a messaging connection to the background. If the connection is
+        // broken by the background, the background ServiceWorker was restarted
+        // and the connection must be recreated + any earlier user actions
+        // resent.
+        const reconnect = () => {
+            connection = browser.runtime.connect({ name: 'popup' })
+            connection.onDisconnect.addListener(reconnect)
+            for (const userAction of userActions) {
+                connection.postMessage(userAction)
+            }
+        }
+        reconnect()
+
         store.subscribe.on('action:site', (event) => {
             if (event.action === 'user-action') {
                 userActions.push(event.data)
+                connection.postMessage(event.data)
             }
         })
-        window.addEventListener('unload', function () {
-            browserUIWrapper.popupUnloaded(userActions)
-        }, false)
     }
 }