Bug 1735825 - [devtools] Pass Watcher's context down to the content process via DevToolsFrame queries and WatcheRegistry. r=jdescottes

This is the second and last step, to propagate the context down to the content process.
So that now, whenever we introduce a new context or add something into it,
it should be automatically made available to the whole codebase.

Differential Revision: https://phabricator.services.mozilla.com/D128491
This commit is contained in:
Alexandre Poirot 2021-11-02 10:57:23 +00:00
parent 02c1a22e99
commit 4465bb9663
8 changed files with 80 additions and 67 deletions

View File

@ -75,6 +75,8 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
* The connection to use in order to communicate back to the client.
* @param {Object} context
* Mandatory argument to define the debugged context of this actor.
* Note that as this object is passed to other processes and thread,
* this should be a serializable object.
* @param {String} context.type: The type of debugged context.
* Can be:
* - "all", to debug everything in the browser.

View File

@ -41,10 +41,8 @@ const { SUPPORTED_DATA } = SessionDataHelpers;
// It is keyed by WatcherActor ID and values contains following attributes:
// - targets: Set of strings, refering to target types to be listened to
// - resources: Set of strings, refering to resource types to be observed
// - browserId: Optional, if set, restrict the observation to one specific Browser Element tree.
// It can be a tab, a top-level window or a top-level iframe (e.g. special privileged iframe)
// See https://searchfox.org/mozilla-central/rev/31d8600b73dc85b4cdbabf45ac3f1a9c11700d8e/dom/chrome-webidl/BrowsingContext.webidl#114-121
// for more information.
// - context: WatcherActor's context. Describe what the watcher should be debugging.
// See watcher actor constructor for more info.
// - connectionPrefix: The DevToolsConnection prefix of the watcher actor. Used to compute new actor ID in the content processes.
//
// Unfortunately, `sharedData` is subject to race condition and may have side effect
@ -111,11 +109,9 @@ const WatcherRegistry = {
let sessionData = sessionDataByWatcherActor.get(watcherActorID);
if (!sessionData && createData) {
sessionData = {
// The Browser ID will be helpful to identify which BrowsingContext should be considered
// when running code in the content process. Browser ID, compared to BrowsingContext ID won't change
// if we navigate to the parent process or if a new BrowsingContext is used for the <browser> element
// we are currently inspecting.
browserId: watcher.context.browserId,
// The "context" object help understand what should be debugged and which target should be created.
// See WatcherActor constructor for more info.
context: watcher.context,
// The DevToolsServerConnection prefix will be used to compute actor IDs created in the content process
connectionPrefix: watcher.conn.prefix,
// Expose watcher traits so we can retrieve them in content process.

View File

@ -134,7 +134,7 @@ async function createTargetForBrowsingContext({
.instantiateTarget({
watcherActorID: watcher.actorID,
connectionPrefix: watcher.conn.prefix,
browserId: watcher.context.browserId,
context: watcher.context,
sessionData: watcher.sessionData,
});
} catch (e) {
@ -191,7 +191,7 @@ function destroyTargets(watcher) {
.getActor("DevToolsFrame")
.destroyTarget({
watcherActorID: watcher.actorID,
browserId: watcher.context.browserId,
context: watcher.context,
});
}
@ -231,7 +231,7 @@ async function addSessionDataEntry({ watcher, type, entries }) {
.getActor("DevToolsFrame")
.addSessionDataEntry({
watcherActorID: watcher.actorID,
browserId: watcher.context.browserId,
context: watcher.context,
type,
entries,
});
@ -258,7 +258,7 @@ function removeSessionDataEntry({ watcher, type, entries }) {
.getActor("DevToolsFrame")
.removeSessionDataEntry({
watcherActorID: watcher.actorID,
browserId: watcher.context.browserId,
context: watcher.context,
type,
entries,
});

View File

@ -29,7 +29,7 @@ async function createTargets(watcher) {
.instantiateWorkerTargets({
watcherActorID: watcher.actorID,
connectionPrefix: watcher.conn.prefix,
browserId: watcher.context.browserId,
context: watcher.context,
sessionData: watcher.sessionData,
});
promises.push(promise);
@ -60,8 +60,8 @@ async function destroyTargets(watcher) {
}
windowActor.destroyWorkerTargets({
watcher,
browserId: watcher.context.browserId,
watcherActorID: watcher.actorID,
context: watcher.context,
});
}
}
@ -84,7 +84,7 @@ async function addSessionDataEntry({ watcher, type, entries }) {
.getActor(DEVTOOLS_WORKER_JS_WINDOW_ACTOR_NAME)
.addSessionDataEntry({
watcherActorID: watcher.actorID,
browserId: watcher.context.browserId,
context: watcher.context,
type,
entries,
});
@ -106,7 +106,7 @@ function removeSessionDataEntry({ watcher, type, entries }) {
.getActor(DEVTOOLS_WORKER_JS_WINDOW_ACTOR_NAME)
.removeSessionDataEntry({
watcherActorID: watcher.actorID,
browserId: watcher.context.browserId,
context: watcher.context,
type,
entries,
});

View File

@ -37,7 +37,7 @@ const SHARED_DATA_KEY_NAME = "DevTools:watchedPerWatcher";
*/
function shouldNotifyWindowGlobal(
windowGlobal,
watchedBrowserId,
context,
{ acceptTopLevelTarget = false }
) {
const browsingContext = windowGlobal.browsingContext;
@ -69,7 +69,10 @@ function shouldNotifyWindowGlobal(
// If we are focusing only on a sub-tree of Browsing Element,
// Ignore the out of the sub tree elements.
if (watchedBrowserId && browsingContext.browserId != watchedBrowserId) {
if (
context.type == "browser-element" &&
browsingContext.browserId != context.browserId
) {
return false;
}
@ -82,7 +85,11 @@ function shouldNotifyWindowGlobal(
//
// `acceptTopLevelTarget` is set both when server side target switching is enabled
// or when navigating to and from pages in the bfcache
if (!acceptTopLevelTarget && watchedBrowserId && !browsingContext.parent) {
if (
!acceptTopLevelTarget &&
context.type == "browser-element" &&
!browsingContext.parent
) {
return false;
}
@ -167,7 +174,7 @@ class DevToolsFrameChild extends JSWindowActorChild {
for (const [watcherActorID, sessionData] of sessionDataByWatcherActor) {
const {
connectionPrefix,
browserId,
context,
isServerTargetSwitchingEnabled,
} = sessionData;
// Always create new targets when server targets are enabled as we create targets for all the WindowGlobal's,
@ -180,7 +187,7 @@ class DevToolsFrameChild extends JSWindowActorChild {
(isBFCache && this.isBfcacheInParentEnabled);
if (
sessionData.targets.includes("frame") &&
shouldNotifyWindowGlobal(this.manager, browserId, {
shouldNotifyWindowGlobal(this.manager, context, {
acceptTopLevelTarget,
})
) {
@ -196,7 +203,7 @@ class DevToolsFrameChild extends JSWindowActorChild {
// - in such case we weren't seeing the issue of Bug 1721398 (the old target can't access the new document)
const existingTarget = this._findTargetActor({
watcherActorID,
browserId,
context,
browsingContextId: this.manager.browsingContext.id,
});
@ -301,7 +308,7 @@ class DevToolsFrameChild extends JSWindowActorChild {
const browsingContext = this.manager.browsingContext;
const isTopLevelTarget =
!browsingContext.parent &&
browsingContext.browserId == sessionData.browserId;
browsingContext.browserId == sessionData.context.browserId;
const { connection, targetActor } = this._createConnectionAndActor(
forwardingPrefix,
@ -456,10 +463,10 @@ class DevToolsFrameChild extends JSWindowActorChild {
// When debugging only a given tab, all messages but "packet" one pass `browserId`
// and are expected to match shouldNotifyWindowGlobal result.
if (
message.data.browserId &&
message.name != "DevToolsFrameParent:packet"
message.name != "DevToolsFrameParent:packet" &&
message.data.context.type == "browser-element"
) {
const { browserId } = message.data;
const { browserId } = message.data.context;
// Re-check here, just to ensure that both parent and content processes agree
// on what should or should not be watched.
if (
@ -492,19 +499,19 @@ class DevToolsFrameChild extends JSWindowActorChild {
return this._destroyTargetActor(watcherActorID);
}
case "DevToolsFrameParent:addSessionDataEntry": {
const { watcherActorID, browserId, type, entries } = message.data;
const { watcherActorID, context, type, entries } = message.data;
return this._addSessionDataEntry(
watcherActorID,
browserId,
context,
type,
entries
);
}
case "DevToolsFrameParent:removeSessionDataEntry": {
const { watcherActorID, browserId, type, entries } = message.data;
const { watcherActorID, context, type, entries } = message.data;
return this._removeSessionDataEntry(
watcherActorID,
browserId,
context,
type,
entries
);
@ -527,13 +534,13 @@ class DevToolsFrameChild extends JSWindowActorChild {
*
* @param {Object} options
* @param {Object} options.watcherActorID
* @param {Object} options.browserId
* @param {Object} options.context
* @param {Object} options.browsingContextId: Optional browsing context id to narrow the
* search to a specific browsing context.
*
* @returns {WindowGlobalTargetActor|null}
*/
_findTargetActor({ watcherActorID, browserId, browsingContextId }) {
_findTargetActor({ watcherActorID, context, browsingContextId }) {
// First let's check if a target was created for this watcher actor in this specific
// DevToolsFrameChild instance.
const connectionInfo = this._connections.get(watcherActorID);
@ -543,13 +550,16 @@ class DevToolsFrameChild extends JSWindowActorChild {
// (watcherActorId,browserId, {browsingContextId}) in another DevToolsFrameChild instance.
// This might be the case if we're navigating to a new page with server side target
// enabled and we want to retrieve the target of the page we're navigating from.
if (!targetActor && this.manager.browsingContext.browserId == browserId) {
if (
!targetActor &&
this.manager.browsingContext.browserId == context.browserId
) {
// Ensure retrieving the one target actor related to this connection.
// This allows to distinguish actors created for various toolboxes.
// For ex, regular toolbox versus browser console versus browser toolbox
const connectionPrefix = watcherActorID.replace(/watcher\d+$/, "");
const targetActors = TargetActorRegistry.getTargetActors(
browserId,
context.browserId,
connectionPrefix
);
@ -565,30 +575,30 @@ class DevToolsFrameChild extends JSWindowActorChild {
return targetActor;
}
_addSessionDataEntry(watcherActorID, browserId, type, entries) {
_addSessionDataEntry(watcherActorID, context, type, entries) {
// /!\ We may have an issue here as there could be multiple targets for a given
// (watcherActorID,browserId) pair.
// This should be clarified as part of Bug 1725623.
const targetActor = this._findTargetActor({
watcherActorID,
browserId,
context,
});
if (!targetActor) {
throw new Error(
`No target actor for this Watcher Actor ID:"${watcherActorID}" / BrowserId:${browserId}`
`No target actor for this Watcher Actor ID:"${watcherActorID}" / BrowserId:${context.browserId}`
);
}
return targetActor.addSessionDataEntry(type, entries);
}
_removeSessionDataEntry(watcherActorID, browserId, type, entries) {
_removeSessionDataEntry(watcherActorID, context, type, entries) {
// /!\ We may have an issue here as there could be multiple targets for a given
// (watcherActorID,browserId) pair.
// This should be clarified as part of Bug 1725623.
const targetActor = this._findTargetActor({
watcherActorID,
browserId,
context,
});
// By the time we are calling this, the target may already have been destroyed.
if (targetActor) {
@ -656,14 +666,14 @@ class DevToolsFrameChild extends JSWindowActorChild {
// It may not be the case if one Watcher isn't having server target switching enabled.
let allActorsAreDestroyed = true;
for (const [watcherActorID, sessionData] of sessionDataByWatcherActor) {
const { browserId, isServerTargetSwitchingEnabled } = sessionData;
const { context, isServerTargetSwitchingEnabled } = sessionData;
// /!\ We may have an issue here as there could be multiple targets for a given
// (watcherActorID,browserId) pair.
// This should be clarified as part of Bug 1725623.
const existingTarget = this._findTargetActor({
watcherActorID,
browserId,
context,
});
if (!existingTarget) {

View File

@ -55,32 +55,32 @@ class DevToolsFrameParent extends JSWindowActorParent {
async instantiateTarget({
watcherActorID,
connectionPrefix,
browserId,
context,
sessionData,
}) {
await this.sendQuery("DevToolsFrameParent:instantiate-already-available", {
watcherActorID,
connectionPrefix,
browserId,
context,
sessionData,
});
}
destroyTarget({ watcherActorID, browserId }) {
destroyTarget({ watcherActorID, context }) {
this.sendAsyncMessage("DevToolsFrameParent:destroy", {
watcherActorID,
browserId,
context,
});
}
/**
* Communicate to the content process that some data have been added.
*/
async addSessionDataEntry({ watcherActorID, browserId, type, entries }) {
async addSessionDataEntry({ watcherActorID, context, type, entries }) {
try {
await this.sendQuery("DevToolsFrameParent:addSessionDataEntry", {
watcherActorID,
browserId,
context,
type,
entries,
});
@ -96,10 +96,10 @@ class DevToolsFrameParent extends JSWindowActorParent {
/**
* Communicate to the content process that some data have been removed.
*/
removeSessionDataEntry({ watcherActorID, browserId, type, entries }) {
removeSessionDataEntry({ watcherActorID, context, type, entries }) {
this.sendAsyncMessage("DevToolsFrameParent:removeSessionDataEntry", {
watcherActorID,
browserId,
context,
type,
entries,
});

View File

@ -119,10 +119,10 @@ class DevToolsWorkerChild extends JSWindowActorChild {
// Create one Target actor for each prefix/client which listen to workers
for (const [watcherActorID, sessionData] of sessionDataByWatcherActor) {
const { targets, connectionPrefix, browserId } = sessionData;
const { targets, connectionPrefix, context } = sessionData;
if (
targets.includes("worker") &&
shouldNotifyWindowGlobal(this.manager, browserId)
shouldNotifyWindowGlobal(this.manager, context)
) {
this._watchWorkerTargets({
watcherActorID,
@ -141,15 +141,15 @@ class DevToolsWorkerChild extends JSWindowActorChild {
* @param {*} message.data
*/
receiveMessage(message) {
// All messages pass `browserId` (except packet) and are expected
// All messages pass `context` (except packet) and are expected
// to match shouldNotifyWindowGlobal result.
if (message.name != "DevToolsWorkerParent:packet") {
const { browserId } = message.data;
const { browserId } = message.data.context;
// Re-check here, just to ensure that both parent and content processes agree
// on what should or should not be watched.
if (
this.manager.browsingContext.browserId != browserId &&
!shouldNotifyWindowGlobal(this.manager, browserId)
!shouldNotifyWindowGlobal(this.manager, message.data.context)
) {
throw new Error(
"Mismatch between DevToolsWorkerParent and DevToolsWorkerChild " +
@ -193,7 +193,7 @@ class DevToolsWorkerChild extends JSWindowActorChild {
/**
* Instantiate targets for existing workers, watch for worker registration and listen
* for resources on those workers, for given connection and browserId. Targets are sent
* for resources on those workers, for given connection and context. Targets are sent
* to the DevToolsWorkerParent via the DevToolsWorkerChild:workerTargetAvailable message.
*
* @param {Object} options
@ -527,12 +527,15 @@ class DevToolsWorkerChild extends JSWindowActorChild {
/**
* Helper function to know if we should watch for workers on a given windowGlobal
*/
function shouldNotifyWindowGlobal(windowGlobal, watchedBrowserId) {
function shouldNotifyWindowGlobal(windowGlobal, context) {
const browsingContext = windowGlobal.browsingContext;
// If we are focusing only on a sub-tree of Browsing Element, ignore elements that are
// not part of it.
if (watchedBrowserId && browsingContext.browserId != watchedBrowserId) {
if (
context.type == "browser-element" &&
browsingContext.browserId != context.browserId
) {
return false;
}

View File

@ -48,13 +48,13 @@ class DevToolsWorkerParent extends JSWindowActorParent {
}
/**
* Request the content process to create Worker Targets if workers matching the browserId
* Request the content process to create Worker Targets if workers matching the context
* are already available.
*/
async instantiateWorkerTargets({
watcherActorID,
connectionPrefix,
browserId,
context,
sessionData,
}) {
try {
@ -63,7 +63,7 @@ class DevToolsWorkerParent extends JSWindowActorParent {
{
watcherActorID,
connectionPrefix,
browserId,
context,
sessionData,
}
);
@ -78,20 +78,21 @@ class DevToolsWorkerParent extends JSWindowActorParent {
}
}
destroyWorkerTargets({ watcher, browserId }) {
destroyWorkerTargets({ watcherActorID, context }) {
return this.sendAsyncMessage("DevToolsWorkerParent:destroy", {
watcherActorID: watcher.actorID,
browserId,
watcherActorID: watcherActorID,
context,
});
}
/**
* Communicate to the content process that some data have been added.
*/
async addSessionDataEntry({ watcherActorID, type, entries }) {
async addSessionDataEntry({ watcherActorID, context, type, entries }) {
try {
await this.sendQuery("DevToolsWorkerParent:addSessionDataEntry", {
watcherActorID,
context,
type,
entries,
});
@ -109,9 +110,10 @@ class DevToolsWorkerParent extends JSWindowActorParent {
/**
* Communicate to the content process that some data have been removed.
*/
removeSessionDataEntry({ watcherActorID, type, entries }) {
removeSessionDataEntry({ watcherActorID, context, type, entries }) {
this.sendAsyncMessage("DevToolsWorkerParent:removeSessionDataEntry", {
watcherActorID,
context,
type,
entries,
});