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

Start web worker from static URL on startup #6058

Closed
ryanbaumann opened this issue Jan 25, 2018 · 9 comments
Closed

Start web worker from static URL on startup #6058

ryanbaumann opened this issue Jan 25, 2018 · 9 comments
Assignees
Labels

Comments

@ryanbaumann
Copy link
Contributor

ryanbaumann commented Jan 25, 2018

Issue

Currently, Mapbox GL JS dynamically generates a worker on startup and hosts it in a local window URL. This startup method causes issues with:

  1. Applications that require a worker:src CSP
  2. Mapbox GL startup in iframe sandbox permissions allow-same-origin on IE11, Safari, and Edge browsers.

Possible implementation

  1. Add another build step that browserifies https://github.com/mapbox/mapbox-gl-js/blob/master/src/source/worker.js
  2. Modify https://github.com/mapbox/mapbox-gl-js/blob/master/src/util/browser/web_worker.js so that instead of using WebWorkify, it takes a real URL (by which that browserified worker build will be accessible)

Ideal solution

Ideally, we'd be able to detect CSP on the client and load the same JS file again, with a different entry point. The entry point can be a static worker URL if CSP worker:src is enforced.

cc @kkaefer

Related: #6056

@anandthakker
Copy link
Contributor

ideally, we'd be able to detect CSP on the client and load the same JS file again, with a different entry point

@ryanbaumann is this preferable over having a separate worker bundle, per #5939?

@ryanbaumann
Copy link
Contributor Author

ryanbaumann commented Jan 25, 2018

@anandthakker Either solution should work fine - whether the developer has to build the specific CSP support version, or the browser could make a run-time decision of which web worker initialization source to use.

@anandthakker
Copy link
Contributor

Quick proof of concept here: https://github.com/mapbox/mapbox-gl-js/tree/blobless, just tries the blob url first, catches the exception and tries again with a (user-provided) network URL instead.

@jfirebaugh
Copy link
Contributor

The fact that Safari disallows Blob reads in a sandboxed iframe is a bug: https://bugs.webkit.org/show_bug.cgi?id=170075

I know of one other place that blobs are used in Mapbox GL JS:

const blob: Blob = new window.Blob([new Uint8Array(imgData.data)], { type: 'image/png' });

Presumably we'd need a workaround for that as well.

@ryanbaumann
Copy link
Contributor Author

ryanbaumann commented Feb 27, 2018

@jfirebaugh thanks for linking the bug in Safari.

Summarizing what we've tried so far to support Mapbox GL on Safari in a sandboxed iframe:

  1. Load the worker source from a blob URL in an https page.
  2. Load the worker source from a local file.
    • Result - does not work in a sandboxed-iframe environment. Chrome, Firefox, and Safari all throw an error requesting the local resource from null origin. This is the expected behavior in sandboxed iframes with allow-same-origin not enabled.
  3. Load the worker source from a data URI.

Given the three options above, the only option we know of to support Mapbox GL in Safari using an iframe sandbox without the allow-same-origin flag is to fix the Blob read URL bug in Safari https://bugs.webkit.org/show_bug.cgi?id=170075

@anandthakker
Copy link
Contributor

As of #6196, we're now building our bundle using Rollup, and while we're still producing a single bundle that loads the worker via a blob URL, we're much closer to being able to build & load the worker as a separate bundle:

  • The main bundle loads the worker from mapboxgl.workerUrl. By default, this is set to the Blob URL, but we could easily allow it to be overwritten by client code to point to the location of a standalone copy of the worker bundle.
  • The build process already has an intermediate step in which the worker and its dependencies are bundled separately from the main entrypoint. Adding a step to build & distribute a standalone worker.js would be straightforward.

@thomasneirynck
Copy link

@anandthakker fwiw, this is an important issue for us, now that Kibana will start shipping with a mapbox-gl dependency. Are there any plans to bump up this issue and add this option to future versions of mapbox-gl? thanks,

@chloekraw
Copy link
Contributor

@thomasneirynck - thanks for raising this issue to us. We'll add this to our planning and get back to you when we have a timeline.

@mourner
Copy link
Member

mourner commented Mar 15, 2019

Looked at this again and turns out most of the groundwork for this was already laid out when we switched to Rollup for bundling. So I JFDI: #8044

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants