Error opening images on remotes

[mjbvz]

@mjbvz this happens for me 100% of the time when using Remote - WSL. If I open a folder locally I can view the images. Trying to open any images when remote give this error.

There are no errors in the console.

Using latest Insiders

Version: 1.46.0-insider (user setup)
Commit: 9e439b13d5784538333bee044ca1d1a992bc1c90
Date: 2020-05-28T18:44:47.151Z
Electron: 7.3.0
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Windows_NT x64 10.0.19041

Originally posted by @chrisdias in #95736 (comment)

[mjbvz]

@chrisdias I extracted this to a new issue. I can reproduce this with containers as well

[mjbvz]

Still happening for me on wsl.

@alexdima When you have a chance, can you please look at my comment b84597e#commitcomment-39521095.

@deepak1556 @bpasero Unless there is a simple fix, I think we need to revert the work to move the webview resource protocol to the main process. The change has been much more complicated than I anticipated and it is late in the release cycle. Since we are still on electron 7, I don't anticipate this revert to cause any issues problems. We will still have to solve this problem for June

@deepak1556 I just want to check again if there is any way we can get service workers supported in our webviews? The service worker based approach we use to implement webviews on the web is quite elegant

[deepak1556]

I am lacking context on the problem for remote scenario, @mjbvz can you please explain what is it we are trying to achieve via the render process that is not possible now with protocol handlers in the main process ?

[mjbvz]

In remote scenarios, files in the workspace have urls such as:

vscode-remote://vscode-ssh+localhost/path/to/file.png

We can read uris that have the vscode-remote scheme in our renderer processes (there is a file system provider registered for them). However it looks like we currently cannot read this sort of url in the main process

[deepak1556]

Thanks for the context @mjbvz

IIUC, previously for remote schemes the data is obtained over the RemoteFileSystemProvider that is registered in the render process and was served over the webview resource protocol handler that was also registered in the render process, is that correct ?

Some thoughts,

• When the protocol handler is registered in the render process, it doesn't handle any IO in the render process (thats a chromium implementation), its just a proxy to the actual handler in the main process. So any data that was served in the render process would be forwarded to the main process over ipc and then served back to the correct frame which is another ipc but this one is an efficient as it is handled by chromium. For file and http protocol there is no ipc cost for the proxy handler since only local or remote endpoints are forwarded, but for buffer protocol there is a considerable cost.

• Now when the protocol handler is moved to the main process, it actually cuts the proxy ipc cost. So if the RemoteFileSystemProvider is also made available in the main process which would be ideal for sandbox mode as well, then the protocol handler can forward the data over stream protocol which is more efficient than buffer protocol. Overall it will improve resource handling for webview.

Now given this is a considerable refactor, I am fine with reverting the protocol change for this iteration and continuing the work in June.

I just want to check again if there is any way we can get service workers supported in our webviews? The service worker based approach we use to implement webviews on the web is quite elegant

Service workers for custom protocol are not in a working state for a while now, it will take some effort to get this working. But I am confident for the problem in hand, the solution can be achieved without relying on service workers.

[alexdima]

@mjbvz I believe the solution is to call DOM.asDomUri. That is what we do for other resources that need to be loaded via the main process.

[mjbvz]

Thanks @alexdima! I tried this but it looks like RemoteAuthoritiesImpl._hosts and _ports are empty in this case, so the resolved uri ends up being:

vscode-remote-resource://undefined:undefined/vscode-remote-resource?path%3D%252Fworkspaces%252Fvscode-docs%252Fdocs%252Fcontainers%252Fimages%252Fregistries%252Fazure-repository-context-menu.png

I'm checking in the change that moves us back to using the render side vscode-resource scheme now. Will continue looking into this for June

[deepak1556]

@mjbvz wouldn't it be better to have this issue open until the root issue is fixed in next iteration.

[mjbvz]

@deepak1556 I've reopened #89038 to track switching back over to use a protocol from the main process. I want someone to verify that this issue has been fixed for May however

[deepak1556]

cool, thanks!

[alexdima]

I tried this but it looks like RemoteAuthoritiesImpl._hosts and _ports are empty in this case, so the resolved uri ends up being

@mjbvz Are you calling this method from some other place than the renderer process that is remote enabled? Are you calling this method at a time when the remote authority is not resolved?

[mjbvz]

@alexdima We actually need to call it from the main processes. This is where the protocol for registering local resources in webviews is registered:

vscode/src/vs/platform/webview/electron-main/webviewProtocolProvider.ts

Line 26 in 75fb631

protocol.registerStreamProtocol(Schemas.vscodeWebviewResource, async (request, callback): Promise<void> => {

[alexdima]

OK. In that case a new approach is needed, but I'm not sure how that should work. Is there any chance these resources can be intercepted and rewritten before they reach the main process? That is the pattern we use around DOM.asDomUri; basically the URIs are replaced in the renderer side with the resolved connection information.

[chrisdias]

i also verified this now works on windows + wsl. thx.