From 327dcc3d53b7536817e751db7811c7710c9539fa Mon Sep 17 00:00:00 2001 From: Shane Caraveo Date: Wed, 29 Apr 2020 23:28:27 +0000 Subject: [PATCH] Bug 1606862 support devtools as an optional extension permission r=rpl Differential Revision: https://phabricator.services.mozilla.com/D71829 --- .../components/extensions/ext-browser.json | 2 +- .../extensions/parent/ext-devtools.js | 48 +++++++- .../extensions/schemas/devtools.json | 2 +- .../extensions/test/browser/browser.ini | 1 + .../browser/browser_ext_devtools_optional.js | 114 ++++++++++++++++++ .../extensions/test/browser/head_devtools.js | 18 ++- toolkit/components/extensions/Extension.jsm | 17 ++- .../test/xpcshell/test_ext_permissions_api.js | 1 + 8 files changed, 194 insertions(+), 9 deletions(-) create mode 100644 browser/components/extensions/test/browser/browser_ext_devtools_optional.js diff --git a/browser/components/extensions/ext-browser.json b/browser/components/extensions/ext-browser.json index df51ec8b6d66..7dab7d45cc13 100644 --- a/browser/components/extensions/ext-browser.json +++ b/browser/components/extensions/ext-browser.json @@ -54,7 +54,7 @@ "url": "chrome://browser/content/parent/ext-devtools.js", "schema": "chrome://browser/content/schemas/devtools.json", "scopes": ["devtools_parent"], - "events": ["uninstall"], + "events": ["uninstall", "startup"], "manifest": ["devtools_page"], "paths": [ ["devtools"] diff --git a/browser/components/extensions/parent/ext-devtools.js b/browser/components/extensions/parent/ext-devtools.js index f4624fc7eb14..f7b3dd2c4cf5 100644 --- a/browser/components/extensions/parent/ext-devtools.js +++ b/browser/components/extensions/parent/ext-devtools.js @@ -29,9 +29,7 @@ function getDevToolsPrefBranchName(extensionId) { } /** - * Retrieve the devtools target for the devtools extension proxy context - * (lazily cloned from the target of the toolbox associated to the context - * the first time that it is accessed). + * Retrieve the tabId for the given devtools toolbox. * * @param {Toolbox} toolbox * A devtools toolbox instance. @@ -257,6 +255,7 @@ class DevToolsPageDefinition { if (this.devtoolsPageForToolbox.size === 0) { DevToolsShim.off("theme-changed", this.onThemeChanged); } + this.extension.emit("devtools-page-shutdown", toolbox); } } @@ -311,11 +310,29 @@ this.devtools = class extends ExtensionAPI { constructor(extension) { super(extension); + this._initialized = false; + // DevToolsPageDefinition instance (created in onManifestEntry). this.pageDefinition = null; this.onToolboxCreated = this.onToolboxCreated.bind(this); this.onToolboxDestroy = this.onToolboxDestroy.bind(this); + + /* eslint-disable mozilla/balanced-listeners */ + extension.on("add-permissions", (ignoreEvent, permissions) => { + if (permissions.permissions.includes("devtools")) { + this._initialize(); + } + }); + extension.on("remove-permissions", (ignoreEvent, permissions) => { + Services.prefs.setBoolPref( + `${getDevToolsPrefBranchName(extension.id)}.enabled`, + false + ); + if (permissions.permissions.includes("devtools")) { + this._uninitialize(); + } + }); } static onUninstall(extensionId) { @@ -327,9 +344,13 @@ this.devtools = class extends ExtensionAPI { prefBranch.deleteBranch(""); } - onManifestEntry(entryName) { + _initialize() { const { extension } = this; + if (!extension.hasPermission("devtools") || this._initialized) { + return; + } + this.initDevToolsPref(); // Create the devtools_page definition. @@ -346,9 +367,17 @@ this.devtools = class extends ExtensionAPI { DevToolsShim.on("toolbox-created", this.onToolboxCreated); DevToolsShim.on("toolbox-destroy", this.onToolboxDestroy); + this._initialized = true; } - onShutdown() { + _uninitialize() { + // devtoolsPrefBranch is set in onManifestEntry, and nullified + // later in onShutdown. If it isn't set, then onManifestEntry + // did not initialize devtools for the extension. + if (!this._initialized) { + return; + } + DevToolsShim.off("toolbox-created", this.onToolboxCreated); DevToolsShim.off("toolbox-destroy", this.onToolboxDestroy); @@ -362,6 +391,15 @@ this.devtools = class extends ExtensionAPI { } this.uninitDevToolsPref(); + this._initialized = false; + } + + onStartup() { + this._initialize(); + } + + onShutdown() { + this._uninitialize(); } getAPI(context) { diff --git a/browser/components/extensions/schemas/devtools.json b/browser/components/extensions/schemas/devtools.json index 960959cd4d7f..7f0b81bad050 100644 --- a/browser/components/extensions/schemas/devtools.json +++ b/browser/components/extensions/schemas/devtools.json @@ -12,7 +12,7 @@ } }, { - "$extend": "Permission", + "$extend": "OptionalPermission", "choices": [{ "type": "string", "enum": [ diff --git a/browser/components/extensions/test/browser/browser.ini b/browser/components/extensions/test/browser/browser.ini index 1aa70056cd91..6fcbd1b17c64 100644 --- a/browser/components/extensions/test/browser/browser.ini +++ b/browser/components/extensions/test/browser/browser.ini @@ -119,6 +119,7 @@ support-files = !/browser/components/places/tests/browser/head.js [browser_ext_devtools_inspectedWindow_targetSwitch.js] [browser_ext_devtools_network.js] skip-if = fission || os == 'linux' || (os == 'mac' && debug) || (debug && os == 'win' && bits == 64) # Bug1570478 +[browser_ext_devtools_optional.js] [browser_ext_devtools_page.js] [browser_ext_devtools_page_incognito.js] [browser_ext_devtools_panel.js] diff --git a/browser/components/extensions/test/browser/browser_ext_devtools_optional.js b/browser/components/extensions/test/browser/browser_ext_devtools_optional.js new file mode 100644 index 000000000000..6986ec85fcab --- /dev/null +++ b/browser/components/extensions/test/browser/browser_ext_devtools_optional.js @@ -0,0 +1,114 @@ +/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */ +/* vim: set sts=2 sw=2 et tw=80: */ +"use strict"; + +loadTestSubscript("head_devtools.js"); + +/** + * This test file ensures that: + * + * - the devtools_page property creates a new WebExtensions context + * - the devtools_page can exchange messages with the background page + */ +add_task(async function test_devtools_page_runtime_api_messaging() { + Services.prefs.setBoolPref( + "extensions.webextOptionalPermissionPrompts", + false + ); + registerCleanupFunction(() => { + Services.prefs.clearUserPref("extensions.webextOptionalPermissionPrompts"); + }); + + let tab = await BrowserTestUtils.openNewForegroundTab( + gBrowser, + "http://mochi.test:8888/" + ); + + function background() { + let perm = { permissions: ["devtools"], origins: [] }; + browser.test.onMessage.addListener(async (msg, sender) => { + if (msg === "request") { + let granted = await new Promise(resolve => { + browser.test.withHandlingUserInput(() => { + resolve(browser.permissions.request(perm)); + }); + }); + browser.test.assertTrue(granted, "permission request succeeded"); + browser.test.sendMessage("done"); + } else if (msg === "revoke") { + browser.permissions.remove(perm); + browser.test.sendMessage("done"); + } + }); + } + + function devtools_page() { + browser.test.sendMessage("devtools_page_loaded"); + } + + let extension = ExtensionTestUtils.loadExtension({ + background, + manifest: { + optional_permissions: ["devtools"], + devtools_page: "devtools_page.html", + }, + files: { + "devtools_page.html": ` + + + + + + + + `, + "devtools_page.js": devtools_page, + }, + }); + + await extension.startup(); + + function checkEnabled(expect = false) { + Assert.equal( + expect, + Services.prefs.getBoolPref( + `devtools.webextensions.${extension.id}.enabled`, + false + ), + "devtools enabled pref is correct" + ); + } + + checkEnabled(false); + + // Open the devtools first, then request permission + info("Open the developer toolbox"); + await openToolboxForTab(tab); + assertDevToolsExtensionEnabled(extension.uuid, false); + + extension.sendMessage("request"); + await extension.awaitMessage("done"); + checkEnabled(true); + + info("Wait the devtools page load"); + await extension.awaitMessage("devtools_page_loaded"); + assertDevToolsExtensionEnabled(extension.uuid, true); + + let policy = WebExtensionPolicy.getByID(extension.id); + let closed = new Promise(resolve => { + // eslint-disable-next-line mozilla/balanced-listeners + policy.extension.on("devtools-page-shutdown", resolve); + }); + + extension.sendMessage("revoke"); + await extension.awaitMessage("done"); + + await closed; + checkEnabled(false); + assertDevToolsExtensionEnabled(extension.uuid, false); + + await extension.unload(); + + await closeToolboxForTab(tab); + BrowserTestUtils.removeTab(tab); +}); diff --git a/browser/components/extensions/test/browser/head_devtools.js b/browser/components/extensions/test/browser/head_devtools.js index c50d1372640b..7b4d602d4cfb 100644 --- a/browser/components/extensions/test/browser/head_devtools.js +++ b/browser/components/extensions/test/browser/head_devtools.js @@ -3,7 +3,7 @@ "use strict"; /* exported openToolboxForTab, closeToolboxForTab, getToolboxTargetForTab, - registerBlankToolboxPanel, TOOLBOX_BLANK_PANEL_ID */ + registerBlankToolboxPanel, TOOLBOX_BLANK_PANEL_ID, assertDevToolsExtensionEnabled */ ChromeUtils.defineModuleGetter( this, @@ -19,6 +19,12 @@ XPCOMUtils.defineLazyGetter(this, "TargetFactory", () => { return TargetFactory; }); +ChromeUtils.defineModuleGetter( + this, + "DevToolsShim", + "chrome://devtools-startup/content/DevToolsShim.jsm" +); + const TOOLBOX_BLANK_PANEL_ID = "testBlankPanel"; // Register a blank custom tool so that we don't need to wait the webconsole @@ -86,3 +92,13 @@ async function closeToolboxForTab(tab) { })}` ); } + +function assertDevToolsExtensionEnabled(uuid, enabled) { + for (let toolbox of DevToolsShim.getToolboxes()) { + is( + enabled, + !!toolbox.isWebExtensionEnabled(uuid), + `extension is ${enabled ? "enabled" : "disabled"} on toolbox` + ); + } +} diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index 51c64f55a934..97cd0bcd3615 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -635,7 +635,10 @@ class ExtensionData { this.manifest.permissions ); - if (this.manifest.devtools_page) { + if ( + this.manifest.devtools_page && + !this.manifest.optional_permissions.includes("devtools") + ) { permissions.add("devtools"); } @@ -2435,6 +2438,18 @@ class Extension extends ExtensionData { } } + // Ensure devtools permission is set + if ( + this.manifest.devtools_page && + !this.manifest.optional_permissions.includes("devtools") + ) { + ExtensionPermissions.add(this.id, { + permissions: ["devtools"], + origins: [], + }); + this.permissions.add("devtools"); + } + GlobalManager.init(this); this.initSharedData(); diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_permissions_api.js b/toolkit/components/extensions/test/xpcshell/test_ext_permissions_api.js index 9239c5761da7..26d41f76bab1 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_permissions_api.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_permissions_api.js @@ -50,6 +50,7 @@ add_task(async function setup() { "activeTab", "clipboardRead", "clipboardWrite", + "devtools", "downloads.open", "geolocation", "management",