Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2026-09-19 18:14:50 +01:00
Code Issues Packages Projects Releases Wiki Activity

Avoid race conditions in handling webview output changes, queue all processing

Browse Source
This commit is contained in:
Rob Lourens
2020-08-28 15:19:43 -07:00
parent f56b64cffb
commit 37b438fe9a
5 changed files with 80 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/vs/base/common/async.ts
+19
View File
@@ -170,6 +170,25 @@ export class Sequencer {
}
}
export class SequencerByKey<TKey> {
private promiseMap = new Map<TKey, Promise<any>>();
queue<T>(key: TKey, promiseTask: ITask<Promise<T>>): Promise<T> {
const runningPromise = this.promiseMap.get(key) ?? Promise.resolve();
const newPromise = runningPromise
.catch(() => { })
.then(promiseTask)
.finally(() => {
if (this.promiseMap.get(key) === newPromise) {
this.promiseMap.delete(key);
}
});
this.promiseMap.set(key, newPromise);
return newPromise;
}
}
/**
* A helper to delay execution of a task that is being requested often.
*
src/vs/base/test/common/async.test.ts
+18
View File
@@ -688,4 +688,22 @@ suite('Async', () => {
assert.ok(Date.now() - now < 100);
assert.equal(timedout, false);
});
test('SequencerByKey', async () => {
const s = new async.SequencerByKey<string>();
const r1 = await s.queue('key1', () => Promise.resolve('hello'));
assert.equal(r1, 'hello');
await s.queue('key2', () => Promise.reject(new Error('failed'))).then(() => {
throw new Error('should not be resolved');
}, err => {
// Expected error
assert.equal(err.message, 'failed');
});
// Still works after a queued promise is rejected
const r3 = await s.queue('key2', () => Promise.resolve('hello'));
assert.equal(r3, 'hello');
});
});
src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts
+33 -17
View File
@@ -8,6 +8,7 @@ import * as DOM from 'vs/base/browser/dom';
import { IMouseWheelEvent, StandardMouseEvent } from 'vs/base/browser/mouseEvent';
import { IListContextMenuEvent } from 'vs/base/browser/ui/list/list';
import { IAction, Separator } from 'vs/base/common/actions';
import { SequencerByKey } from 'vs/base/common/async';
import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { Color, RGBA } from 'vs/base/common/color';
import { onUnexpectedError } from 'vs/base/common/errors';
@@ -51,7 +52,7 @@ import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewMod
import { NotebookEventDispatcher, NotebookLayoutChangedEvent } from 'vs/workbench/contrib/notebook/browser/viewModel/eventDispatcher';
import { CellViewModel, IModelDecorationsChangeAccessor, INotebookEditorViewState, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel';
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
import { CellKind, CellToolbarLocKey, ICellRange, IInsetRenderOutput, INotebookKernelInfo, INotebookKernelInfo2, INotebookKernelInfoDto, IProcessedOutput, NotebookCellRunState, NotebookRunState, ShowCellStatusbarKey } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { CellKind, CellToolbarLocKey, ICellRange, IInsetRenderOutput, INotebookKernelInfo, INotebookKernelInfo2, INotebookKernelInfoDto, IProcessedOutput, isTransformedDisplayOutput, NotebookCellRunState, NotebookRunState, ShowCellStatusbarKey } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { NotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookProvider';
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
import { editorGutterModifiedBackground } from 'vs/workbench/contrib/scm/browser/dirtydiffDecorator';
@@ -101,6 +102,8 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor
private readonly _onWillDispose = this._register(new Emitter<void>());
public readonly onWillDispose: Event<void> = this._onWillDispose.event;
private readonly _insetModifyQueueByOutputId = new SequencerByKey<string>();
set scrollTop(top: number) {
if (this._list) {
this._list.scrollTop = top;
@@ -1577,30 +1580,37 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor
this._list?.triggerScrollFromMouseWheelEvent(event);
}
async createInset(cell: CodeCellViewModel, output: IInsetRenderOutput, offset: number) {
if (!this._webview) {
return;
}
async createInset(cell: CodeCellViewModel, output: IInsetRenderOutput, offset: number): Promise<void> {
this._insetModifyQueueByOutputId.queue(output.source.outputId, async () => {
if (!this._webview) {
return;
}
await this._resolveWebview();
await this._resolveWebview();
if (!this._webview!.insetMapping.has(output.source)) {
const cellTop = this._list?.getAbsoluteTopOfElement(cell) || 0;
await this._webview!.createInset(cell, output, cellTop, offset);
} else {
const cellTop = this._list?.getAbsoluteTopOfElement(cell) || 0;
const scrollTop = this._list?.scrollTop || 0;
if (!this._webview!.insetMapping.has(output.source)) {
const cellTop = this._list?.getAbsoluteTopOfElement(cell) || 0;
await this._webview!.createInset(cell, output, cellTop, offset);
} else {
const cellTop = this._list?.getAbsoluteTopOfElement(cell) || 0;
const scrollTop = this._list?.scrollTop || 0;
this._webview!.updateViewScrollTop(-scrollTop, true, [{ cell, output: output.source, cellTop }]);
}
this._webview!.updateViewScrollTop(-scrollTop, true, [{ cell, output: output.source, cellTop }]);
}
});
}
removeInset(output: IProcessedOutput) {
if (!this._webview || !this._webviewResolved) {
if (!isTransformedDisplayOutput(output)) {
return;
}
this._webview!.removeInset(output);
this._insetModifyQueueByOutputId.queue(output.outputId, async () => {
if (!this._webview || !this._webviewResolved) {
return;
}
this._webview!.removeInset(output);
});
}
hideInset(output: IProcessedOutput) {
@@ -1608,7 +1618,13 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor
return;
}
this._webview!.hideInset(output);
if (!isTransformedDisplayOutput(output)) {
return;
}
this._insetModifyQueueByOutputId.queue(output.outputId, async () => {
this._webview!.hideInset(output);
});
}
getOutputRenderer(): OutputRenderer {
src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts
+4 -6
View File
@@ -33,8 +33,6 @@ export class CodeCell extends Disposable {
private outputResizeListeners = new Map<IProcessedOutput, DisposableStore>();
private outputElements = new Map<IProcessedOutput, IRenderedOutput>();
private modifyInsetQueue = Promise.resolve();
constructor(
private notebookEditor: INotebookEditor,
private viewCell: CodeCellViewModel,
@@ -173,7 +171,7 @@ export class CodeCell extends Disposable {
removedKeys.push(key);
// remove element from DOM
this.templateData?.outputContainer?.removeChild(value.element);
this.modifyInsetQueue = this.modifyInsetQueue.finally(() => this.notebookEditor.removeInset(key));
this.notebookEditor.removeInset(key);
}
});
@@ -326,7 +324,7 @@ export class CodeCell extends Disposable {
const renderedOutput = this.outputElements.get(currOutput);
if (renderedOutput) {
if (renderedOutput.renderResult.type !== RenderOutputType.None) {
this.modifyInsetQueue = this.modifyInsetQueue.finally(() => this.notebookEditor.createInset(this.viewCell, renderedOutput.renderResult as IInsetRenderOutput, this.viewCell.getOutputOffset(index)));
this.notebookEditor.createInset(this.viewCell, renderedOutput.renderResult as IInsetRenderOutput, this.viewCell.getOutputOffset(index));
} else {
// Anything else, just update the height
this.viewCell.updateOutputHeight(index, renderedOutput.element.clientHeight);
@@ -515,7 +513,7 @@ export class CodeCell extends Disposable {
if (result.type !== RenderOutputType.None) {
this.viewCell.selfSizeMonitoring = true;
this.modifyInsetQueue = this.modifyInsetQueue.finally(() => this.notebookEditor.createInset(this.viewCell, result as any, this.viewCell.getOutputOffset(index)));
this.notebookEditor.createInset(this.viewCell, result as any, this.viewCell.getOutputOffset(index));
} else {
DOM.addClass(outputItemDiv, 'foreground');
DOM.addClass(outputItemDiv, 'output-element');
@@ -610,7 +608,7 @@ export class CodeCell extends Disposable {
const element = this.outputElements.get(output)?.element;
if (element) {
this.templateData?.outputContainer?.removeChild(element);
await (this.modifyInsetQueue = this.modifyInsetQueue.finally(() => this.notebookEditor.removeInset(output)));
this.notebookEditor.removeInset(output);
}
output.pickedMimeTypeIndex = pick;
src/vs/workbench/contrib/notebook/common/notebookCommon.ts
+6 -2
View File
@@ -215,6 +215,10 @@ export interface ITransformedDisplayOutputDto {
pickedMimeTypeIndex?: number;
}
export function isTransformedDisplayOutput(thing: unknown): thing is ITransformedDisplayOutputDto {
return (thing as ITransformedDisplayOutputDto).outputKind === CellOutputKind.Rich && !!(thing as ITransformedDisplayOutputDto).outputId;
}
export interface IGenericOutput {
outputKind: CellOutputKind;
pickedMimeType?: string;
@@ -313,14 +317,14 @@ export interface IRenderNoOutput {
export interface IRenderPlainHtmlOutput {
type: RenderOutputType.Html;
source: IProcessedOutput;
source: ITransformedDisplayOutputDto;
htmlContent: string;
hasDynamicHeight: boolean;
}
export interface IRenderOutputViaExtension {
type: RenderOutputType.Extension;
source: IProcessedOutput;
source: ITransformedDisplayOutputDto;
mimeType: string;
renderer: INotebookRendererInfo;
}