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

[websockets] Correct failure case #16354

Closed

Conversation

@jugglinmike
Copy link
Contributor

jugglinmike commented Apr 15, 2019

The test titled 002.html includes a document which references an
undefined property, asset_equals. Correcting this typo would allow
tests to pass spuriously (that is: when the document is not discarded).
Replace the assertion that was originally intended with
assert_unreached so that user agents which execute this code path
report a test failure.

The test titled 001.html also includes a misspelled reference, but it
does not rely on the value. Remove the reference.

The test titled `002.html` includes a document which references an
undefined property, `asset_equals`. Correcting this typo would allow
tests to pass spuriously (that is: when the document is not discarded).
Replace the assertion that was originally intended with
`assert_unreached` so that user agents which execute this code path
report a test failure.

The test titled `001.html` also includes a misspelled reference, but it
does not rely on the value. Remove the reference.
@jugglinmike
Copy link
Contributor Author

jugglinmike commented Apr 15, 2019

Reviewing more closely, it looks like the existing use of assert_unreached is also incorrect: that seems to be the success case. All browsers are failing the test currently, and three of them fail due to that assertion:

https://wpt.fyi/results/websockets/unload-a-document/002.html?sha=3efff9b&label=master&product=chrome%5Bexperimental%5D&product=edge&product=firefox%5Bexperimental%5D&product=safari%5Bexperimental%5D

So I've added another commit to replace the existing assert_unreached with t.done.

@jugglinmike
Copy link
Contributor Author

jugglinmike commented Apr 15, 2019

One more bug: the child document uses the uuid function object in the parent as a key to sessionStorage. In another fixup commit, I've replaced it with the intended string value stored in token.

@jugglinmike
Copy link
Contributor Author

jugglinmike commented Nov 9, 2019

On further inspection, the expectation as written seems correct to me. The document should be salvageable because its only WebSocket has been closed, so being "made to disappear" during unload should have no effect.

The property references continue to be incorrect, though, so the test cannot pass. I've filed a new pull request that only corrects the references: gh-20190.

@jugglinmike jugglinmike closed this Nov 9, 2019
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

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