Bug 1781726 - [devtools] Pass an `isModeSwitching` property to `onTargetDestroyed` callback param when switching modes. r=jdescottes,devtools-backward-compat-reviewers.

We pass `isModeSwitching` to `unwatchTargets` from the target command when the
pref is changed.
On the server, we then pass it to the various places which might call `notifyTargetDestroyed`,
so we can pass the flag in the `target-destroyed-form` event, which we can then
pass to TargetCommand#onDestroyed callbacks.`

Differential Revision: https://phabricator.services.mozilla.com/D152758
This commit is contained in:
Nicolas Chevobbe 2022-07-29 14:30:07 +00:00
Родитель 9c927cafd9
Коммит 9a96297e35
16 изменённых файлов: 210 добавлений и 73 удалений

Просмотреть файл

@ -60,7 +60,7 @@ class WatcherFront extends FrontClassWithSpec(watcherSpec) {
this.emit("target-available", front);
}
_onTargetDestroyed(form) {
_onTargetDestroyed(form, options = {}) {
const front = this._getTargetFront(form);
// When server side target switching is off,
@ -71,7 +71,7 @@ class WatcherFront extends FrontClassWithSpec(watcherSpec) {
// existing targets.
// https://searchfox.org/mozilla-central/rev/af8e5d37fd56be90ccddae2203e7b875d3f3ae87/devtools/shared/commands/target/target-command.js#166-173
if (front) {
this.emit("target-destroyed", front);
this.emit("target-destroyed", front, options);
}
}

Просмотреть файл

@ -658,8 +658,10 @@ const windowGlobalTargetPrototype = {
* @params {Object} options
* @params {Boolean} options.isTargetSwitching: Set to true when this is called during
* a target switch.
* @params {Boolean} options.isModeSwitching: Set to true true when this is called as the
* result of a change to the devtools.browsertoolbox.scope pref.
*/
destroy({ isTargetSwitching = false } = {}) {
destroy({ isTargetSwitching = false, isModeSwitching = false } = {}) {
// Avoid reentrancy. We will destroy the Transport when emitting "destroyed",
// which will force destroying all actors.
if (this.destroying) {
@ -727,7 +729,7 @@ const windowGlobalTargetPrototype = {
// Emit a last event before calling Actor.destroy
// which will destroy the EventEmitter API
this.emit("destroyed");
this.emit("destroyed", { isTargetSwitching, isModeSwitching });
Actor.prototype.destroy.call(this);
TargetActorRegistry.unregisterTargetActor(this);

Просмотреть файл

@ -230,18 +230,29 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
*
* @param {string} targetType
* Type of context to observe. See Targets.TYPES object.
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
unwatchTargets(targetType) {
const isWatchingTargets = WatcherRegistry.unwatchTargets(this, targetType);
unwatchTargets(targetType, options = {}) {
const isWatchingTargets = WatcherRegistry.unwatchTargets(
this,
targetType,
options
);
if (!isWatchingTargets) {
return;
}
const targetHelperModule = TARGET_HELPERS[targetType];
targetHelperModule.destroyTargets(this);
targetHelperModule.destroyTargets(this, options);
// Unregister the JS Window Actor if there is no more DevTools code observing any target/resource
WatcherRegistry.maybeUnregisteringJSWindowActor();
// Unregister the JS Window Actor 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) {
WatcherRegistry.maybeUnregisteringJSWindowActor();
}
},
/**
@ -295,12 +306,18 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
/**
* Called by a Watcher module, whenever a target has been destroyed
*
* @param {object} actor
* the actor form of the target being destroyed
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
async notifyTargetDestroyed(actor) {
async notifyTargetDestroyed(actor, options = {}) {
// Emit immediately for worker, process & extension targets
// as they don't have a parent browsing context.
if (!actor.innerWindowId) {
this.emit("target-destroyed-form", actor);
this.emit("target-destroyed-form", actor, options);
return;
}
// Flush all iframe targets if we are destroying a top level target.
@ -314,7 +331,7 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
// Ignore the top level target itself, because its topInnerWindowId will be its innerWindowId
form.innerWindowId != actor.innerWindowId
);
childrenActors.map(form => this.notifyTargetDestroyed(form));
childrenActors.map(form => this.notifyTargetDestroyed(form, options));
}
if (this._earlyIframeTargets[actor.innerWindowId]) {
delete this._earlyIframeTargets[actor.innerWindowId];
@ -347,7 +364,7 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
) {
await documentEventWatcher.onceWillNavigateIsEmitted(actor.innerWindowId);
}
this.emit("target-destroyed-form", actor);
this.emit("target-destroyed-form", actor, options);
},
/**

Просмотреть файл

@ -183,12 +183,20 @@ const WatcherRegistry = {
/**
* Notify that a given watcher removed an entry in a given data type.
*
* See `addSessionDataEntry` for argument definition.
* @param WatcherActor watcher
* The WatcherActor which stops observing.
* @param string type
* The type of data to be removed
* @param Array<Object> 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;
@ -207,7 +215,12 @@ const WatcherRegistry = {
const isWatchingSomething = SUPPORTED_DATA_TYPES.some(
dataType => sessionData[dataType] && sessionData[dataType].length > 0
);
if (!isWatchingSomething) {
// 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);
}
@ -246,15 +259,23 @@ const WatcherRegistry = {
/**
* Notify that a given watcher stops observing a given target type.
*
* See `watchTargets` for argument definition.
*
* @param WatcherActor watcher
* 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
);
},
/**

Просмотреть файл

@ -158,8 +158,11 @@ async function createTargetForBrowsingContext({
*
* @param WatcherActor watcher
* The Watcher Actor requesting to stop watching for new targets.
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
function destroyTargets(watcher) {
function destroyTargets(watcher, options) {
// Go over all existing BrowsingContext in order to destroy all targets
const browsingContexts = watcher.getAllBrowsingContexts();
@ -178,6 +181,7 @@ function destroyTargets(watcher) {
.destroyTarget({
watcherActorID: watcher.actorID,
sessionContext: watcher.sessionContext,
options,
});
}

Просмотреть файл

@ -118,8 +118,14 @@ function onMessageManagerClose(messageManager, topic, data) {
* - clear up things from `actors` WeakMap,
* - notify all related target actors as being destroyed,
* - close all DevTools Transports being created for each Message Manager.
*
* @param {WatcherActor} watcher
* @param {MessageManager}
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
function unregisterWatcherForMessageManager(watcher, messageManager) {
function unregisterWatcherForMessageManager(watcher, messageManager, options) {
const targetActorDescriptions = actors.get(messageManager);
if (!targetActorDescriptions || targetActorDescriptions.length == 0) {
return;
@ -134,7 +140,7 @@ function unregisterWatcherForMessageManager(watcher, messageManager) {
childTransport,
actor,
} of matchingTargetActorDescriptions) {
watcher.notifyTargetDestroyed(actor);
watcher.notifyTargetDestroyed(actor, options);
childTransport.close();
watcher.conn.cancelForwarding(prefix);
@ -154,11 +160,16 @@ function unregisterWatcherForMessageManager(watcher, messageManager) {
/**
* Destroy everything related to a given watcher that has been created in this module:
* (See unregisterWatcherForMessageManager)
*
* @param {WatcherActor} watcher
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
function closeWatcherTransports(watcher) {
function closeWatcherTransports(watcher, options) {
for (let i = 0; i < Services.ppmm.childCount; i++) {
const messageManager = Services.ppmm.getChildAt(i);
unregisterWatcherForMessageManager(watcher, messageManager);
unregisterWatcherForMessageManager(watcher, messageManager, options);
}
}
@ -189,10 +200,17 @@ function maybeRegisterMessageListeners(watcher) {
}
}
}
function maybeUnregisterMessageListeners(watcher) {
/**
* @param {WatcherActor} watcher
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
function maybeUnregisterMessageListeners(watcher, options = {}) {
const sizeBefore = watchers.size;
watchers.delete(watcher);
closeWatcherTransports(watcher);
closeWatcherTransports(watcher, options);
if (sizeBefore == 1 && watchers.size == 0) {
Services.ppmm.removeMessageListener(
@ -212,7 +230,9 @@ function maybeUnregisterMessageListeners(watcher) {
// module unregister the last watcher of all.
Services.ppmm.removeDelayedProcessScript(CONTENT_PROCESS_SCRIPT);
Services.ppmm.broadcastAsyncMessage("debug:destroy-process-script");
Services.ppmm.broadcastAsyncMessage("debug:destroy-process-script", {
options,
});
}
}
@ -263,8 +283,14 @@ async function createTargets(watcher) {
await onTargetsCreated;
}
function destroyTargets(watcher) {
maybeUnregisterMessageListeners(watcher);
/**
* @param {WatcherActor} watcher
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
function destroyTargets(watcher, options) {
maybeUnregisterMessageListeners(watcher, options);
Services.ppmm.broadcastAsyncMessage("debug:destroy-target", {
watcherActorID: watcher.actorID,

Просмотреть файл

@ -237,9 +237,9 @@ class DevToolsFrameChild extends JSWindowActorChild {
const form = targetActor.form();
// Ensure unregistering and destroying the related DevToolsServerConnection+Transport
// on both content and parent process JSWindowActors.
targetActor.once("destroyed", () => {
targetActor.once("destroyed", options => {
// This will destroy the content process one
this._destroyTargetActor(watcherActorID);
this._destroyTargetActor(watcherActorID, options);
// And this will destroy the parent process one
try {
this.sendAsyncMessage("DevToolsFrameChild:destroy", {
@ -249,6 +249,7 @@ class DevToolsFrameChild extends JSWindowActorChild {
form,
},
],
options,
});
} catch (e) {
// Ignore exception when the JSWindowActorChild has already been destroyed.
@ -295,7 +296,13 @@ class DevToolsFrameChild extends JSWindowActorChild {
}
}
_destroyTargetActor(watcherActorID) {
/**
* @param {string} watcherActorID
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
_destroyTargetActor(watcherActorID, options) {
const connectionInfo = this._connections.get(watcherActorID);
// This connection has already been cleaned?
if (!connectionInfo) {
@ -303,10 +310,10 @@ class DevToolsFrameChild extends JSWindowActorChild {
`Trying to destroy a target actor that doesn't exists, or has already been destroyed. Watcher Actor ID:${watcherActorID}`
);
}
connectionInfo.connection.close();
connectionInfo.connection.close(options);
this._connections.delete(watcherActorID);
if (this._connections.size == 0) {
this.didDestroy();
this.didDestroy(options);
}
}
@ -429,8 +436,8 @@ class DevToolsFrameChild extends JSWindowActorChild {
});
}
case "DevToolsFrameParent:destroy": {
const { watcherActorID } = message.data;
return this._destroyTargetActor(watcherActorID);
const { watcherActorID, options } = message.data;
return this._destroyTargetActor(watcherActorID, options);
}
case "DevToolsFrameParent:addSessionDataEntry": {
const { watcherActorID, sessionContext, type, entries } = message.data;
@ -672,10 +679,10 @@ class DevToolsFrameChild extends JSWindowActorChild {
}
}
didDestroy() {
didDestroy(options) {
logWindowGlobal(this.manager, "Destroy WindowGlobalTarget");
for (const [, connectionInfo] of this._connections) {
connectionInfo.connection.close();
connectionInfo.connection.close(options);
}
this._connections.clear();

Просмотреть файл

@ -70,10 +70,18 @@ class DevToolsFrameParent extends JSWindowActorParent {
});
}
destroyTarget({ watcherActorID, sessionContext }) {
/**
* @param {object} arg
* @param {object} arg.sessionContext
* @param {object} arg.options
* @param {boolean} arg.options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
destroyTarget({ watcherActorID, sessionContext, options }) {
this.sendAsyncMessage("DevToolsFrameParent:destroy", {
watcherActorID,
sessionContext,
options,
});
}
@ -191,10 +199,14 @@ class DevToolsFrameParent extends JSWindowActorParent {
* When navigating away, we will destroy them and call this method.
* Then when navigating back, we will reuse the same instances.
* So that we should be careful to keep the class fully function and only clear all its state.
*
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
_closeAllConnections() {
_closeAllConnections(options) {
for (const { actor, watcher } of this._connections.values()) {
watcher.notifyTargetDestroyed(actor);
watcher.notifyTargetDestroyed(actor, options);
this._unregisterWatcher(watcher.conn.prefix);
}
this._connections.clear();
@ -225,7 +237,7 @@ class DevToolsFrameParent extends JSWindowActorParent {
// we may easily receive the target destruction notification *after*
// the watcher has been removed from the registry.
if (watcher) {
watcher.notifyTargetDestroyed(form);
watcher.notifyTargetDestroyed(form, message.data.options);
this._unregisterWatcher(watcher.conn.prefix);
}
}

Просмотреть файл

@ -89,9 +89,9 @@ DevToolsServerConnection.prototype = {
*/
parentMessageManager: null,
close() {
close(options) {
if (this._transport) {
this._transport.close();
this._transport.close(options);
}
},
@ -471,8 +471,11 @@ DevToolsServerConnection.prototype = {
* @param status nsresult
* The status code that corresponds to the reason for closing
* the stream.
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
onTransportClosed(status) {
onTransportClosed(status, options) {
dumpn("Cleaning up connection.");
if (!this._actorPool) {
// Ignore this call if the connection is already closed.
@ -490,7 +493,7 @@ DevToolsServerConnection.prototype = {
// See test_connection_closes_all_pools.js for practical examples of Pool
// hierarchies.
const topLevelPools = this._extraPools.filter(p => p.isTopPool());
topLevelPools.forEach(p => p.destroy());
topLevelPools.forEach(p => p.destroy(options));
this._extraPools = null;

Просмотреть файл

@ -46,11 +46,11 @@ class ContentProcessStartup {
}
}
destroy() {
destroy(options) {
this.removeListeners();
for (const [, connectionInfo] of this._connections) {
connectionInfo.connection.close();
connectionInfo.connection.close(options);
}
this._connections.clear();
}
@ -133,7 +133,7 @@ class ContentProcessStartup {
);
break;
case "debug:destroy-process-script":
this.destroy();
this.destroy(msg.data.options);
break;
default:
throw new Error(`Unsupported message name ${msg.name}`);

Просмотреть файл

@ -60,7 +60,7 @@ function initContentProcessTarget(msg) {
const response = { watcherActorID, prefix, actor: actor.form() };
mm.sendAsyncMessage("debug:content-process-actor", response);
function onDestroy() {
function onDestroy(options) {
mm.removeMessageListener(
"debug:content-process-disconnect",
onContentProcessDisconnect
@ -75,7 +75,7 @@ function initContentProcessTarget(msg) {
// Call DevToolsServerConnection.close to destroy all child actors. It should end up
// calling DevToolsServerConnection.onTransportClosed that would actually cleanup all actor
// pools.
conn.close();
conn.close(options);
// Destroy the related loader when the target is destroyed
// and we were the last user of the special loader

Просмотреть файл

@ -157,7 +157,10 @@ class TargetCommand extends EventEmitter {
// The related targets will be destroyed by the server
// and reported as destroyed to the frontend.
for (const type of disabledTargetTypes) {
this.stopListeningForType(type, { isTargetSwitching: false });
this.stopListeningForType(type, {
isTargetSwitching: false,
isModeSwitching: true,
});
}
}
}
@ -316,19 +319,26 @@ class TargetCommand extends EventEmitter {
*
* @param {TargetFront} targetFront
* The target that just got destroyed.
* @param Object options
* Dictionary object with:
* - `isTargetSwitching` optional boolean. To be set to true when this
* is about the top level target which is being replaced by a new one.
* The passed target should be still the one store in TargetCommand.targetFront
* and will be replaced via a call to onTargetAvailable with a new target front.
* - `shouldDestroyTargetFront` optional boolean. By default, the passed target
* front will be destroyed. But in some cases like legacy listeners for service workers
* we want to keep the front alive.
* @param {Object} options
* @param {Boolean} [options.isTargetSwitching]
* To be set to true when this is about the top level target which is being replaced
* by a new one.
* The passed target should be still the one store in TargetCommand.targetFront
* and will be replaced via a call to onTargetAvailable with a new target front.
* @param {Boolean} [options.isModeSwitching]
* To be set to true when the target was destroyed was called as the result of a
* change to the devtools.browsertoolbox.scope pref.
* @param {Boolean} [options.shouldDestroyTargetFront]
* By default, the passed target front will be destroyed. But in some cases like
* legacy listeners for service workers we want to keep the front alive.
*/
_onTargetDestroyed(
targetFront,
{ isTargetSwitching = false, shouldDestroyTargetFront = true } = {}
{
isModeSwitching = false,
isTargetSwitching = false,
shouldDestroyTargetFront = true,
} = {}
) {
// The watcher actor may notify us about the destruction of the top level target.
// But second argument to this method, isTargetSwitching is only passed from the frontend.
@ -340,6 +350,7 @@ class TargetCommand extends EventEmitter {
this._destroyListeners.emit(targetFront.targetType, {
targetFront,
isTargetSwitching,
isModeSwitching,
});
this._targets.delete(targetFront);
@ -618,7 +629,21 @@ class TargetCommand extends EventEmitter {
}
}
stopListeningForType(type, { isTargetSwitching }) {
/**
* Stop listening for targets of a given type from the server
*
* @param String type
* target type we want to stop listening for
* @param Object options
* @param Boolean options.isTargetSwitching
* Set to true when this is called while a target switching happens. In such case,
* we won't unregister listener set on the Watcher Actor, but still unregister
* listeners set via Legacy Listeners.
* @param Boolean options.isModeSwitching
* Set to true when this is called as the result of a change to the
* devtools.browsertoolbox.scope pref.
*/
stopListeningForType(type, { isTargetSwitching, isModeSwitching }) {
if (!this._isListening(type)) {
return;
}
@ -633,10 +658,13 @@ class TargetCommand extends EventEmitter {
// Also, TargetCommand.destroy may be called after the client is closed.
// So avoid calling the RDP method in that situation.
if (!isTargetSwitching && !this.watcherFront.isDestroyed()) {
this.watcherFront.unwatchTargets(type);
this.watcherFront.unwatchTargets(type, { isModeSwitching });
}
} else if (this.legacyImplementation[type]) {
this.legacyImplementation[type].unlisten({ isTargetSwitching });
this.legacyImplementation[type].unlisten({
isTargetSwitching,
isModeSwitching,
});
} else {
throw new Error(`Unsupported target type '${type}'`);
}

Просмотреть файл

@ -32,10 +32,12 @@ add_task(async function() {
const { TYPES } = targetCommand;
const targets = new Set();
const destroyedTargetIsModeSwitchingMap = new Map();
const onAvailable = async ({ targetFront }) => {
targets.add(targetFront);
};
const onDestroyed = ({ targetFront }) => {
const onDestroyed = ({ targetFront, isModeSwitching }) => {
destroyedTargetIsModeSwitchingMap.set(targetFront, isModeSwitching);
targets.delete(targetFront);
};
await targetCommand.watchTargets({
@ -91,7 +93,15 @@ add_task(async function() {
);
ok(processTarget.isDestroyed(), "The process target is destroyed");
ok(
destroyedTargetIsModeSwitchingMap.get(processTarget),
"isModeSwitching was passed to onTargetDestroyed and is true for the process target"
);
ok(windowGlobalTarget.isDestroyed(), "The window global target is destroyed");
ok(
destroyedTargetIsModeSwitchingMap.get(windowGlobalTarget),
"isModeSwitching was passed to onTargetDestroyed and is true for the window global target"
);
info("Open a second tab in a new content process");
const parentProcessTargetCount = targets.size;

Просмотреть файл

@ -19,6 +19,7 @@ const watcherSpecPrototype = {
unwatchTargets: {
request: {
targetType: Arg(0, "string"),
options: Arg(1, "nullable:json"),
},
oneway: true,
},
@ -97,6 +98,7 @@ const watcherSpecPrototype = {
"target-destroyed-form": {
type: "target-destroyed-form",
target: Arg(0, "json"),
options: Arg(1, "nullable:json"),
},
"resource-available-form": {

Просмотреть файл

@ -57,10 +57,10 @@ ChildDebuggerTransport.prototype = {
this._addListener();
},
close: function() {
close: function(options) {
this._removeListener();
if (this.hooks.onTransportClosed) {
this.hooks.onTransportClosed();
this.hooks.onTransportClosed(null, options);
}
},

Просмотреть файл

@ -35,10 +35,15 @@ class JsWindowActorTransport {
this._addListener();
}
close() {
/**
* @param {object} options
* @param {boolean} options.isModeSwitching
* true when this is called as the result of a change to the devtools.browsertoolbox.scope pref
*/
close(options) {
this._removeListener();
if (this.hooks.onTransportClosed) {
this.hooks.onTransportClosed();
this.hooks.onTransportClosed(null, options);
}
}