Use .close() instead of the private .destroy() to stop leaking WebContents (#2990)

* Use `.close()` instead of the private `.destroy()` to stop leaking WebContents

* Fix tests
This commit is contained in:
Devin Binnie 2024-03-22 12:26:51 -04:00 committed by GitHub
parent 6c3eced3e9
commit da59e13477
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 8 additions and 22 deletions

View file

@ -364,21 +364,21 @@ describe('main/views/MattermostBrowserView', () => {
it('should remove browser view from window', () => {
const mattermostView = new MattermostBrowserView(view, {}, {});
mattermostView.browserView.webContents.destroy = jest.fn();
mattermostView.browserView.webContents.close = jest.fn();
mattermostView.destroy();
expect(window.removeBrowserView).toBeCalledWith(mattermostView.browserView);
});
it('should clear mentions', () => {
const mattermostView = new MattermostBrowserView(view, {}, {});
mattermostView.browserView.webContents.destroy = jest.fn();
mattermostView.browserView.webContents.close = jest.fn();
mattermostView.destroy();
expect(AppState.clear).toBeCalledWith(mattermostView.view.id);
});
it('should clear outstanding timeouts', () => {
const mattermostView = new MattermostBrowserView(view, {}, {});
mattermostView.browserView.webContents.destroy = jest.fn();
mattermostView.browserView.webContents.close = jest.fn();
const spy = jest.spyOn(global, 'clearTimeout');
mattermostView.retryLoad = 999;
mattermostView.removeLoading = 1000;

View file

@ -258,12 +258,7 @@ export class MattermostBrowserView extends EventEmitter {
WebContentsEventManager.removeWebContentsListeners(this.webContentsId);
AppState.clear(this.id);
MainWindow.get()?.removeBrowserView(this.browserView);
// workaround to eliminate zombie processes
// https://github.com/mattermost/desktop/pull/1519
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
this.browserView.webContents.destroy();
this.browserView.webContents.close();
this.isVisible = false;
if (this.retryLoad) {

View file

@ -15,7 +15,7 @@ jest.mock('electron', () => ({
openDevTools: jest.fn(),
isDevToolsOpened: jest.fn(),
closeDevTools: jest.fn(),
destroy: jest.fn(),
close: jest.fn(),
},
setBounds: jest.fn(),
setAutoResize: jest.fn(),
@ -106,7 +106,7 @@ describe('main/views/modalView', () => {
it('should remove browser view and destroy web contents on hide', () => {
modalView.hide();
expect(modalView.view.webContents.destroy).toBeCalled();
expect(modalView.view.webContents.close).toBeCalled();
expect(window.removeBrowserView).toBeCalledWith(modalView.view);
});

View file

@ -99,12 +99,7 @@ export class ModalView<T, T2> {
this.view.webContents.closeDevTools();
}
this.windowAttached.removeBrowserView(this.view);
// workaround to eliminate zombie processes
// https://github.com/mattermost/desktop/pull/1519
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
this.view.webContents.destroy();
this.view.webContents.close();
delete this.windowAttached;
this.status = Status.ACTIVE;

View file

@ -354,11 +354,7 @@ export class ViewManager {
log.error('Failed to remove URL view', e);
}
// workaround to eliminate zombie processes
// https://github.com/mattermost/desktop/pull/1519
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
urlView.webContents.destroy();
urlView.webContents.close();
};
const adjustWidth = (event: IpcMainEvent, width: number) => {