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

Intermittent failure in /_mozilla/mozilla/window-postmessage-sameorigin.html #25808

Closed
jdm opened this issue Feb 19, 2020 · 5 comments
Closed

Intermittent failure in /_mozilla/mozilla/window-postmessage-sameorigin.html #25808

jdm opened this issue Feb 19, 2020 · 5 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Feb 19, 2020

  â–¶ Unexpected subtest result in /_mozilla/mozilla/window-postmessage-sameorigin.html:
  │ FAIL [expected PASS] explicit invalid url, cloneable string message (handler)
  │   → assert_throws_js: function "function() { postMessage(message, origin) }" threw object "SyntaxError: The string did not match the expected pattern." ("SyntaxError") expected instance of function "function SyntaxError() {
  │   →     [native code]
  │   → }" ("SyntaxError")
  │ 
  │ expect_error_js/</<@http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:15:7
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1867:25
  │ test@http://web-platform.test:8000/resources/testharness.js:548:30
  │ expect_error_js/<@http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:14:5
  │ next_test@http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:77:26
  │ @http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:84:3
  │ Tests.prototype.notify_result/<@http://web-platform.test:8000/resources/testharness.js:2593:21
  │ forEach@http://web-platform.test:8000/resources/testharness.js:3460:26
  │ Tests.prototype.notify_result@http://web-platform.test:8000/resources/testharness.js:2590:9
  │ Tests.prototype.result@http://web-platform.test:8000/resources/testharness.js:2584:14
  │ cleanup_done@http://web-platform.test:8000/resources/testharness.js:2112:15
  │ Test.prototype.cleanup@http://web-platform.test:8000/resources/testharness.js:2051:13
  │ Test.prototype.done@http://web-platform.test:8000/resources/testharness.js:1997:14
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1878:18
  │ test@http://web-platform.test:8000/resources/testharness.js:548:30
  │ expect_error_js/<@http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:14:5
  │ next_test@http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:77:26
  │ @http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:84:3
  │ Tests.prototype.notify_result/<@http://web-platform.test:8000/resources/testharness.js:2593:21
  │ forEach@http://web-platform.test:8000/resources/testharness.js:3460:26
  │ Tests.prototype.notify_result@http://web-platform.test:8000/resources/testharness.js:2590:9
  │ Tests.prototype.result@http://web-platform.test:8000/resources/testharness.js:2584:14
  │ cleanup_done@http://web-platform.test:8000/resources/testharness.js:2112:15
  │ Test.prototype.cleanup@http://web-platform.test:8000/resources/testharness.js:2051:13
  │ Test.prototype.done@http://web-platform.test:8000/resources/testharness.js:1997:14
  â”” Test.prototype.step_func_done/<@http://web-platform.test:8000/resources/testharness.js:1911:23

  â–¶ Unexpected subtest result in /_mozilla/mozilla/window-postmessage-sameorigin.html:
  │ FAIL [expected PASS] explicit invalid url, cloneable string message (listener)
  │   → assert_throws_js: function "function() { postMessage(message, origin) }" threw object "SyntaxError: The string did not match the expected pattern." ("SyntaxError") expected instance of function "function SyntaxError() {
  │   →     [native code]
  │   → }" ("SyntaxError")
  │ 
  │ expect_error_js/</<@http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:15:7
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1867:25
  │ test@http://web-platform.test:8000/resources/testharness.js:548:30
  │ expect_error_js/<@http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:14:5
  │ next_test@http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:77:26
  │ @http://web-platform.test:8000/_mozilla/mozilla/window-postmessage-sameorigin.html:84:3
  │ Tests.prototype.notify_result/<@http://web-platform.test:8000/resources/testharness.js:2593:21
  │ forEach@http://web-platform.test:8000/resources/testharness.js:3460:26
  │ Tests.prototype.notify_result@http://web-platform.test:8000/resources/testharness.js:2590:9
  │ Tests.prototype.result@http://web-platform.test:8000/resources/testharness.js:2584:14
  │ cleanup_done@http://web-platform.test:8000/resources/testharness.js:2112:15
  │ Test.prototype.cleanup@http://web-platform.test:8000/resources/testharness.js:2051:13
  │ Test.prototype.done@http://web-platform.test:8000/resources/testharness.js:1997:14
  â”” Test.prototype.step_func_done/<@http://web-platform.test:8000/resources/testharness.js:1911:23

My suspicion is that this is related to the global that is used to retrieve the SyntaxError constructor, since the objects will be different for different globals.

@gterzian
Copy link
Member

@gterzian gterzian commented Feb 20, 2020

I think it might be the test, that should use assert_throws_js(SyntaxError) as opposed to assert_throws(new SyntaxError()).

function "function() { postMessage(message, origin) }" threw object "SyntaxError: The string did not match the expected pattern." ("SyntaxError") expected instance of function "function SyntaxError()

Although kinda weird this would be intermittent? Maybe there is an underlying issue that is indeed intermittent.

@gterzian
Copy link
Member

@gterzian gterzian commented Feb 20, 2020

Ok sorry I was actually looking at the outdated code.

I think this is a problem with our error bindings.

You can make the test pass by changing it to expect_error_dom('SYNTAX_ERR'

So it looks like we're throwing a dom exception, instead of a JS error.

I think we would need a new helper like https://github.com/servo/mozjs/blob/9b44b1e870cae2d96ea41bf2aa8a7d5eaa4cdce7/mozjs/js/rust/src/error.rs#L66

That would use this error code: https://github.com/servo/mozjs/blob/b2f83932fe9d361face14efd03f2465b9262e687/mozjs/js/public/ErrorReport.h#L52

And use it at

Error::Syntax => DOMErrorName::SyntaxError,

in the same way as is for example currently done with Type errors at

Error::Type(message) => unsafe {

So is this a change in the spec, or were we doing it wrong all along?

@jdm
Copy link
Member Author

@jdm jdm commented Feb 20, 2020

There is no JS syntax error exception object.; I should have used assert_throws_dom instead of assert_throws_js.

@jdm
Copy link
Member Author

@jdm jdm commented Feb 20, 2020

No, that's a lie. There is one, but the spec is clear that we want a dom exception: https://html.spec.whatwg.org/multipage/web-messaging.html#window-post-message-steps

bors-servo added a commit that referenced this issue Feb 20, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 20, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 20, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 20, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 21, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 21, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 21, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 21, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit that referenced this issue Feb 21, 2020
Expect dom syntax error in window postmessage sameorigin test

<!-- Please describe your changes on the following line: -->

FIX #25808

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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