Bug 1721823 - [devtools] Make isServerTargetSwitchingEnabled be a runtime flag from server standpoint. r=jdescottes,devtools-backward-compat-reviewers

This helps enabling server targets only for local tabs descriptors
and so prevents enabling them for about:debugging toolboxes.
They are always using remote tabs descriptors and do not support target switching yet.

Differential Revision: https://phabricator.services.mozilla.com/D120629
This commit is contained in:
Alexandre Poirot 2021-07-27 15:23:24 +00:00
parent a455b36b98
commit cf218bbc87
9 changed files with 142 additions and 106 deletions

View File

@ -83,6 +83,12 @@ class TabDescriptorFront extends DescriptorMixin(
super.destroy();
}
getWatcher() {
return super.getWatcher({
isServerTargetSwitchingEnabled: this.isServerTargetSwitchingEnabled(),
});
}
setLocalTab(localTab) {
this._localTab = localTab;
this._setupLocalTabListeners();

View File

@ -179,9 +179,12 @@ const TabDescriptorActor = ActorClassWithSpec(tabDescriptorSpec, {
* already exists or will be created. It also helps knowing when they
* are destroyed.
*/
getWatcher() {
getWatcher(config) {
if (!this.watcher) {
this.watcher = new WatcherActor(this.conn, { browser: this._browser });
this.watcher = new WatcherActor(this.conn, {
browser: this._browser,
config,
});
this.manage(this.watcher);
}
return this.watcher;

View File

@ -43,6 +43,16 @@ class ParentProcessStorage {
// that got removed
Services.obs.addObserver(this, "window-global-created");
Services.obs.addObserver(this, "window-global-destroyed");
// bfcacheInParent is only enabled when fission is enabled
// and when Session History In Parent is enabled. (all three modes should now enabled all together)
loader.lazyGetter(
this,
"isBfcacheInParentEnabled",
() =>
Services.appinfo.sessionHistoryInParent &&
Services.prefs.getBoolPref("fission.bfcacheInParent", false)
);
}
async watch(watcherActor, { onAvailable }) {
@ -54,8 +64,7 @@ class ParentProcessStorage {
// In such case, the watcher emits specific events that we can use instead.
this._offPageShow = watcherActor.on(
"bf-cache-navigation-pageshow",
({ windowGlobal, isNewTargetCreated }) =>
this._onNewWindowGlobal(windowGlobal, isNewTargetCreated)
({ windowGlobal }) => this._onNewWindowGlobal(windowGlobal, true)
);
const {
@ -177,9 +186,9 @@ class ParentProcessStorage {
* - <bf-cache-navigation-pageshow> (to cover history navications)
*
* @param {WindowGlobal} windowGlobal
* @param {Boolean} isBfCacheNavigationCreatingNewTarget
* @param {Boolean} isBfCacheNavigation
*/
async _onNewWindowGlobal(windowGlobal, isBfCacheNavigationCreatingNewTarget) {
async _onNewWindowGlobal(windowGlobal, isBfCacheNavigation) {
// If the watcher is bound to one browser element (i.e. a tab), ignore
// windowGlobals related to other browser elements
if (
@ -206,7 +215,8 @@ class ParentProcessStorage {
// - target switching is enabled OR bfCacheInParent is enabled, and a bfcache navigation
// is performed (See handling of "pageshow" event in DevToolsFrameChild)
const isNewTargetBeingCreated =
isTargetSwitchingEnabled() || isBfCacheNavigationCreatingNewTarget;
this.watcherActor.isServerTargetSwitchingEnabled ||
(isBfCacheNavigation && this.isBfcacheInParentEnabled);
if (!isNewTargetBeingCreated) {
return;
@ -264,14 +274,15 @@ class StorageActorMock extends EventEmitter {
// We only need to react to those events here if target switching is not enabled; when
// it is enabled, ParentProcessStorage will spawn a whole new actor which will allow
// the client to get the information it needs.
if (!isTargetSwitchingEnabled()) {
if (!this.watcherActor.isServerTargetSwitchingEnabled) {
this._offPageShow = watcherActor.on(
"bf-cache-navigation-pageshow",
({ windowGlobal, isNewTargetCreated }) => {
({ windowGlobal }) => {
// if a new target is created in the content process as a result of the bfcache
// navigation, we don't need to emit window-ready as a new StorageActorMock will
// be created by ParentProcessStorage.
if (isNewTargetCreated) {
// When server targets are disabled, this only happens when bfcache in parent is enabled.
if (this.isBfcacheInParentEnabled) {
return;
}
const windowMock = { location: windowGlobal.documentURI };
@ -383,12 +394,8 @@ class StorageActorMock extends EventEmitter {
// Only notify about remote iframe windows when JSWindowActor based targets are enabled
// We will create a new StorageActor for the top level tab documents when server side target
// switching is enabled
const isTargetSwitching = Services.prefs.getBoolPref(
"devtools.target-switching.server.enabled",
false
);
const isTopContext = subject.browsingContext.top == subject.browsingContext;
if (isTopContext && isTargetSwitching) {
if (isTopContext && this.watcherActor.isServerTargetSwitchingEnabled) {
return;
}
@ -538,10 +545,3 @@ class StorageActorMock extends EventEmitter {
return null;
}
}
function isTargetSwitchingEnabled() {
return Services.prefs.getBoolPref(
"devtools.target-switching.server.enabled",
false
);
}

View File

@ -65,6 +65,7 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
initialize: function(conn, options) {
protocol.Actor.prototype.initialize.call(this, conn);
this._browser = options && options.browser;
this._config = options ? options.config : {};
this.notifyResourceAvailable = this.notifyResourceAvailable.bind(this);
this.notifyResourceDestroyed = this.notifyResourceDestroyed.bind(this);
@ -93,6 +94,10 @@ exports.WatcherActor = protocol.ActorClassWithSpec(watcherSpec, {
return this._browser?.browserId;
},
get isServerTargetSwitchingEnabled() {
return !!this._config.isServerTargetSwitchingEnabled;
},
destroy: function() {
// Force unwatching for all types, even if we weren't watching.
// This is fine as unwatchTarget is NOOP if we weren't already watching for this target type.

View File

@ -121,6 +121,8 @@ const WatcherRegistry = {
// Expose watcher traits so we can retrieve them in content process.
// This should be removed as part of Bug 1700092.
watcherTraits: watcher.form().traits,
// Expose to the content process if we should create top level targets from the server side
isServerTargetSwitchingEnabled: watcher.isServerTargetSwitchingEnabled,
};
// Define empty default array for all data
for (const name of Object.values(SUPPORTED_DATA)) {

View File

@ -19,19 +19,6 @@ const {
const browsingContextAttachedObserverByWatcher = new Map();
// Note: this preference should be read from the client and propagated to the
// server. However since target switching is only supported for local-tab
// debugging scenarios, it is acceptable to temporarily read it both on the
// client and server until we can just enable it by default.
// Do not use a lazy getter in order to help test toggle this pref on/off
// and have it to live update here.
function isServerTargetSwitchingEnabled() {
return Services.prefs.getBoolPref(
"devtools.target-switching.server.enabled",
false
);
}
/**
* Force creating targets for all existing BrowsingContext, that, for a given Watcher Actor.
*
@ -83,7 +70,7 @@ async function createTargets(watcher) {
);
}
if (isServerTargetSwitchingEnabled() && watcher.browserElement) {
if (watcher.isServerTargetSwitchingEnabled && watcher.browserElement) {
// If server side target switching is enabled, process the top level browsing context first,
// so that we guarantee it is notified to the client first.
// If it is disabled, the top level target will be created from the client instead.
@ -147,7 +134,7 @@ function destroyTargets(watcher) {
const browsingContexts = getFilteredRemoteBrowsingContext(
watcher.browserElement
);
if (isServerTargetSwitchingEnabled() && watcher.browserElement) {
if (watcher.isServerTargetSwitchingEnabled && watcher.browserElement) {
// If server side target switching is enabled, we should also destroy the top level browsing context.
// If it is disabled, the top level target will be destroyed from the client instead.
browsingContexts.push(watcher.browserElement.browsingContext);

View File

@ -116,20 +116,9 @@ class DevToolsFrameChild extends JSWindowActorChild {
this._onConnectionChange = this._onConnectionChange.bind(this);
EventEmitter.decorate(this);
// Set the following preferences on the constructor, so that we can easily
// Set the following preference on the constructor, so that we can easily
// toggle these preferences on and off from tests and have the new value being picked up.
// Note: this preference should be read from the client and propagated to the
// server. However since target switching is only supported for local-tab
// debugging scenarios, it is acceptable to temporarily read it both on the
// client and server until we can just enable it by default.
XPCOMUtils.defineLazyGetter(this, "isServerTargetSwitchingEnabled", () =>
Services.prefs.getBoolPref(
"devtools.target-switching.server.enabled",
false
)
);
// bfcache-in-parent changes significantly how navigation behaves.
// We may start reusing previously existing WindowGlobal and so reuse
// previous set of JSWindowActor pairs (i.e. DevToolsFrameParent/DevToolsFrameChild).
@ -146,7 +135,21 @@ class DevToolsFrameChild extends JSWindowActorChild {
);
}
instantiate({ forceOverridingFirstTarget = false } = {}) {
/**
* Try to instantiate new target actors for the current WindowGlobal, and that,
* for all the currently registered Watcher actors.
*
* Read the `sharedData` to get metadata about all registered watcher actors.
* If these watcher actors are interested in the current WindowGlobal,
* instantiate a new dedicated target actor for each of the watchers.
*
* @param Object options
* @param String options.isBFCache
* True, if the request to instantiate a new target comes from a bfcache navigation.
* i.e. when we receive a pageshow event with persisted=true.
* This will be true regardless of bfcacheInParent being enabled or disabled.
*/
instantiate({ isBFCache = false } = {}) {
const { sharedData } = Services.cpmm;
const watchedDataByWatcherActor = sharedData.get(SHARED_DATA_KEY_NAME);
if (!watchedDataByWatcherActor) {
@ -157,12 +160,23 @@ class DevToolsFrameChild extends JSWindowActorChild {
// Create one Target actor for each prefix/client which listen to frames
for (const [watcherActorID, watchedData] of watchedDataByWatcherActor) {
const { connectionPrefix, browserId } = watchedData;
const {
connectionPrefix,
browserId,
isServerTargetSwitchingEnabled,
} = watchedData;
// Always create new targets when server targets are enabled as we create targets for all the WindowGlobal's,
// including all WindowGlobal's of the top target.
// For bfcache navigations, we only create new targets when bfcacheInParent is enabled,
// as this would be the only case where new DocShells will be created. This requires us to spawn a
// new BrowsingContextTargetActor as one such actor is bound to a unique DocShell.
const acceptTopLevelTarget =
isServerTargetSwitchingEnabled ||
(isBFCache && this.isBfcacheInParentEnabled);
if (
watchedData.targets.includes("frame") &&
shouldNotifyWindowGlobal(this.manager, browserId, {
acceptTopLevelTarget:
forceOverridingFirstTarget || this.isServerTargetSwitchingEnabled,
acceptTopLevelTarget,
})
) {
// Bail if there is already an existing BrowsingContextTargetActor.
@ -178,12 +192,11 @@ class DevToolsFrameChild extends JSWindowActorChild {
// unless this target was created from a JSWindowActor target. The old JSWindowActor
// target will still be around for a short time when the new one is
// created.
// Also force overriding the first message manager based target in case of BFCache
// which will set forceOverridingFirstTarget=true
// Also force overriding the first message manager based target in case of BFCache navigations
if (
existingTarget &&
!existingTarget.createdFromJsWindowActor &&
!forceOverridingFirstTarget
!isBFCache
) {
return;
}
@ -528,13 +541,6 @@ class DevToolsFrameChild extends JSWindowActorChild {
return;
}
// Also handle bfcache navigation with new targets when server side target switching is enabled,
// even if bfcacheinParent is false. That's because when isServerTargetSwitchingEnabled=true,
// targets follow WindowGlobal lifecycle and won't emit navigate event, nor debug the page
// restored from the bfcache
const shouldHandleBfCacheEvents =
this.isBfcacheInParentEnabled || this.isServerTargetSwitchingEnabled;
// DOMWindowCreated is registered from FrameWatcher via `ActorManagerParent.addJSWindowActors`
// as a DOM event to be listened to and so is fired by JS Window Actor code platform code.
if (type == "DOMWindowCreated") {
@ -542,65 +548,91 @@ class DevToolsFrameChild extends JSWindowActorChild {
return;
}
// If persisted=true, this is a BFCache navigation.
//
// With Fission enabled and bfcacheInParent, BFCache navigations will spawn a new DocShell
// in the same process:
// * the previous page won't be destroyed, its JSWindowActor will stay alive (`didDestroy` won't be called)
// and a 'pagehide' with persisted=true will be emitted on it.
// * the new page page won't emit any DOMWindowCreated, but instead a pageshow with persisted=true
// will be emitted.
if (type === "pageshow" && persisted) {
this.sendAsyncMessage("DevToolsFrameChild:bf-cache-navigation-pageshow", {
isNewTargetCreated: shouldHandleBfCacheEvents,
// Notify all bfcache navigations, even the one for which we don't create a new target
// as that's being useful for parent process storage resource watchers.
this.sendAsyncMessage("DevToolsFrameChild:bf-cache-navigation-pageshow");
// Here we are going to re-instantiate a target that got destroyed before while processing a pagehide event.
// We force instantiating a new top level target, within `instantiate()` even if server targets are disabled.
// But we only do that if bfcacheInParent is enabled. Otherwise for same-process, same-docshell bfcache navigation,
// we don't want to spawn new targets.
this.instantiate({
isBFCache: true,
});
if (shouldHandleBfCacheEvents) {
// If persisted=true, this is a BFCache navigation.
// With Fission enabled, BFCache navigation will spawn a new DocShell
// in the same process:
// * the previous page won't be destroyed, its JSWindowActor will stay alive (didDestroy won't be called)
// and a pagehide with persisted=true will be emitted on it.
// * the new page page won't emit any DOMWindowCreated, but instead a pageshow with persisted=true
// will be emitted.
this.instantiate({ forceOverridingFirstTarget: true });
}
return;
}
if (type === "pagehide" && persisted) {
// Notify all bfcache navigations, even the one for which we don't create a new target
// as that's being useful for parent process storage resource watchers.
this.sendAsyncMessage("DevToolsFrameChild:bf-cache-navigation-pagehide");
if (shouldHandleBfCacheEvents) {
// We might navigate away for the first top level target,
// which isn't using JSWindowActor (it still uses messages manager and is created by the client, via TabDescriptor.getTarget).
// We have to unregister it from the TargetActorRegistry, otherwise,
// if we navigate back to it, the next DOMWindowCreated won't create a new target for it.
const { sharedData } = Services.cpmm;
const watchedDataByWatcherActor = sharedData.get(SHARED_DATA_KEY_NAME);
if (!watchedDataByWatcherActor) {
throw new Error(
"Request to instantiate the target(s) for the BrowsingContext, but `sharedData` is empty about watched targets"
);
// We might navigate away for the first top level target,
// which isn't using JSWindowActor (it still uses messages manager and is created by the client, via TabDescriptor.getTarget).
// We have to unregister it from the TargetActorRegistry, otherwise,
// if we navigate back to it, the next DOMWindowCreated won't create a new target for it.
const { sharedData } = Services.cpmm;
const watchedDataByWatcherActor = sharedData.get(SHARED_DATA_KEY_NAME);
if (!watchedDataByWatcherActor) {
throw new Error(
"Request to instantiate the target(s) for the BrowsingContext, but `sharedData` is empty about watched targets"
);
}
const actors = [];
// A flag to know if the following for loop ended up destroying all the actors.
// It may not be the case if one Watcher isn't having server target switching enabled.
let allActorsAreDestroyed = true;
for (const [watcherActorID, watchedData] of watchedDataByWatcherActor) {
const { browserId, isServerTargetSwitchingEnabled } = watchedData;
const existingTarget = this._getTargetActorForWatcherActorID(
watcherActorID,
browserId
);
if (!existingTarget) {
continue;
}
if (existingTarget.window.document != target) {
throw new Error("Existing target actor is for a distinct document");
}
// Do not do anything if both bfcache in parent and server targets are disabled
// As history navigations will be handled within the same DocShell and by the
// same BrowsingContextTargetActor. The actor will listen to pageshow/pagehide by itself.
// We should not destroy any target.
if (!this.isBfcacheInParentEnabled && !isServerTargetSwitchingEnabled) {
allActorsAreDestroyed = false;
continue;
}
const actors = [];
for (const [watcherActorID, watchedData] of watchedDataByWatcherActor) {
const { browserId } = watchedData;
const existingTarget = this._getTargetActorForWatcherActorID(
watcherActorID,
browserId
);
actors.push({
watcherActorID,
form: existingTarget.form(),
});
existingTarget.destroy();
}
if (!existingTarget) {
continue;
}
if (existingTarget.window.document != target) {
throw new Error("Existing target actor is for a distinct document");
}
actors.push({ watcherActorID, form: existingTarget.form() });
existingTarget.destroy();
}
if (actors.length > 0) {
// The most important is to unregister the actor from TargetActorRegistry,
// so that it is no longer present in the list when new DOMWindowCreated fires.
// This will also help notify the client that the target has been destroyed.
// And if we navigate back to this target, the client will receive the same target actor ID,
// so that it is really important to destroy it correctly on both server and client.
this.sendAsyncMessage("DevToolsFrameChild:destroy", { actors });
}
if (allActorsAreDestroyed) {
// Completely clear this JSWindow Actor.
// Do this after having called _getTargetActorForWatcherActorID,
// as it would clear the registered target actors.

View File

@ -232,7 +232,6 @@ class DevToolsFrameParent extends JSWindowActorParent {
)) {
watcherActor.emit("bf-cache-navigation-pageshow", {
windowGlobal: this.browsingContext.currentWindowGlobal,
isNewTargetCreated: message.data.isNewTargetCreated,
});
}
return null;

View File

@ -26,7 +26,9 @@ const tabDescriptorSpec = generateActorSpec({
},
},
getWatcher: {
request: {},
request: {
isServerTargetSwitchingEnabled: Option(0, "boolean"),
},
response: RetVal("watcher"),
},
reloadDescriptor: {