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

Use MessageChannel to support Chrome <51, test with localtunnel.me for https #9

Merged
merged 1 commit into from
Jul 20, 2016

Conversation

nolanlawson
Copy link
Owner

@nolanlawson nolanlawson commented Jul 20, 2016

Fixes #7

MessageChannels are preferred in the case of Service Workers, because event.source is null in Chrome < 51 (bug), which means you can receive messages but can't send any back. This can be revisited once Chrome <51 drops to low usage numbers and we can just switch to event.source, but for now this works in Chrome 42+ and in Firefox. For Web Workers, I stick with the old system and avoid unnecessary MessageChannel creation.

I also needed some way of actually testing Service Workers to prevent regressions, meaning I needed Zuul to run over HTTPS. I decided to go with localtunnel.me because ngrok, although it also offered https, was too restrictive with the concurrent number of tunnels. I tested manually and confirmed that this is actually testing HTTPS and actually testing Service Worker.

Unfortunately I had to set the tested Chrome versions to 42+ because, although Service Worker is in 40 and 41, it has some bug in MessageChannels that breaks the test (no time to debug to figure out what the issue was). Luckily Chrome 40 and 41 have very low usage.

@nolanlawson
Copy link
Owner Author

Background: #7 #8

@coveralls
Copy link

coveralls commented Jul 20, 2016

Coverage Status

Changes Unknown when pulling c60ba24 on 7 into * on master*.

@nolanlawson
Copy link
Owner Author

Sweeeet, finally passing

@nolanlawson nolanlawson merged commit c60ba24 into master Jul 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants