#579: Deduplicate injection and make it more straightforward

package.json

@@ -218,7 +218,7 @@
       "^.+\\.txt$": "jest-raw-loader"
     },
     "transformIgnorePatterns": [
-      "node_modules/(?!@cfworker|webext-detect-page|idb|webext-patterns|webext-polyfill-kinda)"
+      "node_modules/(?!@cfworker|webext-detect-page|idb|webext-patterns|webext-polyfill-kinda|webext-additional-permissions)"
     ],
     "setupFiles": [
       "<rootDir>/src/test-env.js",

src/background/browserAction.ts

@@ -18,10 +18,10 @@
 import { isBackgroundPage } from "webext-detect-page";
 import { toggleActionPanel } from "@/contentScript/browserAction";
 import { reportError } from "@/telemetry/logging";
-import { injectContentScript } from "@/background/util";
 import { browser, Runtime } from "webextension-polyfill-ts";
 import { allowSender } from "@/actionPanel/protocol";
-import { isErrorObject, sleep } from "@/utils";
+import { sleep } from "@/utils";
+import { isPrivatePageError } from "./util";
 
 export const MESSAGE_PREFIX = "@@pixiebrix/background/browserAction/";
 
@@ -49,31 +49,21 @@ async function handleBrowserAction(tab: chrome.tabs.Tab): Promise<void> {
   tabFrames.delete(tab.id);
 
   try {
-    await injectContentScript({ tabId: tab.id, frameId: 0 });
     const nonce = await toggleActionPanel({ tabId: tab.id, frameId: 0 });
     tabNonces.set(tab.id, nonce);
   } catch (error: unknown) {
-    if (isErrorObject(error)) {
-      // Example error messages:
-      // Cannot access a chrome:// URL
-      // Cannot access a chrome-extension:// URL of different extension
-      // Cannot access contents of url "chrome-extension://mpjjildhmpddojocokjkgmlkkkfjnepo/options.html#/". Extension manifest must request permission to access this host.
-      // The extensions gallery cannot be scripted.
-      if (
-        /cannot be scripted|(chrome|about|extension):\/\//.test(error.message)
-      ) {
-        await showErrorInOptions(
-          "ERR_BROWSER_ACTION_TOGGLE_SPECIAL_PAGE",
-          tab.index
-        );
-        return;
-      }
-
-      // Firefox does not catch injection errors so we don't get a specific error message
-      // https://github.com/pixiebrix/pixiebrix-extension/issues/579#issuecomment-866451242
-      await showErrorInOptions("ERR_BROWSER_ACTION_TOGGLE", tab.index);
+    if (isPrivatePageError(error)) {
+      void showErrorInOptions(
+        "ERR_BROWSER_ACTION_TOGGLE_SPECIAL_PAGE",
+        tab.index
+      );
+      return;
     }
 
+    // Firefox does not catch injection errors so we don't get a specific error message
+    // https://github.com/pixiebrix/pixiebrix-extension/issues/579#issuecomment-866451242
+    await showErrorInOptions("ERR_BROWSER_ACTION_TOGGLE", tab.index);
+
     // Only report unknown-reason errors
     reportError(error);
   }

src/background/contextMenus.ts

@@ -21,7 +21,6 @@ import { isBackgroundPage } from "webext-detect-page";
 import { reportError } from "@/telemetry/logging";
 import { handleMenuAction } from "@/contentScript/contextMenus";
 import { showNotification } from "@/contentScript/notify";
-import { injectContentScript, waitReady } from "@/background/util";
 import { reportEvent } from "@/telemetry/events";
 import { hasCancelRootCause } from "@/errors";
 import { getErrorMessage } from "@/extensionPoints/helpers";
@@ -32,7 +31,6 @@ type MenuItemId = number | string;
 const extensionMenuItems = new Map<ExtensionId, MenuItemId>();
 
 const MENU_PREFIX = "pixiebrix-";
-const CONTEXT_SCRIPT_INSTALL_MS = 1000;
 const CONTEXT_MENU_INSTALL_MS = 1000;
 
 interface SelectionMenuOptions {
@@ -58,10 +56,6 @@ async function dispatchMenu(
     throw new TypeError(`Not a PixieBrix menu item: ${info.menuItemId}`);
   }
 
-  // Using the context menu gives temporary access to the page
-  await injectContentScript(target);
-  await waitReady(target, { maxWaitMillis: CONTEXT_SCRIPT_INSTALL_MS });
-
   try {
     await handleMenuAction(target, {
       extensionId: info.menuItemId.slice(MENU_PREFIX.length),

src/background/devtools/contract.ts

@@ -43,6 +43,7 @@ export interface BackgroundResponse {
 export type Target = {
   tabId: number;
   frameId: number;
+  url: string;
 };
 
 export interface HandlerEntry {
@@ -57,4 +58,5 @@ export interface Meta {
   nonce: Nonce;
   tabId: TabId;
   frameId: number;
+  url: string;
 }

src/background/devtools/internal.ts

@@ -38,7 +38,7 @@ import { reportError } from "@/telemetry/logging";
 import { isBackgroundPage } from "webext-detect-page";
 import { v4 as uuidv4 } from "uuid";
 import { callBackground } from "@/background/devtools/external";
-import { testTabPermissions, injectContentScript } from "@/background/util";
+import { injectContentScript } from "@/background/util";
 import * as nativeEditorProtocol from "@/nativeEditor";
 import { reactivate } from "@/background/navigation";
 
@@ -76,7 +76,7 @@ function backgroundMessageListener(
     const handlerPromise = new Promise((resolve) => {
       resolve(
         handler(
-          { tabId: meta.tabId, frameId: meta.frameId ?? 0 },
+          { tabId: meta.tabId, frameId: meta.frameId ?? 0, url: meta.url },
           port
         )(...payload)
       );
@@ -271,16 +271,18 @@ export function registerPort(tabId: TabId, port: Runtime.Port): void {
 async function injectTemporaryAccess({
   tabId,
   frameId,
+  url,
 }: WebNavigation.OnDOMContentLoadedDetailsType): Promise<void> {
-  if (connections.has(tabId)) {
-    const hasPermissions = await testTabPermissions({ tabId, frameId });
-    if (hasPermissions) {
-      await injectContentScript({ tabId, frameId });
-    } else {
-      console.debug(
-        `Skipping injectDevtoolsContentScript because no activeTab permissions for tab: ${tabId}`
-      );
-    }
+  if (!connections.has(tabId)) {
+    return;
+  }
+
+  try {
+    await injectContentScript({ tabId, frameId, url });
+  } catch (error: unknown) {
+    console.warn(`injectDevtoolsContentScript failed on tab ${tabId}`, {
+      error,
+    });
   }
 }
 

src/background/devtools/protocol.ts

@@ -60,8 +60,8 @@ export const getTabInfo = liftBackground(
 
 export const injectScript = liftBackground(
   "INJECT_SCRIPT",
-  (target: Target) => async ({ file }: { file: string }) => {
-    return injectContentScript(target, file);
+  (target: Target) => async () => {
+    return injectContentScript(target);
   }
 );
 

src/background/util.ts

@@ -17,12 +17,15 @@
 
 import { isBackgroundPage } from "webext-detect-page";
 import { browser } from "webextension-polyfill-ts";
+import { getAdditionalPermissions } from "webext-additional-permissions";
+import { patternToRegex } from "webext-patterns";
 import * as contentScriptProtocol from "@/contentScript/devTools";
-import { sleep } from "@/utils";
+import { isErrorObject, sleep } from "@/utils";
 
 export type Target = {
   tabId: number;
   frameId: number;
+  url: string;
 };
 
 export async function testTabPermissions(target: Target): Promise<boolean> {
@@ -41,11 +44,9 @@ export async function testTabPermissions(target: Target): Promise<boolean> {
       runAt: "document_start",
     });
     return true;
-  } catch (error) {
-    if ((error.message as string)?.includes("Cannot access contents")) {
-      // no permissions
-    } else {
-      console.warn("testTabPermissions failed", { reason: error });
+  } catch (error: unknown) {
+    if (!isErrorObject(error) || !/Cannot access/.test(error.message)) {
+      console.warn("testTabPermissions failed", { error });
     }
   }
   return false;
@@ -66,36 +67,68 @@ export async function waitReady(
   throw new Error(`contentScript not ready in ${maxWaitMillis}ms`);
 }
 
+/** Checks whether a URL has permanent permissions and therefore whether `webext-dynamic-content-scripts` already registered the scripts */
+export async function isContentScriptRegistered(url: string): Promise<boolean> {
+  const { permissions } = await getAdditionalPermissions({
+    strictOrigins: false,
+  });
+
+  return patternToRegex(...permissions).test(url);
+}
+
+/** Checks whether a tab has the activeTab permission to inject scripts and CSS */
+export async function isActiveTab(
+  tabId: number,
+  frameId = 0
+): Promise<boolean> {
+  return browser.tabs.insertCSS(tabId, { code: "@media {}", frameId }).then(
+    () => true,
+    () => false
+  );
+}
+
 /**
  * Inject a contentScript into a page if the contentScript is not already available on the page
  * @param target the tab frame to inject the contentScript into
  * @param file the contentScript file
  * @return true if the content script was injected
  */
-export async function injectContentScript(
-  target: Target,
-  file = "contentScript.js"
-): Promise<boolean> {
+export async function injectContentScript(target: Target): Promise<boolean> {
   if (!isBackgroundPage()) {
     throw new Error(
       "injectContentScript can only be called from the background page"
     );
   }
 
+  // Already has permanent access
+  if (await isContentScriptRegistered(target.url)) {
+    return;
+  }
+
+  // Has temporary access
+  if (!(await isActiveTab(target.tabId, target.frameId))) {
+    console.debug(
+      `Skipping content script injection because no activeTab permissions for tab: ${target.tabId}`
+    );
+    return;
+  }
+
   const { installed } = await contentScriptProtocol.isInstalled(target);
 
   if (!installed) {
     console.debug(
       `Injecting devtools contentScript for tab ${target.tabId}, frame ${
         target.frameId ?? 0
-      }: ${file}`
+      }`
     );
 
     // inject in the top-level frame
+    console.log("injectContentScript");
+
     await browser.tabs.executeScript(target.tabId, {
       frameId: target.frameId ?? 0,
       allFrames: false,
-      file,
+      file: "contentScript.js",
       runAt: "document_end",
     });
 
@@ -109,3 +142,22 @@ export async function injectContentScript(
     return false;
   }
 }
+
+/**
+ * Some pages are off-limits to extension. This function can find out if an error is due to this limitation.
+ *
+ * Example error messages:
+ * Cannot access a chrome:// URL
+ * Cannot access a chrome-extension:// URL of different extension
+ * Cannot access contents of url "chrome-extension://mpjjildhmpddojocokjkgmlkkkfjnepo/options.html#/". Extension manifest must request permission to access this host.
+ * The extensions gallery cannot be scripted.
+ *
+ * @param error
+ * @returns
+ */
+export async function isPrivatePageError(error: unknown): Promise<boolean> {
+  return (
+    isErrorObject(error) &&
+    /cannot be scripted|(chrome|about|extension):\/\//.test(error.message)
+  );
+}

src/devTools/context.ts

@@ -132,7 +132,7 @@ async function connectToFrame(port: Runtime.Port): Promise<FrameMeta> {
     throw new PermissionsError(`No access to URL: ${url}`);
   }
   console.debug(`connectToFrame: ensuring contentScript for ${url}`);
-  await injectScript(port, { file: "contentScript.js" });
+  await injectScript(port);
 
   console.debug(
     `connectToFrame: waiting for contentScript to be ready for ${url}`

src/devtools.ts

@@ -90,7 +90,7 @@ async function initialize() {
   let injected = false;
 
   try {
-    await injectScript(port, { file: "contentScript.js" });
+    await injectScript(port);
     injected = true;
   } catch (error) {
     // Can install without having content script on the page; they just won't do much