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 service workers for loading webview resources on desktop #120654

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

mjbvz
Copy link
Collaborator

@mjbvz mjbvz commented Apr 6, 2021

This switches us from using a custom protocol to using a service worker to load resources inside webviews. Previously we had only been using service workers on web (since custom protocols are not supported on web). The service worker based approach is much cleaner to than our custom protocol work and lets us avoid some extra roundtrips between the main process and the renderer

We were previously blocked from using service workers on desktop due to an electron issue. However this has now been resolved

/cc @deepak1556

@mjbvz mjbvz self-assigned this Apr 6, 2021
.yarnrc Outdated
@@ -1,3 +1,3 @@
disturl "https://electronjs.org/headers"
target "11.4.1"
target "11.4.2"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deepak1556 I bumped the electron version for testing but this should likely be part of a separate PR

@mjbvz mjbvz added this to the April 2021 milestone Apr 6, 2021
mjbvz added a commit to mjbvz/vscode that referenced this pull request Apr 6, 2021
This brings in the service worker support required for microsoft#120654
@mjbvz mjbvz mentioned this pull request Apr 6, 2021
deepak1556 pushed a commit that referenced this pull request Apr 7, 2021
* Pick up electorn 11.4.2

This brings in the service worker support required for #120654

* Update cgmanifest
This switches us from using a custom protocol to using a service worker to load resources inside webviews. Previously we had only been using service workers on web (since custom protocols are not supported on web). The service worker based approach is much cleaner to than our custom protocol work and lets us avoid some extra roundtrips between the main process and the renderer

We were previously blocked from using service workers on desktop due to an electron issue. However this has now been resolved
@mjbvz mjbvz force-pushed the dev/mjbvz/service-workers-desktop branch from 92c97a4 to ae3aeac Compare April 7, 2021 22:12
@mjbvz mjbvz merged commit d05ded6 into main Apr 7, 2021
@mjbvz mjbvz deleted the dev/mjbvz/service-workers-desktop branch April 7, 2021 22:14
@mjbvz
Copy link
Collaborator Author

mjbvz commented Apr 7, 2021

Merging this for testing in insiders

It's possible this change will cause regressions due to an oversight the implementation or for specific extensions that are using the webview in interesting ways. Will continue testing today and look for more feedback once this hits insiders

mjbvz added a commit that referenced this pull request Apr 8, 2021
Mistakenly added as part of #120654

Doesn't seem to cause issues in the current builds but breaks our electron 12 build
@github-actions github-actions bot locked and limited conversation to collaborators May 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant