Test that MessagePort owners are current, not incumbent #24566
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
domenic
added a commit
to whatwg/html
that referenced
this pull request
Jul 10, 2020
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
3 tasks
domenic
added a commit
to whatwg/html
that referenced
this pull request
Jul 10, 2020
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
annevk
reviewed
Jul 13, 2020
| messageChannel.port1.onmessageerror = t.unreached_func("messageerror event recieved"); | ||
| messageChannel.port2.postMessage("boo"); | ||
|
|
||
| await new Promise(resolve => t.step_timeout(resolve, 3000)); |
There was a problem hiding this comment.
If you create another messageChannel in the entry global and use that, can we expect anything about its speed relative to the other one? That might allow using less resources on CI.
There was a problem hiding this comment.
I don't believe so, because each MessagePort has its own independent message queue, with no ordering guarantees between them. (Unlike BroadcastChannel.)
There was a problem hiding this comment.
I think it would be good to have a comment here explaining the timeout so those looking into removing timeouts know roughly what kind of API would have to exist.
|
For test(() => {
const c = new self[0].MessageChannel();
let obj = c.port1;
assert_global(obj);
obj = Object.getOwnPropertyDescriptor(MessageChannel.prototype, "port1").get.call(c);
assert_global(obj);
}, "MessageChannel's ports");to convince myself current and relevant are indeed the same for constructors. All browsers pass this so I'm not sure it's worth adding. |
domenic
added a commit
to whatwg/html
that referenced
this pull request
Jul 13, 2020
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 (or relevant settings object of this) 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
annevk
approved these changes
Jul 14, 2020
| messageChannel.port1.onmessageerror = t.unreached_func("messageerror event recieved"); | ||
| messageChannel.port2.postMessage("boo"); | ||
|
|
||
| await new Promise(resolve => t.step_timeout(resolve, 3000)); |
There was a problem hiding this comment.
I think it would be good to have a comment here explaining the timeout so those looking into removing timeouts know roughly what kind of API would have to exist.
mfreed7
pushed a commit
to mfreed7/html
that referenced
this pull request
Sep 11, 2020
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 (or relevant settings object of this) 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 whatwg#4340. Helps with whatwg#1430. [1]: web-platform-tests/wpt#24566
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follows whatwg/html#5728.