Skip to content

Commit

Permalink
Fix design and implementation issues surrounding CustomEditorOpener
Browse files Browse the repository at this point in the history 
This commit ensures that the promise returned by `CustomEditorOpener.open`
will only resolve to a properly initialized and opened `CustomEditorWidget`.
In particular, it ensures that the widget is opened according to the specified
`WidgetOpenerOptions`, including `widgetOptions.ref` and `mode`.

Essentially, it revises the work done in eclipse-theia#9671 and eclipse-theia#10580
to fix eclipse-theia#9670 and eclipse-theia#10583.
  • Loading branch information
@pisv
pisv committed Jul 10, 2024
1 parent 715a3ba commit 1e71f0b
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 122 deletions.
5 changes: 4 additions & 1 deletion packages/core/src/browser/widget-open-handler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import { WidgetManager } from './widget-manager';

export type WidgetOpenMode = 'open' | 'reveal' | 'activate';
/**
* `WidgetOpenerOptions` define serializable generic options used by the {@link WidgetOpenHandler}.
* `WidgetOpenerOptions` define generic options used by the {@link WidgetOpenHandler}.
*
* _Note:_ This object may contain references to widgets (e.g. `widgetOptions.ref`);
* these need to be transformed before it can be serialized.
*/
export interface WidgetOpenerOptions extends OpenerOptions {
/**
Expand Down
3 changes: 1 addition & 2 deletions packages/plugin-ext/src/common/plugin-api-rpc.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1880,7 +1880,7 @@ export interface CustomEditorsExt {
newWebviewHandle: string,
viewType: string,
title: string,
widgetOpenerOptions: object | undefined,
position: number,
options: theia.WebviewPanelOptions,
cancellation: CancellationToken): Promise<void>;
$createCustomDocument(resource: UriComponents, viewType: string, openContext: theia.CustomDocumentOpenContext, cancellation: CancellationToken): Promise<{ editable: boolean }>;
Expand All @@ -1903,7 +1903,6 @@ export interface CustomEditorsMain {
$registerTextEditorProvider(viewType: string, options: theia.WebviewPanelOptions, capabilities: CustomTextEditorCapabilities): void;
$registerCustomEditorProvider(viewType: string, options: theia.WebviewPanelOptions, supportsMultipleEditorsPerDocument: boolean): void;
$unregisterEditorProvider(viewType: string): void;
$createCustomEditorPanel(handle: string, title: string, widgetOpenerOptions: object | undefined, options: theia.WebviewPanelOptions & theia.WebviewOptions): Promise<void>;
$onDidEdit(resource: UriComponents, viewType: string, editId: number, label: string | undefined): void;
$onContentChange(resource: UriComponents, viewType: string): void;
}
Expand Down
60 changes: 37 additions & 23 deletions packages/plugin-ext/src/main/browser/custom-editors/custom-editor-opener.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
// *****************************************************************************

import { inject } from '@theia/core/shared/inversify';
import URI from '@theia/core/lib/common/uri';
import { ApplicationShell, OpenHandler, Widget, WidgetManager, WidgetOpenerOptions } from '@theia/core/lib/browser';
import { ApplicationShell, OpenHandler, WidgetManager, WidgetOpenerOptions } from '@theia/core/lib/browser';
import { CustomEditor, CustomEditorPriority, CustomEditorSelector } from '../../../common';
import { CustomEditorWidget } from './custom-editor-widget';
import { PluginCustomEditorRegistry } from './plugin-custom-editor-registry';
import { generateUuid } from '@theia/core/lib/common/uuid';
import { Emitter } from '@theia/core';
import { match } from '@theia/core/lib/common/glob';
Expand All @@ -33,8 +33,9 @@ export class CustomEditorOpener implements OpenHandler {

constructor(
private readonly editor: CustomEditor,
@inject(ApplicationShell) protected readonly shell: ApplicationShell,
@inject(WidgetManager) protected readonly widgetManager: WidgetManager
protected readonly shell: ApplicationShell,
protected readonly widgetManager: WidgetManager,
protected readonly editorRegistry: PluginCustomEditorRegistry
) {
this.id = CustomEditorOpener.toCustomEditorId(this.editor.viewType);
this.label = this.editor.displayName;
Expand Down Expand Up @@ -62,31 +63,44 @@ export class CustomEditorOpener implements OpenHandler {
}

protected readonly pendingWidgetPromises = new Map<string, Promise<CustomEditorWidget>>();
async open(uri: URI, options?: WidgetOpenerOptions): Promise<Widget | undefined> {
async open(uri: URI, options?: WidgetOpenerOptions): Promise<CustomEditorWidget> {
let widget: CustomEditorWidget | undefined;
const widgets = this.widgetManager.getWidgets(CustomEditorWidget.FACTORY_ID) as CustomEditorWidget[];
widget = widgets.find(w => w.viewType === this.editor.viewType && w.resource.toString() === uri.toString());

if (widget?.isVisible) {
return this.shell.revealWidget(widget.id);
}
if (widget?.isAttached) {
return this.shell.activateWidget(widget.id);
}
if (!widget) {
const uriString = uri.toString();
let widgetPromise = this.pendingWidgetPromises.get(uriString);
if (!widgetPromise) {
let shouldNotify = false;
const uriString = uri.toString();
let widgetPromise = this.pendingWidgetPromises.get(uriString);
if (widgetPromise) {
widget = await widgetPromise;
} else {
const widgets = this.widgetManager.getWidgets(CustomEditorWidget.FACTORY_ID) as CustomEditorWidget[];
widget = widgets.find(w => w.viewType === this.editor.viewType && w.resource.toString() === uriString);
if (!widget) {
shouldNotify = true;
const id = generateUuid();
widgetPromise = this.widgetManager.getOrCreateWidget<CustomEditorWidget>(CustomEditorWidget.FACTORY_ID, { id });
widgetPromise = this.widgetManager.getOrCreateWidget<CustomEditorWidget>(CustomEditorWidget.FACTORY_ID, { id }).then(async w => {
try {
w.viewType = this.editor.viewType;
w.resource = uri;
await this.editorRegistry.resolveWidget(w);
await this.shell.addWidget(w, options?.widgetOptions);
return w;
} catch (e) {
w.dispose();
throw e;
}
}).finally(() => this.pendingWidgetPromises.delete(uriString));
this.pendingWidgetPromises.set(uriString, widgetPromise);
widget = await widgetPromise;
this.pendingWidgetPromises.delete(uriString);
widget.viewType = this.editor.viewType;
widget.resource = uri;
this.onDidOpenCustomEditorEmitter.fire([widget, options]);
}
}
const mode = options?.mode ?? 'activate';
if (mode === 'activate') {
await this.shell.activateWidget(widget.id);
} else if (mode === 'reveal') {
await this.shell.revealWidget(widget.id);
}
if (shouldNotify) {
this.onDidOpenCustomEditorEmitter.fire([widget, options]);
}
return widget;
}

Expand Down
81 changes: 11 additions & 70 deletions packages/plugin-ext/src/main/browser/custom-editors/custom-editors-main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import { MAIN_RPC_CONTEXT, CustomEditorsMain, CustomEditorsExt, CustomTextEditor
import { RPCProtocol } from '../../../common/rpc-protocol';
import { HostedPluginSupport } from '../../../hosted/browser/hosted-plugin';
import { PluginCustomEditorRegistry } from './plugin-custom-editor-registry';
import { CustomEditorWidget } from './custom-editor-widget';
import { Emitter } from '@theia/core';
import { UriComponents } from '../../../common/uri-components';
import { URI } from '@theia/core/shared/vscode-uri';
Expand All @@ -39,11 +38,9 @@ import { FileService } from '@theia/filesystem/lib/browser/file-service';
import { UndoRedoService } from '@theia/editor/lib/browser/undo-redo-service';
import { WebviewsMainImpl } from '../webviews-main';
import { WidgetManager } from '@theia/core/lib/browser/widget-manager';
import { ApplicationShell, DefaultUriLabelProviderContribution, Saveable, SaveOptions, WidgetOpenerOptions } from '@theia/core/lib/browser';
import { WebviewOptions, WebviewPanelOptions, WebviewPanelShowOptions } from '@theia/plugin';
import { WebviewWidgetIdentifier } from '../webview/webview';
import { ApplicationShell, LabelProvider, Saveable, SaveOptions } from '@theia/core/lib/browser';
import { WebviewPanelOptions } from '@theia/plugin';
import { EditorPreferences } from '@theia/editor/lib/browser';
import { ViewColumn, WebviewPanelTargetArea } from '../../../plugin/types-impl';

const enum CustomEditorModelType {
Custom,
Expand All @@ -58,7 +55,7 @@ export class CustomEditorsMainImpl implements CustomEditorsMain, Disposable {
protected readonly customEditorService: CustomEditorService;
protected readonly undoRedoService: UndoRedoService;
protected readonly customEditorRegistry: PluginCustomEditorRegistry;
protected readonly labelProvider: DefaultUriLabelProviderContribution;
protected readonly labelProvider: LabelProvider;
protected readonly widgetManager: WidgetManager;
protected readonly editorPreferences: EditorPreferences;
private readonly proxy: CustomEditorsExt;
Expand All @@ -75,7 +72,7 @@ export class CustomEditorsMainImpl implements CustomEditorsMain, Disposable {
this.customEditorService = container.get(CustomEditorService);
this.undoRedoService = container.get(UndoRedoService);
this.customEditorRegistry = container.get(PluginCustomEditorRegistry);
this.labelProvider = container.get(DefaultUriLabelProviderContribution);
this.labelProvider = container.get(LabelProvider);
this.editorPreferences = container.get(EditorPreferences);
this.widgetManager = container.get(WidgetManager);
this.proxy = rpc.getProxy(MAIN_RPC_CONTEXT.CUSTOM_EDITORS_EXT);
Expand Down Expand Up @@ -111,7 +108,8 @@ export class CustomEditorsMainImpl implements CustomEditorsMain, Disposable {
const disposables = new DisposableCollection();

disposables.push(
this.customEditorRegistry.registerResolver(viewType, async (widget, widgetOpenerOptions) => {
this.customEditorRegistry.registerResolver(viewType, async widget => {

const { resource, identifier } = widget;
widget.options = options;

Expand Down Expand Up @@ -144,13 +142,16 @@ export class CustomEditorsMainImpl implements CustomEditorsMain, Disposable {
});
}

this.webviewsMain.hookWebview(widget);
widget.title.label = this.labelProvider.getName(resource);

const _cancellationSource = new CancellationTokenSource();
await this.proxy.$resolveWebviewEditor(
resource.toComponents(),
identifier.id,
viewType,
this.labelProvider.getName(resource)!,
widgetOpenerOptions,
widget.title.label,
widget.viewState.position,
options,
_cancellationSource.token
);
Expand Down Expand Up @@ -213,66 +214,6 @@ export class CustomEditorsMainImpl implements CustomEditorsMain, Disposable {
const model = await this.getCustomEditorModel(resourceComponents, viewType);
model.changeContent();
}

async $createCustomEditorPanel(
panelId: string,
title: string,
widgetOpenerOptions: WidgetOpenerOptions | undefined,
options: WebviewPanelOptions & WebviewOptions
): Promise<void> {
const view = await this.widgetManager.getOrCreateWidget<CustomEditorWidget>(CustomEditorWidget.FACTORY_ID, <WebviewWidgetIdentifier>{ id: panelId });
this.webviewsMain.hookWebview(view);
view.title.label = title;
const { enableFindWidget, retainContextWhenHidden, enableScripts, enableForms, localResourceRoots, ...contentOptions } = options;
view.viewColumn = ViewColumn.One; // behaviour might be overridden later using widgetOpenerOptions (if available)
view.options = { enableFindWidget, retainContextWhenHidden };
view.setContentOptions({
allowScripts: enableScripts,
allowForms: enableForms,
localResourceRoots: localResourceRoots && localResourceRoots.map(root => root.toString()),
...contentOptions,
...view.contentOptions
});
if (view.isAttached) {
if (view.isVisible) {
this.shell.revealWidget(view.id);
}
return;
}
const showOptions: WebviewPanelShowOptions = {
preserveFocus: true
};

if (widgetOpenerOptions) {
if (widgetOpenerOptions.mode === 'reveal') {
showOptions.preserveFocus = false;
}

if (widgetOpenerOptions.widgetOptions) {
let area: WebviewPanelTargetArea;
switch (widgetOpenerOptions.widgetOptions.area) {
case 'main':
area = WebviewPanelTargetArea.Main;
case 'left':
area = WebviewPanelTargetArea.Left;
case 'right':
area = WebviewPanelTargetArea.Right;
case 'bottom':
area = WebviewPanelTargetArea.Bottom;
default: // includes 'top' and 'secondaryWindow'
area = WebviewPanelTargetArea.Main;
}
showOptions.area = area;

if (widgetOpenerOptions.widgetOptions.mode === 'split-right' ||
widgetOpenerOptions.widgetOptions.mode === 'open-to-right') {
showOptions.viewColumn = ViewColumn.Beside;
}
}
}

this.webviewsMain.addOrReattachWidget(view, showOptions);
}
}

export interface CustomEditorModel extends Saveable, Disposable {
Expand Down
28 changes: 15 additions & 13 deletions packages/plugin-ext/src/main/browser/custom-editors/plugin-custom-editor-registry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@
import { injectable, inject, postConstruct } from '@theia/core/shared/inversify';
import { CustomEditor, DeployedPlugin } from '../../../common';
import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable';
import { Deferred } from '@theia/core/lib/common/promise-util';
import { CustomEditorOpener } from './custom-editor-opener';
import { Emitter } from '@theia/core';
import { ApplicationShell, DefaultOpenerService, OpenWithService, WidgetManager, WidgetOpenerOptions } from '@theia/core/lib/browser';
import { ApplicationShell, DefaultOpenerService, OpenWithService, WidgetManager } from '@theia/core/lib/browser';
import { CustomEditorWidget } from './custom-editor-widget';

@injectable()
export class PluginCustomEditorRegistry {
private readonly editors = new Map<string, CustomEditor>();
private readonly pendingEditors = new Set<CustomEditorWidget>();
private readonly resolvers = new Map<string, (widget: CustomEditorWidget, options?: WidgetOpenerOptions) => void>();
private readonly pendingEditors = new Map<CustomEditorWidget, { deferred: Deferred<void>, disposable: Disposable }>();
private readonly resolvers = new Map<string, (widget: CustomEditorWidget) => Promise<void>>();

private readonly onWillOpenCustomEditorEmitter = new Emitter<string>();
readonly onWillOpenCustomEditor = this.onWillOpenCustomEditorEmitter.event;
Expand Down Expand Up @@ -74,7 +75,8 @@ export class PluginCustomEditorRegistry {
const editorOpenHandler = new CustomEditorOpener(
editor,
this.shell,
this.widgetManager
this.widgetManager,
this
);
toDispose.push(this.defaultOpenerService.addHandler(editorOpenHandler));
toDispose.push(
Expand All @@ -86,30 +88,30 @@ export class PluginCustomEditorRegistry {
open: uri => editorOpenHandler.open(uri)
})
);
toDispose.push(
editorOpenHandler.onDidOpenCustomEditor(event => this.resolveWidget(event[0], event[1]))
);
return toDispose;
}

resolveWidget = (widget: CustomEditorWidget, options?: WidgetOpenerOptions) => {
async resolveWidget(widget: CustomEditorWidget): Promise<void> {
const resolver = this.resolvers.get(widget.viewType);
if (resolver) {
resolver(widget, options);
await resolver(widget);
} else {
this.pendingEditors.add(widget);
const deferred = new Deferred<void>();
const disposable = widget.onDidDispose(() => this.pendingEditors.delete(widget));
this.pendingEditors.set(widget, { deferred, disposable });
this.onWillOpenCustomEditorEmitter.fire(widget.viewType);
return deferred.promise;
}
};

registerResolver(viewType: string, resolver: (widget: CustomEditorWidget, options?: WidgetOpenerOptions) => void): Disposable {
registerResolver(viewType: string, resolver: (widget: CustomEditorWidget) => Promise<void>): Disposable {
if (this.resolvers.has(viewType)) {
throw new Error(`Resolver for ${viewType} already registered`);
}

for (const editorWidget of this.pendingEditors) {
for (const [editorWidget, { deferred, disposable }] of this.pendingEditors.entries()) {
if (editorWidget.viewType === viewType) {
resolver(editorWidget);
resolver(editorWidget).then(() => deferred.resolve(), err => deferred.reject(err)).finally(() => disposable.dispose());
this.pendingEditors.delete(editorWidget);
}
}
Expand Down
2 changes: 0 additions & 2 deletions packages/plugin-ext/src/main/browser/webview/webview.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import { isFirefox } from '@theia/core/lib/browser/browser';
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';
import { ViewColumn } from '../../../plugin/types-impl';
import { ExtractableWidget } from '@theia/core/lib/browser/widgets/extractable-widget';
import { BadgeWidget } from '@theia/core/lib/browser/view-container';
import { MenuPath } from '@theia/core';
Expand Down Expand Up @@ -185,7 +184,6 @@ export class WebviewWidget extends BaseWidget implements StatefulWidget, Extract
}

viewType: string;
viewColumn: ViewColumn;
options: WebviewPanelOptions = {};

protected ready = new Deferred<void>();
Expand Down
Loading

0 comments on commit 1e71f0b

Please sign in to comment.