-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 #5339
Conversation
I'm not sure if this may be missing something to give Realms sufficiently concrete behavior, so reviews will be appreciated if folks have time. In particular, in the February 2020 TC39 meeting, the Realm proposal champions announced that they would be seeking Stage 3 in the March 2020 meeting, and I think it'd be best if we had the HTML interactions basically worked out and agreed on before Stage 3. See also tc39/proposal-shadowrealm#225. (Note that the current rendered Realm spec text is a bit out of date; for now, you can cross-reference the source.) |
/cc @bzbarsky. At a glance, the proposed semantics of inheriting from the "parent settings object" seem most likely to integrate well without major issues. (I'm glad these have a settings object at all; that wasn't clear from the proposal.) Here is a mix of vague concerns and concrete questions that I've been thinking about in this space. I think this proposal gives reasonable answers to most of them, but perhaps not all (especially the vague concerns).
|
Thanks for the prompt and detailed feedback. It looks like we have some more plumbing to do. Overall, this leads to an editorial question for HTML experts: Would it be better to associate a "parent" Document/Worker/Worklet with each Realm (and reference it in all the appropriate places), or to deepen the use of environment settings objects, so that more logic indirects through their algorithms (thus avoiding assembling ever-larger switch statements for global type)? I'm fine with writing up either; the former sounds like less work/a smaller delta.
Note that the Realm champion group agreed on these semantics in tc39/proposal-shadowrealm#225. It wasn't clear to me either at first, so I asked.
Thanks for pointing out a bunch of these cases below; I'll be happy to have help identifying more of them. Given successful past work on adding Worker and Worklet, I was hoping that we'll be able to figure everything out before too long. If we are left with errors or omissions (e.g., in unmaintained specs or earlier proposals), I'd want to make it clear to future spec readers and editors that the intention is to forward all behavior and state from the enclosing environment. Maybe we could write this in a note somewhere? As far as agent clusters go, Realms created through the Realm API are in the same agent as their parent Realm. I'm not sure how/where to formalize/document this. Do you see any particular difficulties with integration with agent clusters?
Yep, I see how this PR fails to handle this case. I think they should act like their parent Realm, which must (eventually, recursively) be something with already-defined semantics, e.g., a Window, Worker, or Worklet.
Another good catch; again, it should forward to the error handler for the parent Realm (previous discussion).
In general, my understanding is that we only end up using the "associated Realm" of functions and platform objects, so I'm failing to see where an addition mismatch would come from. I'm having trouble finding where the "call a user object's operation" algorithm assumes that the global is a Window or Worker; I just see its use of the relevant exection context, which is defined in this PR. Maybe there's some other algorithm that runs into this issue?
Heh, I agree with that reading of this PR, which obviously isn't the intention. Again, this should forward up through the parent Realm.
I'm a big fan of new things in the web platform that allow splits into multiple processes/agent clusters--these mechanisms enable really strong isolation. I see the benefit of Realms to be complementary: they enable scenarios like emulating one environment on top of another, as well as creating sub-environments, e.g., for testing, as well as gradually migrating legacy code. So, I'm having trouble understanding what you're getting at with "general move"--these features seem complementary. About implementer interest: maybe we could ask about this at the TC39 presentation where Realms is proposed for Stage 3? I wouldn't mind collecting positions and other kinds of feedback beforehand, but overall, I think the Realms proposal repository would be a better place to do so than this issue. |
Well, I think https://html.spec.whatwg.org/#integration-with-the-javascript-agent-formalism and https://html.spec.whatwg.org/#integration-with-the-javascript-agent-cluster-formalism make a lot of assumptions about everything being a window or worker (or worklet). Those would need extensive updates. And those sections have a bit of preexisting foundational shakiness which makes this hard:
Nope, we use it for user-provided objects, in https://heycam.github.io/webidl/#call-a-user-objects-operation step 4. This shows up in real-world code via
I think you're right. I may have been thinking of the error reporting in https://dom.spec.whatwg.org/#concept-event-listener-inner-invoke step 2.10.1, which builds on more shaky foundations (#958).
The intention is to prevent creation of sync-accessible realms, so there's a conflict, not a complement.
I think you'll separately need implementer interest for the web platform implications before we can consider this for HTML. Implementation in JavaScript engines is one thing, but exposure to the web is another, often involving a different constituency (depending on the browser engine). |
Where can I find more documentation about this intention, and how it relates to the use cases I mentioned? It'd be good to forward this feedback to TC39! |
I'm not sure it's documented centrally. It's more of a guiding principle in the development of new web APIs like workers, worklets, portals, etc. And disallowdocumentaccess allows us to "fix" the incorrect default that iframes have. Ideally we'll be able to move to a world where all realms are isolated from each other, with separate event loops. And there are very-early-stage experiments with rewarding this behavior (or punishing its opposite), e.g. never slow mode imagines a world where users have to give permission for a page to use slow APIs like sync realm creation, or you could use document policy to get that guarantee (and potentially get access to powerful features in exchange), etc. You can also see it in the design of "meta-frameworks" like AMP which only allow executing JavaScript in a worker, and ensure all iframes are third-party so that they are isolated into separate event loops. These techniques are making their way into standards as well. (Although some of the AMP techniques are controversial, from what I can tell the ones about prohibiting performance worst-practices like |
Edit: Let's have the conversation about whether we should go ahead with Realms, or whether they contradict web platform goals, in tc39/proposal-shadowrealm#238. We can use this thread to work out the HTML integration on a technical level. I think the proposal repository is a better place to give this kind of high-level feedback because it is more likely to be visible to the people who are working on this proposal and the rest of TC39. (Previous response collapsed in details) Realms are definitely designed to have synchronous access to each other, sharing a common heap. Is this an "incorrect default"? Is the idea that Workers should supersede proposed Realm use cases?
I'd be interested to understand why sync realm creation is considered slow. (I'd imagine that the Realm API would be less overhead than iframe creation, but who knows.) It would indeed be unfortunate to introduce an API at the same time as introducing a mode which bans it. |
I don't plan to engage much on the TC39 repository; I'll leave that to others.
Yes; I've been giving this feedback to the realms champions for years. (This is part of why I am not excited about engaging in TC39 on the subject.)
I agree. |
I'm trying to wrap my head around the missing parts here if we keep with the forwarding settings object architecture. As I'm slowly doing this Domenic's complexity objections become more reasonable to me. Conceptually I still find the forwarding mechanic to be simple and generalizable, but a big effort seems to be needed to ensure that the resulting web specs remain actually coherent. 1. The invariant that all globals are windows/workers is broken.An audit is needed here and in other web specs. This seems like the big outstanding TODO. In 2. Implications of user Realms having their own module mapI'm not exactly clear what the implications are. Step 2 https://html.spec.whatwg.org/#fetch-a-single-module-script has an implicit assumption currently that each document (and thus event loop) has one fetch task per module map url key. This PR is morally equivalent in having the module map key be something like url+realm, is that right? That might have implementation concerns, we should follow up with @yuki3. The user-realm module map entries are GCable, presumably, since user Realms have independent lifetimes, and the map would need to be additionally weakly keyed off the Realms, or something. Might be complicated. 3. The incumbent/entry settings object calculationWhen determining the incumbent, the settings object is pulled off topmost execution context's Realm. In case the topmost execution context is a user Realm, it'll get that user Realm's forwarding settings object. That settings object behaves like the root settings object except in 3 cases AFAICT:
I'm not seeing any obvious problems with this setup, e.g. the "prepare to run script" and "prepare to run callback" algorithms and their cleanup counterparts seem to be work as intended. Does this extend to all algorithms that use these concepts? 4. Error reportingAt first I thought But I guess that's wrong, as 5. Events and addEventListener() specThis is beyond my expertise. Perusing https://dom.spec.whatwg.org/#dom-eventtarget-addeventlistener makes it seem like it might "just work" currently, since there's already a check if the callback Realm's global is a Window. An audit is also called for where places that have Realm and global-related logic should be explicitly reasoned through. 6. Realms cannot run any script since they do not have an active document.My reading is that currently this PR is incorrect in the other way in that always allows a script to run in a user Realm, even if the root Document has disabled scripting. Both https://html.spec.whatwg.org/#check-if-we-can-run-script and https://html.spec.whatwg.org/#concept-environment-script have checks for the global being a Window. In user realms' case, the global isn't a Window, so my reading is that step 1 of "check if we can run script" does not apply, step 2 does not apply since per the 3rd bullet point of "Scripting is enabled", scripting is enabled if the setting's global object is not a Window. |
Yeah, that should almost certainly refer to a But yes, the thing stored in |
These comments make me think that the environment settings object was the wrong layer to do this "forwarding" at, and instead, we should frame this integration as, "synthetic" Realms created with the Realm API refer to a "principal realm" which is used for all related web specs. This would avoid the need to do the large amount of refactoring that the approach in this PR implies. For example, for the source property, I was imagining that the source would be the outer Window or Worker where the Realm was created, for the current Realm which made the call to postMessage. I'm working on a new PR revision to frame Realm/HTML integration this way. |
Thanks! This helps me a lot putting some perspective on how I can work this out going through the details of this PR. I'll be in sync with @littledan to provide any support necessary. |
I've pushed a rebase and refactor of this PR, described in its commit message. This new version reflects the semantics that I was intending originally (that the Realms created with the Realm API never leak out into Web specs in a non-trivial way, that you're always deferring to their parent) but which I failed to express editorially, as has been pointed out above. To answer the questions @syg listed above, in terms of this version:
I guess this is already broken, since we added Worklets :) I fixed a little reference to that, as a drive-by. More types of global objects have been added in the past. I'm having trouble understanding what the invariant is here. This PR preserves the (modified) invariant that all principal global objects implement WindowProxy, WorkerGlobalScope or WorkletGlobalScope. However, it also exposes synthetic global objects which do not implement any WebIDL interface.
This is true, but I think the scope is a bit reduced. We mostly have to worry about if other specs reach into JavaScript-internal data structures to get the [[Realm]] (fairly unlikely) or ask for the "current Realm" (more likely). In the latter case, this PR includes a note indicating that "current Realm" should be replaced by "current principal Realm" in other specs, generally. We'll have to do an audit of other specs for these two issues before landing.
The source comes from the incumbent settings object in https://html.spec.whatwg.org/#posting-messages:dom-messageevent-source . This version of the PR ensures that the incumbent Realm is always a principal Realm (so it is either a Window, Worker or Worklet).
Yes, the module map is used as a synchronization point, so either we need the realm as part of the module map key, or to give each realm its own separate module map. I think these are equivalent.
The current PR hangs the module map off of the Realm, whether it's a principal or synthetic Realm, so no need for complicated GC semantics I think.
Note, the execution context ended up not being needed, so the synthetic realm settings object omits it.
Since no synthetic Realms don't implement any WebIDL interface and cannot be used in
The "target" for "report an error" is always a principal Realm, and "report an exception" will always find the corresponding principal, not synthetic, Realm, since it goes via the environment settings object. I'm not sure if this covers the path that you're thinking of; if it doesn't, the solution is probably through adding "the principal realm of" somewhere.
This is beyond my expertise as well, but skimming the DOM spec, I think one thing that may need to be updated is WebIDL's associated Realm definition: the associated Realm of platform objects will always be a principal Realm, but for function objects, we'll need to reference function.[[Realm]]'s principal realm. This issue comes up in the inner invoke algorithm. I'm not really sure what the issue is that you're referring to with addEventListener--I think the Realms there are already all principal Realms by construction (since they have WebIDL interfaces implemented on them).
I'm having trouble understanding how this scenario would come up. It looks to me like the entrypoints to the "scripting is enabled"/"scripting is disabled" algorithms come from dealing with elements or documents, so they'll always pass in the environments settings object of a Window, Worker or Worklet. I think if we leave these algorithms as is, in terms of expecting an environment settings object as an argument, then this amounts to an implicit assertion that it's dealing with a principal Realm, as synthetic Realms no longer have an environment settings object. One thing I could do is sprinkle the text with assertions that various realms being passed around are principal. I'm not sure whether or not this is necessary or helpful. Thanks for your reviews and patience as I figure out how to frame this PR. |
I'd like to understand what this means. This PR contains the spec update. Are there more spec updates expected? Could you elaborate on the implementation concerns? I'd like to understand what the problem is with forwarding to the relevant principal Realm, with the associated properties you mention. |
Discussion about exposing Web APIs from Realms' globals by default has been ongoing; see tc39/proposal-shadowrealm#284 (comment) . Tentatively, it seems reasonable to expose certain Web APIs (such as |
To be clear, I'd expect that to be part of the work done before entering stage 3. |
@domenic I agree at the point I want the thread for tc39/proposal-shadowrealm#284 (comment) to be resolved before Stage 3 to tell a clear direction for how this integration and implementation should go. At the same time, I feel like TC39 should not exercise any decision making or governance over specific host APIs. The specifics of this work depends on the Web Platform side. I believe we have the current options being at stake there:
I'm in favor of 2, possibly 2.a. I recognize the item 1 is a no go for to make this integration possible and I believe it won't reflect other platforms as well such as Node, XS, etc. The idea of not allowing any host properties seems, IMO, weakened by the fact we already have ECMAScript built-ins already. A synthetic realm is not super "clean" by nature. Saying that, I hope we can find a good working direction for what should be added in this Web Platform integration, but I hope it doesn't become a blocker alone. |
My understanding of the feedback from Chrome, delivered by @syg in the May 2021 TC39 meeting, was that for Realms to be acceptable for Stage 3, the necessary change in this patch is that we move towards the model where there is one module map per document, rather than per Realm, and this module map is keyed by both the module specifier and Realm. The observable impact of this change will be to guarantee that there be only one fetch per module specifier per document (without relying on unspecified network caching behavior which probably exists reliably cross-browser in practice). I don't think this change has any effect on whether a future "compartments" proposal can define a different module map within a document--it's just that we don't need such a mechanism for Realms. My understanding of Chrome's position is that the WebIDL work to expose the right set of globals needs to be done before landing this PR and before TC39 Stage 4, but is not a TC39 Stage 3 prerequisite. @syg please correct me if I'm misunderstanding any of this. I'm working on a rebase + change to keying, which should be ready some time this week; apologies for my delay here, but I think the agreed-on semantics are well-understood among those active in this conversation. |
Huh, that's odd. db370e7...a40b80a seems to show a reasonable diff, at least. |
Yeah, that does look more promising... unfortunately you can't commit on commit ranges like that, from what I can see :-/. |
587e049
to
53908c1
Compare
This patch gives the TC39 JavaScript ShadowRealm proposal semantics in HTML. ShadowRealms are given an environment settings object; in general, ShadowRealms inherit their settings from the outer environment. When modules are used in ShadowRealms, module specifiers are interpreted with respect to the base URL of the surrounding environment, but when a module is imported within a ShadowRealm, it is a separate copy from what may be loaded in the surrounding environment or other ShadowRealms. This patch implements the plan described earlier at <tc39/proposal-shadowrealm#225 (comment)>. The ShadowRealm proposal is currently at Stage 3.
This should be reviewable now. (Fwiw, some work seems to be ongoing at https://bugs.chromium.org/p/chromium/issues/detail?id=1281880.) |
ShadowRealms will be under discussion at TC39 this week; it would be great to have input from HTML folks here. |
I could remind of some existing open issues, at least whatwg/webidl#1119 and #7591 |
@smaug---- @annevk we have provided answers to both questions: |
Import maps were recently merged into the HTML spec, and there doesn't seem to have been any discussion about how they interact with shadow realm module maps. From what I can gather, "as written" (or rather, as it would be once the PR is updated to track the main branch), an import map only applies to principal realms, and imports in shadow realms will not prevent an import map from applying to the principal realm. Furthermore, the "caching" going on between the principal and shadow realms only applies to specifiers after the import map resolution. Am I understanding correctly? cc @domenic |
The forwarding mechanism from the SR to the principal realm for the resolution of the settings object give you sufficient information to resolve modules from within the ShadowRealm using the principal realm's map. Another good example is a ServiceWorker, which will be able to capture requests from a SR just fine. The virtualization of the module graph resolution is not a goal of the ShadowRealm proposal, there are other proposals under TC39 to attempt to solve that. |
Apologies for the delay here. Nominating this for the HTML triage call to see how to move this forward, though it'll have to wait until @domenic is back so probably Feb 23? (Which unfortunately will be too late for me.) @littledan @Ms2ger IPR-wise I think all the work here can be attributed to Igalia. Unfortunately our bot is not that smart so @Ms2ger will have to create a fresh PR. We should still acknowledge @littledan by using a suitable |
I can do that, but I'll wait until your meeting. If you'd be able to ping me on matrix, that'd be helpful. |
Note that we had some discussion about moving this forward in #8873 (which was not the right place; sorry all). Quoting from myself there:
|
Rebased and partially updated at #9893 |
This patch gives the TC39 JavaScript ShadowRealm proposal semantics in HTML. ShadowRealms are given an environment settings object; in general, ShadowRealms inherit their settings from the outer environment.
When modules are used in ShadowRealms, module specifiers are interpreted with respect to the base URL of the surrounding environment, but when a module is imported within a ShadowRealm, it is a separate copy from what may be loaded in the surrounding environment or other ShadowRealms.
This patch implements the plan described earlier at tc39/proposal-shadowrealm#225 (comment).
The ShadowRealm proposal is currently at Stage 3.
(See WHATWG Working Mode: Changes for more details.)
/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 )
/rendering.html ( diff )
/structured-data.html ( diff )
/webappapis.html ( diff )
/workers.html ( diff )