-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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 [[CanBlock]] in various agents #5569
Conversation
Notifying @jdm, @jgraham, @zcorpan, and @zqzhang. (Learn how reviewing works.) |
295a5e1
to
694e04e
Compare
LintPassed |
Firefox (nightly channel)Testing web-platform-tests at revision c83536c All results4 tests ran/html/webappapis/scripting/processing-model-2/integration-with-the-javascript-agent-formalism/canblock-dedicatedworker.html
/html/webappapis/scripting/processing-model-2/integration-with-the-javascript-agent-formalism/canblock-serviceworker.https.html
/html/webappapis/scripting/processing-model-2/integration-with-the-javascript-agent-formalism/canblock-sharedworker.html
/html/webappapis/scripting/processing-model-2/integration-with-the-javascript-agent-formalism/canblock-window.html
|
Chrome (unstable channel)Testing web-platform-tests at revision c83536c All results4 tests ran/html/webappapis/scripting/processing-model-2/integration-with-the-javascript-agent-formalism/canblock-dedicatedworker.html
/html/webappapis/scripting/processing-model-2/integration-with-the-javascript-agent-formalism/canblock-serviceworker.https.html
/html/webappapis/scripting/processing-model-2/integration-with-the-javascript-agent-formalism/canblock-sharedworker.html
/html/webappapis/scripting/processing-model-2/integration-with-the-javascript-agent-formalism/canblock-window.html
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks okay in general, but I wish we could annotate these tests somehow for removal if we ever get proper test262 integration going.
Also, should the .js
resources be in a resources directory? It's never clear to me what the convention is there, if anything.
const ta = new Int32Array(sab); | ||
|
||
// Test passes if this doesn't throw | ||
Atomics.wait(ta, 0, 0, 10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we assert the return value to be "ok"?
@binji @lars-t-hansen can you help get this test passing? It seems that in a dedicated worker, in both Chrome and Firefox, the following code gives "timed-out" instead of "ok": "use strict";
importScripts("/resources/testharness.js");
test(() => {
const sab = new SharedArrayBuffer(16);
const ta = new Int32Array(sab);
assert_equals(Atomics.wait(ta, 0, 0, 10), "ok");
}, `[[CanBlock]] in a ${self.constructor.name}`);
done(); Presumably this is me not understanding something about SABs/Atomics.wait. However this test passes in Chrome in shared workers, somehow. |
@domenic |
Oh, I thought it would return "ok" if the value successfully became the target value (which it already is). So "timed-out" is correct? Any suggestions for a better test? Is it a Chrome bug that it returns "ok" in shared workers? |
@domenic "timed-out" is fine here, I think, if the point is that it doesn't throw. BTW if the TA value isn't equal to the supplied value, you get "not-equal". |
Hm, I'm not sure why this test would return "ok" in any context, it should either timeout or throw, as syg mentioned. |
What @syg said. Additionally, observe that [[CanBlock]] is tested early so (a) a timeout of 0 is fine here, and you should still get "timed-out"; or (b) leaving off the timeout while first setting ta[0]=1 so that you get "not-equal" is also a valid test of [[CanBlock]]. |
3b53e25
to
d80dcb7
Compare
Thanks for the help all. I just updated the test to require "timed-out". Chrome seems to be passing now; I'm not sure what was going on there locally, but maybe I had an old version of the test open or something and was being caught out by shared workers being... shared... between tabs. |
Define the infrastructure for SharedArrayBuffer. This also clarifies along which boundaries a browser implementation can use processes and threads. Tests: web-platform-tests/wpt#5569. Follow-up to define similar-origin window agents upon a less shaky foundation is #2528. Because of that, similar-origin window agents are the best place to store state that would formerly go on unit of related similar-origin browsing contexts. tc39/ecma262#882 is follow-up to define agents in more detail; in particular make their implicit realms slot explicit. w3c/css-houdini-drafts#224 is follow-up to define worklet ownership better which is needed to define how they relate to agent (sub)clusters. Fixes part of #2260. Fixes #851. Fixes w3c/ServiceWorker#1115. Fixes most of w3c/css-houdini-drafts#380 (no tests and no nice grouping of multiple realms in a single agent as that is not needed).
Since service workers require HTTPS I renamed the resource to include .https. I believe that's the correct thing to do. |
Follows whatwg/html#2521. Worklets are not tested at this time as their API is still in flux.
Define the infrastructure for SharedArrayBuffer. This also clarifies along which boundaries a browser implementation can use processes and threads. Tests: web-platform-tests/wpt#5569. Follow-up to define similar-origin window agents upon a less shaky foundation is #2528. Because of that, similar-origin window agents are the best place to store state that would formerly go on unit of related similar-origin browsing contexts. Follow-up for better agent shutdown notifications: #2581. tc39/ecma262#882 is follow-up to define agents in more detail; in particular make their implicit realms slot explicit. w3c/css-houdini-drafts#224 is follow-up to define worklet ownership better which is needed to define how they relate to agent (sub)clusters. Fixes part of #2260. Fixes #851. Fixes w3c/ServiceWorker#1115. Fixes most of w3c/css-houdini-drafts#380 (no tests and no nice grouping of multiple realms in a single agent as that is not needed).
I had to force push because I had to rebase to get past the failing Chrome bot that was disabled by a newer commit. I'll let someone else double check before landing. See whatwg/html#2521 (comment) for the browser bug analysis. Led to one bug against Firefox. |
Atomics.wait is a blocking call, and is only allowed on certain workers. See the new web platform tests here: web-platform-tests/wpt#5569 BUG=chromium:711809 Review-Url: https://codereview.chromium.org/2841003002 Cr-Commit-Position: refs/heads/master@{#468188}
Define the infrastructure for SharedArrayBuffer. This also clarifies along which boundaries a browser implementation can use processes and threads. Tests: web-platform-tests/wpt#5569. Follow-up to define similar-origin window agents upon a less shaky foundation is whatwg#2528. Because of that, similar-origin window agents are the best place to store state that would formerly go on unit of related similar-origin browsing contexts. Follow-up for better agent shutdown notifications: whatwg#2581. tc39/ecma262#882 is follow-up to define agents in more detail; in particular make their implicit realms slot explicit. w3c/css-houdini-drafts#224 is follow-up to define worklet ownership better which is needed to define how they relate to agent (sub)clusters. Fixes part of whatwg#2260. Fixes whatwg#851. Fixes w3c/ServiceWorker#1115. Fixes most of w3c/css-houdini-drafts#380 (no tests and no nice grouping of multiple realms in a single agent as that is not needed).
Define the infrastructure for SharedArrayBuffer. This also clarifies along which boundaries a browser implementation can use processes and threads. Tests: web-platform-tests/wpt#5569. Follow-up to define similar-origin window agents upon a less shaky foundation is whatwg#2528. Because of that, similar-origin window agents are the best place to store state that would formerly go on unit of related similar-origin browsing contexts. Follow-up for better agent shutdown notifications: whatwg#2581. tc39/ecma262#882 is follow-up to define agents in more detail; in particular make their implicit realms slot explicit. w3c/css-houdini-drafts#224 is follow-up to define worklet ownership better which is needed to define how they relate to agent (sub)clusters. Fixes part of whatwg#2260. Fixes whatwg#851. Fixes w3c/ServiceWorker#1115. Fixes most of w3c/css-houdini-drafts#380 (no tests and no nice grouping of multiple realms in a single agent as that is not needed).
Define the infrastructure for SharedArrayBuffer. This also clarifies along which boundaries a browser implementation can use processes and threads. Tests: web-platform-tests/wpt#5569. Follow-up to define similar-origin window agents upon a less shaky foundation is whatwg#2528. Because of that, similar-origin window agents are the best place to store state that would formerly go on unit of related similar-origin browsing contexts. Follow-up for better agent shutdown notifications: whatwg#2581. tc39/ecma262#882 is follow-up to define agents in more detail; in particular make their implicit realms slot explicit. w3c/css-houdini-drafts#224 is follow-up to define worklet ownership better which is needed to define how they relate to agent (sub)clusters. Fixes part of whatwg#2260. Fixes whatwg#851. Fixes w3c/ServiceWorker#1115. Fixes most of w3c/css-houdini-drafts#380 (no tests and no nice grouping of multiple realms in a single agent as that is not needed).
Follows whatwg/html#2521. This downgrades the resources submodule due to #5568 which prevents these tests (and many, many other tests in WPT) from working.
FYI @binji @mtrofin on the Blink side, @lars-t-hansen on the Gecko side. Failures:
Worklets are not tested at this time due to spec uncertainty.
This change is