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

stream webview resources to avoid blocking a web socket #8359

Merged
merged 1 commit into from
Aug 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
- On the frontend `EnvVariableServer` should be used instead to access the current user home and available drives.
<a name="1.5.0_usestorage_as_fs_provider"></a>
- [[userstorage]](#1.5.0_usestorage_as_fs_provider) `UserStorageService` was replaced by the user data fs provider [#7908](https://github.com/eclipse-theia/theia/pull/7908)
<a name="1.5.0_webview_resource_streaming"></a>
- [[webview]](#1.5.0_webview_resource_streaming) webview resources are streamed instead of loading one by one the entire content and blocking the web socket [#8359](https://github.com/eclipse-theia/theia/pull/8359)
- Consequently, `WebviewResourceLoader` is removed. One should change `DiskFileSystemProvider` to customize resource loading instead.
<a name="1.5.0_root_user_storage_uri"></a>
- [[user-storage]](#1.5.0_root_user_storage_uri) settings URI must start with `/user` root to satisfy expectations of `FileService` []()
- If you implement a custom user storage make sure to check old relative locations, otherwise it can cause user data loss.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ import { WebviewWidget } from './webview/webview';
import { WebviewEnvironment } from './webview/webview-environment';
import { WebviewThemeDataProvider } from './webview/webview-theme-data-provider';
import { bindWebviewPreferences } from './webview/webview-preferences';
import { WebviewResourceLoader, WebviewResourceLoaderPath } from '../common/webview-protocol';
import { WebviewResourceCache } from './webview/webview-resource-cache';
import { PluginIconThemeService, PluginIconThemeFactory, PluginIconThemeDefinition, PluginIconTheme } from './plugin-icon-theme-service';
import { PluginTreeViewNodeLabelProvider } from './view/plugin-tree-view-node-label-provider';
Expand Down Expand Up @@ -152,9 +151,6 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
bindWebviewPreferences(bind);
bind(WebviewEnvironment).toSelf().inSingletonScope();
bind(WebviewThemeDataProvider).toSelf().inSingletonScope();
bind(WebviewResourceLoader).toDynamicValue(ctx =>
WebSocketConnectionProvider.createProxy(ctx.container, WebviewResourceLoaderPath)
).inSingletonScope();
bind(WebviewResourceCache).toSelf().inSingletonScope();
bind(WebviewWidget).toSelf();
bind(WebviewWidgetFactory).toDynamicValue(ctx => new WebviewWidgetFactory(ctx.container)).inSingletonScope();
Expand Down
20 changes: 13 additions & 7 deletions packages/plugin-ext/src/main/browser/webview/webview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ import { WebviewThemeDataProvider } from './webview-theme-data-provider';
import { ExternalUriService } from '@theia/core/lib/browser/external-uri-service';
import { OutputChannelManager } from '@theia/output/lib/common/output-channel';
import { WebviewPreferences } from './webview-preferences';
import { WebviewResourceLoader } from '../../common/webview-protocol';
import { WebviewResourceCache } from './webview-resource-cache';
import { Endpoint } from '@theia/core/lib/browser/endpoint';
import { FileService } from '@theia/filesystem/lib/browser/file-service';
import { FileOperationError, FileOperationResult } from '@theia/filesystem/lib/common/files';
import { BinaryBufferReadableStream } from '@theia/core/lib/common/buffer';

// Style from core
const TRANSPARENT_OVERLAY_STYLE = 'theia-transparent-overlay';
Expand Down Expand Up @@ -130,8 +132,8 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget {
@inject(WebviewPreferences)
protected readonly preferences: WebviewPreferences;

@inject(WebviewResourceLoader)
protected readonly resourceLoader: WebviewResourceLoader;
@inject(FileService)
protected readonly fileService: FileService;

@inject(WebviewResourceCache)
protected readonly resourceCache: WebviewResourceCache;
Expand Down Expand Up @@ -432,11 +434,15 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget {
continue;
}
let cached = await this.resourceCache.match(cacheUrl);
const response = await this.resourceLoader.load({ uri: normalizedUri.toString(), eTag: cached && cached.eTag });
if (response) {
const { buffer, eTag } = response;
cached = { body: () => new Uint8Array(buffer), eTag: eTag };
try {
const result = await this.fileService.readFileStream(normalizedUri, { etag: cached?.eTag });
const { buffer } = await BinaryBufferReadableStream.toBuffer(result.value);
cached = { body: () => buffer, eTag: result.etag };
this.resourceCache.put(cacheUrl, cached);
} catch (e) {
if (!(e instanceof FileOperationError && e.fileOperationResult === FileOperationResult.FILE_NOT_MODIFIED_SINCE)) {
throw e;
}
}
if (cached) {
const data = await cached.body();
Expand Down
21 changes: 0 additions & 21 deletions packages/plugin-ext/src/main/common/webview-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,3 @@ export namespace WebviewExternalEndpoint {
export const pattern = 'THEIA_WEBVIEW_EXTERNAL_ENDPOINT';
export const defaultPattern = '{{uuid}}.webview.{{hostname}}';
}

export interface LoadWebviewResourceParams {
uri: string
eTag?: string
}

export interface LoadWebviewResourceResult {
buffer: number[]
eTag: string
}

export const WebviewResourceLoader = Symbol('WebviewResourceLoader');
export interface WebviewResourceLoader {
/**
* Loads initial webview resource data.
* Returns `undefined` if a resource has not beed modified.
* Throws if a resource cannot be loaded.
*/
load(params: LoadWebviewResourceParams): Promise<LoadWebviewResourceResult | undefined>
}
export const WebviewResourceLoaderPath = '/services/webview-resource-loader';
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,10 @@ import { PluginPathsService, pluginPathsServicePath } from '../common/plugin-pat
import { PluginPathsServiceImpl } from './paths/plugin-paths-service';
import { PluginServerHandler } from './plugin-server-handler';
import { PluginCliContribution } from './plugin-cli-contribution';
import { WebviewResourceLoaderImpl } from './webview-resource-loader-impl';
import { WebviewResourceLoaderPath } from '../common/webview-protocol';
import { PluginTheiaEnvironment } from '../common/plugin-theia-environment';
import { PluginTheiaDeployerParticipant } from './plugin-theia-deployer-participant';

export function bindMainBackend(bind: interfaces.Bind): void {
bind(WebviewResourceLoaderImpl).toSelf().inSingletonScope();
bind(ConnectionHandler).toDynamicValue(ctx =>
new JsonRpcConnectionHandler(WebviewResourceLoaderPath, () =>
ctx.container.get(WebviewResourceLoaderImpl)
)
).inSingletonScope();

bind(PluginApiContribution).toSelf().inSingletonScope();
bind(BackendApplicationContribution).toService(PluginApiContribution);

Expand Down
43 changes: 0 additions & 43 deletions packages/plugin-ext/src/main/node/webview-resource-loader-impl.ts

This file was deleted.