From d3fda81af3bc17f688c31e92487673869a464557 Mon Sep 17 00:00:00 2001 From: Zhihe Li Date: Tue, 4 Mar 2025 11:50:19 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20=F0=9F=90=9B=20Fix=20race=20condition=20?= =?UTF-8?q?in=20browser=20window=20loading=20(#2701)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ui/desktop/electron-app/src/index.js | 18 ++++++++++-------- ui/desktop/electron-app/src/ipc/handlers.js | 2 +- .../src/models/runtime-settings.js | 15 ++++++++++----- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/ui/desktop/electron-app/src/index.js b/ui/desktop/electron-app/src/index.js index e5118aa7a..2094c0af4 100644 --- a/ui/desktop/electron-app/src/index.js +++ b/ui/desktop/electron-app/src/index.js @@ -85,7 +85,7 @@ if (isWindows()) { app.setAppUserModelId(app.name); } -const createWindow = (partition, closeWindowCB) => { +const createWindow = async (partition, closeWindowCB) => { /** * Enable electron OS window frame/chrome for MacOS only. * Disable frame/chrome regardless of OS when @@ -128,15 +128,17 @@ const createWindow = (partition, closeWindowCB) => { // If the user-specified cluster URL changes, reload the page so that // the CSP can be refreshed with the this source allowed - runtimeSettings.onClusterUrlChange(() => browserWindow.loadURL(emberAppURL)); + runtimeSettings.onClusterUrlChange( + async () => await browserWindow.loadURL(emberAppURL), + ); // Load the ember application - browserWindow.loadURL(emberAppURL); + await browserWindow.loadURL(emberAppURL); // If a loading operation goes wrong, we'll send Electron back to // Ember App entry point - browserWindow.webContents.on('did-fail-load', () => { - browserWindow.loadURL(emberAppURL); + browserWindow.webContents.on('did-fail-load', async () => { + await browserWindow.loadURL(emberAppURL); }); browserWindow.webContents.on('render-process-gone', () => { @@ -259,7 +261,7 @@ app.on('ready', async () => { mainWindow = null; }; - mainWindow = createWindow(partition, closeWindowCB); + mainWindow = await createWindow(partition, closeWindowCB); // Check for updates on launch appUpdater.run({ suppressNoUpdatePrompt: true }); @@ -270,9 +272,9 @@ app.on('ready', async () => { * while the app is not close. * More info: https://www.electronjs.org/docs/latest/tutorial/quick-start#open-a-window-if-none-are-open-macos */ - app.on('activate', () => { + app.on('activate', async () => { if (BrowserWindow.getAllWindows().length === 0) { - mainWindow = createWindow(partition, closeWindowCB); + mainWindow = await createWindow(partition, closeWindowCB); } }); diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index a68dd9eb8..2807be586 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -33,7 +33,7 @@ handle('getClusterUrl', () => runtimeSettings.clusterUrl); handle('setClusterUrl', async (requestOrigin) => { const clusterUrl = sanitizer.urlValidate(requestOrigin); await runtimeSettings.validateClusterUrl(clusterUrl); - runtimeSettings.clusterUrl = clusterUrl; + await runtimeSettings.setClusterUrl(clusterUrl); }); /** diff --git a/ui/desktop/electron-app/src/models/runtime-settings.js b/ui/desktop/electron-app/src/models/runtime-settings.js index efa1271a7..59addf25d 100644 --- a/ui/desktop/electron-app/src/models/runtime-settings.js +++ b/ui/desktop/electron-app/src/models/runtime-settings.js @@ -18,10 +18,11 @@ class RuntimeSettings { get clusterUrl() { return this.#clusterUrl; } - set clusterUrl(clusterUrl) { + + async setClusterUrl(clusterUrl) { if (this.#clusterUrl !== clusterUrl) { this.#clusterUrl = clusterUrl; - this.triggerClusterUrlChanged(); + await this.triggerClusterUrlChanged(); } } @@ -50,7 +51,7 @@ class RuntimeSettings { /** * Sets the clusterUrl to null. */ - async resetClusterUrl() { + resetClusterUrl() { this.clusterUrl = null; } @@ -62,8 +63,12 @@ class RuntimeSettings { this.#clusterUrlWatchers.push(fn); } - triggerClusterUrlChanged() { - this.#clusterUrlWatchers.forEach((fn) => fn(this.#clusterUrl)); + async triggerClusterUrlChanged() { + // We currently only store one function but this will execute serially + // which may or may not be the desired behavior if there ever is more + for (const fn of this.#clusterUrlWatchers) { + await fn(this.#clusterUrl); + } } }