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

Support web workers in webviews #87282

Closed
shanejonas opened this issue Dec 18, 2019 · 25 comments
Closed

Support web workers in webviews #87282

shanejonas opened this issue Dec 18, 2019 · 25 comments
Assignees
Labels
custom-editors Custom editor API (webview based editors) feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded webview Webview issues

Comments

@shanejonas
Copy link

shanejonas commented Dec 18, 2019

I'm using the monaco-editor-webpack-plugin to load monaco within a vscode extension, but I'm running into an error around the webworkers when it loads the json language:

Uncaught ReferenceError: importScripts is not defined

image

are webworkers just not supported?

this is the code to set the vscode-resource in my extension

  getWebviewContent(extensionPath: string) {
    const manifest = require(path.join(extensionPath, 'build', 'asset-manifest.json'));

    // get all generated chunks names
    const chunksRegex = /^((?!\.map|\.css|\.html).)*$/;
    const chunkNames = Object.keys(manifest.files).filter(key => chunksRegex.test(key));

    // Use a nonce to whitelist which scripts can be run
    const nonce = v4();

    const scripts = [...chunkNames]
      .map((scriptName) => {
        const scriptUri = vscode.Uri
          .file(path.join(extensionPath, 'build', manifest.files[scriptName]))
          .with({ scheme: 'vscode-resource' });

        return `<script nonce="${nonce}" src="${scriptUri}"></script>`;
      })
      .join('');

    return `<!DOCTYPE html>
			<html lang="en">
			<head>
				<meta charset="utf-8">
				<meta name="viewport" content="width=device-width,initial-scale=1,shrink-to-fit=no">
				<meta name="theme-color" content="#000000">
				<title>React App</title>
        <meta http-equiv="Content-Security-Policy" content="default-src 'none'; img-src vscode-resource: https:; script-src 'nonce-${nonce}';style-src vscode-resource: 'unsafe-inline' http: https: data:;">
				<base href="${vscode.Uri.file(path.join(extensionPath, 'build')).with({ scheme: 'vscode-resource' })}/">
				<style>
					body {
						background: white;
					}
				</style>
			</head>

			<body>
				<noscript>You need to enable JavaScript to run this app.</noscript>
				<div id="root"></div>
				${scripts}
			</body>
			</html>`;
  }

and how it gets used:

      ReactWebView.currentPanel.webview.html = this.getWebviewContent(context.extensionPath);
@vscodebot vscodebot bot added the html HTML support issues label Dec 18, 2019
@dbaeumer dbaeumer removed the html HTML support issues label Dec 19, 2019
@mjbvz mjbvz added the webview Webview issues label Dec 20, 2019
@alexdima
Copy link
Member

@mjbvz Are web workers available inside web views ?

@mjbvz
Copy link
Contributor

mjbvz commented Jan 17, 2020

I don't think we can enable them at the moment. We serve up the WebViews in its own origin, this causes chrome to throw a security error:

[Embedded Page] Uncaught SecurityError: Failed to construct 'Worker': Script at 'vscode-resource://file///Users/matb/projects/vscode-extension-samples/webview-sample/media/worker.js' cannot be accessed from origin 'null'.

@alexdima alexdima added the feature-request Request for new features or functionality label Jan 17, 2020
@alexdima alexdima removed their assignment Jan 17, 2020
@vscodebot vscodebot bot added this to the Backlog Candidates milestone Jan 17, 2020
@vscodebot
Copy link

vscodebot bot commented Jan 17, 2020

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@vscodebot
Copy link

vscodebot bot commented Mar 7, 2020

This feature request has not yet received the 20 community upvotes it takes to make to our backlog. 10 days to go. To learn more about how we handle feature requests, please see our documentation.

Happy Coding

@vscodebot
Copy link

vscodebot bot commented Mar 11, 2020

🙂 This feature request received a sufficient number of community upvotes and we moved it to our backlog. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@vscodebot vscodebot bot modified the milestones: Backlog Candidates, Backlog Mar 11, 2020
@MulverineX
Copy link

MulverineX commented Aug 18, 2020

