-
Notifications
You must be signed in to change notification settings - Fork 172
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
State that by default all objects are created in the relevant realm of this
#135
Comments
This looks like a duplicate of https://www.w3.org/Bugs/Public/show_bug.cgi?id=24652. |
See also web-platform-tests/wpt#1381 for tests which I'm about to land. |
Can we provide a default for in parallel blocks created from IDL methods and constructors too? We'd want to capture the relevant or current realm, respectively, at the point where the in parallel block starts, and use it as the "object-creation realm" within the block. Tasks posted from a context with an "object-creation realm" could inherit the same realm. We still have some cases without an "object-creation realm", like https://webbluetoothcg.github.io/web-bluetooth/#notification-events. These tasks actually have an obvious realm to use, but it's lost when we pull the event loop out of the realm's settings object. We could extend "queue a task" to take its object-creation realm in order to have a default here too. |
That seems not great to me. When you're in a background thread, you should be very explicit about what realm you're creating objects in. Just inheriting a default is not going to work out well in general, I think. IMO it's better to just state the realm when creating the object, and rely on ambient defaults as little as possible---only for the most obvious of cases. We should not be trying to create more situations that have ambient defaults available in non-obvious ways. |
Given:
Do you have any cases in mind where you'd want |x| and |y| to have different globals? |
Yes, definitely. For example if |y| was an event that was going to be dispatched to a different global. The various postMessage specs do this, for example. |
Cool, thanks for the example. https://html.spec.whatwg.org/multipage/comms.html#dom-window-postmessage creates the events inside a task, rather than in parallel, and it'd be straightforward to reset the default when queuing a task... My main concern is that verbosity and repetition hurt readability. If we insist that every object creation in parallel code specify a realm, and that realm is the same for most calls, it makes it harder to follow the rest of the algorithm and obscures the few cases where the realm is importantly different. Are we differing in how often we expect parallel code to need to change the realm? Most of the new specs I'm seeing do a lot of work in parallel but don't shift realms much if ever. When you and @bzbarsky go through existing specs to specify their new objects' realms, do you tend to find that you'd want them to shift realms when they go parallel? |
In general, staying in the same realm when you go parallel makes sense. The main footguns with object creation after going parallel are:
Apart from that, using the same global unless explicitly stated otherwise makes sense to me. |
I don't understand why you'd ever create an object in parallel. Object creation should always happen in an environment with a global. You can schedule a task that ends up creating the object. Otherwise you're inventing new JavaScript semantics and we've been told off for that many times. (FWIW, I think "in parallel" is a bit of an anti-pattern as it often doesn't deal well with races or the constraints of that parallel place. Having explicit task-based isolated-threads would be much clearer.) |
I mean, in practice scheduling a task is the right thing to do, and might happen implicitly (e.g. if you do your work off a promise getting resolved). But you then need to define which global to use anyway.... |
It'd be fine with me if we wrote somewhere that objects cannot be created from parallel contexts, and I agree with @annevk's goal to replace "in parallel" with better-specified threads. I tentatively propose the following resolution to this issue:
|
I still strongly object to trying to make object creation realm implicit. I think specs should use an explicit realm whenever possible. Ideally I would prefer not even having the exceptions outlined in the OP, but they are there as a compromise. This is something spec authors should be aware of and making conscious decisions on. I'd like to get us to a state where things are done explicitly in a widespread way. If we find this leads to a lot of repetition then maybe we can consider setting some defaults. But my suspicion is that it's generally not going to be obvious which realm to create things in when operating in parallel/inside a task. I'd like us to try this before considering a radical step such as implicit propagation of object creation realms. |
I think the object creation realm is one of the things that spec authors are just not going to consider, and so if we don't default it, we'll just wind up with underspecification. However, I'm happy to take the consensus subset for now. Does everyone like the idea to define the object-creation realm and "new object" term, and have "in parallel" and "queue a task" kill off the default? I can't send a PR until next week, but I'll try to do it then. |
I think that's why we should formalize object creation. So that it's an easy-to-review thing in standards and something implementers can more easily flag. Perhaps even lintable some day. The main problem at the moment is the lack of a formalized approach (for many things) where everyone just copy-and-pastes things that mostly work if you squint hard enough without really thinking through the implications (and without being able to think them through since we don't even say what "new" means or link it). |
Agreed with at least formalizing object creation - we've got a lot of object creation in the Typed OM spec, for instance, and didn't realize that we had to specify the global for each. (BZ is flagging every instance of it for us. ^_^) |
Another issue is static methods. At least with Chrome and Firefox and at least with // $0 is a same-origin synchronously-accessible iframe
rect = $0.contentWindow.DOMRect.fromRect.call(DOMRect);
rect.constructor === $0.contentWindow.DOMRect // true Firefox also has this weird bug where |
Per current Web IDL spec, static methods aren't even passed a "this" at the post-IDL-spec level, so don't have a concept of "relevant global" or "relevant realm". |
That's quite fair, and I would be satisfied if this is spec'd as such. However, newer ES primitives like |
We did have the ambition to support subclassing of platform objects and make that work nicely in a way that's compatible with JavaScript, but that kind of wavered and in general it does not seem like subclassing has taken of. |
We could change static methods to pass along "this", sure. I'm just saying that right now they do not. |
@annevk tried tackling this again recently. Some discussion in https://freenode.logbot.info/whatwg/20210203#c6775366 . Here's one proposal, motivated largely by the fact that we still haven't solved this. (So, my previous appetite for making things explicit is dampened.)
(*): what does "the current realm" and "the relevant realm" mean when you're inside a queued task? We want to formalize its intuitive meaning: whatever the current/relevant realm was when the original main-thread task kicked off. Proposal:
|
Does that include promises that are then cached? |
Yeah, although I'm not sure how many of those there are (if any)? I think the concern there is not very large, compared to the cost of inconsistency if we continue using current for so many other promises. (Notably, all implementations today use current at least for for synchronously rejected promises, and probably more promises than that to the extent bindings code is involved.) |
This is most critically a concern for Promise-valued attributes (e.g. |
"Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves webmachinelearning#793.
* Ensure object creation specifies the realm "Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves #793. * Don't double-init realm; and don't need realm for dicts * Variable name improvement from @fdwr
* Ensure object creation specifies the realm "Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves webmachinelearning#793. * Don't double-init realm; and don't need realm for dicts * Variable name improvement from @fdwr
* Ensure object creation specifies the realm "Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves webmachinelearning#793. * Don't double-init realm; and don't need realm for dicts * Variable name improvement from @fdwr
* Ensure object creation specifies the realm "Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves webmachinelearning#793. * Don't double-init realm; and don't need realm for dicts * Variable name improvement from @fdwr
* Ensure object creation specifies the realm "Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves webmachinelearning#793. * Don't double-init realm; and don't need realm for dicts * Variable name improvement from @fdwr
* Ensure object creation specifies the realm "Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves webmachinelearning#793. * Don't double-init realm; and don't need realm for dicts * Variable name improvement from @fdwr
* Ensure object creation specifies the realm "Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves webmachinelearning#793. * Don't double-init realm; and don't need realm for dicts * Variable name improvement from @fdwr
* Ensure object creation specifies the realm "Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so! This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic. Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135. Resolves webmachinelearning#793. * Don't double-init realm; and don't need realm for dicts * Variable name improvement from @fdwr
This has come up recently in w3c/webcrypto#85 and also in some internal work Blink is doing on refactoring their bindings layer.
It appears that in all browsers, document.createElement, innerHTML, and an Event created through button.click() are created in the relevant realm of
this
. All of these cases are currently unspecified and use spec text equivalent to "create a new X object".We should specify a few things:
this
(At first I thought that we should follow the ES spec and use the current realm for all cases. That would better match e.g.
Array.prototype.map
orPromise.prototype.then
. But it seems like that's not what browsers do, so we'll just have to live with the inconsistency.)The text was updated successfully, but these errors were encountered: