Roll WPT for abort reason

[MattiasBuelens]

It looks like we forgot to roll WPT for the new tests in #1182. We rolled it to web-platform-tests/wpt@4905296, but that's behind web-platform-tests/wpt@b724cac where we actually landed the new tests. 😬

Update: there were some bugs in the new tests, so I opened web-platform-tests/wpt#31738 to fix them.

[domenic]

Oh no, the _pageShowingFlag jsdom error again... what did we do last time??

[MattiasBuelens]

Looks like we also have some failing tests. Fun! 😛

  × (reason: 'null') all the error objects should be the same object
  
    promise_rejects_dom: pipeTo rejects with AbortError function "function() { throw e }" threw null, not an object
        at processTicksAndRejections (internal/process/task_queues.js:95:5)
        at async Test.<anonymous> (http://127.0.0.1:39171/streams/piping/abort.any.js:67:7)

  × (reason: 'null') abort should prevent further reads
  
    promise_rejects_dom: pipeTo rejects with AbortError function "function() { throw e }" threw null, not an object
        at processTicksAndRejections (internal/process/task_queues.js:95:5)
        at async Test.<anonymous> (http://127.0.0.1:39171/streams/piping/abort.any.js:137:7)

  × (reason: 'null') all pending writes should complete on abort
  
    promise_rejects_dom: pipeTo rejects with AbortError function "function() { throw e }" threw null, not an object
        at async Test.<anonymous> (http://127.0.0.1:39171/streams/piping/abort.any.js:176:7)

It seems like the tests expect controller.abort(null) to behave like controller.abort(undefined), meaning that the AbortSignal becomes aborted with an AbortError. However, the spec says:

2. Set signal’s abort reason to reason if it is given; otherwise to a new "AbortError" DOMException.

AFAIK "if it is given" means "if it is not undefined", so null should be passed through unmodified and signal.reason becomes null. At least, that's how it's implemented in jsdom and in Deno. Node.js doesn't seem to create an AbortError yet, instead it always passes reason unmodified.

[domenic]

Yep, I think those are test bugs.

[MattiasBuelens]

Yep, I think those are test bugs.

Should be easy enough to fix. On it. 👷‍♂️

Oh no, the _pageShowingFlag jsdom error again... what did we do last time??

It looks like those happen as a result of some of the tests failing (and thus not completing properly). Making the tests pass also fixes those errors.

[MattiasBuelens]

Opened web-platform-tests/wpt#31738.

[MattiasBuelens]

Hmm, tests are now passing, but the _pageShowingFlag thingy still shows up in the logs... 😕

I believe what's happening is that wpt-runner calls window.close() before the load event fires on that window. That means that jsdom has already deleted window._document by the time it tries to handle the load event.

The easiest solution is probably to add an if (!window._document) { return } statement right before this line. I think that aligns with step 9.2 of 13.2.7 The end?

2. If the Document object's browsing context is null, then abort these steps.

[MattiasBuelens]

Never mind, it's worse. 😛

1. jsdom fires a load event on window. (source)
2. testharness.js has a load event listener, which calls tests.complete(). (source)
3. testharness.js eventually calls tests.notify_complete(), which runs all done callbacks. (source)
4. wpt-runner has a done callback, which calls window.close(). (source)
5. jsdom's Window.close() deletes the window's document. (source)
6. Finally we return from all these nested function calls and come back to jsdom's load event listener. We now try to retrieve the wrapper of window._document, but it's already undefined. (source)

In other words, while dispatching the load event, the window is closed synchronously. jsdom doesn't expect this, and I'm not entirely sure if the HTML spec even covers this? 😕

[domenic]

Great diagnosis! Let's move that over to the jsdom repo and consider the fix there, while we merge this PR :)