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

remote: allow loading resources for managed remote authorities #185720

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

connor4312
Copy link
Member

This implements basically the same 'loopback' that vscode.dev does via
a service worker to load resources for managed remote authorities in
Electron. Except instead of using a service worker, it uses a buffer
protocol via a new 'vscode-managed-remote-resource' scheme. It finds
the window that the request came from and asks it to load the remote
file.

I initially looked at folding it into the existing 'vscode-remote-resource'
scheme, but we needed a "buffer protocol" for this and it seems http
protocols can only respond with an HTTP URL, which we don't want here.

This implements basically the same 'loopback' that vscode.dev does via
a service worker to load resources for managed remote authorities in
Electron. Except instead of using a service worker, it uses a buffer
protocol via a new 'vscode-managed-remote-resource' scheme. It finds
the window that the request came from and asks it to load the remote
file.

I initially looked at folding it into the existing 'vscode-remote-resource'
scheme, but we needed a "buffer protocol" for this and it seems http
protocols can _only_ respond with an HTTP URL, which we don't want here.
@connor4312 connor4312 enabled auto-merge (squash) June 21, 2023 03:17
@connor4312 connor4312 self-assigned this Jun 21, 2023
@VSCodeTriageBot VSCodeTriageBot added this to the June 2023 milestone Jun 21, 2023
return callback(notFound());
}

remoteResourceChannel.value.call<NodeRemoteResourceResponse>(NODE_REMOTE_RESOURCE_IPC_METHOD_NAME, [url]).then(
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not familiar with the details of the issue this PR is trying to solve, but just asking from Electron API adoption perspective. In the current implementation, when the renderer process encounters a resource with vscode-managed-remote-resource scheme, the main process is tasked with sending an ipc to renderer for resolving and loading the resource via some fileservice which then again streams the data back into the renderer via another ipc internally for the protocol handler implementation.

Is there any way the resource can be read from the main process to make one less IPC call ?

@connor4312 connor4312 merged commit 55bb8f1 into main Jun 21, 2023
7 checks passed
@connor4312 connor4312 deleted the connor4312/desktop-load-managed-resources branch June 21, 2023 09:07
@github-actions github-actions bot locked and limited conversation to collaborators Aug 5, 2023
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.

None yet

4 participants