fix: guard `browser` + reuse more globals

* use async mode in onMessageListener to prevent interception of results via `then` hook (another approach was to use isPromise, but it's defined only in `content` whereas this code runs in extension pages too, so in addition to a ternary we would have to pacify eslint, which is meh)
* reuse `chrome`
* reuse icon path from manifest
* globalThis is used only by tests

src/background/index.js

@@ -115,7 +115,7 @@ initialize(() => {
   sync.initialize();
   checkRemove();
   setInterval(checkRemove, TIMEOUT_24HOURS);
-  const api = global.chrome.declarativeContent;
+  const api = chrome.declarativeContent;
   if (api) {
     // Using declarativeContent to run content scripts earlier than document_start
     api.onPageChanged.getRules(/* for old Chrome */ null, async ([rule]) => {

src/background/utils/icon.js

@@ -26,7 +26,6 @@ const iconCache = {};
 // Firefox Android does not support such APIs, use noop
 
 const browserAction = (() => {
-  const { chrome } = global;
   // Using `chrome` namespace in order to skip our browser.js polyfill in Chrome
   const api = chrome.browserAction;
   // Suppress the "no tab id" error when setting an icon/badge as it cannot be reliably prevented

src/common/browser.js

@@ -5,7 +5,7 @@ let { browser } = global;
 // https://github.com/mozilla/webextension-polyfill/pull/153
 // https://html.spec.whatwg.org/multipage/window-object.html#named-access-on-the-window-object
 if (!IS_FIREFOX && !browser?.runtime) {
-  const { chrome, Proxy: SafeProxy } = global;
+  const { Proxy: SafeProxy } = global;
   const { bind } = SafeProxy;
   const MESSAGE = 'message';
   const STACK = 'stack';
@@ -32,7 +32,8 @@ if (!IS_FIREFOX && !browser?.runtime) {
     target[key] = res;
     return res;
   };
-  const proxifyGroup = (src, meta) => new SafeProxy({}, {
+  const proxifyGroup = (src, meta) => new SafeProxy({ __proto__: null }, {
+    __proto__: null,
     get: (group, key) => group[key] ?? proxifyValue(group, key, src, meta?.[key]),
   });
   /**
@@ -86,50 +87,33 @@ if (!IS_FIREFOX && !browser?.runtime) {
       return promise;
     }
   );
-  // Both result and error must be explicitly specified to avoid prototype eavesdropping
-  const wrapSuccess = result => [
-    result,
-    null,
-  ];
-  // Both result and error must be explicitly specified to avoid prototype eavesdropping
-  const wrapError = err => process.env.DEBUG && console.warn(err) || [
-    null,
-    err?.[MESSAGE]
-      ? [err[MESSAGE], err[STACK]]
-      : [err, new SafeError()[STACK]],
-  ];
-  const sendResponseAsync = async (result, sendResponse) => {
+  const sendResponseAsync = async (listener, message, sender, sendResponse) => {
+    let result;
+    let error;
     try {
-      result = await result;
+      result = await listener(message, sender);
       if (process.env.DEBUG) console.info('send', result);
-      sendResponse(wrapSuccess(result));
     } catch (err) {
-      sendResponse(wrapError(err));
+      if (process.env.DEBUG) console.warn(err);
+      error = err?.[MESSAGE]
+        ? [err[MESSAGE], err[STACK]]
+        : [err, new SafeError()[STACK]];
     }
+    // `undefined` in arrays is received as `null` in Chrome, but we always use `== null` so it's ok
+    sendResponse([result, error]);
   };
   const onMessageListener = (listener, message, sender, sendResponse) => {
     if (process.env.DEBUG) console.info('receive', message);
-    try {
-      const result = listener(message, sender);
-      if (result && isFunction(result.then)) {
-        sendResponseAsync(result, sendResponse);
-        return true;
-      }
-      // In some browsers (e.g Chrome 56, Vivaldi), the listener in
-      // popup pages are not properly cleared after closed.
-      // They may send `undefined` before the real response is sent.
-      if (result !== undefined) {
-        sendResponse(wrapSuccess(result));
-      }
-    } catch (err) {
-      sendResponse(wrapError(err));
-    }
+    // TODO: use a port with a timeout for each tab/frame to speed up frequent calls to GM API?
+    // Always using async mode because the difference in performance is negligible.
+    sendResponseAsync(listener, message, sender, sendResponse);
+    return true;
   };
   /** @type {WrapAsyncPreprocessorFunc} */
   const unwrapResponse = (resolve, response) => (
     !response && 'null response'
-    || response[1] // error created in wrapError
-    || resolve(response[0]) // result created in wrapSuccess
+    || response[1] // error from sendResponseAsync
+    || resolve(response[0]) // result from sendResponseAsync
   );
   const wrapSendMessage = (runtime, sendMessage) => (
     wrapAsync(runtime, sendMessage, unwrapResponse)
@@ -152,7 +136,7 @@ if (!IS_FIREFOX && !browser?.runtime) {
       },
       sendMessage: wrapSendMessage,
     },
-    tabs: {
+    tabs: !process.env.IS_INJECTED && {
       connect: 0,
       sendMessage: wrapSendMessage,
     },

src/common/safe-globals-shared.js

@@ -7,7 +7,7 @@
  */
 
 const global = (function _() {
-  return this || globalThis; // eslint-disable-line no-undef
+  return process.env.TEST ? globalThis : this; // eslint-disable-line no-undef
 }());
 /** These two are unforgeable so we extract them primarily to improve minification.
  * The document's value can change only in about:blank but we don't inject there. */

src/common/safe-globals.js

@@ -12,6 +12,7 @@ const {
   Error,
   Object,
   Promise,
+  chrome,
   performance,
 } = global;
 export const SafePromise = Promise; // alias used by browser.js
@@ -19,10 +20,10 @@ export const SafeError = Error; // alias used by browser.js
 export const { apply: safeApply } = Reflect;
 export const hasOwnProperty = safeApply.call.bind(({}).hasOwnProperty);
 export const safeCall = Object.call.bind(Object.call);
-export const IS_FIREFOX = !global.chrome.app;
+export const IS_FIREFOX = !chrome.app;
 export const ROUTE_SCRIPTS = `#scripts`;
-export const extensionRoot = global.chrome.runtime.getURL('/');
+export const extensionRoot = chrome.runtime.getURL('/');
 export const extensionOrigin = extensionRoot.slice(0, -1);
-export const extensionManifest = global.chrome.runtime.getManifest();
+export const extensionManifest = chrome.runtime.getManifest();
 export const extensionOptionsPage = extensionRoot + extensionManifest.options_page;
-export const ICON_PREFIX = `${extensionRoot}public/images/icon`;
+export const ICON_PREFIX = extensionRoot + extensionManifest.icons[16].split('16')[0];

src/injected/content/safe-globals.js

@@ -17,6 +17,7 @@ export const {
   atob: safeAtob,
   addEventListener: on,
   cloneInto,
+  chrome,
   dispatchEvent: fire,
   removeEventListener: off,
 } = global;
@@ -46,7 +47,7 @@ export const { stopImmediatePropagation } = Event[PROTO];
 export const getDetail = describeProperty(SafeCustomEvent[PROTO], 'detail').get;
 export const getRelatedTarget = describeProperty(SafeMouseEvent[PROTO], 'relatedTarget').get;
 export const logging = nullObjFrom(console);
-export const VM_UUID = global.chrome.runtime.getURL('');
+export const VM_UUID = chrome.runtime.getURL('');
 /** Unlike the built-in `instanceof` operator this doesn't call @@hasInstance which may be spoofed */
 export const isInstance = (instance, safeOriginalProto) => {
   for (let obj = instance; isObject(obj) && (obj = getPrototypeOf(obj));) {
@@ -56,4 +57,4 @@ export const isInstance = (instance, safeOriginalProto) => {
   }
 };
 export const isPromise = (proto => val => isInstance(val, proto))(SafePromise[PROTO]);
-export let IS_FIREFOX = !global.chrome.app;
+export let IS_FIREFOX = !chrome.app;

src/options/views/edit/index.vue

@@ -131,7 +131,7 @@ const savePosition = async wnd => {
 /** @param {chrome.windows.Window} _ */
 const setupSavePosition = ({ id: curWndId, tabs }) => {
   if (tabs.length === 1) {
-    const { onBoundsChanged } = global.chrome.windows;
+    const { onBoundsChanged } = chrome.windows;
     if (onBoundsChanged) {
       // triggered on moving/resizing, Chrome 86+
       onBoundsChanged.addListener(wnd => {

src/options/views/tab-settings/index.vue

@@ -241,7 +241,7 @@ export default {
   },
   computed: {
     editorWindowHint() {
-      return global.chrome.windows?.onBoundsChanged ? null : this.i18n('optionEditorWindowHint');
+      return chrome.windows?.onBoundsChanged ? null : this.i18n('optionEditorWindowHint');
     },
     isCustomBadgeColor() {
       return badgeColorNames.some(name => settings[name] !== optionsDefaults[name]);

test/mock/polyfill.js

@@ -16,7 +16,9 @@ global.browser = {
   },
   runtime: {
     getURL: path => path,
-    getManifest: () => ({}),
+    getManifest: () => ({
+      icons: { 16: '' },
+    }),
   },
 };
 if (!window.Response) window.Response = { prototype: {} };