Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JavaScript ShadowRealm proposal integration #9893

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

Conversation

Ms2ger
Copy link
Member

@Ms2ger Ms2ger commented Oct 30, 2023

To replace #5339.

(See WHATWG Working Mode: Changes for more details.)

TODO:

  • Prepare PRs for dependent specs to adjust to terminology changes

/browsing-the-web.html ( diff )
/dom.html ( diff )
/embedded-content.html ( diff )
/form-elements.html ( diff )
/index.html ( diff )
/infrastructure.html ( diff )
/media.html ( diff )
/nav-history-apis.html ( diff )
/references.html ( diff )
/scripting.html ( diff )
/structured-data.html ( diff )
/timers-and-user-prompts.html ( diff )
/webappapis.html ( diff )
/workers.html ( diff )

Copy link
Contributor

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes to the module loading algorithms look ok.

For the record, we (people working on modules in TC39) have some ideas for splitting the stateless and stateful parts of Module Records in ECMA-262. If/once that will happen, it would be great to share a module map for the stateless parts across principal realms and shadow realms, so that basically in this case:

import "/foo";
new ShadowRealm().importValue("/foo", "bar");

we only call into fetch once (to get the stateless part) and then we create the stateful part twice in both realms.

source Outdated Show resolved Hide resolved
@Ms2ger
Copy link
Member Author

Ms2ger commented Feb 2, 2024

I think this could benefit from review at this point

ptomato added a commit to ptomato/whatwg-dom that referenced this pull request Feb 7, 2024
Delete the Exposed attribute from AbortSignal.timeout, letting it
inherit [Exposed=*] from AbortSignal.

Cf. whatwg/html#9893 where other timers are
being exposed in ShadowRealm global contexts.
@littledan
Copy link
Contributor

littledan commented Feb 7, 2024

ShadowRealms reached 2.7 in TC39. Acceptance of this PR by WHATWG editors is a blocking requirement for Stage 3, though it probably shouldn't be landed until the Web API set is finalized.

@domenic @annevk @zcorpan Any thoughts/concerns?

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@Ms2ger
Copy link
Member Author

Ms2ger commented Oct 8, 2024

@domenic or @annevk - it seems to me that the we're making progress on the questions about exposure; is there anything else I can do to help move this PR forward?

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
shannonbooth added a commit to shannonbooth/ladybird that referenced this pull request Oct 23, 2024
This aligns with the lastest version of the PR implementing the
javascript shadow realm proposal:

whatwg/html#9893

This commit simply performs the rename before implementing the behaviour
change.
source Outdated Show resolved Hide resolved
ptomato added a commit to ptomato/wpt that referenced this pull request Nov 18, 2024
As per whatwg/html#9893, ShadowRealmGlobalScope
should have a `self` attribute already. There is no need to monkeypatch it
for the test harness.
ptomato added a commit to ptomato/wpt that referenced this pull request Nov 18, 2024
As per whatwg/html#9893, ShadowRealmGlobalScope
should have a `self` attribute already. There is no need to monkeypatch it
for the test harness.
ptomato added a commit to web-platform-tests/wpt that referenced this pull request Nov 18, 2024
As per whatwg/html#9893, ShadowRealmGlobalScope
should have a `self` attribute already. There is no need to monkeypatch it
for the test harness.
@Ms2ger
Copy link
Member Author

Ms2ger commented Nov 19, 2024

Let me reproduce @domenic's #5339 (comment) comment here:

I think we have two paths that would be acceptable from my perspective:

  1. Update ECMAScript so that "current Realm" is always a non-shadow Realm.

    • Continue defining "current global object" / "current settings object" as 1:1:1 with "current Realm".
    • Very few specs need updating besides ECMAScript; maybe some stuff in core parts of HTML or Web IDL.
  2. Do not update "current Realm".

    • Introduce "current principal Realm" in either ECMAScript or HTML.
    • Define "current principal global object" / "current principal settings object" in HTML as 1:1:1 with "current principal Realm".
    • Maybe remove the definitions of "current global object" and "current settings object", or at least don't export them.
    • Update all web specs that reference "current Realm" / "current global object" / "current settings object". The latter two seem to be indexed: current global, current settings but for some reason "current Realm" isn't appearing there; not sure why. You can try asking dontcallmedom for help on that (purposefully not pinging him yet though).

