Skip to content

Commit 4784cd8

Browse files
Dispose webviewhost on panel load failure (microsoft#13129)
1 parent 1a1a7fd commit 4784cd8

File tree

6 files changed

+79
-50
lines changed

6 files changed

+79
-50
lines changed

news/2 Fixes/13106.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
If a webpanel fails to load, dispose our webviewhost so that it can try again.

src/client/common/application/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,6 +1060,10 @@ export type WebPanelMessage = {
10601060
// Wraps the VS Code webview panel
10611061
export const IWebPanel = Symbol('IWebPanel');
10621062
export interface IWebPanel {
1063+
/**
1064+
* Event is fired when the load for a web panel fails
1065+
*/
1066+
readonly loadFailed: Event<void>;
10631067
/**
10641068
* Convert a uri for the local file system to one that can be used inside webviews.
10651069
*

src/client/common/application/webPanels/webPanel.ts

Lines changed: 48 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
import '../../extensions';
55

66
import * as path from 'path';
7-
import { Uri, Webview, WebviewOptions, WebviewPanel, window } from 'vscode';
7+
import { Event, EventEmitter, Uri, Webview, WebviewOptions, WebviewPanel, window } from 'vscode';
88
import { Identifiers } from '../../../datascience/constants';
9+
import { traceError } from '../../logger';
910
import { IFileSystem } from '../../platform/types';
1011
import { IDisposableRegistry } from '../../types';
1112
import * as localize from '../../utils/localize';
@@ -14,6 +15,7 @@ import { IWebPanel, IWebPanelOptions, WebPanelMessage } from '../types';
1415
export class WebPanel implements IWebPanel {
1516
private panel: WebviewPanel | undefined;
1617
private loadPromise: Promise<void>;
18+
private loadFailedEmitter = new EventEmitter<void>();
1719

1820
constructor(
1921
private fs: IFileSystem,
@@ -43,6 +45,9 @@ export class WebPanel implements IWebPanel {
4345
this.loadPromise = this.load();
4446
}
4547

48+
public get loadFailed(): Event<void> {
49+
return this.loadFailedEmitter.event;
50+
}
4651
public async show(preserveFocus: boolean) {
4752
await this.loadPromise;
4853
if (this.panel) {
@@ -89,42 +94,49 @@ export class WebPanel implements IWebPanel {
8994

9095
// tslint:disable-next-line:no-any
9196
private async load() {
92-
if (this.panel) {
93-
const localFilesExist = await Promise.all(this.options.scripts.map((s) => this.fs.fileExists(s)));
94-
if (localFilesExist.every((exists) => exists === true)) {
95-
// Call our special function that sticks this script inside of an html page
96-
// and translates all of the paths to vscode-resource URIs
97-
this.panel.webview.html = await this.generateLocalReactHtml(this.panel.webview);
98-
99-
// Reset when the current panel is closed
100-
this.disposableRegistry.push(
101-
this.panel.onDidDispose(() => {
102-
this.panel = undefined;
103-
this.options.listener.dispose().ignoreErrors();
104-
})
105-
);
106-
107-
this.disposableRegistry.push(
108-
this.panel.webview.onDidReceiveMessage((message) => {
109-
// Pass the message onto our listener
110-
this.options.listener.onMessage(message.type, message.payload);
111-
})
112-
);
113-
114-
this.disposableRegistry.push(
115-
this.panel.onDidChangeViewState((_e) => {
116-
// Pass the state change onto our listener
117-
this.options.listener.onChangeViewState(this);
118-
})
119-
);
120-
121-
// Set initial state
122-
this.options.listener.onChangeViewState(this);
123-
} else {
124-
// Indicate that we can't load the file path
125-
const badPanelString = localize.DataScience.badWebPanelFormatString();
126-
this.panel.webview.html = badPanelString.format(this.options.scripts.join(', '));
97+
try {
98+
if (this.panel) {
99+
const localFilesExist = await Promise.all(this.options.scripts.map((s) => this.fs.fileExists(s)));
100+
if (localFilesExist.every((exists) => exists === true)) {
101+
// Call our special function that sticks this script inside of an html page
102+
// and translates all of the paths to vscode-resource URIs
103+
this.panel.webview.html = await this.generateLocalReactHtml(this.panel.webview);
104+
105+
// Reset when the current panel is closed
106+
this.disposableRegistry.push(
107+
this.panel.onDidDispose(() => {
108+
this.panel = undefined;
109+
this.options.listener.dispose().ignoreErrors();
110+
})
111+
);
112+
113+
this.disposableRegistry.push(
114+
this.panel.webview.onDidReceiveMessage((message) => {
115+
// Pass the message onto our listener
116+
this.options.listener.onMessage(message.type, message.payload);
117+
})
118+
);
119+
120+
this.disposableRegistry.push(
121+
this.panel.onDidChangeViewState((_e) => {
122+
// Pass the state change onto our listener
123+
this.options.listener.onChangeViewState(this);
124+
})
125+
);
126+
127+
// Set initial state
128+
this.options.listener.onChangeViewState(this);
129+
} else {
130+
// Indicate that we can't load the file path
131+
const badPanelString = localize.DataScience.badWebPanelFormatString();
132+
this.panel.webview.html = badPanelString.format(this.options.scripts.join(', '));
133+
}
127134
}
135+
} catch (error) {
136+
// If our web panel failes to load, report that out so whatever
137+
// is hosting the panel can clean up
138+
traceError(`Error Loading WebPanel: ${error}`);
139+
this.loadFailedEmitter.fire();
128140
}
129141
}
130142

src/client/datascience/webViewHost.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,9 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
2929
private webPanel: IWebPanel | undefined;
3030
private webPanelInit: Deferred<void> | undefined = createDeferred<void>();
3131
private messageListener: IWebPanelMessageListener;
32-
private themeChangeHandler: IDisposable | undefined;
33-
private settingsChangeHandler: IDisposable | undefined;
3432
private themeIsDarkPromise: Deferred<boolean> | undefined = createDeferred<boolean>();
3533
private startupStopwatch = new StopWatch();
34+
private readonly _disposables: IDisposable[] = [];
3635

3736
constructor(
3837
@unmanaged() protected configService: IConfigurationService,
@@ -62,12 +61,12 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
6261
);
6362

6463
// Listen for settings changes from vscode.
65-
this.themeChangeHandler = this.workspaceService.onDidChangeConfiguration(this.onPossibleSettingsChange, this);
64+
this._disposables.push(this.workspaceService.onDidChangeConfiguration(this.onPossibleSettingsChange, this));
6665

6766
// Listen for settings changes
68-
this.settingsChangeHandler = this.configService
69-
.getSettings(undefined)
70-
.onDidChange(this.onDataScienceSettingsChanged.bind(this));
67+
this._disposables.push(
68+
this.configService.getSettings(undefined).onDidChange(this.onDataScienceSettingsChanged.bind(this))
69+
);
7170
}
7271

7372
public async show(preserveFocus: boolean): Promise<void> {
@@ -91,14 +90,9 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
9190
this.webPanel.close();
9291
this.webPanel = undefined;
9392
}
94-
if (this.themeChangeHandler) {
95-
this.themeChangeHandler.dispose();
96-
this.themeChangeHandler = undefined;
97-
}
98-
if (this.settingsChangeHandler) {
99-
this.settingsChangeHandler.dispose();
100-
this.settingsChangeHandler = undefined;
101-
}
93+
94+
this._disposables.forEach((item) => item.dispose());
95+
10296
this.webPanelInit = undefined;
10397
this.themeIsDarkPromise = undefined;
10498
}
@@ -272,6 +266,9 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
272266
additionalPaths: workspaceFolder ? [workspaceFolder.fsPath] : []
273267
});
274268

269+
// Track to seee if our web panel fails to load
270+
this._disposables.push(this.webPanel.loadFailed(this.onWebPanelLoadFailed, this));
271+
275272
traceInfo('Web view created.');
276273
}
277274

@@ -294,6 +291,11 @@ export abstract class WebViewHost<IMapping> implements IDisposable {
294291
this.postMessageInternal(SharedMessages.UpdateSettings, dsSettings).ignoreErrors();
295292
};
296293

294+
// If our webpanel fails to load then just dispose ourselves
295+
private onWebPanelLoadFailed = async () => {
296+
this.dispose();
297+
};
298+
297299
private getValue<T>(workspaceConfig: WorkspaceConfiguration, section: string, defaultValue: T): T {
298300
if (workspaceConfig) {
299301
return workspaceConfig.get(section, defaultValue);

src/test/datascience/mountedWebView.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export class MountedWebView implements IMountedWebView, IDisposable {
5858
private active = true;
5959
private visible = true;
6060
private disposedEvent = new EventEmitter<void>();
61+
private loadFailedEmitter = new EventEmitter<void>();
6162

6263
constructor(mount: () => ReactWrapper<any, Readonly<{}>, React.Component>, public readonly id: string) {
6364
// Setup the acquireVsCodeApi. The react control will cache this value when it's mounted.
@@ -97,6 +98,9 @@ export class MountedWebView implements IMountedWebView, IDisposable {
9798
public get onDisposed() {
9899
return this.disposedEvent.event;
99100
}
101+
public get loadFailed(): Event<void> {
102+
return this.loadFailedEmitter.event;
103+
}
100104
public attach(options: IWebPanelOptions) {
101105
this.webPanelListener = options.listener;
102106

src/test/datascience/uiTests/webBrowserPanel.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ export class WebBrowserPanel implements IWebPanel, IDisposable {
134134
private panel?: WebviewPanel;
135135
private server?: IWebServer;
136136
private serverUrl: string | undefined;
137+
private loadFailedEmitter = new EventEmitter<void>();
137138
constructor(private readonly disposableRegistry: IDisposableRegistry, private readonly options: IWebPanelOptions) {
138139
this.disposableRegistry.push(this);
139140
const webViewOptions: WebviewOptions = {
@@ -174,6 +175,11 @@ export class WebBrowserPanel implements IWebPanel, IDisposable {
174175
console.error('Failed to start Web Browser Panel', ex)
175176
);
176177
}
178+
179+
public get loadFailed(): Event<void> {
180+
return this.loadFailedEmitter.event;
181+
}
182+
177183
public asWebviewUri(localResource: Uri): Uri {
178184
const filePath = localResource.fsPath;
179185
const name = path.basename(path.dirname(filePath));

0 commit comments

Comments
 (0)