From da59e134775ec0402415214104edad7d06d9472c Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Fri, 22 Mar 2024 12:26:51 -0400 Subject: [PATCH] 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 --- src/main/views/MattermostBrowserView.test.js | 6 +++--- src/main/views/MattermostBrowserView.ts | 7 +------ src/main/views/modalView.test.js | 4 ++-- src/main/views/modalView.ts | 7 +------ src/main/views/viewManager.ts | 6 +----- 5 files changed, 8 insertions(+), 22 deletions(-) diff --git a/src/main/views/MattermostBrowserView.test.js b/src/main/views/MattermostBrowserView.test.js index f434c2f1..2bb6b78d 100644 --- a/src/main/views/MattermostBrowserView.test.js +++ b/src/main/views/MattermostBrowserView.test.js @@ -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; diff --git a/src/main/views/MattermostBrowserView.ts b/src/main/views/MattermostBrowserView.ts index 0c8adf97..3b4bb641 100644 --- a/src/main/views/MattermostBrowserView.ts +++ b/src/main/views/MattermostBrowserView.ts @@ -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) { diff --git a/src/main/views/modalView.test.js b/src/main/views/modalView.test.js index ed78174e..dd54e6a7 100644 --- a/src/main/views/modalView.test.js +++ b/src/main/views/modalView.test.js @@ -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); }); diff --git a/src/main/views/modalView.ts b/src/main/views/modalView.ts index 052745d5..cc983ac0 100644 --- a/src/main/views/modalView.ts +++ b/src/main/views/modalView.ts @@ -99,12 +99,7 @@ export class ModalView { 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; diff --git a/src/main/views/viewManager.ts b/src/main/views/viewManager.ts index fca53dd4..818045e1 100644 --- a/src/main/views/viewManager.ts +++ b/src/main/views/viewManager.ts @@ -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) => {