Fix race condition on switching calls (#2591)

src/main/windows/callsWidgetWindow.test.js

@@ -75,13 +75,15 @@ describe('main/windows/callsWidgetWindow', () => {
         baseWindow.setAlwaysOnTop = jest.fn();
         baseWindow.setBackgroundColor = jest.fn();
         baseWindow.setMenuBarVisibility = jest.fn();
-        baseWindow.setBounds = jest.fn();
         baseWindow.webContents = {
             setWindowOpenHandler: jest.fn(),
             on: jest.fn(),
         };
+        baseWindow.isDestroyed = jest.fn(() => false);
 
         beforeEach(() => {
+            baseWindow.setBounds = jest.fn();
+
             mainWindow.getBounds.mockImplementation(() => {
                 return {
                     x: 0,
@@ -100,6 +102,14 @@ describe('main/windows/callsWidgetWindow', () => {
                 };
             });
 
+            baseWindow.show = jest.fn(() => {
+                baseWindow.emit('show');
+            });
+
+            baseWindow.close = jest.fn(() => {
+                baseWindow.emit('closed');
+            });
+
             baseWindow.loadURL.mockImplementation(() => ({
                 catch: jest.fn(),
             }));
@@ -108,6 +118,8 @@ describe('main/windows/callsWidgetWindow', () => {
 
         afterEach(() => {
             jest.resetAllMocks();
+            baseWindow.removeAllListeners('show');
+            baseWindow.removeAllListeners('ready-to-show');
         });
 
         it('verify initial configuration', () => {
@@ -175,14 +187,24 @@ describe('main/windows/callsWidgetWindow', () => {
             expect(widgetWindow.win.webContents.openDevTools).toHaveBeenCalled();
         });
 
-        it('closing window', () => {
-            baseWindow.close = jest.fn(() => {
-                baseWindow.emit('closed');
-            });
+        it('closing window', async () => {
+            const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
+
+            await widgetWindow.close();
+
+            expect(widgetWindow.win.close).toHaveBeenCalled();
+            expect(widgetWindow.win.isDestroyed).toHaveBeenCalled();
+        });
+
+        it('closing window - already closed', async () => {
+            baseWindow.isDestroyed = jest.fn().mockReturnValue(true);
 
             const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
-            widgetWindow.close();
-            expect(widgetWindow.win.close).toHaveBeenCalled();
+
+            await widgetWindow.close();
+
+            expect(widgetWindow.win.isDestroyed).toHaveBeenCalled();
+            expect(widgetWindow.win.close).not.toHaveBeenCalled();
         });
 
         it('resize', () => {
@@ -191,10 +213,6 @@ describe('main/windows/callsWidgetWindow', () => {
                 id: 'windowID',
             };
 
-            baseWindow.show = jest.fn(() => {
-                baseWindow.emit('show');
-            });
-
             let winBounds = {
                 x: 0,
                 y: 0,
@@ -209,12 +227,18 @@ describe('main/windows/callsWidgetWindow', () => {
                 winBounds = bounds;
             });
 
+            baseWindow.show = jest.fn(() => {
+                baseWindow.emit('show');
+            });
+
+            expect(baseWindow.setBounds).not.toHaveBeenCalled();
+
             baseWindow.webContents.getZoomFactor = jest.fn(() => 1.0);
 
             const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig);
             widgetWindow.win.emit('ready-to-show');
 
-            expect(baseWindow.setBounds).toHaveBeenCalledTimes(2);
+            expect(baseWindow.setBounds).toHaveBeenCalledTimes(1);
 
             expect(baseWindow.setBounds).toHaveBeenCalledWith({
                 x: 12,
@@ -260,10 +284,6 @@ describe('main/windows/callsWidgetWindow', () => {
                 id: 'windowID',
             };
 
-            baseWindow.show = jest.fn(() => {
-                baseWindow.emit('show');
-            });
-
             let winBounds = {
                 x: 0,
                 y: 0,

src/main/windows/callsWidgetWindow.ts

@@ -89,9 +89,16 @@ export default class CallsWidgetWindow extends EventEmitter {
         this.load();
     }
 
-    public close() {
+    public async close() {
         log.debug('CallsWidgetWindow.close');
-        this.win.close();
+        return new Promise<void>((resolve) => {
+            if (this.win.isDestroyed()) {
+                resolve();
+                return;
+            }
+            this.once('closed', resolve);
+            this.win.close();
+        });
     }
 
     public getServerName() {

src/main/windows/windowManager.test.js

@@ -1032,6 +1032,23 @@ describe('main/windows/windowManager', () => {
                 },
             },
         };
+
+        beforeEach(() => {
+            CallsWidgetWindow.mockImplementation(() => {
+                return {
+                    win: {
+                        isDestroyed: jest.fn(() => true),
+                    },
+                    on: jest.fn(),
+                    close: jest.fn(),
+                };
+            });
+        });
+
+        afterEach(() => {
+            jest.resetAllMocks();
+        });
+
         const windowManager = new WindowManager();
         windowManager.viewManager = {
             views: new Map([
@@ -1039,25 +1056,25 @@ describe('main/windows/windowManager', () => {
             ]),
         };
 
-        it('should create calls widget window', () => {
+        it('should create calls widget window', async () => {
             expect(windowManager.callsWidgetWindow).toBeUndefined();
-            windowManager.createCallsWidgetWindow('server-1_tab-messaging', {callID: 'test'});
+            await windowManager.createCallsWidgetWindow('server-1_tab-messaging', {callID: 'test'});
             expect(windowManager.callsWidgetWindow).toBeDefined();
         });
 
-        it('should not create a new window if call is the same', () => {
+        it('should not create a new window if call is the same', async () => {
             const widgetWindow = windowManager.callsWidgetWindow;
             expect(widgetWindow).toBeDefined();
             widgetWindow.getCallID = jest.fn(() => 'test');
-            windowManager.createCallsWidgetWindow('server-1_tab-messaging', {callID: 'test'});
+            await windowManager.createCallsWidgetWindow('server-1_tab-messaging', {callID: 'test'});
             expect(windowManager.callsWidgetWindow).toEqual(widgetWindow);
         });
 
-        it('should create a new window if switching calls', () => {
+        it('should create a new window if switching calls', async () => {
             const widgetWindow = windowManager.callsWidgetWindow;
             expect(widgetWindow).toBeDefined();
             widgetWindow.getCallID = jest.fn(() => 'test');
-            windowManager.createCallsWidgetWindow('server-1_tab-messaging', {callID: 'test2'});
+            await windowManager.createCallsWidgetWindow('server-1_tab-messaging', {callID: 'test2'});
             expect(windowManager.callsWidgetWindow).not.toEqual(widgetWindow);
         });
     });

src/main/windows/windowManager.ts

@@ -127,14 +127,17 @@ export class WindowManager {
         };
     }
 
-    createCallsWidgetWindow = (viewName: string, msg: CallsJoinCallMessage) => {
+    createCallsWidgetWindow = async (viewName: string, msg: CallsJoinCallMessage) => {
         log.debug('WindowManager.createCallsWidgetWindow');
         if (this.callsWidgetWindow) {
             // trying to join again the call we are already in should not be allowed.
             if (this.callsWidgetWindow.getCallID() === msg.callID) {
                 return;
             }
-            this.callsWidgetWindow.close();
+
+            // to switch from one call to another we need to wait for the existing
+            // window to be fully closed.
+            await this.callsWidgetWindow.close();
         }
         const currentView = this.viewManager?.views.get(viewName);
         if (!currentView) {