From 621bf1a656c65781509e740b9f1c0ff54503a410 Mon Sep 17 00:00:00 2001 From: Cristian Tuns Date: Wed, 22 May 2024 09:02:19 -0400 Subject: [PATCH] Backed out 8 changesets (bug 1651518, bug 1883847) dt failures in nsTArray.h. CLOSED TREE Backed out changeset 481558275c1d (bug 1651518) Backed out changeset 5337435e8bca (bug 1651518) Backed out changeset 8fe0c1a4da7f (bug 1651518) Backed out changeset 840a0d9db260 (bug 1651518) Backed out changeset eaab9de5819d (bug 1651518) Backed out changeset cf71de92d782 (bug 1651518) Backed out changeset d778691464db (bug 1651518) Backed out changeset af28e5f719e8 (bug 1883847) --- .../mochitest/browser_dbg-es-module-worker.js | 3 +- ...rowser_dbg-features-source-text-content.js | 2 +- .../browser_dbg-features-source-tree.js | 2 +- .../browser-toolbox/Launcher.sys.mjs | 11 ++-- .../framework/browser-toolbox/window.js | 3 +- .../client/framework/test/allocations/head.js | 3 +- devtools/client/fronts/targets/worker.js | 6 -- devtools/client/shared/test/shared-head.js | 6 +- devtools/client/webconsole/actions/filters.js | 10 +-- .../enhancers/css-error-reporting.js | 43 +++++++++++++ .../client/webconsole/enhancers/moz.build | 1 + devtools/client/webconsole/store.js | 2 + ...ser_console_evaluation_context_selector.js | 2 +- .../client/webconsole/test/node/helpers.js | 2 +- .../test/node/store/filters.test.js | 8 +-- devtools/client/webconsole/webconsole-ui.js | 64 ++++++++----------- .../client/webconsole/webconsole-wrapper.js | 16 ----- devtools/server/actors/resources/index.js | 2 - devtools/server/actors/watcher.js | 13 +++- .../ParentProcessWatcherRegistry.sys.mjs | 32 ++++++++-- .../server/actors/watcher/session-context.js | 8 +-- .../ContentProcessWatcherRegistry.sys.mjs | 24 ++----- .../DevToolsProcessChild.sys.mjs | 29 ++------- .../target-watchers/moz.build | 1 - .../target-watchers/shared_worker.sys.mjs | 13 ---- .../target-watchers/worker.sys.mjs | 44 ++++++------- devtools/server/devtools-server.js | 6 -- .../server/startup/content-process.sys.mjs | 3 +- devtools/server/startup/frame.js | 6 +- devtools/server/tests/xpcshell/head_dbg.js | 3 +- .../commands/resource/resource-command.js | 4 -- ...rowser_resources_watch_unwatch_multiple.js | 7 +- devtools/shared/loader/Loader.sys.mjs | 11 +--- .../xpcshell/test_devtools_socket_status.js | 3 +- .../tests/xpcshell/test_invisible_loader.js | 3 +- devtools/shared/tests/xpcshell/test_loader.js | 3 +- devtools/startup/DevToolsStartup.sys.mjs | 3 +- 37 files changed, 179 insertions(+), 223 deletions(-) create mode 100644 devtools/client/webconsole/enhancers/css-error-reporting.js delete mode 100644 devtools/server/connectors/js-process-actor/target-watchers/shared_worker.sys.mjs diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-es-module-worker.js b/devtools/client/debugger/test/mochitest/browser_dbg-es-module-worker.js index ee5f055b3a86..f9b299991f8d 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-es-module-worker.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-es-module-worker.js @@ -38,8 +38,7 @@ add_task(async function () { await waitForThreadCount(dbg, 1); const threads = dbg.selectors.getThreads(); is(threads.length, 1, "Got the page and the worker threads"); - is(threads[0].name, "worker.js", "Thread name is correct"); - is(threads[0].url, WORKER_URL, "Thread URL is correct"); + is(threads[0].name, WORKER_URL, "Thread name is correct"); const source = await waitForSource(dbg, "worker.js"); await selectSource(dbg, source); diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-features-source-text-content.js b/devtools/client/debugger/test/mochitest/browser_dbg-features-source-text-content.js index 5cb343df35a1..2b853f2c4f04 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-features-source-text-content.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-features-source-text-content.js @@ -317,7 +317,7 @@ add_task(async function testSourceTextContent() { const workerThread = dbg.selectors .getAllThreads() - .find(thread => thread.url == `${BASE_URL}same-url.js`); + .find(thread => thread.name == `${BASE_URL}same-url.js`); is( sourceActors.filter(actor => actor.thread == workerThread.actor).length, diff --git a/devtools/client/debugger/test/mochitest/browser_dbg-features-source-tree.js b/devtools/client/debugger/test/mochitest/browser_dbg-features-source-tree.js index 121e4a0284d3..4590f0c2b32f 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg-features-source-tree.js +++ b/devtools/client/debugger/test/mochitest/browser_dbg-features-source-tree.js @@ -341,7 +341,7 @@ add_task(async function testSourceTreeOnTheIntegrationTestPage() { const workerThread = dbg.selectors .getAllThreads() - .find(thread => thread.url == testServer.urlFor("same-url.sjs")); + .find(thread => thread.name == testServer.urlFor("same-url.sjs")); is( sourceActors.filter(actor => actor.thread == workerThread.actor).length, diff --git a/devtools/client/framework/browser-toolbox/Launcher.sys.mjs b/devtools/client/framework/browser-toolbox/Launcher.sys.mjs index 6e6c0b211e36..a619fbdff20f 100644 --- a/devtools/client/framework/browser-toolbox/Launcher.sys.mjs +++ b/devtools/client/framework/browser-toolbox/Launcher.sys.mjs @@ -12,16 +12,13 @@ const CHROME_DEBUGGER_PROFILE_NAME = "chrome_debugger_profile"; import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs"; import { require } from "resource://devtools/shared/loader/Loader.sys.mjs"; +import { + useDistinctSystemPrincipalLoader, + releaseDistinctSystemPrincipalLoader, +} from "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs"; import { Subprocess } from "resource://gre/modules/Subprocess.sys.mjs"; import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; -const { - useDistinctSystemPrincipalLoader, - releaseDistinctSystemPrincipalLoader, -} = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } -); const lazy = {}; ChromeUtils.defineESModuleGetters(lazy, { BackgroundTasksUtils: "resource://gre/modules/BackgroundTasksUtils.sys.mjs", diff --git a/devtools/client/framework/browser-toolbox/window.js b/devtools/client/framework/browser-toolbox/window.js index 98dc8a5717f0..d4d65af78d46 100644 --- a/devtools/client/framework/browser-toolbox/window.js +++ b/devtools/client/framework/browser-toolbox/window.js @@ -10,8 +10,7 @@ var { loader, require } = ChromeUtils.importESModule( var { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); // Require this module to setup core modules diff --git a/devtools/client/framework/test/allocations/head.js b/devtools/client/framework/test/allocations/head.js index 122d46048a04..a7e7f56a36fe 100644 --- a/devtools/client/framework/test/allocations/head.js +++ b/devtools/client/framework/test/allocations/head.js @@ -16,8 +16,7 @@ let tracker, releaseTrackerLoader; useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); const requester = {}; diff --git a/devtools/client/fronts/targets/worker.js b/devtools/client/fronts/targets/worker.js index 5aeab94c1e4f..769706577d2a 100644 --- a/devtools/client/fronts/targets/worker.js +++ b/devtools/client/fronts/targets/worker.js @@ -28,12 +28,6 @@ class WorkerTargetFront extends TargetMixin( get isServiceWorker() { return this._type === Ci.nsIWorkerDebugger.TYPE_SERVICE; } - - // Display file name instead of absolute URL in the context selector/threads panel - get name() { - return this._url.split("/").pop(); - } - form(json) { this.actorID = json.actor; diff --git a/devtools/client/shared/test/shared-head.js b/devtools/client/shared/test/shared-head.js index 7241a6675310..0657ede75e0b 100644 --- a/devtools/client/shared/test/shared-head.js +++ b/devtools/client/shared/test/shared-head.js @@ -32,8 +32,7 @@ if (DEBUG_ALLOCATIONS) { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); const requester = {}; const loader = useDistinctSystemPrincipalLoader(requester); @@ -79,8 +78,7 @@ if (DEBUG_STEP) { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); const requester = {}; const loader = useDistinctSystemPrincipalLoader(requester); diff --git a/devtools/client/webconsole/actions/filters.js b/devtools/client/webconsole/actions/filters.js index 7468f6741a02..108614ee9f74 100644 --- a/devtools/client/webconsole/actions/filters.js +++ b/devtools/client/webconsole/actions/filters.js @@ -24,18 +24,12 @@ function filterTextSet(text) { } function filterToggle(filter) { - return async ({ dispatch, getState, webConsoleUI, prefsService }) => { - // When enabling CSS Warning message, we have to start listening for it - let filterState = getAllFilters(getState()); - if (filter == FILTERS.CSS && !filterState[FILTERS.CSS]) { - await webConsoleUI.watchCssMessages(); - } - + return ({ dispatch, getState, prefsService }) => { dispatch({ type: FILTER_TOGGLE, filter, }); - filterState = getAllFilters(getState()); + const filterState = getAllFilters(getState()); prefsService.setBoolPref( PREFS.FILTER[filter.toUpperCase()], filterState[filter] diff --git a/devtools/client/webconsole/enhancers/css-error-reporting.js b/devtools/client/webconsole/enhancers/css-error-reporting.js new file mode 100644 index 000000000000..513552db4df8 --- /dev/null +++ b/devtools/client/webconsole/enhancers/css-error-reporting.js @@ -0,0 +1,43 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +const { + INITIALIZE, + FILTER_TOGGLE, + FILTERS, +} = require("resource://devtools/client/webconsole/constants.js"); + +/** + * This is responsible for ensuring that error reporting is enabled if the CSS + * filter is toggled on. + */ +function ensureCSSErrorReportingEnabled(webConsoleUI) { + let watchingCSSMessages = false; + return next => (reducer, initialState, enhancer) => { + function ensureErrorReportingEnhancer(state, action) { + state = reducer(state, action); + + // If we're already watching CSS messages, or if the CSS filter is disabled, + // we don't do anything. + if (!webConsoleUI || watchingCSSMessages || !state.filters.css) { + return state; + } + + const cssFilterToggled = + action.type == FILTER_TOGGLE && action.filter == FILTERS.CSS; + + if (cssFilterToggled || action.type == INITIALIZE) { + watchingCSSMessages = true; + webConsoleUI.watchCssMessages(); + } + + return state; + } + return next(ensureErrorReportingEnhancer, initialState, enhancer); + }; +} + +module.exports = ensureCSSErrorReportingEnabled; diff --git a/devtools/client/webconsole/enhancers/moz.build b/devtools/client/webconsole/enhancers/moz.build index 76cb4a3a055a..79bb7dd1910e 100644 --- a/devtools/client/webconsole/enhancers/moz.build +++ b/devtools/client/webconsole/enhancers/moz.build @@ -6,5 +6,6 @@ DevToolsModules( "actor-releaser.js", "batching.js", + "css-error-reporting.js", "message-cache-clearing.js", ) diff --git a/devtools/client/webconsole/store.js b/devtools/client/webconsole/store.js index 60c5261cfdad..029d46f439c4 100644 --- a/devtools/client/webconsole/store.js +++ b/devtools/client/webconsole/store.js @@ -46,6 +46,7 @@ const { // Enhancers const enableBatching = require("resource://devtools/client/webconsole/enhancers/batching.js"); const enableActorReleaser = require("resource://devtools/client/webconsole/enhancers/actor-releaser.js"); +const ensureCSSErrorReportingEnabled = require("resource://devtools/client/webconsole/enhancers/css-error-reporting.js"); const enableMessagesCacheClearing = require("resource://devtools/client/webconsole/enhancers/message-cache-clearing.js"); /** @@ -118,6 +119,7 @@ function configureStore(webConsoleUI, options = {}) { middleware, enableActorReleaser(webConsoleUI), enableMessagesCacheClearing(webConsoleUI), + ensureCSSErrorReportingEnabled(webConsoleUI), // ⚠️ Keep this one last so it will be executed before all the other ones. This is // needed so batched actions can be "unbatched" and handled in the other enhancers. enableBatching() diff --git a/devtools/client/webconsole/test/browser/browser_console_evaluation_context_selector.js b/devtools/client/webconsole/test/browser/browser_console_evaluation_context_selector.js index b9762be3dcbf..98faa3a92a82 100644 --- a/devtools/client/webconsole/test/browser/browser_console_evaluation_context_selector.js +++ b/devtools/client/webconsole/test/browser/browser_console_evaluation_context_selector.js @@ -139,7 +139,7 @@ add_task(async function () { }); checkContextSelectorMenuItemAt(hud, workerIndex, { label: workerFile, - tooltip: workerUrl, + tooltip: workerFile, checked: true, }); diff --git a/devtools/client/webconsole/test/node/helpers.js b/devtools/client/webconsole/test/node/helpers.js index eb876a387e6e..b9d8aa9c14c4 100644 --- a/devtools/client/webconsole/test/node/helpers.js +++ b/devtools/client/webconsole/test/node/helpers.js @@ -58,7 +58,7 @@ function setupStore( } const store = configureStore(webConsoleUI, { ...storeOptions, - thunkArgs: { toolbox: {}, webConsoleUI }, + thunkArgs: { toolbox: {} }, telemetry: new Telemetry(), }); diff --git a/devtools/client/webconsole/test/node/store/filters.test.js b/devtools/client/webconsole/test/node/store/filters.test.js index ca27196838ba..eddf59e5376a 100644 --- a/devtools/client/webconsole/test/node/store/filters.test.js +++ b/devtools/client/webconsole/test/node/store/filters.test.js @@ -100,7 +100,7 @@ describe("Filtering", () => { expect(messages.length).toEqual(numUnfilterableMessages + 5); }); - it("filters css messages", async () => { + it("filters css messages", () => { const message = stubPreparedMessages.get( "Unknown property ‘such-unknown-property’. Declaration dropped." ); @@ -109,7 +109,7 @@ describe("Filtering", () => { let messages = getVisibleMessages(store.getState()); expect(messages.length).toEqual(numUnfilterableMessages); - await store.dispatch(actions.filterToggle("css")); + store.dispatch(actions.filterToggle("css")); messages = getVisibleMessages(store.getState()); expect(messages.length).toEqual(numUnfilterableMessages + 1); }); @@ -251,12 +251,12 @@ describe("Filtering", () => { }); describe("Clear filters", () => { - it("clears all filters", async () => { + it("clears all filters", () => { const store = setupStore(); // Setup test case store.dispatch(actions.filterToggle(FILTERS.ERROR)); - await store.dispatch(actions.filterToggle(FILTERS.CSS)); + store.dispatch(actions.filterToggle(FILTERS.CSS)); store.dispatch(actions.filterToggle(FILTERS.NET)); store.dispatch(actions.filterToggle(FILTERS.NETXHR)); store.dispatch(actions.filterTextSet("foobar")); diff --git a/devtools/client/webconsole/webconsole-ui.js b/devtools/client/webconsole/webconsole-ui.js index dd40bc99f7da..b562752ccb64 100644 --- a/devtools/client/webconsole/webconsole-ui.js +++ b/devtools/client/webconsole/webconsole-ui.js @@ -17,10 +17,7 @@ const { getAdHocFrontOrPrimitiveGrip, } = require("resource://devtools/client/fronts/object.js"); -const { - PREFS, - FILTERS, -} = require("resource://devtools/client/webconsole/constants.js"); +const { PREFS } = require("resource://devtools/client/webconsole/constants.js"); const FirefoxDataProvider = require("resource://devtools/client/netmonitor/src/connector/firefox-data-provider.js"); @@ -182,11 +179,21 @@ class WebConsoleUI { }); const resourceCommand = this.hud.resourceCommand; - if (this._watchedResources) { - resourceCommand.unwatchResources(this._watchedResources, { - onAvailable: this._onResourceAvailable, - }); - } + resourceCommand.unwatchResources( + [ + resourceCommand.TYPES.CONSOLE_MESSAGE, + resourceCommand.TYPES.ERROR_MESSAGE, + resourceCommand.TYPES.PLATFORM_MESSAGE, + resourceCommand.TYPES.DOCUMENT_EVENT, + resourceCommand.TYPES.LAST_PRIVATE_CONTEXT_EXIT, + resourceCommand.TYPES.JSTRACER_TRACE, + resourceCommand.TYPES.JSTRACER_STATE, + ], + { onAvailable: this._onResourceAvailable } + ); + resourceCommand.unwatchResources([resourceCommand.TYPES.CSS_MESSAGE], { + onAvailable: this._onResourceAvailable, + }); this.stopWatchingNetworkResources(); @@ -325,26 +332,18 @@ class WebConsoleUI { onDestroyed: this._onTargetDestroyed, }); - this._watchedResources = [ - resourceCommand.TYPES.CONSOLE_MESSAGE, - resourceCommand.TYPES.ERROR_MESSAGE, - resourceCommand.TYPES.PLATFORM_MESSAGE, - resourceCommand.TYPES.DOCUMENT_EVENT, - resourceCommand.TYPES.LAST_PRIVATE_CONTEXT_EXIT, - resourceCommand.TYPES.JSTRACER_TRACE, - resourceCommand.TYPES.JSTRACER_STATE, - ]; - - // CSS Warnings are only enabled when the user explicitely requested to show them - // as it can slow down page load. - const shouldShowCssWarnings = this.wrapper.getFilterState(FILTERS.CSS); - if (shouldShowCssWarnings) { - this._watchedResources.push(resourceCommand.TYPES.CSS_MESSAGE); - } - - await resourceCommand.watchResources(this._watchedResources, { - onAvailable: this._onResourceAvailable, - }); + await resourceCommand.watchResources( + [ + resourceCommand.TYPES.CONSOLE_MESSAGE, + resourceCommand.TYPES.ERROR_MESSAGE, + resourceCommand.TYPES.PLATFORM_MESSAGE, + resourceCommand.TYPES.DOCUMENT_EVENT, + resourceCommand.TYPES.LAST_PRIVATE_CONTEXT_EXIT, + resourceCommand.TYPES.JSTRACER_TRACE, + resourceCommand.TYPES.JSTRACER_STATE, + ], + { onAvailable: this._onResourceAvailable } + ); if (this.isBrowserConsole || this.isBrowserToolboxConsole) { const shouldEnableNetworkMonitoring = Services.prefs.getBoolPref( @@ -452,18 +451,11 @@ class WebConsoleUI { this.wrapper.dispatchTabWillNavigate({ timeStamp, url }); } - /** - * Called when the CSS Warning filter is enabled, in order to start observing for them in the backend. - */ async watchCssMessages() { const { resourceCommand } = this.hud; - if (this._watchedResources.includes(resourceCommand.TYPES.CSS_MESSAGE)) { - return; - } await resourceCommand.watchResources([resourceCommand.TYPES.CSS_MESSAGE], { onAvailable: this._onResourceAvailable, }); - this._watchedResources.push(resourceCommand.TYPES.CSS_MESSAGE); } _onResourceAvailable(resources) { diff --git a/devtools/client/webconsole/webconsole-wrapper.js b/devtools/client/webconsole/webconsole-wrapper.js index dfd0649d6f45..acfc43c3cd04 100644 --- a/devtools/client/webconsole/webconsole-wrapper.js +++ b/devtools/client/webconsole/webconsole-wrapper.js @@ -31,9 +31,6 @@ const EventEmitter = require("resource://devtools/shared/event-emitter.js"); const App = createFactory( require("resource://devtools/client/webconsole/components/App.js") ); -const { - getAllFilters, -} = require("resource://devtools/client/webconsole/selectors/filters.js"); loader.lazyGetter(this, "AppErrorBoundary", () => createFactory( @@ -174,19 +171,6 @@ class WebConsoleWrapper { } } - /** - * Query the reducer store for the current state of filtering - * a given type of message - * - * @param {String} filter - * Type of message to be filtered. - * @return {Boolean} - * True if this type of message should be displayed. - */ - getFilterState(filter) { - return getAllFilters(this.getStore().getState())[filter]; - } - dispatchMessageAdd(packet) { this.batchedMessagesAdd([packet]); } diff --git a/devtools/server/actors/resources/index.js b/devtools/server/actors/resources/index.js index f988695d4643..cfc941a1615f 100644 --- a/devtools/server/actors/resources/index.js +++ b/devtools/server/actors/resources/index.js @@ -256,8 +256,6 @@ function getResourceTypeDictionaryForTargetType(targetType) { return WorkerTargetResources; case Targets.TYPES.SERVICE_WORKER: return WorkerTargetResources; - case Targets.TYPES.SHARED_WORKER: - return WorkerTargetResources; default: throw new Error(`Unsupported target actor typeName '${targetType}'`); } diff --git a/devtools/server/actors/watcher.js b/devtools/server/actors/watcher.js index f680b1fc556e..10de1022295c 100644 --- a/devtools/server/actors/watcher.js +++ b/devtools/server/actors/watcher.js @@ -279,7 +279,8 @@ exports.WatcherActor = class WatcherActor extends Actor { unwatchTargets(targetType, options = {}) { const isWatchingTargets = ParentProcessWatcherRegistry.unwatchTargets( this, - targetType + targetType, + options ); if (!isWatchingTargets) { return; @@ -293,6 +294,13 @@ exports.WatcherActor = class WatcherActor extends Actor { options, }); } + + // Unregister the JS Actors if there is no more DevTools code observing any target/resource, + // unless we're switching mode (having both condition at the same time should only + // happen in tests). + if (!options.isModeSwitching) { + ParentProcessWatcherRegistry.maybeUnregisterJSActors(); + } } /** @@ -653,6 +661,9 @@ exports.WatcherActor = class WatcherActor extends Actor { ); targetActor.removeSessionDataEntry("resources", targetActorResourceTypes); } + + // Unregister the JS Window Actor if there is no more DevTools code observing any target/resource + ParentProcessWatcherRegistry.maybeUnregisterJSActors(); } clearResources(resourceTypes) { diff --git a/devtools/server/actors/watcher/ParentProcessWatcherRegistry.sys.mjs b/devtools/server/actors/watcher/ParentProcessWatcherRegistry.sys.mjs index 6e591454ea71..e9b3a9d50d86 100644 --- a/devtools/server/actors/watcher/ParentProcessWatcherRegistry.sys.mjs +++ b/devtools/server/actors/watcher/ParentProcessWatcherRegistry.sys.mjs @@ -198,11 +198,14 @@ export const ParentProcessWatcherRegistry = { * The type of data to be removed * @param Array entries * The values to be removed to this type of data + * @params {Object} options + * @params {Boolean} options.isModeSwitching: Set to true true when this is called as the + * result of a change to the devtools.browsertoolbox.scope pref. * * @return boolean * True if we such entry was already registered, for this watcher actor. */ - removeSessionDataEntry(watcher, type, entries) { + removeSessionDataEntry(watcher, type, entries, options) { const sessionData = this.getSessionData(watcher); if (!sessionData) { return false; @@ -218,6 +221,19 @@ export const ParentProcessWatcherRegistry = { return false; } + const isWatchingSomething = SUPPORTED_DATA_TYPES.some( + dataType => sessionData[dataType] && !!sessionData[dataType].length + ); + + // Remove the watcher reference if it's not watching for anything anymore, unless we're + // doing a mode switch; in such case we don't mean to end the DevTools session, so we + // still want to have access to the underlying data (furthermore, such case should only + // happen in tests, in a regular workflow we'd still be watching for resources). + if (!isWatchingSomething && !options?.isModeSwitching) { + sessionDataByWatcherActor.delete(watcher.actorID); + watcherActors.delete(watcher.actorID); + } + persistMapToSharedData(); return true; @@ -271,13 +287,19 @@ export const ParentProcessWatcherRegistry = { * The WatcherActor which stops observing. * @param string targetType * The new target type to stop listening to. + * @params {Object} options + * @params {Boolean} options.isModeSwitching: Set to true true when this is called as the + * result of a change to the devtools.browsertoolbox.scope pref. * @return boolean * True if we were watching for this target type, for this watcher actor. */ - unwatchTargets(watcher, targetType) { - return this.removeSessionDataEntry(watcher, SUPPORTED_DATA.TARGETS, [ - targetType, - ]); + unwatchTargets(watcher, targetType, options) { + return this.removeSessionDataEntry( + watcher, + SUPPORTED_DATA.TARGETS, + [targetType], + options + ); }, /** diff --git a/devtools/server/actors/watcher/session-context.js b/devtools/server/actors/watcher/session-context.js index 15f1b40934d3..645739945567 100644 --- a/devtools/server/actors/watcher/session-context.js +++ b/devtools/server/actors/watcher/session-context.js @@ -159,10 +159,10 @@ function getWatcherSupportedTargets(type) { return { [Targets.TYPES.FRAME]: true, [Targets.TYPES.PROCESS]: true, - [Targets.TYPES.WORKER]: true, - [Targets.TYPES.SERVICE_WORKER]: - type == SESSION_TYPES.BROWSER_ELEMENT || type == SESSION_TYPES.ALL, - [Targets.TYPES.SHARED_WORKER]: type == SESSION_TYPES.ALL, + [Targets.TYPES.WORKER]: + type == SESSION_TYPES.BROWSER_ELEMENT || + type == SESSION_TYPES.WEBEXTENSION, + [Targets.TYPES.SERVICE_WORKER]: type == SESSION_TYPES.BROWSER_ELEMENT, }; } diff --git a/devtools/server/connectors/js-process-actor/ContentProcessWatcherRegistry.sys.mjs b/devtools/server/connectors/js-process-actor/ContentProcessWatcherRegistry.sys.mjs index 74fe93ba93d8..41ce80c9fd6d 100644 --- a/devtools/server/connectors/js-process-actor/ContentProcessWatcherRegistry.sys.mjs +++ b/devtools/server/connectors/js-process-actor/ContentProcessWatcherRegistry.sys.mjs @@ -3,14 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ const lazy = {}; -ChromeUtils.defineESModuleGetters( - lazy, - { - loader: "resource://devtools/shared/loader/Loader.sys.mjs", - }, - { global: "contextual" } -); - ChromeUtils.defineESModuleGetters( lazy, { @@ -18,8 +10,9 @@ ChromeUtils.defineESModuleGetters( "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", useDistinctSystemPrincipalLoader: "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", + loader: "resource://devtools/shared/loader/Loader.sys.mjs", }, - { global: "shared" } + { global: "contextual" } ); // Name of the attribute into which we save data in `sharedData` object. @@ -164,17 +157,11 @@ export const ContentProcessWatcherRegistry = { return { connection, loader }; } - // When debugging a privileged page, like about:addons, this module will run in the same compartment - // as the debugged page. Both will run in the shared system compartment. + const { sessionContext, forwardingPrefix } = watcherDataObject; + // For the browser toolbox, we need to use a distinct loader in order to debug privileged JS. // The thread actor ultimately need to be in a distinct compartments from its debuggees. - // So we are using a special loader, which will use a distinct privileged global and compartment - // to load itself as well as all its modules. - // - // Note that when we are running the Browser Toolbox, this module will already be loaded in a special, distinct global and compartment - // Thanks to `loadInDevToolsLoader` flag of BrowserToolboxDevToolsProcess's JS Process Actor configuration. - // So that the Loader will also be loaded in the right, distinct compartment. loader = - useDistinctLoader || watcherDataObject.sessionContext.type == "all" + useDistinctLoader || sessionContext.type == "all" ? lazy.useDistinctSystemPrincipalLoader(watcherDataObject) : lazy.loader; watcherDataObject.loader = loader; @@ -192,7 +179,6 @@ export const ContentProcessWatcherRegistry = { // Instantiate a DevToolsServerConnection which will pipe all its outgoing RDP packets // up to the parent process manager via DevToolsProcess JS Actor messages. - const { forwardingPrefix } = watcherDataObject; connection = DevToolsServer.connectToParentWindowActor( watcherDataObject.jsProcessActor, forwardingPrefix, diff --git a/devtools/server/connectors/js-process-actor/DevToolsProcessChild.sys.mjs b/devtools/server/connectors/js-process-actor/DevToolsProcessChild.sys.mjs index b3e2d6d4711c..d98c416e346f 100644 --- a/devtools/server/connectors/js-process-actor/DevToolsProcessChild.sys.mjs +++ b/devtools/server/connectors/js-process-actor/DevToolsProcessChild.sys.mjs @@ -15,8 +15,6 @@ ChromeUtils.defineESModuleGetters( "resource://devtools/server/actors/watcher/SessionDataHelpers.sys.mjs", ServiceWorkerTargetWatcher: "resource://devtools/server/connectors/js-process-actor/target-watchers/service_worker.sys.mjs", - SharedWorkerTargetWatcher: - "resource://devtools/server/connectors/js-process-actor/target-watchers/shared_worker.sys.mjs", WorkerTargetWatcher: "resource://devtools/server/connectors/js-process-actor/target-watchers/worker.sys.mjs", WindowGlobalTargetWatcher: @@ -79,13 +77,6 @@ export class DevToolsProcessChild extends JSProcessActorChild { return lazy.ServiceWorkerTargetWatcher; }, }, - - shared_worker: { - activeListener: 0, - get watcher() { - return lazy.SharedWorkerTargetWatcher; - }, - }, }; #initialized = false; @@ -432,14 +423,6 @@ export class DevToolsProcessChild extends JSProcessActorChild { updateType ); } - if (watchingTargetTypes.includes("shared_worker")) { - await this.#watchers.shared_worker.watcher.addOrSetSessionDataEntry( - watcherDataObject, - type, - entries, - updateType - ); - } } /** @@ -496,14 +479,14 @@ export class DevToolsProcessChild extends JSProcessActorChild { * but also within this class when the last watcher stopped watching for targets. */ didDestroy() { - // Unregister all the active watchers. - // This will destroy all the active target actors and unregister the target observers. - for (const watcherDataObject of ContentProcessWatcherRegistry.getAllWatchersDataObjects()) { - this.#destroyWatcher(watcherDataObject); + // Stop watching for all target types + for (const entry of Object.values(this.#watchers)) { + if (entry.activeListener > 0) { + entry.watcher.unwatch(); + entry.activeListener = 0; + } } - // The previous for loop should have removed all the elements, - // but just to be safe, wipe all stored data to avoid any possible leak. ContentProcessWatcherRegistry.clear(); } } diff --git a/devtools/server/connectors/js-process-actor/target-watchers/moz.build b/devtools/server/connectors/js-process-actor/target-watchers/moz.build index ca5252762292..0574b0399ec1 100644 --- a/devtools/server/connectors/js-process-actor/target-watchers/moz.build +++ b/devtools/server/connectors/js-process-actor/target-watchers/moz.build @@ -7,7 +7,6 @@ DevToolsModules( "process.sys.mjs", "service_worker.sys.mjs", - "shared_worker.sys.mjs", "window-global.sys.mjs", "worker.sys.mjs", ) diff --git a/devtools/server/connectors/js-process-actor/target-watchers/shared_worker.sys.mjs b/devtools/server/connectors/js-process-actor/target-watchers/shared_worker.sys.mjs deleted file mode 100644 index b9309ca69213..000000000000 --- a/devtools/server/connectors/js-process-actor/target-watchers/shared_worker.sys.mjs +++ /dev/null @@ -1,13 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -import { WorkerTargetWatcherClass } from "resource://devtools/server/connectors/js-process-actor/target-watchers/worker.sys.mjs"; - -class SharedWorkerTargetWatcherClass extends WorkerTargetWatcherClass { - constructor() { - super("shared_worker"); - } -} - -export const SharedWorkerTargetWatcher = new SharedWorkerTargetWatcherClass(); diff --git a/devtools/server/connectors/js-process-actor/target-watchers/worker.sys.mjs b/devtools/server/connectors/js-process-actor/target-watchers/worker.sys.mjs index 92d3e334e118..0b67e8b03813 100644 --- a/devtools/server/connectors/js-process-actor/target-watchers/worker.sys.mjs +++ b/devtools/server/connectors/js-process-actor/target-watchers/worker.sys.mjs @@ -229,13 +229,6 @@ export class WorkerTargetWatcherClass { workerInfo.workerTargetForm = workerTargetForm; workerInfo.transport = transport; - // Bail out and cleanup the actor by closing the transport, - // if we stopped listening for workers while waiting for onConnectToWorker resolution. - if (!watcherDataObject.workers.includes(workerInfo)) { - transport.close(); - return; - } - const { forwardingPrefix } = watcherDataObject; // Immediately queue a message for the parent process, before applying any SessionData // as it may start emitting RDP events on the target actor and be lost if the client @@ -280,11 +273,23 @@ export class WorkerTargetWatcherClass { destroyTargetsForWatcher(watcherDataObject) { // Notify to all worker threads to destroy their target actor running in them - for (const { transport } of watcherDataObject.workers) { - // The transport may not be set if the worker is still being connected to from createWorkerTargetActor. + for (const { + dbg, + workerThreadServerForwardingPrefix, + transport, + } of watcherDataObject.workers) { + if (isWorkerDebuggerAlive(dbg)) { + try { + dbg.postMessage( + JSON.stringify({ + type: "disconnect", + forwardingPrefix: workerThreadServerForwardingPrefix, + }) + ); + } catch (e) {} + } + // Also cleanup the DevToolsTransport created in the main thread to bridge RDP to the worker thread if (transport) { - // Clean the DevToolsTransport created in the main thread to bridge RDP to the worker thread. - // This will also send a last message to the worker to clean things up in the other thread. transport.close(); } } @@ -317,16 +322,6 @@ export class WorkerTargetWatcherClass { return false; } - // subprocess workers are ignored because they take several seconds to - // attach to when opening the browser toolbox. See bug 1594597. - if ( - /resource:\/\/gre\/modules\/subprocess\/subprocess_.*\.worker\.js/.test( - dbg.url - ) - ) { - return false; - } - const { type: sessionContextType } = sessionData.sessionContext; if (sessionContextType == "all") { return true; @@ -377,9 +372,10 @@ export class WorkerTargetWatcherClass { } if (dbg.type === TYPE_SHARED) { - // Don't expose shared workers when debugging a tab. - // For now, they are only exposed in the browser toolbox, when Session Context Type is set to "all". - return false; + // We still don't fully support instantiating targets for shared workers from the server side + throw new Error( + "Server side listening for shared workers isn't supported" + ); } return false; diff --git a/devtools/server/devtools-server.js b/devtools/server/devtools-server.js index 89bcab68039a..e1dd7994d16d 100644 --- a/devtools/server/devtools-server.js +++ b/devtools/server/devtools-server.js @@ -258,12 +258,6 @@ var DevToolsServer = { // Remove connections that were accepted in the listener. for (const connID of Object.getOwnPropertyNames(this._connections)) { const connection = this._connections[connID]; - // When calling connection.close on a previous element, - // this may unregister some of the following other connections in `_connections` - // and make them be null here. - if (!connection) { - continue; - } if (connection.isAcceptedBy(listener)) { connection.close(); } diff --git a/devtools/server/startup/content-process.sys.mjs b/devtools/server/startup/content-process.sys.mjs index e740f42f0d16..fd974e8c4aa6 100644 --- a/devtools/server/startup/content-process.sys.mjs +++ b/devtools/server/startup/content-process.sys.mjs @@ -21,8 +21,7 @@ export function initContentProcessTarget(msg) { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); // Use a unique object to identify this one usage of the loader diff --git a/devtools/server/startup/frame.js b/devtools/server/startup/frame.js index 3924ed013ab2..db14f03c1546 100644 --- a/devtools/server/startup/frame.js +++ b/devtools/server/startup/frame.js @@ -28,8 +28,7 @@ try { customLoader = false; if (content.document.nodePrincipal.isSystemPrincipal) { const { useDistinctSystemPrincipalLoader } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); loader = useDistinctSystemPrincipalLoader(chromeGlobal); customLoader = true; @@ -182,8 +181,7 @@ try { if (customLoader) { const { releaseDistinctSystemPrincipalLoader } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); releaseDistinctSystemPrincipalLoader(chromeGlobal); } diff --git a/devtools/server/tests/xpcshell/head_dbg.js b/devtools/server/tests/xpcshell/head_dbg.js index eb3440bb13b4..e8547f15b8da 100644 --- a/devtools/server/tests/xpcshell/head_dbg.js +++ b/devtools/server/tests/xpcshell/head_dbg.js @@ -89,8 +89,7 @@ function getDistinctDevToolsServer() { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); const requester = {}; const distinctLoader = useDistinctSystemPrincipalLoader(requester); diff --git a/devtools/shared/commands/resource/resource-command.js b/devtools/shared/commands/resource/resource-command.js index 7d7ffd95ba88..5b34bf4442ee 100644 --- a/devtools/shared/commands/resource/resource-command.js +++ b/devtools/shared/commands/resource/resource-command.js @@ -179,10 +179,6 @@ class ResourceCommand { } } - // Copy the array in order to avoid the callsite to modify the list of watched resources by mutating the array. - // You have to call (un)watchResources to update the list of resources being watched! - resources = [...resources]; - // Pending watchers are used in unwatchResources to remove watchers which // are not fully registered yet. Store `onAvailable` which is the unique key // for a watcher, as well as the resources array, so that unwatchResources diff --git a/devtools/shared/commands/resource/tests/browser_resources_watch_unwatch_multiple.js b/devtools/shared/commands/resource/tests/browser_resources_watch_unwatch_multiple.js index a8ece5f09a3a..cc45e7bf7fd5 100644 --- a/devtools/shared/commands/resource/tests/browser_resources_watch_unwatch_multiple.js +++ b/devtools/shared/commands/resource/tests/browser_resources_watch_unwatch_multiple.js @@ -19,8 +19,7 @@ add_task(async function () { }; info("Watch for error messages resources"); - const watchedResources = [resourceCommand.TYPES.ERROR_MESSAGE]; - await resourceCommand.watchResources(watchedResources, { + await resourceCommand.watchResources([resourceCommand.TYPES.ERROR_MESSAGE], { onAvailable, }); @@ -35,10 +34,6 @@ add_task(async function () { "no resources were received after the first watchResources call" ); - // Clear the array which was passed to watchResources. - // It should not impact ResourceCommand behavior! - watchedResources.length = 0; - info("Trigger an error in the page"); await ContentTask.spawn(tab.linkedBrowser, [], function frameScript() { const document = content.document; diff --git a/devtools/shared/loader/Loader.sys.mjs b/devtools/shared/loader/Loader.sys.mjs index bc603d29a3f7..e34810dde842 100644 --- a/devtools/shared/loader/Loader.sys.mjs +++ b/devtools/shared/loader/Loader.sys.mjs @@ -72,14 +72,9 @@ export function DevToolsLoader({ "toolkit/locales": "chrome://global/locale", }; - // In case the Loader ESM is loaded in DevTools global, - // also reuse this global for all CommonJS modules. - const sharedGlobal = - useDevToolsLoaderGlobal || - // eslint-disable-next-line mozilla/reject-globalThis-modification - Cu.getRealmLocation(globalThis) == "DevTools global" - ? Cu.getGlobalForObject({}) - : undefined; + const sharedGlobal = useDevToolsLoaderGlobal + ? Cu.getGlobalForObject({}) + : undefined; this.loader = new Loader({ paths, sharedGlobal, diff --git a/devtools/shared/security/tests/xpcshell/test_devtools_socket_status.js b/devtools/shared/security/tests/xpcshell/test_devtools_socket_status.js index fdd6feea8661..2e41583b0512 100644 --- a/devtools/shared/security/tests/xpcshell/test_devtools_socket_status.js +++ b/devtools/shared/security/tests/xpcshell/test_devtools_socket_status.js @@ -7,8 +7,7 @@ const { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); const { DevToolsSocketStatus } = ChromeUtils.importESModule( diff --git a/devtools/shared/tests/xpcshell/test_invisible_loader.js b/devtools/shared/tests/xpcshell/test_invisible_loader.js index 260042bd7d47..d83efd9c2a83 100644 --- a/devtools/shared/tests/xpcshell/test_invisible_loader.js +++ b/devtools/shared/tests/xpcshell/test_invisible_loader.js @@ -60,8 +60,7 @@ function distinct_system_principal_loader() { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); const requester = {}; diff --git a/devtools/shared/tests/xpcshell/test_loader.js b/devtools/shared/tests/xpcshell/test_loader.js index e812e46264f5..f2d3b912ca5c 100644 --- a/devtools/shared/tests/xpcshell/test_loader.js +++ b/devtools/shared/tests/xpcshell/test_loader.js @@ -7,8 +7,7 @@ const { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); function run_test() { diff --git a/devtools/startup/DevToolsStartup.sys.mjs b/devtools/startup/DevToolsStartup.sys.mjs index cf9539b6ff58..e9e24af41e89 100644 --- a/devtools/startup/DevToolsStartup.sys.mjs +++ b/devtools/startup/DevToolsStartup.sys.mjs @@ -1091,8 +1091,7 @@ DevToolsStartup.prototype = { useDistinctSystemPrincipalLoader, releaseDistinctSystemPrincipalLoader, } = ChromeUtils.importESModule( - "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs", - { global: "shared" } + "resource://devtools/shared/loader/DistinctSystemPrincipalLoader.sys.mjs" ); try {