I prefer (1) because I think (2) is pretty disruptive to the spec ecosystem, but I can understand how (2) might be more pure from the perspective of the ECMAScript editors. The next step is probably getting their preference.

I think we are going with 2 here, but that means we need PRs for all of https://dontcallmedom.github.io/webdex/c.html#current%20global%20object%40%40html%25%25dfn and similar terms. We would not be able to land this without all of that work being done.

I'm happy willing to take that on, but I'd like to postpone that until this PR is otherwise ready to land. We can't land the PRs to dependent specs anyway (or their links would break), and the changes are highly mechanical, so I don't think there's much of a need to look at them while we do the review here.

source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
source Show resolved Hide resolved
@nicolo-ribaudo
Copy link
Contributor

nicolo-ribaudo commented Nov 20, 2024

Is there any reason to ever use the current principal realm, other than to grab the current principal global object and current principal settings object? If not, we probably should not <dfn> it at all.


EDIT: I'm pasting here a message I sent to Ms2ger, since it might help clarify.

The reasons you might want to get to the realm are 4:

  • to actually use the realm to create objects/functions (in which case, you want the current realm)
  • to get to the settings object (except for module loading you always want the principal) — and we have the utility defined for this
  • to get to the global object:
    • to read properties from it (in which case you want the current)
    • to check in which kind of global we are (and it seems like you might sometimes want the principal and sometimes want the current)

So we should easily expose what is more likely to be the correct choice, and make it a bit harder to get the one you likely don't want so that you have to explicitly think about it

@nicolo-ribaudo
Copy link
Contributor

nicolo-ribaudo commented Nov 20, 2024

Looking through all the usages of the current principal global object, it seems like it's also not meaningfully different from the global object associated to the current realm (i.e. the old current global object). Out of 8 usages:

  • 5 are in DOM constructors like new Image. These constructors are not exposed in ShadowRealms, and their current realm and their current principal realm are always the same.
  • 2 are in internal object operations of the WindowProxy, which is obviously not exposed in ShadowRealm
  • 1 is in the pause steps (Ctrl+F for "require the user agent to pause while running" in 8.1.7.3). The current principal global object here is passed to https://w3c.github.io/hr-time/#dfn-current-high-resolution-time. That spec only uses the global object to get its relevant settings object: we could just update that spec to receive the current principal settings object directly.

Looking at the other specs that reference the curent global object, none of them are about APIs exposed in ShadowRealms.

The only places where we actually need the current principal global object is when we enable/disable some features based on which global we are running in:

  • one is at the beginning of HostLoadImportedModule, where we disallow dynamic import in service workers and worklets. However, in this case you are already using realm's principal realm's global object
  • the other one is in "resolve a module specifier", where we only allow import maps if the referrer's global object implements Window. This already checks the realm's global and not the realm's principal realm's global.

So again, do we actually need the concept of the current principal global object, or can we continue using the current global object (with the manual handling in HostLoadImportedModule to actually grab the global object of the principal realm)?


the other one is in "resolve a module specifier", where we only allow import maps if the referrer's global object implements Window. This already checks the realm's global and not the realm's principal realm's global.

Is it intentional that ShadowRealms ignore the principal realm's import map?

@Ms2ger
Copy link
Member Author

Ms2ger commented Nov 20, 2024

Talking to Nic, it seems that when you need a realm, you're most likely to want to use the (potentially synthetic) current realm, and when you need a settings object, you're most likely to want the current principal settings object. Practically the suggestion is that we'd remove the "current principal realm" definition, and where it's really needed, people can go through the current principal settings object. Any objections to that, @domenic (or others)?

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 20, 2024
…, a=testonly

