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

Enable ShadowRealm testing for ErrorEvent and queueMicrotask #49325

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Nov 22, 2024

Adapt the tests for the onerror event on Worker global scopes to cover ShadowRealm global scopes as well.

One of the tests uses setTimeout() directly to fire the global onerror event. Use queueMicrotask() instead, since that is [Exposed=*]. We cannot use t.step_timeout() because that wraps the callback in a t.step_func() which catches the error and fails the test.

Run the microtask evaluation order tests in ShadowRealm scopes.

Add a new test ensuring that the behaviour of wrapping exceptions thrown from outer realm functions in a fresh TypeError extends to the onerror event as well.

(This PR was originally for just ErrorEvent and onerror. I added my queueMicrotask coverage as well, because most of the tests are quite related. EDIT: added reportError coverage as well, pending whatwg/html#9893 (comment))

@ptomato ptomato changed the title Enable ShadowRealm testing for ErrorEvent Enable ShadowRealm testing for ErrorEvent and queueMicrotask Nov 22, 2024
// META: title=Microtask checkpoint after ShadowRealm onerror events
// META: global=shadowrealm

// Adapted from first part of ./resources/checkpoint-after-error-event.js.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not verified that the expected behavior is correct, but I assume it is if it matches the existing test

Adapt the tests for the onerror event on Worker global scopes to cover
ShadowRealm global scopes as well.

One of the tests uses setTimeout() directly to fire the global onerror
event. Use queueMicrotask() instead, since that is [Exposed=*]. We cannot
use t.step_timeout() because that wraps the callback in a t.step_func()
which catches the error and fails the test.
This test is similar to window-onerror-runtime-error-throw.html and
window-onerror-runtime-error.html, but ensuring that the ShadowRealm
global's onerror is triggered and not the Window's onerror.

By my reading of the spec, we cannot have a test equivalent to
window-onerror-parse-error.html in ShadowRealm because
ShadowRealm.prototype.evaluate() will exit early if the given code fails
to parse.
reportError seems to be minimally tested. ShadowRealm doesn't have the
location exposed, so just assume it will be blank. This may need to be
changed depending on what browsers do. It is not specified what the value
should be in https://html.spec.whatwg.org/#extract-error:

  "Set _attributes_[message], _attributes_[filename],
  _attributes_[lineno], and _attributes_[colno] to implementation-defined
  values derived from _exception_.

  NOTE: Browsers implement behavior not specified here or in the
  JavaScript specification to gather values which are helpful, including
  in unusual cases (e.g., `eval`). In the future, this might be specified
  in greater detail.

This seems like exactly such an unusual case.
@@ -1,4 +1,4 @@
// META: global=dedicatedworker-module,sharedworker-module
// META: global=dedicatedworker-module,sharedworker-module,shadowrealm-in-window,shadowrealm-in-shadowrealm,shadowrealm-in-dedicatedworker,shadowrealm-in-sharedworker
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the improved error reporting in #49387 I realized these didn't run in shadowrealm-in-serviceworker and shadowrealm-in-audioworklet because the fake dynamic import isn't actually module code 😖 I'm not sure how important that is though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants