diff --git a/src/ElectronNET.API/API/WindowManager.cs b/src/ElectronNET.API/API/WindowManager.cs index 9b1ffd6..ff67578 100644 --- a/src/ElectronNET.API/API/WindowManager.cs +++ b/src/ElectronNET.API/API/WindowManager.cs @@ -19,6 +19,7 @@ namespace ElectronNET.API internal WindowManager() { + EnsureBrowserWindowClosedSubscription(); } internal static WindowManager Instance @@ -106,17 +107,6 @@ namespace ElectronNET.API tcs.SetResult(browserWindow); }); - BridgeConnector.Socket.Once("BrowserWindowClosed", (ids) => - { - for (int index = 0; index < _browserWindows.Count; index++) - { - if (!ids.Contains(_browserWindows[index].Id)) - { - _browserWindows.RemoveAt(index); - } - } - }); - if (loadUrl.Equals("http://localhost", StringComparison.OrdinalIgnoreCase) && ElectronNetRuntime.AspNetWebPort.HasValue) { loadUrl = $"{loadUrl}:{ElectronNetRuntime.AspNetWebPort}"; @@ -149,6 +139,40 @@ namespace ElectronNET.API return await tcs.Task.ConfigureAwait(false); } + private readonly object _browserWindowSubscriptionSync = new(); + private bool _browserWindowClosedSubscribed; + + private void EnsureBrowserWindowClosedSubscription() + { + if (_browserWindowClosedSubscribed) + { + return; + } + + lock (_browserWindowSubscriptionSync) + { + if (_browserWindowClosedSubscribed) + { + return; + } + + BridgeConnector.Socket.On("BrowserWindowClosed", HandleBrowserWindowClosed); + _browserWindowClosedSubscribed = true; + } + } + + private void HandleBrowserWindowClosed(int[] ids) + { + if (ids == null || ids.Length == 0) + { + _browserWindows.Clear(); + return; + } + + var existingIds = ids.ToHashSet(); + _browserWindows.RemoveAll(window => !existingIds.Contains(window.Id)); + } + private bool IsWindows10() { return RuntimeInformation.OSDescription.Contains("Windows 10"); diff --git a/src/ElectronNET.Host/api/browserWindows.js b/src/ElectronNET.Host/api/browserWindows.js index b4894a8..5ed71e8 100644 --- a/src/ElectronNET.Host/api/browserWindows.js +++ b/src/ElectronNET.Host/api/browserWindows.js @@ -230,7 +230,10 @@ module.exports = (socket, app) => { window = app["mainWindow"]; if (window) { window.reload(); - windows.push(window); + synchronizeWindowRegistry(); + if (!windows.some((entry) => tryGetWindowId(entry) === window.id)) { + windows.push(window); + } electronSocket.emit("BrowserWindowCreated", window.id); return; } @@ -253,21 +256,9 @@ module.exports = (socket, app) => { } }); lastOptions = options; - window.on("closed", (sender) => { - for (let index = 0; index < windows.length; index++) { - const windowItem = windows[index]; - try { - windowItem.id; - } - catch (error) { - if (error.message === "Object has been destroyed") { - windows.splice(index, 1); - const ids = []; - windows.forEach((x) => ids.push(x.id)); - electronSocket.emit("BrowserWindowClosed", ids); - } - } - } + window.on("closed", () => { + synchronizeWindowRegistry(); + emitBrowserWindowClosed(); }); app.on("activate", () => { // On macOS it's common to re-create a window in the app when the @@ -703,12 +694,47 @@ module.exports = (socket, app) => { getWindowById(id).setBrowserView((0, browserView_1.browserViewMediateService)(browserViewId)); }); function getWindowById(id) { + const runtimeWindow = electron_1.BrowserWindow.fromId(id); + if (runtimeWindow) { + return runtimeWindow; + } + synchronizeWindowRegistry(); for (let index = 0; index < windows.length; index++) { const element = windows[index]; - if (element.id === id) { + if (tryGetWindowId(element) === id) { return element; } } + throw new Error(`BrowserWindow with id '${id}' was not found.`); + } + function tryGetWindowId(element) { + try { + return element.id; + } + catch { + return null; + } + } + function synchronizeWindowRegistry() { + const runtimeWindows = electron_1.BrowserWindow.getAllWindows(); + const runtimeWindowIds = new Set(runtimeWindows.map((entry) => entry.id)); + for (let index = windows.length - 1; index >= 0; index--) { + const windowId = tryGetWindowId(windows[index]); + if (windowId === null || !runtimeWindowIds.has(windowId)) { + windows.splice(index, 1); + } + } + readyToShowWindowsIds = readyToShowWindowsIds.filter((entryId) => runtimeWindowIds.has(entryId)); + } + function emitBrowserWindowClosed() { + const ids = []; + for (const entry of windows) { + const windowId = tryGetWindowId(entry); + if (windowId !== null) { + ids.push(windowId); + } + } + electronSocket.emit("BrowserWindowClosed", ids); } }; //# sourceMappingURL=browserWindows.js.map \ No newline at end of file diff --git a/src/ElectronNET.Host/api/browserWindows.ts b/src/ElectronNET.Host/api/browserWindows.ts index e52f729..4434f81 100644 --- a/src/ElectronNET.Host/api/browserWindows.ts +++ b/src/ElectronNET.Host/api/browserWindows.ts @@ -255,7 +255,10 @@ export = (socket: Socket, app: Electron.App) => { window = app["mainWindow"]; if (window) { window.reload(); - windows.push(window); + synchronizeWindowRegistry(); + if (!windows.some((entry) => tryGetWindowId(entry) === window.id)) { + windows.push(window); + } electronSocket.emit("BrowserWindowCreated", window.id); return; } @@ -283,21 +286,9 @@ export = (socket: Socket, app: Electron.App) => { lastOptions = options; - window.on("closed", (sender) => { - for (let index = 0; index < windows.length; index++) { - const windowItem = windows[index]; - try { - windowItem.id; - } catch (error) { - if (error.message === "Object has been destroyed") { - windows.splice(index, 1); - - const ids = []; - windows.forEach((x) => ids.push(x.id)); - electronSocket.emit("BrowserWindowClosed", ids); - } - } - } + window.on("closed", () => { + synchronizeWindowRegistry(); + emitBrowserWindowClosed(); }); app.on("activate", () => { @@ -907,11 +898,57 @@ export = (socket: Socket, app: Electron.App) => { }); function getWindowById(id: number): Electron.BrowserWindow { + const runtimeWindow = BrowserWindow.fromId(id); + if (runtimeWindow) { + return runtimeWindow; + } + + synchronizeWindowRegistry(); + for (let index = 0; index < windows.length; index++) { const element = windows[index]; - if (element.id === id) { + if (tryGetWindowId(element) === id) { return element; } } + + throw new Error(`BrowserWindow with id '${id}' was not found.`); + } + + function tryGetWindowId(element: Electron.BrowserWindow): number | null { + try { + return element.id; + } catch { + return null; + } + } + + function synchronizeWindowRegistry(): void { + const runtimeWindows = BrowserWindow.getAllWindows(); + const runtimeWindowIds = new Set(runtimeWindows.map((entry) => entry.id)); + + for (let index = windows.length - 1; index >= 0; index--) { + const windowId = tryGetWindowId(windows[index]); + if (windowId === null || !runtimeWindowIds.has(windowId)) { + windows.splice(index, 1); + } + } + + readyToShowWindowsIds = readyToShowWindowsIds.filter((entryId) => + runtimeWindowIds.has(entryId), + ); + } + + function emitBrowserWindowClosed(): void { + const ids: number[] = []; + + for (const entry of windows) { + const windowId = tryGetWindowId(entry); + if (windowId !== null) { + ids.push(windowId); + } + } + + electronSocket.emit("BrowserWindowClosed", ids); } }; diff --git a/src/ElectronNET.IntegrationTests/Tests/WindowManagerLifecycleTests.cs b/src/ElectronNET.IntegrationTests/Tests/WindowManagerLifecycleTests.cs new file mode 100644 index 0000000..59679a7 --- /dev/null +++ b/src/ElectronNET.IntegrationTests/Tests/WindowManagerLifecycleTests.cs @@ -0,0 +1,55 @@ +namespace ElectronNET.IntegrationTests.Tests; + +/// +/// Regression checks for BrowserWindow lifecycle cleanup in WindowManager. +/// Covers GitHub issue #1008. +/// +public class WindowManagerLifecycleTests +{ + private static readonly string WindowManagerFilePath = FindWindowManagerFile(); + + [Fact] + public void WindowManager_ShouldUsePersistentBrowserWindowClosedSubscription() + { + File.Exists(WindowManagerFilePath).Should().BeTrue( + $"WindowManager source must exist at '{WindowManagerFilePath}'."); + + var content = File.ReadAllText(WindowManagerFilePath); + + content.Should().Contain( + "Socket.On(\"BrowserWindowClosed\"", + "closed-window cleanup must be wired for every close event, not just the first one."); + + content.Should().NotContain( + "Socket.Once(\"BrowserWindowClosed\"", + "a one-shot subscription causes stale BrowserWindow references after subsequent closes (issue #1008)."); + } + + private static string FindWindowManagerFile() + { + const string RelativeFromRepoRoot = "src/ElectronNET.API/API/WindowManager.cs"; + const string RelativeFromSrc = "ElectronNET.API/API/WindowManager.cs"; + + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir != null) + { + var fromRepoRoot = Path.Combine(dir.FullName, RelativeFromRepoRoot); + if (File.Exists(fromRepoRoot)) + { + return Path.GetFullPath(fromRepoRoot); + } + + var fromSrc = Path.Combine(dir.FullName, RelativeFromSrc); + if (File.Exists(fromSrc)) + { + return Path.GetFullPath(fromSrc); + } + + dir = dir.Parent; + } + + throw new FileNotFoundException( + "Could not locate WindowManager.cs by walking up from " + + $"'{AppContext.BaseDirectory}'."); + } +}