Change MessagePort owner from incumbent to current #5728
Conversation
The "owner" of a MessagePort is set a lot, but only *used* for determining what document (if any) should be associated with that MessagePort's tasks. I.e., it ensures that when that document becomes not-fully-active, the tasks are not run. Tests [1] show that the use of incumbent settings object for the owner of MessageChannel-created MessagePorts was incorrect. In fact, the current settings object is used. This allows for a dramatic simplification, as it means that the MessagePort's owner is always equal to its relevant settings object. Thus, we can entirely delete the owner concept, and the associated specialized "create a new MessagePort object" algorithm. This does require tweaking the examples of how the incumbent concept works, now that a simple constructor is no longer available to demonstrate the point. This also modernizes the definition of MessageChannel. Closes #4340. Helps with #1430. [1]: web-platform-tests/wpt#24566
domenic
force-pushed
the
messageport-fixes
branch
from
889d3f4 to
e89627f
Compare
| <ol> | ||
| <li><p>Set <span>this</span>'s <span>port 1</span> to a <span>new</span> | ||
| <code>MessagePort</code> in the <span data-x="current Realm Record">current | ||
| Realm</span>.</p></li> |
There was a problem hiding this comment.
Can we make this relevant realm instead? I worry that folks might copy from this for non-constructor creators and get it wrong.
There was a problem hiding this comment.
We use current in constructors currently, per Web IDL. Technically we could use relevant of this (which would be done if Web IDL used the relevant realm of new.target), but it would be different in strange Reflect.construct() cases, in undesirable ways. I can't find where we decided that...
There was a problem hiding this comment.
So does that mean the difference can be observed using Reflect.construct()? If so we should probably add tests.
There was a problem hiding this comment.
I'm a bit hesitant to do so since there's not much unique about this constructor compared to every other one, but if it's important, then I can try.
There was a problem hiding this comment.
We don't have that many constructors allocating tearoffs I think and it's important for those. Or is there a more general property I'm missing?
There was a problem hiding this comment.
Well, it's both.
There's the more general property, that Web IDL establishes, which is basically that if you mix together multiple realms using Reflect.construct(), the resulting value uses the current realm, not the relevant realm of new.target. That's the case for every constructor.
Then, if the constructor has tearoffs, I guess we could in theory decide to be inconsistent with Web IDL and use the relevant realm of new.target, but that'd be pretty unnatural.
There was a problem hiding this comment.
I think I've convinced myself that relevant realm of this === current, in the new-ish Web IDL setup. You would need to step into ES directly by referencing NewTarget to get access to the other realm.
So we could make this change. However it'd be inconsistent with all the other usage of current in constructors, e.g. in https://fetch.spec.whatwg.org/#dom-response step 10 or https://fetch.spec.whatwg.org/#dom-request step 4 and step 8, which predate the introduction of this in constructors.
What would you prefer?
There was a problem hiding this comment.
Am I correct that WPT webidl/current-realm.html ends up requiring relevant for factory methods? If so, I have a slight preference for always using relevant, perhaps explaining somewhere that it doesn't matter for constructors. That will just make it easier for people as they don't have to learn the distinction between factory methods and constructors as far as this thing is concerned.
There was a problem hiding this comment.
Yes, that looks to be what's tested. I'll change it then, although perhaps you could also update Fetch similarly.
…, not incumbent, a=testonly Automatic update from web-platform-tests Test that MessagePort owners are current, not incumbent Follows whatwg/html#5728. -- wpt-commits: 299084220865342716773d5116880a63711339d7 wpt-pr: 24566
…, not incumbent, a=testonly Automatic update from web-platform-tests Test that MessagePort owners are current, not incumbent Follows whatwg/html#5728. -- wpt-commits: 299084220865342716773d5116880a63711339d7 wpt-pr: 24566
The "owner" of a MessagePort is set a lot, but only used for
determining what document (if any) should be associated with that
MessagePort's tasks. I.e., it ensures that when that document becomes
not-fully-active, the tasks are not run.
Tests 1 show that the use of incumbent settings object for the owner
of MessageChannel-created MessagePorts was incorrect. In fact, the
current settings object is used.
This allows for a dramatic simplification, as it means that the
MessagePort's owner is always equal to its relevant settings object.
Thus, we can entirely delete the owner concept, and the associated
specialized "create a new MessagePort object" algorithm.
This does require tweaking the examples of how the incumbent concept
works, now that a simple constructor is no longer available to
demonstrate the point.
This also modernizes the definition of MessageChannel.
Closes #4340. Helps with #1430.
I suspect a similar thing is true for BroadcastChannel, and may allow deleting BroadcastChannel settings object. I'll work on that next.
(See WHATWG Working Mode: Changes for more details.)
/dynamic-markup-insertion.html ( diff )
/infrastructure.html ( diff )
/web-messaging.html ( diff )
/webappapis.html ( diff )
/workers.html ( diff )