Automatic update from web-platform-tests
Change 'shadowrealm' global into a shorthand for all possible ShadowRealm scopes

In order to automatically run tests not only in a ShadowRealm created in a
window scope, but also in ShadowRealms created in other realms, change the
'shadowrealm' global type to a collection, and rename the existing
ShadowRealm handler to 'shadowrealm-in-window'.

--
Remove monkeypatch of globalThis.self in ShadowRealm

As per whatwg/html#9893, ShadowRealmGlobalScope
should have a `self` attribute already. There is no need to monkeypatch it
for the test harness.

--
Factor out JS code that will be common to multiple ShadowRealm handlers

We will add multiple ShadowRealm handlers, and they will all need to set
up certain global properties. Avoid repeating this code in each handler
as well as in idlharness-shadowrealm.js.

This should also increase readability, which is good since the ShadowRealm
setup code can be confusing.

--
Add 'shadowrealm-in-shadowrealm' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-shadowrealm.html variant.

The test wrapper creates an outer ShadowRealm, which creates an inner
ShadowRealm and runs the tests inside that, relaying the results through
the outer ShadowRealm.

--
Add 'shadowrealm-in-dedicatedworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-dedicatedworker.html variant.

The test loads an intermediate .any.worker-shadowrealm.js wrapper into a
Worker, and forwards the message port to the Worker's message port so that
fetch_tests_from_worker can receive the results.

--
Add 'shadowrealm-in-sharedworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-sharedworker.html variant.

The test loads the same intermediate .any.worker-shadowrealm.js wrapper as
.any.shadowrealm-in-dedicatedworker.html, but populates a 'port' variable
with the port received from the connect event, instead of calling the
global postMessage since that won't work in a SharedWorker.

--
Add 'shadowrealm-in-serviceworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-serviceworker.html variant.

We have to use a slightly different .any.serviceworker-shadowrealm.js
wrapper from the wrapper used for the other types of workers, because
dynamic import() is forbidden in ServiceWorker scopes. Instead, add a
utility function to set up a fakeDynamicImport() function inside the
ShadowRealm which uses the fetch adaptor to get the module's source text
and evaluate it in the shadowRealm.

Also add a case for ServiceWorkers to getPostMessageFunc(), which returns
a postMessage() drop-in replacement that broadcasts the message to all
clients, since test result messages from the ShadowRealm are not in
response to any particular message received by the ServiceWorker.

Note '.https.' needs to be added to the test path.

--
Add 'shadowrealm-in-audioworklet' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-audioworklet.html variant.

The wrapper here is similar to the one for ServiceWorkers, since dynamic
import() is also forbidden in worklet scopes. But additionally fetch() is
not exposed, so we add a utility function to set up the ability to call
the window realm's fetch() through the AudioWorklet's message port.

We also add /resources/testharness-shadowrealm-audioworkletprocessor.js to
contain most of the AudioWorklet setup boilerplate, so that it isn't
written inline in serve.py.

Note '.https.' needs to be added to the test path.

--

wpt-commits: 9c8db8af89efbe0f67b215af2a6b49e9564e2971, 65a205aea5d02ff5bea7b1a0579287035d02d6c4, eb9c8e7259ef8bd5cca5019c1ca15ccd430e81dc, 3a20c56893472783b5e20c0d61cbb7b7b278cc6d, 7d8458ed291b139307430a102180c9a617d7876e, 42160ae827c863ac6787c8451fe377901c8f0652, 59367bb21d053abb9ed6de3cca5409486816acc9, 60d6c48e5fa76876bc3924b9d6185dfb56c9ab1c
wpt-pr: 49108
@domenic
Copy link
Member

domenic commented Nov 21, 2024

I'm fine adding such guidance, but I don't think it makes sense to only define some subset of concepts; there should be the full matrix of 2x3 concepts available.

