Use ExtendableMessageEvent for messageerror in service workers

[gterzian]

In the light of w3c/ServiceWorker#1485

code:

servo/components/script/dom/serviceworkerglobalscope.rs

Line 418 in b98a8fc

MessageEvent::dispatch_error(target, scope.upcast());

Also requires adding a dispatch_error method to

servo/components/script/dom/extendablemessageevent.rs

Line 85 in b98a8fc

impl ExtendableMessageEvent {

[nosark]

I'd like to work on this issue!

[gterzian]

Thanks @nosark, let me know if you have an questions.

[nosark]

@gterzian I want to clarify the necessary changes to be made. Am I adding the dispatch_error functionality to ExtendableMessageEvent and then replacing the MessageEvent::dispatch_error call in serviceworkerglobalscope.rs with the ExtendableMessageEvent::dispatch_error call? Do I need to create any new tests for the added dispatch_error function? Is there anything I'm missing? Thanks @gterzian !

[gterzian]

Yes that sounds like a correct description.

Regarding the tests, it looks like they were already added/changed in the WPT test suite by Chromium in https://chromium-review.googlesource.com/c/chromium/src/+/1921521 although it looks we haven't synced yet(which I believe will be done at some point semi-automatically). So I don't think you need to change anything. If your change makes a test fail, we will have to look into it, perhaps add a partial sync with the upstream test suite to your PR.

[jdm]

Those test changes were made last December, so we have definitely synced since then. That being said, we can't run most service worker tests because our implementation is very incomplete.

[nosark]

@gterzian @jdm Awesome, thank you for all of the info! Just a heads up, I was just assigned a concussion recovery protocol (hockey) so it may take me a little longer than usual, but I'll get this out asap!

[nosark]

Hey guys, I've started implementing this, but I am receiving a linking error similar to issue #20736 If I can't get around this, should I setup my windows machine and test and submit from that machine instead of my macbook?

[jdm]

Yes, it's important to be able to build when submitting code changes. Sorry we don't have any good suggestions for the linking error.

[nosark]

@jdm Awesome, thanks for the quick response, I'll get right on it!

[nosark]

@jdm Everything is building again on my mac, And I'm trying to test my solution. Is there a specific test I could run my solution against? I've tried running against the WPT test suite and most of the tests throw a navigator.serviceWorker.getRegistration() is not a function error. I imagine this error could be from the incomplete service worker tests implementation mentioned above. Thanks in advance!

[jdm]

Our service worker implementation is still incomplete, so I doubt there are any automated tests that run to the point where they can verify this change. @gterzian may know better than me, though.

[gterzian]

Yes so currently the SW globalscope actually doesn't run, so we can't test it with javascript and I think the change will not make a difference to the WPT test expectations, but I would say if it compiles it should be good enough for now. @nosark Do you want to open a PR with your work?

[nosark]

@jdm ok. @gterzian yeah I'll put in for PR. Thanks guys!

[kootoopas]

Someone with access should add the "has open pr" label to this.