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

Test that MessagePort owners are current, not incumbent #24566

Merged
merged 4 commits into from Jul 14, 2020

Conversation

@domenic
Copy link
Member

domenic commented Jul 10, 2020

Follows whatwg/html#5728.

@wpt-pr-bot wpt-pr-bot requested review from annevk, jdm, mkruisselbrink and zqzhang Jul 10, 2020
@wpt-pr-bot wpt-pr-bot requested a deployment to wpt-preview-24566 Jul 10, 2020 Pending
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
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
webmessaging/multi-globals/messageport-current.html Outdated Show resolved Hide resolved
webmessaging/multi-globals/messageport-current.html Outdated Show resolved Hide resolved
messageChannel.port1.onmessageerror = t.unreached_func("messageerror event recieved");
messageChannel.port2.postMessage("boo");

await new Promise(resolve => t.step_timeout(resolve, 3000));

This comment has been minimized.

@annevk

annevk Jul 13, 2020 Member

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.

This comment has been minimized.

@domenic

domenic Jul 13, 2020 Author Member

I don't believe so, because each MessagePort has its own independent message queue, with no ordering guarantees between them. (Unlike BroadcastChannel.)

This comment has been minimized.

@annevk

annevk Jul 14, 2020 Member

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.

@annevk
Copy link
Member

annevk commented Jul 13, 2020

For webidl/current-realm.html I wrote

   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.

@wpt-pr-bot wpt-pr-bot requested a deployment to wpt-preview-24566 Jul 13, 2020 Pending
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
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));

This comment has been minimized.

@annevk

annevk Jul 14, 2020 Member

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.

@domenic domenic merged commit 2990842 into master Jul 14, 2020
19 checks passed
19 checks passed
Azure Pipelines Build #20200714.22 succeeded
Details
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
download-firefox-nightly Community-TC (pull_request)
Details
lint Community-TC (pull_request)
Details
sink-task Community-TC (pull_request)
Details
wpt-chrome-dev-results Community-TC (pull_request)
Details
wpt-chrome-dev-results-without-changes Community-TC (pull_request)
Details
wpt-chrome-dev-stability Community-TC (pull_request)
Details
wpt-decision-task Community-TC (pull_request)
Details
wpt-firefox-nightly-results Community-TC (pull_request)
Details
wpt-firefox-nightly-results-without-changes Community-TC (pull_request)
Details
wpt-firefox-nightly-stability Community-TC (pull_request)
Details
wpt.fyi - chrome[experimental] Chrome results
Details
wpt.fyi - firefox[experimental] Firefox results
Details
wpt.fyi - safari[experimental] Safari results
Details
@domenic domenic deleted the messageport-incumbent branch Jul 14, 2020
mfreed7 added 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.