[MM-68738][MM-68739][MM-68741][MM-68742][MM-68747][MM-68748][MM-68750][MM-68814][MM-68880][MM-68908][MM-69030][MM-69783] Guard against destroyed Electron objects and premature window creation

src/app/mainWindow/mainWindow.test.js

@@ -586,6 +586,7 @@ describe('main/windows/mainWindow', () => {
                 isVisible: jest.fn(() => mainWindow.win.browserWindow.visible),
                 show: jest.fn(),
                 focus: jest.fn(),
+                isDestroyed: jest.fn(() => false),
             },
         };
         mainWindow.init = jest.fn();

src/app/mainWindow/mainWindow.ts

@@ -131,28 +131,32 @@ export class MainWindow extends EventEmitter {
     }
 
     get = () => {
-        return this.win?.browserWindow;
+        if (!this.win || this.win.browserWindow.isDestroyed()) {
+            return undefined;
+        }
+        return this.win.browserWindow;
     };
 
     get window() {
         return this.win;
     }
 
     show = () => {
-        if (this.win && this.isReady) {
+        const browserWindow = this.get();
+        if (browserWindow && this.isReady) {
             // There's a bug on Windows in Electron where if the window is snapped, it will unsnap when you call show()
             // See here: https://github.com/electron/electron/issues/25359
             // So to make sure we always show the window on macOS/Linux (need for workspace switching)
             // We make an exception here
             if (process.platform === 'win32') {
-                if (this.win.browserWindow.isVisible()) {
-                    this.win.browserWindow.focus();
+                if (browserWindow.isVisible()) {
+                    browserWindow.focus();
                 } else {
-                    this.win.browserWindow.show();
+                    browserWindow.show();
                 }
             } else {
                 log.info('showing main window');
-                this.win.browserWindow.show();
+                browserWindow.show();
             }
         } else {
             this.init();
@@ -164,7 +168,7 @@ export class MainWindow extends EventEmitter {
     };
 
     getBounds = () => {
-        return this.win?.browserWindow.getContentBounds();
+        return this.get()?.getContentBounds();
     };
 
     private shouldStartFullScreen = () => {
@@ -217,6 +221,10 @@ export class MainWindow extends EventEmitter {
     };
 
     private saveWindowState = (file: string, window: BrowserWindow) => {
+        if (window.isDestroyed()) {
+            return;
+        }
+
         const windowState: SavedWindowState = {
             ...window.getBounds(),
             maximized: window.isMaximized(),
@@ -263,19 +271,20 @@ export class MainWindow extends EventEmitter {
     };
 
     private onBlur = () => {
-        if (!this.win) {
+        const browserWindow = this.get();
+        if (!browserWindow) {
             return;
         }
 
-        this.emit(MAIN_WINDOW_RESIZED, this.win?.browserWindow.getContentBounds());
+        this.emit(MAIN_WINDOW_RESIZED, browserWindow.getContentBounds());
         ipcMain.emit(TOGGLE_SECURE_INPUT, null, false);
 
         // App should save bounds when a window is closed.
         // However, 'close' is not fired in some situations(shutdown, ctrl+c)
         // because main process is killed in such situations.
         // 'blur' event was effective in order to avoid this.
         // Ideally, app should detect that OS is shutting down.
-        this.saveWindowState(boundsInfoPath, this.win.browserWindow);
+        this.saveWindowState(boundsInfoPath, browserWindow);
     };
 
     private onClose = (event: Event) => {

src/app/system/tray/tray.ts

@@ -107,7 +107,12 @@ export class TrayIcon {
         }
     };
 
-    setMenu = (tMenu: Electron.Menu) => this.tray?.setContextMenu(tMenu);
+    setMenu = (tMenu: Electron.Menu) => {
+        if (!this.tray || this.tray.isDestroyed()) {
+            return;
+        }
+        this.tray.setContextMenu(tMenu);
+    };
 
     private update = (status: string, message: string) => {
         if (!this.tray || this.tray.isDestroyed()) {

src/app/views/MattermostWebContentsView.test.js

@@ -124,7 +124,7 @@ const view = new MattermostView(server, ViewType.TAB);
 
 describe('main/views/MattermostWebContentsView', () => {
     describe('load', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         const mattermostView = new MattermostWebContentsView(view, {}, window);
 
         beforeEach(() => {
@@ -190,7 +190,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('retry', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         const mattermostView = new MattermostWebContentsView(view, {}, window);
         const retryInBackgroundFn = jest.fn();
 
@@ -254,7 +254,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('retryInBackground', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         const mattermostView = new MattermostWebContentsView(view, {}, window);
         mattermostView.reload = jest.fn();
 
@@ -272,7 +272,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('goToOffset', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         const mattermostView = new MattermostWebContentsView(view, {}, window);
         mattermostView.reload = jest.fn();
 
@@ -302,7 +302,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('loadSuccess', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         const mattermostView = new MattermostWebContentsView(view, {}, window);
 
         beforeEach(() => {
@@ -331,7 +331,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('updateHistoryButton', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         const mattermostView = new MattermostWebContentsView(view, {}, window);
 
         beforeEach(() => {
@@ -463,7 +463,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('handleUpdateTarget', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         const mattermostView = new MattermostWebContentsView(view, {}, window);
 
         beforeEach(() => {
@@ -495,7 +495,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('handlePageTitleUpdated', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         const mattermostView = new MattermostWebContentsView(view, {}, window);
 
         beforeEach(() => {
@@ -556,7 +556,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('useLastPath', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         let mattermostView;
 
         beforeEach(() => {
@@ -632,21 +632,21 @@ describe('main/views/MattermostWebContentsView', () => {
         });
 
         it('should register devtools-focused listener permanently', () => {
-            const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+            const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
             const mattermostView = new MattermostWebContentsView(view, {}, window);
             const onCalls = mattermostView.webContentsView.webContents.on.mock.calls;
             expect(onCalls.some(([e]) => e === 'devtools-focused')).toBe(true);
         });
 
         it('should register devtools-closed listener permanently', () => {
-            const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+            const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
             const mattermostView = new MattermostWebContentsView(view, {}, window);
             const onCalls = mattermostView.webContentsView.webContents.on.mock.calls;
             expect(onCalls.some(([e]) => e === 'devtools-closed')).toBe(true);
         });
 
         it('should emit UPDATE_SHORTCUT_MENU when devtools-focused fires', () => {
-            const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+            const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
             const mattermostView = new MattermostWebContentsView(view, {}, window);
             const onCall = mattermostView.webContentsView.webContents.on.mock.calls.find(([e]) => e === 'devtools-focused');
             expect(onCall).toBeDefined();
@@ -655,7 +655,7 @@ describe('main/views/MattermostWebContentsView', () => {
         });
 
         it('should emit UPDATE_SHORTCUT_MENU when devtools-closed fires', () => {
-            const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+            const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
             const mattermostView = new MattermostWebContentsView(view, {}, window);
             const onCall = mattermostView.webContentsView.webContents.on.mock.calls.find(([e]) => e === 'devtools-closed');
             expect(onCall).toBeDefined();
@@ -665,7 +665,7 @@ describe('main/views/MattermostWebContentsView', () => {
     });
 
     describe('openDevTools', () => {
-        const window = {on: jest.fn(), webContents: {send: jest.fn()}};
+        const window = {on: jest.fn(), webContents: {send: jest.fn()}, isDestroyed: jest.fn(() => false)};
         let mattermostView;
 
         beforeEach(() => {

src/app/views/MattermostWebContentsView.ts

@@ -202,7 +202,7 @@ export class MattermostWebContentsView extends EventEmitter {
         const loading = this.webContentsView.webContents.loadURL(loadURL, {userAgent: composeUserAgent(DeveloperMode.get('browserOnly'))});
         loading.then(this.loadSuccess(loadURL)).catch((err) => {
             if (err.code && err.code.startsWith('ERR_CERT')) {
-                this.parentWindow.webContents.send(LOAD_FAILED, this.id, err.toString(), loadURL.toString());
+                this.sendToParentWindow(LOAD_FAILED, this.id, err.toString(), loadURL.toString());
                 this.emit(LOAD_FAILED, this.id, err.toString(), loadURL.toString());
                 this.log.info(`Invalid certificate, stop retrying until the user decides what to do: ${err}.`);
                 this.status = Status.ERROR;
@@ -353,6 +353,12 @@ export class MattermostWebContentsView extends EventEmitter {
         this.webContents?.send(channel, ...args);
     };
 
+    private sendToParentWindow = (channel: string, ...args: any[]) => {
+        if (this.parentWindow && !this.parentWindow.isDestroyed()) {
+            this.parentWindow.webContents.send(channel, ...args);
+        }
+    };
+
     isDestroyed = () => {
         return this.webContentsView?.webContents?.isDestroyed() ?? true;
     };
@@ -382,7 +388,7 @@ export class MattermostWebContentsView extends EventEmitter {
                 if (this.maxRetries-- > 0) {
                     this.loadRetry(loadURL, err);
                 } else {
-                    this.parentWindow.webContents.send(LOAD_FAILED, this.id, err.toString(), loadURL.toString());
+                    this.sendToParentWindow(LOAD_FAILED, this.id, err.toString(), loadURL.toString());
                     this.emit(LOAD_FAILED, this.id, err.toString(), loadURL.toString());
                     this.log.info('Could not establish a connection, will continue to retry in the background', {err});
                     this.status = Status.ERROR;
@@ -426,7 +432,7 @@ export class MattermostWebContentsView extends EventEmitter {
             return;
         }
         this.retryLoad = setTimeout(this.retry(loadURL), RELOAD_INTERVAL);
-        this.parentWindow.webContents.send(LOAD_RETRY, this.id, Date.now() + RELOAD_INTERVAL, err.toString(), loadURL.toString());
+        this.sendToParentWindow(LOAD_RETRY, this.id, Date.now() + RELOAD_INTERVAL, err.toString(), loadURL.toString());
         this.log.info(`failed loading URL: ${err}, retrying in ${RELOAD_INTERVAL / SECOND} seconds`);
     };
 
@@ -438,7 +444,7 @@ export class MattermostWebContentsView extends EventEmitter {
             const serverInfo = ServerManager.getRemoteInfo(this.view.serverId);
             if (!serverInfo?.serverVersion || semver.gte(serverInfo.serverVersion, '9.4.0')) {
                 this.log.verbose('finished loading URL');
-                this.parentWindow.webContents.send(LOAD_SUCCESS, this.id);
+                this.sendToParentWindow(LOAD_SUCCESS, this.id);
                 this.maxRetries = MAX_SERVER_RETRIES;
                 this.status = Status.WAITING_MM;
                 this.removeLoading = setTimeout(this.setInitialized, MAX_LOADING_SCREEN_SECONDS, true);
@@ -447,7 +453,7 @@ export class MattermostWebContentsView extends EventEmitter {
                     this.setBounds(getWindowBoundaries(this.parentWindow));
                 }
             } else {
-                this.parentWindow.webContents.send(LOAD_INCOMPATIBLE_SERVER, this.id, loadURL.toString());
+                this.sendToParentWindow(LOAD_INCOMPATIBLE_SERVER, this.id, loadURL.toString());
                 this.emit(LOAD_FAILED, this.id, 'Incompatible server version', loadURL.toString());
                 this.status = Status.ERROR;
             }

src/main/app/app.test.js

@@ -6,7 +6,7 @@ import {app, dialog} from 'electron';
 import MainWindow from 'app/mainWindow/mainWindow';
 import WebContentsManager from 'app/views/webContentsManager';
 import ServerManager from 'common/servers/serverManager';
-import {handleAppWillFinishLaunching, handleAppCertificateError, certificateErrorCallbacks} from 'main/app/app';
+import {handleAppActivate, handleAppWillFinishLaunching, handleAppCertificateError, certificateErrorCallbacks} from 'main/app/app';
 import {getDeeplinkingURL, openDeepLink} from 'main/app/utils';
 import CertificateStore from 'main/security/certificateStore';
 
@@ -60,6 +60,32 @@ jest.mock('common/views/viewManager', () => ({
 }));
 
 describe('main/app/app', () => {
+    describe('handleAppActivate', () => {
+        afterEach(() => {
+            jest.resetAllMocks();
+        });
+
+        it('should show the main window if app is ready', () => {
+            app.isReady.mockReturnValue(true);
+            handleAppActivate();
+            expect(MainWindow.show).toHaveBeenCalled();
+        });
+
+        it('should wait until app is ready to show the main window', () => {
+            let callback;
+            app.once.mockImplementation((event, cb) => {
+                if (event === 'ready') {
+                    callback = cb;
+                }
+            });
+            app.isReady.mockReturnValue(false);
+            handleAppActivate();
+            expect(MainWindow.show).not.toHaveBeenCalled();
+            callback();
+            expect(MainWindow.show).toHaveBeenCalled();
+        });
+    });
+
     describe('handleAppWillFinishLaunching', () => {
         const deepLinkURL = 'mattermost://server-1.com';
         const testURL = 'http://server-1.com';

src/main/app/app.ts

@@ -38,6 +38,15 @@ export function handleAppSecondInstance(event: Event, argv: string[]) {
     }
 }
 
+export function handleAppActivate() {
+    if (app.isReady()) {
+        MainWindow.show();
+    } else {
+        // On macOS 'activate' can fire before 'ready' (e.g. clicking the dock icon while still launching).
+        app.once('ready', () => MainWindow.show());
+    }
+}
+
 export function handleAppWindowAllClosed() {
     log.debug('handleAppWindowAllClosed');
 

src/main/app/initialize.ts

@@ -65,6 +65,7 @@ import updateNotifier from 'main/updateNotifier';
 import UserActivityMonitor from 'main/UserActivityMonitor';
 
 import {
+    handleAppActivate,
     handleAppBeforeQuit,
     handleAppBrowserWindowCreated,
     handleAppCertificateError,
@@ -187,7 +188,7 @@ function initializeAppEventListeners() {
     app.on('second-instance', handleAppSecondInstance);
     app.on('window-all-closed', handleAppWindowAllClosed);
     app.on('browser-window-created', handleAppBrowserWindowCreated);
-    app.on('activate', () => MainWindow.show());
+    app.on('activate', handleAppActivate);
     app.on('before-quit', handleAppBeforeQuit);
     app.on('certificate-error', handleAppCertificateError);
     app.on('child-process-gone', handleChildProcessGone);