[MM-68749] Guard webContents finish-load handlers against destroyed objects

src/app/callsWidgetWindow.test.js

@@ -448,6 +448,7 @@ describe('main/windows/callsWidgetWindow', () => {
                 id: 'webContentsId',
                 getURL: () => ('http://myurl.com'),
                 removeListener: jest.fn(),
+                isDestroyed: jest.fn(() => false),
             },
             off: jest.fn(),
             loadURL: jest.fn(),

src/app/callsWidgetWindow.ts

@@ -359,7 +359,10 @@ export class CallsWidgetWindow {
         // 'did-frame-finish-load' is the earliest moment that allows us to call loadURL without throwing an error.
         // https://mattermost.atlassian.net/browse/MM-52756 is the proper fix for this.
         this.popOut.webContents.once('did-frame-finish-load', async () => {
-            const url = this.popOut?.webContents.getURL() || '';
+            if (!this.popOut || this.popOut.isDestroyed() || this.popOut.webContents.isDestroyed()) {
+                return;
+            }
+            const url = this.popOut.webContents.getURL() || '';
             if (!url) {
                 return;
             }

src/app/mainWindow/mainWindow.test.js

@@ -110,6 +110,7 @@ describe('main/windows/mainWindow', () => {
                 send: jest.fn(),
                 setWindowOpenHandler: jest.fn(),
                 zoomLevel: 0,
+                isDestroyed: jest.fn(() => false),
             },
             contentView: {
                 on: jest.fn(),
@@ -118,6 +119,7 @@ describe('main/windows/mainWindow', () => {
             isFullScreen: jest.fn(),
             getBounds: jest.fn(),
             isMinimized: jest.fn().mockReturnValue(false),
+            isDestroyed: jest.fn(() => false),
         };
 
         beforeEach(() => {

src/app/mainWindow/mainWindow.ts

@@ -91,7 +91,7 @@ export class MainWindow extends EventEmitter {
         }
 
         this.win.browserWindow.webContents.once('did-finish-load', () => {
-            if (!this.win) {
+            if (!this.win || this.win.browserWindow.isDestroyed()) {
                 return;
             }
 

src/app/mainWindow/modals/modalView.test.js

@@ -17,6 +17,7 @@ jest.mock('electron', () => ({
             isDevToolsOpened: jest.fn(),
             closeDevTools: jest.fn(),
             close: jest.fn(),
+            isDestroyed: jest.fn(() => false),
         },
         setBounds: jest.fn(),
         setAutoResize: jest.fn(),

src/app/mainWindow/modals/modalView.ts

@@ -80,9 +80,12 @@ export class ModalView<T, T2> {
         this.status = Status.SHOWING;
         if (this.view.webContents.isLoading()) {
             this.view.webContents.once('did-finish-load', () => {
+                if (this.view.webContents.isDestroyed()) {
+                    return;
+                }
                 this.view.webContents.focus();
             });
-        } else {
+        } else if (!this.view.webContents.isDestroyed()) {
             this.view.webContents.focus();
         }
 

src/app/views/MattermostWebContentsView.test.js

@@ -4,7 +4,7 @@
 'use strict';
 
 import AppState from 'common/appState';
-import {LOAD_FAILED, UPDATE_TARGET_URL} from 'common/communication';
+import {BROWSER_HISTORY_PUSH, LOAD_FAILED, UPDATE_TARGET_URL} from 'common/communication';
 import {MattermostServer} from 'common/servers/MattermostServer';
 import ServerManager from 'common/servers/serverManager';
 import {MattermostView, ViewType} from 'common/views/MattermostView';
@@ -26,6 +26,8 @@ jest.mock('electron', () => ({
         webContents: {
             loadURL: jest.fn(),
             on: jest.fn(),
+            once: jest.fn(),
+            reload: jest.fn(),
             getTitle: () => 'title',
             getURL: () => 'http://server-1.com',
             send: jest.fn(),
@@ -36,7 +38,7 @@ jest.mock('electron', () => ({
                 goToOffset: jest.fn(),
                 canGoToOffset: jest.fn(),
             },
-            isDestroyed: jest.fn(),
+            isDestroyed: jest.fn(() => false),
         },
     })),
     ipcMain: {
@@ -99,6 +101,7 @@ jest.mock('main/server/serverAPI', () => ({
 }));
 jest.mock('common/views/viewManager', () => ({
     updateViewTitle: jest.fn(),
+    isPrimaryView: jest.fn(),
     getViewLog: jest.fn().mockReturnValue({
         info: jest.fn(),
         verbose: jest.fn(),
@@ -469,4 +472,75 @@ describe('main/views/MattermostWebContentsView', () => {
             expect(ViewManager.updateViewTitle).toHaveBeenCalledWith(mattermostView.id, 'Just Channel Name');
         });
     });
+
+    describe('useLastPath', () => {
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        let mattermostView;
+
+        beforeEach(() => {
+            MainWindow.get.mockReturnValue(window);
+            mattermostView = new MattermostWebContentsView(view, {}, window);
+        });
+
+        it('should send BROWSER_HISTORY_PUSH immediately for the primary view', () => {
+            ViewManager.isPrimaryView.mockReturnValue(true);
+            mattermostView.setLastPath('/team/channel');
+
+            mattermostView.useLastPath();
+
+            expect(mattermostView.webContentsView.webContents.send).toHaveBeenCalledWith(BROWSER_HISTORY_PUSH, '/team/channel');
+            expect(mattermostView.webContentsView.webContents.reload).not.toHaveBeenCalled();
+            expect(mattermostView.lastPath).toBeUndefined();
+        });
+
+        it('should send the captured path after reload, even though lastPath was cleared synchronously', () => {
+            ViewManager.isPrimaryView.mockReturnValue(false);
+
+            let didFinishLoadCb;
+            mattermostView.webContentsView.webContents.once.mockImplementation((event, cb) => {
+                if (event === 'did-finish-load') {
+                    didFinishLoadCb = cb;
+                }
+            });
+
+            mattermostView.setLastPath('/team/channel');
+            mattermostView.useLastPath();
+
+            expect(mattermostView.webContentsView.webContents.reload).toHaveBeenCalled();
+            expect(mattermostView.lastPath).toBeUndefined();
+
+            didFinishLoadCb();
+
+            expect(mattermostView.webContentsView.webContents.send).toHaveBeenCalledWith(BROWSER_HISTORY_PUSH, '/team/channel');
+        });
+
+        it('should not send when the webContents is destroyed before did-finish-load fires', () => {
+            ViewManager.isPrimaryView.mockReturnValue(false);
+
+            let didFinishLoadCb;
+            mattermostView.webContentsView.webContents.once.mockImplementation((event, cb) => {
+                if (event === 'did-finish-load') {
+                    didFinishLoadCb = cb;
+                }
+            });
+
+            mattermostView.setLastPath('/team/channel');
+            mattermostView.useLastPath();
+
+            mattermostView.webContentsView.webContents.isDestroyed.mockReturnValue(true);
+            didFinishLoadCb();
+
+            expect(mattermostView.webContentsView.webContents.send).not.toHaveBeenCalledWith(BROWSER_HISTORY_PUSH, expect.anything());
+        });
+
+        it('should be a no-op when lastPath is not set', () => {
+            ViewManager.isPrimaryView.mockReturnValue(false);
+
+            mattermostView.useLastPath();
+
+            expect(mattermostView.webContentsView.webContents.once).not.toHaveBeenCalled();
+            expect(mattermostView.webContentsView.webContents.reload).not.toHaveBeenCalled();
+            expect(mattermostView.webContentsView.webContents.send).not.toHaveBeenCalledWith(BROWSER_HISTORY_PUSH, expect.anything());
+        });
+    });
 });

src/app/views/MattermostWebContentsView.ts

@@ -291,8 +291,12 @@ export class MattermostWebContentsView extends EventEmitter {
             if (ViewManager.isPrimaryView(this.view.id)) {
                 this.webContentsView.webContents.send(BROWSER_HISTORY_PUSH, this.lastPath);
             } else {
+                const pathToPush = this.lastPath;
                 this.webContentsView.webContents.once('did-finish-load', () => {
-                    this.webContentsView.webContents.send(BROWSER_HISTORY_PUSH, this.lastPath);
+                    if (this.isDestroyed()) {
+                        return;
+                    }
+                    this.webContentsView.webContents.send(BROWSER_HISTORY_PUSH, pathToPush);
                 });
                 this.webContentsView.webContents.reload();
             }

src/app/views/loadingScreen.test.js

@@ -19,6 +19,7 @@ jest.mock('electron', () => {
             mockWebContents.send = jest.fn();
             mockWebContents.loadURL = jest.fn();
             mockWebContents.isLoading = jest.fn();
+            mockWebContents.isDestroyed = jest.fn(() => false);
 
             return {
                 webContents: mockWebContents,
@@ -50,6 +51,7 @@ describe('main/views/loadingScreen', () => {
             webContents: {
                 id: 123,
             },
+            isDestroyed: jest.fn(() => false),
         };
         const loadingScreen = new LoadingScreen(mainWindow);
 

src/app/views/loadingScreen.ts

@@ -51,6 +51,9 @@ export class LoadingScreen {
 
         if (this.view.webContents.isLoading()) {
             this.view.webContents.once('did-finish-load', () => {
+                if (this.view.webContents.isDestroyed() || this.parent.isDestroyed()) {
+                    return;
+                }
                 if (this.state !== LoadingScreenState.VISIBLE) {
                     return;
                 }
@@ -67,6 +70,9 @@ export class LoadingScreen {
                 }
             });
         } else {
+            if (this.view.webContents.isDestroyed() || this.parent.isDestroyed()) {
+                return;
+            }
             this.view.webContents.send(TOGGLE_LOADING_SCREEN_VISIBILITY, true);
             log.debug('show: not loading, adding loading screen view');
             if (condition?.()) {
@@ -83,7 +89,9 @@ export class LoadingScreen {
         if (this.state === LoadingScreenState.VISIBLE) {
             log.debug('fade: fading loading screen');
             this.state = LoadingScreenState.FADING;
-            this.view.webContents.send(TOGGLE_LOADING_SCREEN_VISIBILITY, false);
+            if (!this.view.webContents.isDestroyed()) {
+                this.view.webContents.send(TOGGLE_LOADING_SCREEN_VISIBILITY, false);
+            }
         }
     };
 

src/app/windows/baseWindow.test.js

@@ -61,6 +61,7 @@ jest.mock('electron', () => {
             mockWebContents.on = jest.fn(mockWebContents.on);
             mockWebContents.once = jest.fn(mockWebContents.once);
             mockWebContents.emit = jest.fn(mockWebContents.emit);
+            mockWebContents.isDestroyed = jest.fn(() => false);
             mockBrowserWindow.webContents = mockWebContents;
             mockBrowserWindow.getContentBounds = jest.fn(() => ({x: 0, y: 0, width: 800, height: 600}));
             mockBrowserWindow.getSize = jest.fn(() => [800, 600]);

src/app/windows/baseWindow.ts

@@ -71,6 +71,9 @@ export default class BaseWindow {
         this.win.setMenuBarVisibility(false);
 
         this.win.webContents.once('did-finish-load', () => {
+            if (!this.win || this.win.isDestroyed() || this.win.webContents.isDestroyed()) {
+                return;
+            }
             this.win.webContents.zoomLevel = 0;
             this.ready = true;
         });

src/app/windows/popoutManager.test.js

@@ -55,6 +55,7 @@ jest.mock('app/windows/baseWindow', () => {
     mockWebContents.on = jest.fn(mockWebContents.on);
     mockWebContents.once = jest.fn(mockWebContents.once);
     mockWebContents.emit = jest.fn(mockWebContents.emit);
+    mockWebContents.isDestroyed = jest.fn(() => false);
     const mockBrowserWindow = {
         webContents: mockWebContents,
         contentView: {
@@ -70,6 +71,7 @@ jest.mock('app/windows/baseWindow', () => {
         close: jest.fn(),
         setTitle: jest.fn(),
         loadURL: jest.fn(() => Promise.resolve()),
+        isDestroyed: jest.fn(() => false),
     };
 
     return jest.fn(() => ({
@@ -150,6 +152,7 @@ describe('PopoutManager', () => {
     mockWebContents.on = jest.fn(mockWebContents.on);
     mockWebContents.once = jest.fn(mockWebContents.once);
     mockWebContents.emit = jest.fn(mockWebContents.emit);
+    mockWebContents.isDestroyed = jest.fn(() => false);
     const mockBaseWindow = {
         browserWindow: {
             webContents: mockWebContents,
@@ -166,6 +169,7 @@ describe('PopoutManager', () => {
             close: jest.fn(),
             setTitle: jest.fn(),
             loadURL: jest.fn(() => Promise.resolve()),
+            isDestroyed: jest.fn(() => false),
         },
         showLoadingScreen: jest.fn(),
         fadeLoadingScreen: jest.fn(),

src/app/windows/popoutManager.ts

@@ -139,6 +139,9 @@ export class PopoutManager {
 
     private startPopoutWindow = (viewId: string, window: BaseWindow) => {
         window.browserWindow.webContents.once('did-finish-load', () => {
+            if (!window.browserWindow || window.browserWindow.isDestroyed() || window.browserWindow.webContents.isDestroyed()) {
+                return;
+            }
             this.handleViewUpdated(viewId);
             window.browserWindow.show();
         });
@@ -244,7 +247,7 @@ export class PopoutManager {
         const view = ViewManager.getView(viewId);
         if (view && view.type === ViewType.WINDOW) {
             const window = this.popoutWindows.get(viewId);
-            if (window) {
+            if (window?.browserWindow && !window.browserWindow.isDestroyed() && !window.browserWindow.webContents.isDestroyed()) {
                 const title = ViewManager.getViewTitle(viewId);
                 window.browserWindow.setTitle(title);
                 window.browserWindow.webContents.send(UPDATE_POPOUT_TITLE, viewId, title);

src/main/performanceMonitor.test.js

@@ -43,7 +43,7 @@ describe('main/performanceMonitor', () => {
             }
         });
 
-        makeWebContents = (id, resolve) => ({
+        makeWebContents = (id, resolve, {destroyed = false} = {}) => ({
             send: jest.fn().mockImplementation((channel, arg1, arg2) => {
                 if (channel === METRICS_REQUEST) {
                     cb({sender: {id}}, arg1, {serverId: arg2, cpu: id, memory: id * 100});
@@ -53,6 +53,7 @@ describe('main/performanceMonitor', () => {
                 }
             }),
             on: (_, listener) => listener(),
+            isDestroyed: jest.fn(() => destroyed),
             id,
         });
     });
@@ -252,4 +253,62 @@ describe('main/performanceMonitor', () => {
             expect(await sendValue2).toEqual(new Map([['view-1', {cpu: 1, memory: 100, serverId: 'server-1'}]]));
         });
     });
+
+    describe('destroyed view handling', () => {
+        it('should not register a view when its webContents is already destroyed at did-finish-load', () => {
+            const performanceMonitor = new PerformanceMonitor();
+            performanceMonitor.init();
+
+            const destroyedWebContents = makeWebContents(42, jest.fn(), {destroyed: true});
+            performanceMonitor.registerView('view-1', destroyedWebContents);
+            performanceMonitor.registerServerView('view-2', destroyedWebContents, 'server-1');
+
+            expect(performanceMonitor.views.has(42)).toBe(false);
+            expect(performanceMonitor.serverViews.has(42)).toBe(false);
+        });
+
+        it('runMetrics should unregister and skip destroyed views', async () => {
+            const performanceMonitor = new PerformanceMonitor();
+            performanceMonitor.init();
+
+            const liveResolve = jest.fn();
+            const liveWebContents = makeWebContents(1, liveResolve);
+            const destroyedWebContents = makeWebContents(2, jest.fn());
+
+            performanceMonitor.registerServerView('view-live', liveWebContents, 'server-1');
+            performanceMonitor.registerServerView('view-destroyed', destroyedWebContents, 'server-1');
+
+            // Mark as destroyed only after registration so the views map ends up with both entries
+            destroyedWebContents.isDestroyed.mockReturnValue(true);
+
+            const metrics = await performanceMonitor.runMetrics();
+
+            expect(performanceMonitor.serverViews.has(2)).toBe(false);
+            expect(performanceMonitor.serverViews.has(1)).toBe(true);
+            expect(destroyedWebContents.send).not.toHaveBeenCalled();
+            expect(liveWebContents.send).toHaveBeenCalledWith(METRICS_REQUEST, 'view-live', 'server-1');
+            expect(metrics.has('view-live')).toBe(true);
+            expect(metrics.has('view-destroyed')).toBe(false);
+        });
+
+        it('sendMetrics should unregister destroyed serverViews and not call send on them', async () => {
+            const performanceMonitor = new PerformanceMonitor();
+            performanceMonitor.init();
+
+            const sendValue = new Promise((resolve) => {
+                performanceMonitor.registerServerView('view-1', makeWebContents(1, resolve), 'server-1');
+            });
+            const destroyedWebContents = makeWebContents(2, jest.fn());
+            performanceMonitor.registerServerView('view-2', destroyedWebContents, 'server-1');
+
+            // After registration, mark view-2 as destroyed before the next interval fires
+            destroyedWebContents.isDestroyed.mockReturnValue(true);
+
+            jest.runOnlyPendingTimers();
+            await sendValue;
+
+            expect(performanceMonitor.serverViews.has(2)).toBe(false);
+            expect(destroyedWebContents.send).not.toHaveBeenCalledWith(METRICS_SEND, expect.anything());
+        });
+    });
 });