From 9e819e0f3d06bb94c58b334b77761fde0bd237ef Mon Sep 17 00:00:00 2001 From: Martin Fleck Date: Thu, 26 Oct 2023 10:56:52 +0200 Subject: [PATCH] Improve resolution of webview views (#12998) - Ensure webview views are not only resolved during registration - Ensure webview views are re-created once closed - Ensure we resolve the webview view before any other operation Fixes https://github.com/eclipse-theia/theia/issues/12985 --- .../main/browser/view/plugin-view-registry.ts | 148 ++++++++++++------ .../webview-views/webview-views-main.ts | 7 +- .../browser/webview-views/webview-views.ts | 1 + 3 files changed, 104 insertions(+), 52 deletions(-) diff --git a/packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts b/packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts index 3b7a94ccf7fd6..27ec5950870a1 100644 --- a/packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts +++ b/packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts @@ -47,6 +47,7 @@ import { CancellationToken } from '@theia/core/lib/common/cancellation'; import { v4 } from 'uuid'; import { nls } from '@theia/core'; import { TheiaDockPanel } from '@theia/core/lib/browser/shell/theia-dock-panel'; +import { Deferred } from '@theia/core/lib/common/promise-util'; export const PLUGIN_VIEW_FACTORY_ID = 'plugin-view'; export const PLUGIN_VIEW_CONTAINER_FACTORY_ID = 'plugin-view-container'; @@ -103,6 +104,10 @@ export class PluginViewRegistry implements FrontendApplicationContribution { private readonly viewDataState = new Map(); private readonly webviewViewResolvers = new Map(); + protected readonly onNewResolverRegisteredEmitter = new Emitter<{ readonly viewType: string }>(); + readonly onNewResolverRegistered = this.onNewResolverRegisteredEmitter.event; + + private readonly webviewViewRevivals = new Map }>(); private static readonly ID_MAPPINGS: Map = new Map([ // VS Code Viewlets @@ -382,47 +387,51 @@ export class PluginViewRegistry implements FrontendApplicationContribution { return toDispose; } + async resolveWebviewView(viewId: string, webview: WebviewView, cancellation: CancellationToken): Promise { + const resolver = this.webviewViewResolvers.get(viewId); + if (resolver) { + return resolver.resolve(webview, cancellation); + } + const pendingRevival = this.webviewViewRevivals.get(viewId); + if (pendingRevival) { + return pendingRevival.revival.promise; + } + const pending = new Deferred(); + this.webviewViewRevivals.set(viewId, { webview, revival: pending }); + return pending.promise; + } + async registerWebviewView(viewId: string, resolver: WebviewViewResolver): Promise { if (this.webviewViewResolvers.has(viewId)) { throw new Error(`View resolver already registered for ${viewId}`); } this.webviewViewResolvers.set(viewId, resolver); + this.onNewResolverRegisteredEmitter.fire({ viewType: viewId }); - const webviewView = await this.createNewWebviewView(); - const token = CancellationToken.None; - this.getView(viewId).then(async view => { - if (view) { - if (view.isVisible) { - await this.prepareView(view, webviewView.webview.identifier.id); - } else { - const toDisposeOnDidExpandView = new DisposableCollection(this.onDidExpandView(async id => { - if (id === viewId) { - dispose(); - await this.prepareView(view, webviewView.webview.identifier.id); - } - })); - const dispose = () => toDisposeOnDidExpandView.dispose(); - view.disposed.connect(dispose); - toDisposeOnDidExpandView.push(Disposable.create(() => view.disposed.disconnect(dispose))); - } - } - }); + const toDispose = new DisposableCollection(Disposable.create(() => this.webviewViewResolvers.delete(viewId))); + this.initView(viewId, toDispose); - resolver.resolve(webviewView, token); + const pendingRevival = this.webviewViewRevivals.get(viewId); + if (pendingRevival) { + resolver.resolve(pendingRevival.webview, CancellationToken.None).then(() => { + this.webviewViewRevivals.delete(viewId); + pendingRevival.revival.resolve(); + }); + } - return Disposable.create(() => { - this.webviewViewResolvers.delete(viewId); - }); + return toDispose; } - async createNewWebviewView(): Promise { + protected async createNewWebviewView(viewId: string): Promise { const webview = await this.widgetManager.getOrCreateWidget( WebviewWidget.FACTORY_ID, { id: v4() }); webview.setContentOptions({ allowScripts: true }); let _description: string | undefined; + let _resolved = false; + let _pendingResolution: Promise | undefined; - return { + const webviewView: WebviewView = { webview, get onDidChangeVisibility(): Event { return webview.onDidChangeVisibility; }, @@ -442,9 +451,36 @@ export class PluginViewRegistry implements FrontendApplicationContribution { onDidChangeBadge: webview.onDidChangeBadge, onDidChangeBadgeTooltip: webview.onDidChangeBadgeTooltip, - dispose: webview.dispose, + dispose: () => { + _resolved = false; + webview.dispose(); + toDispose.dispose(); + }, + resolve: async () => { + if (_resolved) { + return; + } + if (_pendingResolution) { + return _pendingResolution; + } + _pendingResolution = this.resolveWebviewView(viewId, webviewView, CancellationToken.None).then(() => { + _resolved = true; + _pendingResolution = undefined; + }); + return _pendingResolution; + }, show: webview.show }; + + const toDispose = this.onNewResolverRegistered(resolver => { + if (resolver.viewType === viewId) { + // Potentially re-activate if we have a new resolver + webviewView.resolve(); + } + }); + + webviewView.resolve(); + return webviewView; } registerViewWelcome(viewWelcome: ViewWelcome): Disposable { @@ -526,7 +562,7 @@ export class PluginViewRegistry implements FrontendApplicationContribution { return this.getView(viewId); } - protected async prepareView(widget: PluginViewWidget, webviewId?: string): Promise { + protected async prepareView(widget: PluginViewWidget): Promise { const data = this.views.get(widget.options.viewId); if (!data) { return; @@ -536,6 +572,7 @@ export class PluginViewRegistry implements FrontendApplicationContribution { widget.title.label = view.name; } const currentDataWidget = widget.widgets[0]; + const webviewId = currentDataWidget instanceof WebviewWidget ? currentDataWidget.identifier?.id : undefined; const viewDataWidget = await this.createViewDataWidget(view.id, webviewId); if (widget.isDisposed) { viewDataWidget?.dispose(); @@ -803,35 +840,37 @@ export class PluginViewRegistry implements FrontendApplicationContribution { this.viewDataProviders.delete(viewId); this.viewDataState.delete(viewId); })); - this.getView(viewId).then(async view => { - if (toDispose.disposed) { - return; - } - if (view) { - if (view.isVisible) { - await this.prepareView(view); - } else { - const toDisposeOnDidExpandView = new DisposableCollection(this.onDidExpandView(async id => { - if (id === viewId) { - unsubscribe(); - await this.prepareView(view); - } - })); - const unsubscribe = () => toDisposeOnDidExpandView.dispose(); - view.disposed.connect(unsubscribe); - toDisposeOnDidExpandView.push(Disposable.create(() => view.disposed.disconnect(unsubscribe))); - toDispose.push(toDisposeOnDidExpandView); - } - } - }); + this.initView(viewId, toDispose); return toDispose; } + protected async initView(viewId: string, toDispose: DisposableCollection): Promise { + const view = await this.getView(viewId); + if (toDispose.disposed) { + return; + } + if (view) { + if (view.isVisible) { + await this.prepareView(view); + } else { + const toDisposeOnDidExpandView = new DisposableCollection(this.onDidExpandView(async id => { + if (id === viewId) { + unsubscribe(); + await this.prepareView(view); + } + })); + const unsubscribe = () => toDisposeOnDidExpandView.dispose(); + view.disposed.connect(unsubscribe); + toDisposeOnDidExpandView.push(Disposable.create(() => view.disposed.disconnect(unsubscribe))); + toDispose.push(toDisposeOnDidExpandView); + } + } + } + protected async createViewDataWidget(viewId: string, webviewId?: string): Promise { const view = this.views.get(viewId); if (view?.[1]?.type === PluginViewType.Webview) { - const webviewWidget = this.widgetManager.getWidget(WebviewWidget.FACTORY_ID, { id: webviewId }); - return webviewWidget; + return this.createWebviewWidget(viewId, webviewId); } const provider = this.viewDataProviders.get(viewId); if (!view || !provider) { @@ -849,6 +888,15 @@ export class PluginViewRegistry implements FrontendApplicationContribution { return widget; } + protected async createWebviewWidget(viewId: string, webviewId?: string): Promise { + if (!webviewId) { + const webviewView = await this.createNewWebviewView(viewId); + webviewId = webviewView.webview.identifier.id; + } + const webviewWidget = this.widgetManager.getWidget(WebviewWidget.FACTORY_ID, { id: webviewId }); + return webviewWidget; + } + protected storeViewDataStateOnDispose(viewId: string, widget: Widget & StatefulWidget): void { const dispose = widget.dispose.bind(widget); widget.dispose = () => { diff --git a/packages/plugin-ext/src/main/browser/webview-views/webview-views-main.ts b/packages/plugin-ext/src/main/browser/webview-views/webview-views-main.ts index e18ce34144d4a..6cb23f1521933 100644 --- a/packages/plugin-ext/src/main/browser/webview-views/webview-views-main.ts +++ b/packages/plugin-ext/src/main/browser/webview-views/webview-views-main.ts @@ -83,7 +83,10 @@ export class WebviewViewsMainImpl implements WebviewViewsMain, Disposable { webviewView.webview.options = options; } - webviewView.onDidChangeVisibility(visible => { + webviewView.onDidChangeVisibility(async visible => { + if (visible) { + await webviewView.resolve(); + } this.proxy.$onDidChangeWebviewViewVisibility(handle, visible); }); @@ -93,7 +96,7 @@ export class WebviewViewsMainImpl implements WebviewViewsMain, Disposable { }); try { - this.proxy.$resolveWebviewView(handle, viewType, webviewView.title, state, cancellation); + await this.proxy.$resolveWebviewView(handle, viewType, webviewView.title, state, cancellation); } catch (error) { this.logger.error(`Error resolving webview view '${viewType}': ${error}`); webviewView.webview.setHTML('failed to load plugin webview view'); diff --git a/packages/plugin-ext/src/main/browser/webview-views/webview-views.ts b/packages/plugin-ext/src/main/browser/webview-views/webview-views.ts index f660aa0d47d51..ebc00ee33d6b5 100644 --- a/packages/plugin-ext/src/main/browser/webview-views/webview-views.ts +++ b/packages/plugin-ext/src/main/browser/webview-views/webview-views.ts @@ -35,6 +35,7 @@ export interface WebviewView { dispose(): void; show(preserveFocus: boolean): void; + resolve(): Promise; } export interface WebviewViewResolver {