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

Update ServiceWorker webidl #22540

Merged
merged 1 commit into from Dec 23, 2018
Merged

Update ServiceWorker webidl #22540

merged 1 commit into from Dec 23, 2018

Conversation

@CYBAI
Copy link
Collaborator

CYBAI commented Dec 23, 2018

About the void postMessage, I found the spec doesn't have [Throws]; however, as the implementation for postMessage in spec, we should have [Throws] for it because it will throw errors. Then, I realized that [Throws] is not part of standard webidl. Btw, there's a issue discussing about if [Throws] should be standardized or not at heycam/webidl#603.

Also, per the spec, ServiceWorker will includes AbstractWorker instead of implements so I filed #22539 for it. We can update it after the issue is fixed.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix part of #19302
  • These changes do not require tests because it just updates the webidl and the changes should have no impact to current implementation.

This change is Reviewable

@highfive
Copy link

highfive commented Dec 23, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/ServiceWorker.webidl
  • @KiChjang: components/script/dom/webidls/ServiceWorker.webidl
@highfive
Copy link

highfive commented Dec 23, 2018

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@CYBAI CYBAI mentioned this pull request Dec 23, 2018
5 of 6 tasks complete
@jdm
Copy link
Member

jdm commented Dec 23, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Dec 23, 2018

📌 Commit 370ee89 has been approved by jdm

@highfive highfive assigned jdm and unassigned Manishearth Dec 23, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Dec 23, 2018

Testing commit 370ee89 with merge 96cd051...

bors-servo added a commit that referenced this pull request Dec 23, 2018
Update ServiceWorker webidl

About the `void postMessage`, I found the spec doesn't have `[Throws]`; however, as the implementation for `postMessage` in spec, we should have `[Throws]` for it because it will throw errors. Then, I realized that `[Throws]` is not part of standard webidl. Btw, there's a issue discussing about if `[Throws]` should be standardized or not at heycam/webidl#603.

Also, per the spec, `ServiceWorker` will `includes` `AbstractWorker` instead of `implements` so I filed #22539 for it. We can update it after the issue is fixed.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #19302
- [x] These changes do not require tests because it just updates the webidl and the changes should have no impact to current implementation.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22540)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 23, 2018

💔 Test failed - linux-rel-css

@jdm
Copy link
Member

jdm commented Dec 23, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Dec 23, 2018

💣 Failed to start rebuilding: Unknown error

@bors-servo
Copy link
Contributor

bors-servo commented Dec 23, 2018

Testing commit 370ee89 with merge 6840c18...

bors-servo added a commit that referenced this pull request Dec 23, 2018
Update ServiceWorker webidl

About the `void postMessage`, I found the spec doesn't have `[Throws]`; however, as the implementation for `postMessage` in spec, we should have `[Throws]` for it because it will throw errors. Then, I realized that `[Throws]` is not part of standard webidl. Btw, there's a issue discussing about if `[Throws]` should be standardized or not at heycam/webidl#603.

Also, per the spec, `ServiceWorker` will `includes` `AbstractWorker` instead of `implements` so I filed #22539 for it. We can update it after the issue is fixed.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix part of #19302
- [x] These changes do not require tests because it just updates the webidl and the changes should have no impact to current implementation.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22540)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Dec 23, 2018

@bors-servo bors-servo merged commit 370ee89 into servo:master Dec 23, 2018
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@CYBAI CYBAI deleted the CYBAI:update-sw-webidl branch Dec 24, 2018
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

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