Copy link
Contributor

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine adding such guidance, but I don't think it makes sense to only define some subset of concepts; there should be the full matrix of 2x3 concepts available.

As of today in this PR there are only 4 concepts:

  • current realm (from ECMAScript)
  • current principal realm
  • current principal settings object
  • current principal global object

The places where it would HTML would have used the "current settings object" are just either to grab the realm from it (i.e. in the modules algorithms), so this PR just uses the "current realm" directly, or to grab the global object (in HostPromiseRejectionTracker), so this PR just grabs it from the "current realm" instead.

As per #9893 (comment), I believe in all current usages of the "current principal global object", except for https://html.spec.whatwg.org/#pause, it would be cleaner (*) to use the "current global object": it probably makes sense to at least re-introduce it.

  • "cleaner" = they are equivalent because those APIs depend on the DOM, the DOM is not available in ShadowRealm, and thus they are not exposed there. However, the rule of thumb I'm applying is, for example, "if ShadowRealms had their own DOM, should customElements.define define an element in the ShadowRealm itself or in the principal realm?"

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Nov 21, 2024
…, a=testonly

Automatic update from web-platform-tests
Change 'shadowrealm' global into a shorthand for all possible ShadowRealm scopes

In order to automatically run tests not only in a ShadowRealm created in a
window scope, but also in ShadowRealms created in other realms, change the
'shadowrealm' global type to a collection, and rename the existing
ShadowRealm handler to 'shadowrealm-in-window'.

--
Remove monkeypatch of globalThis.self in ShadowRealm

As per whatwg/html#9893, ShadowRealmGlobalScope
should have a `self` attribute already. There is no need to monkeypatch it
for the test harness.

--
Factor out JS code that will be common to multiple ShadowRealm handlers

We will add multiple ShadowRealm handlers, and they will all need to set
up certain global properties. Avoid repeating this code in each handler
as well as in idlharness-shadowrealm.js.

This should also increase readability, which is good since the ShadowRealm
setup code can be confusing.

--
Add 'shadowrealm-in-shadowrealm' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-shadowrealm.html variant.

The test wrapper creates an outer ShadowRealm, which creates an inner
ShadowRealm and runs the tests inside that, relaying the results through
the outer ShadowRealm.

--
Add 'shadowrealm-in-dedicatedworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-dedicatedworker.html variant.

The test loads an intermediate .any.worker-shadowrealm.js wrapper into a
Worker, and forwards the message port to the Worker's message port so that
fetch_tests_from_worker can receive the results.

--
Add 'shadowrealm-in-sharedworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-sharedworker.html variant.

The test loads the same intermediate .any.worker-shadowrealm.js wrapper as
.any.shadowrealm-in-dedicatedworker.html, but populates a 'port' variable
with the port received from the connect event, instead of calling the
global postMessage since that won't work in a SharedWorker.

--
Add 'shadowrealm-in-serviceworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-serviceworker.html variant.

We have to use a slightly different .any.serviceworker-shadowrealm.js
wrapper from the wrapper used for the other types of workers, because
dynamic import() is forbidden in ServiceWorker scopes. Instead, add a
utility function to set up a fakeDynamicImport() function inside the
ShadowRealm which uses the fetch adaptor to get the module's source text
and evaluate it in the shadowRealm.

Also add a case for ServiceWorkers to getPostMessageFunc(), which returns
a postMessage() drop-in replacement that broadcasts the message to all
clients, since test result messages from the ShadowRealm are not in
response to any particular message received by the ServiceWorker.

Note '.https.' needs to be added to the test path.

--
Add 'shadowrealm-in-audioworklet' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-audioworklet.html variant.

The wrapper here is similar to the one for ServiceWorkers, since dynamic
import() is also forbidden in worklet scopes. But additionally fetch() is
not exposed, so we add a utility function to set up the ability to call
the window realm's fetch() through the AudioWorklet's message port.