It is very likely the common usecase of this would be to embed an instance of Monaco in the webview (as seen in the original post). I know several people who want to do this as well. Wouldn't it be better for performance & usability to instead provide an API to use VSCode's native Monaco & editing tools for your custom file editor?

  • Editor receives a plain text input & language id (support for JSON language schema's as well) from the extension
  • User edits like any other file (with snippets, language server, color scheme/theme, etc)
  • Extension is given updated plain text output on save.

SW support would provide that option for other things, but it is not far off to assume that the primary use of this feature would be to get the Monaco editor. Therefore, a (presumably) more performant & usage friendly feature should be considered.

@Tyriar
Copy link
Member

Tyriar commented Dec 15, 2020

@mjbvz should we rename this issue to support web workers in webviews? I'd like this as well, just not for monaco.

@deepak1556
Copy link
Contributor

deepak1556 commented Sep 10, 2021

September plan has this assigned to me #132467 which I just noticed :)

@mjbvz regarding the error in #87282 (comment),

  1. The URL for the worker resource will be assigned null as vscode-resource is not a standard custom protocol. Where do we register the handler for vscode-resource ? Also the URL should follow https://datatracker.ietf.org/doc/html/rfc3986#section-3 if chromium has to assign non-null origin for the resource. For ex:
vscode-resource://some-uuid/worker.js // -> vscode-resource://some-uuid will become the origin

and internally we map uuid to the extension path to provide the resource via custom handler.

  1. https://gist.github.com/deepak1556/36c31867ad3a11c3b03a56ca8bd3f536 demonstrates runtime support for web worker created inside OOP frame which is loaded over custom protocol.

This is purely an issue on our end with the type of URL created and not the runtime.

@mjbvz
Copy link
Contributor

mjbvz commented Sep 10, 2021

@deepak1556 I believe the url for worker scripts should now use https and look something like:

https://file%2B.vscode-resource.vscode-webview.net/Users/matb/projects/vscode-extension-samples/webview-sample/media/worker.js

Trying to load this worker still fails though since the origin of the worker doesn't match the origin of the webview's iframe:

main.js:3 Uncaught DOMException: Failed to construct 'Worker': Script at 'https://file+.vscode-resource.vscode-webview.net/Users/matb/projects/vscode-extension-samples/webview-sample/media/worker.js' cannot be accessed from origin 'vscode-webview://b121a65a-63f4-4a12-8cf1-633f2d1c4a95'.

@deepak1556
Copy link
Contributor

It can be worked around by making the CORS fetch first on the main thread and then creating a blob URL for the worker script which adheres to the same origin policy declared in the spec, here is an updated version of the frame file https://gist.github.com/deepak1556/36c31867ad3a11c3b03a56ca8bd3f536#file-frame-html

Don't we already do something similar on the web ?

@mjbvz
Copy link
Contributor

mjbvz commented Sep 14, 2021

Ah yes good call about blob urls! This works in the webview example extension:

const workerSource = `console.log('hello worker');`

const blob = new Blob([workerSource], { type: 'application/javascript' });
const blobUrl = URL.createObjectURL(blob)

new Worker(blobUrl);

So it looks like we do support workers but they effectively have to be inlined

@Tyriar
Copy link
Member

Tyriar commented Sep 14, 2021

I guess this wouldn't support dynamic imports of the form await import('./folderRelativeToWorkerMain/file.js')? If so, is there a way we can get it working without all these hoops to jump through? It would be nice if it just worked like in a browser.

@mjbvz
Copy link
Contributor

mjbvz commented Sep 14, 2021

@Tyriar You can using fetch. Not too complicated but not pretty either:

// Get the url to the worker 
const workerSource = document.currentScript.getAttribute('src').replace('main.js', 'worker.js'); 

fetch(workerSource).then(result => result.blob()).then(blob => {
	const blobUrl = URL.createObjectURL(blob)
	new Worker(blobUrl);
});

@Tyriar
Copy link
Member

Tyriar commented Sep 14, 2021

@mjbvz I mean using dynamic imports from inside the worker, I don't think that's what you mean in your example?

@mjbvz
Copy link
Contributor

mjbvz commented Sep 14, 2021

Yes that won't work: you'd need to bundle the worker into a single file

The only way I can see to work around this would be updating the urls we use for loading resources from extensions to be on the same origin as the webview:

On web for example, a webview will be on something like https://59f57046-480c-462d-ae42-1caf2c4b7cb6.vscode-webview.net'.

While a resource will come from something like https://vscodeweb.azureedge.net/insider/f66a3e06bcb9f000e5dc0ad0040ff9b32fc75c78/extensions/markdown-language-features/media/markdown.css

This may be possible to do but we would need to make sure it wouldn't regress anything related to security. Also would add back a lot of complexity we were able to remove when we switched to use a static origin for webview resources

@Tyriar
Copy link
Member

Tyriar commented Sep 14, 2021

@mjbvz can we expose the whole extension's contents on `https://59f57046-480c-462d-ae42-1caf2c4b7cb6.vscode-webview.net'?

While I certainly could do this, it adds a lot of work on the extension's side to handle this and it will probably slow down webpack quite a bit as I'll have heaps of targets and extra overhead when adding more "tasks" (what I use for resize image, fill, etc.)

@Tyriar
Copy link
Member

Tyriar commented Sep 14, 2021

Additionally all of this extra work will be VS Code specific and my server-based target which I use for tests will use a separate mechanism and I'd need to maintain both.

@mjbvz
Copy link
Contributor

mjbvz commented Sep 14, 2021

Maybe, but like I said we'd have to think if this introduces any new security concerns and I really don't want to add back in the quite fragile code we needed to support resources under the same dynamic origin that the webview itself is hosted on

I don't think it's worth it if we have a reasonable workaround

@Tyriar
Copy link
Member

Tyriar commented Sep 14, 2021

Depends on your definition of reasonable as we're essentially creating a branch new vscode-specific worker API, will monaco support our new "VS Code webview-based worker API"? What about any other unaffiliated library that gets used in webviews?

FWIW I don't understand what the security concerns are though.

@deepak1556
Copy link
Contributor

I guess this wouldn't support dynamic imports of the form await import('./folderRelativeToWorkerMain/file.js')?

Yeah if we are using CORS worker then relative importScripts cannot be used, as they are resolved to the document base URL.

If so, is there a way we can get it working without all these hoops to jump through? It would be nice if it just worked like in a browser.

You would have to make them absolute urls dynamically, for ex:

// From the top-level document

myWorker.postMessage({origin: workerResourceOrigin})


// In the worker file

self.onmessage = (ev) => {
  if (ev.origin) {
   importScripts(`${ev.origin}` + path)  // Since these resolve synchronously, all these imports have to be pushed back till we know the origin to rewrite.
 }
}

The only way I can see to work around this would be updating the urls we use for loading resources from extensions to be on the same origin as the webview:

Yup this would be the best way forward, it is not ideal for extensions to implement above hacks to get working on the desktop client.

This may be possible to do but we would need to make sure it wouldn't regress anything related to security. Also would add back a lot of complexity we were able to remove when we switched to use a static origin for webview resources

Did we have extension resources loaded over custom protocol before ? It would be good to understand concerns related to this switch.

@mjbvz
Copy link
Contributor

mjbvz commented Sep 14, 2021

Did we have extension resources loaded over custom protocol before ? It would be good to understand concerns related to this switch.

We did, and it caused a lot of headaches. When I was commenting before, I actually forgot that on desktop the webview's origin is a custom protocol: vscode-webview://ID. electron/electron#28528 may make it possible for service workers to intercept a request to a custom protocol now (I haven't tested yet). We definitely do not want to go back to serving the resources over an electron custom protocol again

