Support multiple user script worlds in the user scripts API

[rdcronin]

The user scripts API allows scripts to inject in a new "user script" world (in addition to the main world), which has slightly different properties than the isolated world content scripts inject in. From early discussions (e.g. this comment from issue #279 ), we have supported the idea of multiple user script worlds.

This issue tracks that work.

[Rob--W]

I am currently working on the implementation of multiple user script worlds in Firefox, and while examining Chrome's implementation more closely, I found an issue with the currently specified/implemented behavior for worldId.

The current spec is at https://github.com/w3c/webextensions/blob/main/proposals/multiple_user_script_worlds.md and describes worldId as follows:

 /**
+     * If specified, specifies a specific user script world ID to execute in.
+     * Only valid if `world` is omitted or is `USER_SCRIPT`. If `worldId` is
+     * omitted, the script will execute in the default user script world.
+     * Values with leading underscores (`_`) are reserved.
+     */
+    worldId?: string;

This description (and the spec in general) does not specify the worldId to use for the "default user script world".

Chrome's current implementation seems to use null for that. This causes issues in cases where null is ambiguous: the userScripts.update method ought to replace some properties, but because null is interpreted as "not explicitly specified", it is not possible to change worldId back to the default world.

Example (requires starting Chrome with --enable-features=ApiUserScriptsMultipleWorlds):

await chrome.userScripts.register([{
  id: "id",
  js: [{ code: "alert(0)" }],
  matches: ["https://example.com/*"],
  worldId: "x",
}]);
await chrome.userScripts.update([{
  id: "id",
  // Also weird: Chrome requires js to be set in update()
  js: [{ code: "alert(1)" }],
  worldId: null, // intent to change to default world
}]);
var scripts = await chrome.userScripts.getScripts({ ids: ["id"] });
console.log(scripts[0].worldId);
// Expected: null (default?)
// Actual: "x" (unchanged)

If we want to keep the default at null, it is technically possible to distinguish "property set" (=set to null) from "property not set" (= do not update) in Firefox's implementation. However, this is unusual and I would prefer a different solution that is easier to explain.

Last time we discussed the worldId format, I mentioned two options at #560 (comment)

• Defaulting to "" (empty string). Chrome's implementation currently throws.
• Using a constant such as "_DEFAULT". Devlin was not fond of exposing _.

Given this all, I propose to establish "" as the value for the default world's worldId.

[rdcronin]

Thanks, @Rob--W ! Agreed this is something we should fix, and I'm supportive of using the empty string ("") to represent the default world. If there are no objections, I'll make the change in Chromium.

[Rob--W]

Thanks, @Rob--W ! Agreed this is something we should fix, and I'm supportive of using the empty string ("") to represent the default world. If there are no objections, I'll make the change in Chromium.

Looks like this change was made in https://issues.chromium.org/issues/331680187#comment22

[Rob--W]

Another aspect that becomes particularly more relevant with multiple worlds is what should happen when userScripts.configureWorld() is invoked when a field is omitted. Firefox only updates non-null fields, whereas Chrome resets fields that were not specified explicitly. This odd behavior was questioned in https://issues.chromium.org/issues/40938749 before ("This is not great, because configureWorld has other properties and the extension may call it to update those thinking the csp will persist.").

Considering that there is now a dedicated resetWorldConfigurations method, it would make sense for configureWorld() to only update non-null fields, and to recommend resetWorldConfigurations() if developers want to reset to the defaults.

See the following example. Note that I pass a worldId in this example, but the same issue also occurs when worldId is not specified.

// Relax CSP:
await browser.userScripts.configureWorld({ worldId: "hey", csp: "script-src 'unsafe-eval'" });
// Enable messaging:
await browser.userScripts.configureWorld({ worldId: "hey", messaging: true });

// Check whether CSP and messaging have been configured:
console.log(await browser.userScripts.getWorldConfigurations());
// Chrome: [{ messaging: true, worldId: "hey" }]  // csp is gone!
// Firefox: [{ worldId: "hey", csp: "script-src 'unsafe-eval'", messaging: true }]

[Rob--W]

I chatted with @rdcronin and he agreed with the behavior proposed above (not overwriting properties that are omitted/null).

[Rob--W]

I chatted with @rdcronin and he agreed with the behavior proposed above (not overwriting properties that are omitted/null).

... and that has been implemented at https://issues.chromium.org/issues/40938749#comment7

I have been working on creating a demo extension to demonstrate the use of the userScripts API with multiple worlds, and based on that I have some feedback:

• resetWorldConfiguration(null) is currently implemented in Chrome (and in Firefox because I followed Chrome's implementation) to reset the default world. The current proposal text specifies worldId as non-optional:

  webextensions/proposals/multiple_user_script_worlds.md

  Line 168 in 17d7b97

  + export function resetWorldConfiguration(worldId: string): Promise<void>;

  
  I guess that Chrome implemented it as optional because null was originally the ID of the default world. Now that the default world is represented by "" (empty string), I think that it would make sense to reset ALL worlds instead of just the default world when resetWorldConfiguration() is called without a worldId. If one wants to reset the default world only, they can pass resetWorldConfiguration(""). This would be internally more consistent with userScripts.unregister(), which unregisters all scripts, unless a filter with specific worldIds is passed.

• configureWorld and resetWorldConfiguration can currently modify one world only. While it is possible for extensions to iterate over each worldId and call these methods, I wonder whether we should change the signature to accept multiple world configurations. World configurations propagate to all renderer processes and are persisted in storage, so the ability to bulk-edit them could enable optimizations and faster processing by the browser.
  configureWorld() has already shipped, so for that to be backcompat, the signature would become configureWorld(WorldProperties | WorldProperties[]) instead of configureWorld(WorldProperties).
  For resetWorldConfiguration, it could be string[] or string | string[] or even an object { worldIds: string[] }.

Firefox has not enabled the userScripts API by default yet, so if Chrome is willing to change these behaviors, I would be willing to make the same changes in Firefox.

[rdcronin]

Now that the default world is represented by "" (empty string), I think that it would make sense to reset ALL worlds instead of just the default world when resetWorldConfiguration() is called without a worldId. If one wants to reset the default world only, they can pass resetWorldConfiguration("")

I'm opposed to this change. Everywhere else, an empty worldId corresponds to the default world, and I think it would be confusing to have this handle that differently. If we wanted to support resetting all worlds (to save extensions from needing to call get getWorldConfigurations and iterate over them), I would much prefer we just add a new resetAllWorldConfigurations() method. (I'm not opposed to that, but also don't think that extensions are going to be resetting all existing worlds so frequently that it's a flow that needs to be optimized. I'm open to being persuaded otherwise.)

configureWorld and resetWorldConfiguration can currently modify one world only. While it is possible for extensions to iterate over each worldId and call these methods, I wonder whether we should change the signature to accept multiple world configurations.

Similar to above, I don't know how often extensions are going to be bulk-editing these worlds. I'm not opposed to relaxing the signature to take a single-or-an-array, but I'd want to see some concrete data on how often that happens. If this is relatively rare, there's not going to be a meaningful performance benefit by batching the calls vs serially calling them (and, in Chrome, we'd probably just do a serial update anyway, unless we found that these were on a "hot path" frequently). Since adding an array option to the signature is backwards compatible (and since configureWorld has already shipped), I'd propose we hold off on these as a potential future enhancement if we see compelling data indicating their value.

WDYT?

[Rob--W]

Preface: I am fine with either outcome; I merely wanted to double-check that this is the direction that we want with the API before shipping (and then being locked into the API signatures).

Now that the default world is represented by "" (empty string), I think that it would make sense to reset ALL worlds instead of just the default world when resetWorldConfiguration() is called without a worldId. If one wants to reset the default world only, they can pass resetWorldConfiguration("")

I'm opposed to this change. Everywhere else, an empty worldId corresponds to the default world, and I think it would be confusing to have this handle that differently.

The way I look at it, an empty worldId means unspecified, and the default value is filled in, which is "". Just like an empty runAt being mapped to document_idle, an empty world being mapped to USER_SCRIPT, etc.

In Firefox, when userScripts.getScripts() is called, the return value includes all default values where not specified.
When userScripts.getWorldConfigurations() is called, the return value includes worldId: "" if the worldId was not passed to userScripts.configureWorld().

If we wanted to support resetting all worlds (to save extensions from needing to call get getWorldConfigurations and iterate over them), I would much prefer we just add a new resetAllWorldConfigurations() method. (I'm not opposed to that, but also don't think that extensions are going to be resetting all existing worlds so frequently that it's a flow that needs to be optimized. I'm open to being persuaded otherwise.)

I'd rather not introduce yet another method. Like you said, extensions can iterate over getWorldConfigurations() and remove all if they really want to.

configureWorld and resetWorldConfiguration can currently modify one world only. While it is possible for extensions to iterate over each worldId and call these methods, I wonder whether we should change the signature to accept multiple world configurations.

Similar to above, I don't know how often extensions are going to be bulk-editing these worlds. I'm not opposed to relaxing the signature to take a single-or-an-array, but I'd want to see some concrete data on how often that happens.

I expect proper user script managers to generate a new world for each script to achieve full isolation between individual user scripts to minimize conflicts (and avoid the need to somehow capture and sanitize prototypes). Thus, as part of registering N scripts, there would be up to N world configurations (minus scripts that run in the main world, which is the default in user script managers).

An alternative approach to avoid the need to configure worlds individually is to just configure the default world (with enableMessaging:true), and let each world do its own thing. With enableMessaging that works, but if there are ever more significant world properties, then at that point bulk-editing would allow for quick initialization.

I'm not opposed to tabling support for bulk-editing world configurations. I just noticed the inconsistency in these methods taking one input only, whereas userScripts.register/update/remove methods all support bulk-calls.

In Firefox I have currently not implemented world limits. In Chrome, I would imagine extensions to want to know whether they are hitting a limit, and perhaps regroup scripts by worldId based on availability. For this, bulk-editing would be useful, to quickly tell whether a proposed configuration would fit within the limits.

[rdcronin]

The way I look at it, an empty worldId means unspecified, and the default value is filled in, which is "". Just like an empty runAt being mapped to document_idle, an empty world being mapped to USER_SCRIPT, etc.

I agree. And here, if omitted, it should use the default value, which is for the default world. : ) (runAt being omitted defaults to document_idle, not to running in all worlds)

I'd rather not introduce yet another method. Like you said, extensions can iterate over getWorldConfigurations() and remove all if they really want to.

I'm definitely fine holding off on this and revisiting in the future.

Thus, as part of registering N scripts, there would be up to N world configurations (minus scripts that run in the main world, which is the default in user script managers).

I agree. However, my expectation is also that, most of the time, users are (un)installing one user script at a time, so the delta of registration is still 1 world. There are exceptions to this -- e.g., user script managers that may support sync state or otherwise need to reinstantiate state; however, I expect these to be relatively rare (on new machines, after updates, etc), where performing a serial update flow is not going to meaningfully affect performance.

In Chrome, I would imagine extensions to want to know whether they are hitting a limit, and perhaps regroup scripts by worldId based on availability. For this, bulk-editing would be useful, to quickly tell whether a proposed configuration would fit within the limits.

Yes, this is one case I do think can potentially happen, and could theoretically benefit from batched updates. However, I'd like to wait for data before we make that optimization. We should determine both how often this happens as well as if bulk-registering/unregistering worlds is the approach managers would want to take (I think a better approach for most use cases would be to instead intelligently group user scripts so that multiple can run in the same world if necessary. This isn't new; I think every major browser does this today for e.g. tab grouping in processing. It's nice when you can have everything in its own process, but resource constraints require letting grouping things in different circumstances).

Preface: I am fine with either outcome; I merely wanted to double-check that this is the direction that we want with the API before shipping (and then being locked into the API signatures).

Understood, and it's good to discuss! It seems like we're okay with the following:

• Table discussion around bulk updates. These can be made in a backwards-compatible way in a future change if we evaluate them to be useful.
• Have resetWorldConfiguration() (with no world ID) continue to reset the main world, not all worlds. We can revisit resetting all worlds if we see compelling use cases (agreed this would lock us into not being able to reset all worlds with the same resetWorldConfiguration() method and would either need to say it should be done through bulk-updates -- which still requires knowing all the world configurations -- or through introducing a new method).

Are you good with that, Rob?

[Rob--W]

Preface: I am fine with either outcome; I merely wanted to double-check that this is the direction that we want with the API before shipping (and then being locked into the API signatures).

Understood, and it's good to discuss! It seems like we're okay with the following:

• Table discussion around bulk updates. These can be made in a backwards-compatible way in a future change if we evaluate them to be useful.
• Have resetWorldConfiguration() (with no world ID) continue to reset the main world, not all worlds. We can revisit resetting all worlds if we see compelling use cases (agreed this would lock us into not being able to reset all worlds with the same resetWorldConfiguration() method and would either need to say it should be done through bulk-updates -- which still requires knowing all the world configurations -- or through introducing a new method).

Are you good with that, Rob?

SGTM!

I see potential issues with the lack of control over the world that the script is executing in when a world limit is reached, but that can be overcome by the userscript manager extensions by wrapping the actual JS code in a function call that does some pre-processing. E.g.

(() => {
  if (some_var_only_defined_in_default_userscript_world) {
    // Oops, over quota. This script is sensitive and should
    // not share execution environment, so return now.
    return;
  }
  // Copy-paste actual user script code here
})();