We also add /resources/testharness-shadowrealm-audioworkletprocessor.js to
contain most of the AudioWorklet setup boilerplate, so that it isn't
written inline in serve.py.

Note '.https.' needs to be added to the test path.

--

wpt-commits: 9c8db8af89efbe0f67b215af2a6b49e9564e2971, 65a205aea5d02ff5bea7b1a0579287035d02d6c4, eb9c8e7259ef8bd5cca5019c1ca15ccd430e81dc, 3a20c56893472783b5e20c0d61cbb7b7b278cc6d, 7d8458ed291b139307430a102180c9a617d7876e, 42160ae827c863ac6787c8451fe377901c8f0652, 59367bb21d053abb9ed6de3cca5409486816acc9, 60d6c48e5fa76876bc3924b9d6185dfb56c9ab1c
wpt-pr: 49108
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Nov 22, 2024
…, a=testonly

Automatic update from web-platform-tests
Change 'shadowrealm' global into a shorthand for all possible ShadowRealm scopes

In order to automatically run tests not only in a ShadowRealm created in a
window scope, but also in ShadowRealms created in other realms, change the
'shadowrealm' global type to a collection, and rename the existing
ShadowRealm handler to 'shadowrealm-in-window'.

--
Remove monkeypatch of globalThis.self in ShadowRealm

As per whatwg/html#9893, ShadowRealmGlobalScope
should have a `self` attribute already. There is no need to monkeypatch it
for the test harness.

--
Factor out JS code that will be common to multiple ShadowRealm handlers

We will add multiple ShadowRealm handlers, and they will all need to set
up certain global properties. Avoid repeating this code in each handler
as well as in idlharness-shadowrealm.js.

This should also increase readability, which is good since the ShadowRealm
setup code can be confusing.

--
Add 'shadowrealm-in-shadowrealm' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-shadowrealm.html variant.

The test wrapper creates an outer ShadowRealm, which creates an inner
ShadowRealm and runs the tests inside that, relaying the results through
the outer ShadowRealm.

--
Add 'shadowrealm-in-dedicatedworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-dedicatedworker.html variant.

The test loads an intermediate .any.worker-shadowrealm.js wrapper into a
Worker, and forwards the message port to the Worker's message port so that
fetch_tests_from_worker can receive the results.

--
Add 'shadowrealm-in-sharedworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-sharedworker.html variant.

The test loads the same intermediate .any.worker-shadowrealm.js wrapper as
.any.shadowrealm-in-dedicatedworker.html, but populates a 'port' variable
with the port received from the connect event, instead of calling the
global postMessage since that won't work in a SharedWorker.

--
Add 'shadowrealm-in-serviceworker' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-serviceworker.html variant.

We have to use a slightly different .any.serviceworker-shadowrealm.js
wrapper from the wrapper used for the other types of workers, because
dynamic import() is forbidden in ServiceWorker scopes. Instead, add a
utility function to set up a fakeDynamicImport() function inside the
ShadowRealm which uses the fetch adaptor to get the module's source text
and evaluate it in the shadowRealm.

Also add a case for ServiceWorkers to getPostMessageFunc(), which returns
a postMessage() drop-in replacement that broadcasts the message to all
clients, since test result messages from the ShadowRealm are not in
response to any particular message received by the ServiceWorker.

Note '.https.' needs to be added to the test path.

--
Add 'shadowrealm-in-audioworklet' global

This will add to any test with global=shadowrealm in its metadata, an
.any.shadowrealm-in-audioworklet.html variant.

The wrapper here is similar to the one for ServiceWorkers, since dynamic
import() is also forbidden in worklet scopes. But additionally fetch() is
not exposed, so we add a utility function to set up the ability to call
the window realm's fetch() through the AudioWorklet's message port.

We also add /resources/testharness-shadowrealm-audioworkletprocessor.js to
contain most of the AudioWorklet setup boilerplate, so that it isn't
written inline in serve.py.

Note '.https.' needs to be added to the test path.

--