However even before switching to service workers on desktop, I believe we served the webview content that we own in a distinct origin from the webview resources that come from extensions/the workspace

@mjbvz
Copy link
Contributor

mjbvz commented Sep 16, 2021

I documented how to use web workers in webviews here: https://github.com/Microsoft/vscode-docs/blob/eb58fbbf6c26e781f33aec963eeba0139337ba87/api/extension-guides/webview.md#L798

I'm closing this issue since workers do work not. If enough extension authors start running into the current limitations , we can see what we can do to relax them in the future

@mjbvz mjbvz closed this as completed Sep 16, 2021
@deepak1556
Copy link
Contributor

I am good with above solution for now, based on #87282 (comment) it looks like we don't support workers on the web out of box because of the origin restrictions. But if we come to relax the origin policy on the web, it would be great to follow-up on the desktop as well.

@rzhao271 rzhao271 added the verification-needed Verification of issue is requested label Sep 29, 2021
@rchiodo
Copy link
Contributor

rchiodo commented Sep 29, 2021

@mjbvz I'm not 100% sure but it sounds like the verification work here is to just verify the documentation? No changes were made, web workers aren't supported in webviews unless you stick everything into a single js file?

@rchiodo rchiodo added the verification-steps-needed Steps to verify are needed for verification label Sep 29, 2021
@mjbvz
Copy link
Contributor

mjbvz commented Sep 29, 2021

Yes that's correct: the fix was documenting the proper approach so I'm going to mark as verified based on the docs existing

@mjbvz mjbvz added verified Verification succeeded and removed verification-steps-needed Steps to verify are needed for verification labels Sep 29, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
custom-editors Custom editor API (webview based editors) feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded webview Webview issues
Projects
None yet
Development

No branches or pull requests

10 participants