wpt-commits: 9c8db8af89efbe0f67b215af2a6b49e9564e2971, 65a205aea5d02ff5bea7b1a0579287035d02d6c4, eb9c8e7259ef8bd5cca5019c1ca15ccd430e81dc, 3a20c56893472783b5e20c0d61cbb7b7b278cc6d, 7d8458ed291b139307430a102180c9a617d7876e, 42160ae827c863ac6787c8451fe377901c8f0652, 59367bb21d053abb9ed6de3cca5409486816acc9, 60d6c48e5fa76876bc3924b9d6185dfb56c9ab1c
wpt-pr: 49108
<h6>Creating a Shadow Realm</h6>

<p>When performing <span data-x="js-InitializeHostDefinedRealm">InitializeHostDefinedRealm</span>
in the <code data-x="js-ShadowRealm-constructor">ShadowRealm</code> constructor, the steps to
Copy link

@shannonbooth shannonbooth Nov 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally preferred the old steps for global object creation before dc9fddc as the steps if followed explicitly were more easily implementable without needing to add web specific changes into the javascript implementation. There is no clear hook for InitializeHostDefinedRealm to say that it is being invoked in the ShadowRealm constructor.

I think this type of customization should ideally be explicitly stated at the call site of HostInitializeShadowRealm, similar to how https://whatpr.org/html/9893/webappapis.html#creating-a-new-javascript-realm does where it is used like:

Let realm execution context be the result of creating a new realm given agent and the following customizations:

For the global object, create a new Window object.

For the global this binding, use browsingContext's WindowProxy object.

What makes this awkward is that HostInitializeShadowRealm is being invoked in JS spec, where we would like to make that customization.

@@ -113690,25 +114036,15 @@ interface mixin <dfn interface>WindowOrWorkerGlobalScope</dfn> {

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if we really need to have things conditionally exposed depending on whether the incumbent realm is a secure context, we'll also need to move isSecureContext to UniversalGlobalScope. (2 lines above this)


// structured cloning
any <span data-x="dom-structuredClone">structuredClone</span>(any value, optional <span>StructuredSerializeOptions</span> options = {});
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still missing the event handlers that go along with the universally exposed stuff - onerror, onrejectionhandled, onunhandledrejection.

I'm not sure if they should go here or in ShadowRealmGlobalScope. Adding them here might make things complicated. onerror is part of GlobalEventHandlers which Document also includes. The latter two are part of WindowEventHandlers which HTMLBodyElement also includes. So either we add these to ShadowRealmGlobalScope, or we add them here and remove them from WorkerGlobalScope, GlobalEventHandlers, and WindowEventHandlers, then additionally re-add them to Document, HTMLBodyElement, and anywhere else they're supposed to be included.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(We'll also have to decide whether to have the weird signature that the onerror handler has on Window and Worker globals)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to associate / plumb the shadow realm through the unhandled rejection hooks? Unhandled rejections for shadow realm promises cannot be reported in the main realm event because of the callable boundary. I see that HostPromiseRejectionTracker only takes the promise, which may not be sufficient for the host to figure out which realm it belongs to.

Btw, there is still some pending implementation issues about which realm reactions are invoked in, but in the case of ShadowRealm they shouldn't apply since promises in these realms cannot interact with other realms.

I guess technically the host hook can lookup the current realm record.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll defer to @Ms2ger but I believe this is handled correctly by the current state of this PR's HostPromiseRejectionTracker. We get the ShadowRealm global from the running script's realm and otherwise from the current realm (although I'm not sure in which case the running script would be null during ShadowRealm execution.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok so HTML is already relying on the current realm.

@@ -113690,25 +114036,15 @@ interface mixin <dfn interface>WindowOrWorkerGlobalScope</dfn> {

undefined <span data-x="dom-reportError">reportError</span>(any e);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On review, I think reportError probably meets the criteria to be exposed in w3ctag/design-principles#510. Maybe move this to UniversalGlobalScope unless we have a good reason not to?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

10 participants