From 9f6cdfdbf685da36d702835cf1fb03b8a04d777c Mon Sep 17 00:00:00 2001 From: Mike Sawka Date: Fri, 6 Dec 2024 15:42:29 -0800 Subject: [PATCH] closetab / tab destroy fixes (#1424) --- emain/emain-tabview.ts | 74 ++++++++++++++----------- emain/emain-window.ts | 119 ++++++++++++++++++++++++++--------------- 2 files changed, 121 insertions(+), 72 deletions(-) diff --git a/emain/emain-tabview.ts b/emain/emain-tabview.ts index 9797db836..9f13278f4 100644 --- a/emain/emain-tabview.ts +++ b/emain/emain-tabview.ts @@ -4,11 +4,11 @@ import { FileService } from "@/app/store/services"; import { adaptFromElectronKeyEvent } from "@/util/keyutil"; import { Rectangle, shell, WebContentsView } from "electron"; +import { getWaveWindowById } from "emain/emain-window"; import path from "path"; import { configureAuthKeyRequestInjection } from "./authkey"; import { setWasActive } from "./emain-activity"; import { handleCtrlShiftFocus, handleCtrlShiftState, shFrameNavHandler, shNavHandler } from "./emain-util"; -import { waveWindowMap } from "./emain-window"; import { getElectronAppBasePath, isDevVite } from "./platform"; function computeBgColor(fullConfig: FullConfigType): string { @@ -31,8 +31,8 @@ export function getWaveTabViewByWebContentsId(webContentsId: number): WaveTabVie } export class WaveTabView extends WebContentsView { + waveWindowId: string; // this will be set for any tabviews that are initialized. (unset for the hot spare) isActiveTab: boolean; - waveWindowId: string; // set when showing in an active window private _waveTabId: string; // always set, WaveTabViews are unique per tab lastUsedTs: number; // ts milliseconds createdTs: number; // ts milliseconds @@ -43,9 +43,7 @@ export class WaveTabView extends WebContentsView { waveReadyResolve: () => void; isInitialized: boolean = false; isWaveReady: boolean = false; - - // used to destroy the tab if it is not initialized within a certain time after being assigned a tabId - private destroyTabTimeout: NodeJS.Timeout; + isDestroyed: boolean = false; constructor(fullConfig: FullConfigType) { console.log("createBareTabView"); @@ -67,13 +65,8 @@ export class WaveTabView extends WebContentsView { this.waveReadyPromise = new Promise((resolve, _) => { this.waveReadyResolve = resolve; }); - - // Once the frontend is ready, we can cancel the destroyTabTimeout, assuming the tab hasn't been destroyed yet - // Only after a tab is ready will we add it to the wcvCache this.waveReadyPromise.then(() => { this.isWaveReady = true; - clearTimeout(this.destroyTabTimeout); - setWaveTabView(this.waveTabId, this); }); wcIdToWaveTabMap.set(this.webContents.id, this); if (isDevVite) { @@ -84,6 +77,7 @@ export class WaveTabView extends WebContentsView { this.webContents.on("destroyed", () => { wcIdToWaveTabMap.delete(this.webContents.id); removeWaveTabView(this.waveTabId); + this.isDestroyed = true; }); this.setBackgroundColor(computeBgColor(fullConfig)); } @@ -94,9 +88,6 @@ export class WaveTabView extends WebContentsView { set waveTabId(waveTabId: string) { this._waveTabId = waveTabId; - this.destroyTabTimeout = setTimeout(() => { - this.destroy(); - }, 1000); } positionTabOnScreen(winBounds: Rectangle) { @@ -128,14 +119,11 @@ export class WaveTabView extends WebContentsView { destroy() { console.log("destroy tab", this.waveTabId); - this.webContents?.close(); removeWaveTabView(this.waveTabId); - - // TODO: circuitous - const waveWindow = waveWindowMap.get(this.waveWindowId); - if (waveWindow) { - waveWindow.allLoadedTabViews.delete(this.waveTabId); + if (!this.isDestroyed) { + this.webContents?.close(); } + this.isDestroyed = true; } } @@ -155,6 +143,31 @@ export function getWaveTabView(waveTabId: string): WaveTabView | undefined { return rtn; } +function tryEvictEntry(waveTabId: string): boolean { + const tabView = wcvCache.get(waveTabId); + if (!tabView) { + return false; + } + if (tabView.isActiveTab) { + return false; + } + const lastUsedDiff = Date.now() - tabView.lastUsedTs; + if (lastUsedDiff < 1000) { + return false; + } + const ww = getWaveWindowById(tabView.waveWindowId); + if (!ww) { + // this shouldn't happen, but if it does, just destroy the tabview + console.log("[error] WaveWindow not found for WaveTabView", tabView.waveTabId); + tabView.destroy(); + return true; + } else { + // will trigger a destroy on the tabview + ww.removeTabView(tabView.waveTabId, false); + return true; + } +} + function checkAndEvictCache(): void { if (wcvCache.size <= MaxCacheSize) { return; @@ -167,13 +180,9 @@ function checkAndEvictCache(): void { // Otherwise, sort by lastUsedTs return a.lastUsedTs - b.lastUsedTs; }); + const now = Date.now(); for (let i = 0; i < sorted.length - MaxCacheSize; i++) { - if (sorted[i].isActiveTab) { - // don't evict WaveTabViews that are currently showing in a window - continue; - } - const tabView = sorted[i]; - tabView?.destroy(); + tryEvictEntry(sorted[i].waveTabId); } } @@ -181,22 +190,21 @@ export function clearTabCache() { const wcVals = Array.from(wcvCache.values()); for (let i = 0; i < wcVals.length; i++) { const tabView = wcVals[i]; - if (tabView.isActiveTab) { - continue; - } - tabView?.destroy(); + tryEvictEntry(tabView.waveTabId); } } // returns [tabview, initialized] -export async function getOrCreateWebViewForTab(tabId: string): Promise<[WaveTabView, boolean]> { +export async function getOrCreateWebViewForTab(waveWindowId: string, tabId: string): Promise<[WaveTabView, boolean]> { let tabView = getWaveTabView(tabId); if (tabView) { return [tabView, true]; } const fullConfig = await FileService.GetFullConfig(); tabView = getSpareTab(fullConfig); + tabView.waveWindowId = waveWindowId; tabView.lastUsedTs = Date.now(); + setWaveTabView(tabId, tabView); tabView.waveTabId = tabId; tabView.webContents.on("will-navigate", shNavHandler); tabView.webContents.on("will-frame-navigate", shFrameNavHandler); @@ -231,11 +239,17 @@ export async function getOrCreateWebViewForTab(tabId: string): Promise<[WaveTabV } export function setWaveTabView(waveTabId: string, wcv: WaveTabView): void { + if (waveTabId == null) { + return; + } wcvCache.set(waveTabId, wcv); checkAndEvictCache(); } function removeWaveTabView(waveTabId: string): void { + if (waveTabId == null) { + return; + } wcvCache.delete(waveTabId); } diff --git a/emain/emain-window.ts b/emain/emain-window.ts index 550b27b21..3e511eae1 100644 --- a/emain/emain-window.ts +++ b/emain/emain-window.ts @@ -38,13 +38,17 @@ async function getClientId() { type TabSwitchQueueEntry = | { - createTab: false; + op: "switch"; tabId: string; setInBackend: boolean; } | { - createTab: true; + op: "create"; pinned: boolean; + } + | { + op: "close"; + tabId: string; }; export class WaveBrowserWindow extends BaseWindow { @@ -252,6 +256,11 @@ export class WaveBrowserWindow extends BaseWindow { console.log("win quitting or updating", this.waveWindowId); return; } + waveWindowMap.delete(this.waveWindowId); + if (focusedWaveWindow == this) { + focusedWaveWindow = null; + } + this.removeAllChildViews(); if (getGlobalIsRelaunching()) { console.log("win relaunching", this.waveWindowId); this.destroy(); @@ -266,17 +275,19 @@ export class WaveBrowserWindow extends BaseWindow { console.log("win removing window from backend DB", this.waveWindowId); fireAndForget(() => WindowService.CloseWindow(this.waveWindowId, true)); } - for (const tabView of this.allLoadedTabViews.values()) { - tabView?.destroy(); - } - waveWindowMap.delete(this.waveWindowId); - if (focusedWaveWindow == this) { - focusedWaveWindow = null; - } }); waveWindowMap.set(waveWindow.oid, this); } + removeAllChildViews() { + for (const tabView of this.allLoadedTabViews.values()) { + if (!this.isDestroyed()) { + this.contentView.removeChildView(tabView); + } + tabView?.destroy(); + } + } + async switchWorkspace(workspaceId: string) { console.log("switchWorkspace", workspaceId, this.waveWindowId); if (workspaceId == this.workspaceId) { @@ -311,12 +322,7 @@ export class WaveBrowserWindow extends BaseWindow { return; } console.log("switchWorkspace newWs", newWs); - if (this.allLoadedTabViews.size) { - for (const tab of this.allLoadedTabViews.values()) { - this.contentView.removeChildView(tab); - tab?.destroy(); - } - } + this.removeAllChildViews(); console.log("destroyed all tabs", this.waveWindowId); this.workspaceId = workspaceId; this.allLoadedTabViews = new Map(); @@ -329,22 +335,7 @@ export class WaveBrowserWindow extends BaseWindow { } async closeTab(tabId: string) { - console.log(`closeTab tabid=${tabId} ws=${this.workspaceId} window=${this.waveWindowId}`); - const rtn = await WorkspaceService.CloseTab(this.workspaceId, tabId, true); - if (rtn == null) { - console.log("[error] closeTab: no return value", tabId, this.workspaceId, this.waveWindowId); - return; - } - if (rtn.closewindow) { - this.close(); - return; - } - if (!rtn.newactivetabid) { - console.log("[error] closeTab, no new active tab", tabId, this.workspaceId, this.waveWindowId); - return; - } - await this.setActiveTab(rtn.newactivetabid, false); - this.allLoadedTabViews.delete(tabId); + await this.queueCloseTab(tabId); } async initializeTab(tabView: WaveTabView) { @@ -447,11 +438,15 @@ export class WaveBrowserWindow extends BaseWindow { } async queueTabSwitch(tabId: string, setInBackend: boolean) { - await this._queueTabSwitchInternal({ createTab: false, tabId, setInBackend }); + await this._queueTabSwitchInternal({ op: "switch", tabId, setInBackend }); } async queueCreateTab(pinned = false) { - await this._queueTabSwitchInternal({ createTab: true, pinned }); + await this._queueTabSwitchInternal({ op: "create", pinned }); + } + + async queueCloseTab(tabId: string) { + await this._queueTabSwitchInternal({ op: "close", tabId }); } async _queueTabSwitchInternal(entry: TabSwitchQueueEntry) { @@ -466,6 +461,12 @@ export class WaveBrowserWindow extends BaseWindow { } } + removeTabViewLater(tabId: string, delayMs: number) { + setTimeout(() => { + this.removeTabView(tabId, false); + }, 1000); + } + // the queue and this function are used to serialize tab switches // [0] => the tab that is currently being switched to // [1] => the tab that will be switched to next @@ -478,10 +479,10 @@ export class WaveBrowserWindow extends BaseWindow { const entry = this.tabSwitchQueue[0]; let tabId: string = null; // have to use "===" here to get the typechecker to work :/ - if (entry.createTab === true) { + if (entry.op === "create") { const { pinned } = entry; tabId = await WorkspaceService.CreateTab(this.workspaceId, null, true, pinned); - } else if (entry.createTab === false) { + } else if (entry.op === "switch") { let setInBackend: boolean = false; ({ tabId, setInBackend } = entry); if (this.activeTabView?.waveTabId == tabId) { @@ -490,11 +491,28 @@ export class WaveBrowserWindow extends BaseWindow { if (setInBackend) { await WorkspaceService.SetActiveTab(this.workspaceId, tabId); } + } else if (entry.op === "close") { + console.log("processTabSwitchQueue closeTab", entry.tabId); + tabId = entry.tabId; + const rtn = await WorkspaceService.CloseTab(this.workspaceId, tabId, true); + if (rtn == null) { + console.log("[error] closeTab: no return value", tabId, this.workspaceId, this.waveWindowId); + return; + } + this.removeTabViewLater(tabId, 1000); + if (rtn.closewindow) { + this.close(); + return; + } + if (!rtn.newactivetabid) { + return; + } + tabId = rtn.newactivetabid; } if (tabId == null) { return; } - const [tabView, tabInitialized] = await getOrCreateWebViewForTab(tabId); + const [tabView, tabInitialized] = await getOrCreateWebViewForTab(this.waveWindowId, tabId); await this.setTabViewIntoWindow(tabView, tabInitialized); } catch (e) { console.log("error caught in processTabSwitchQueue", e); @@ -520,6 +538,22 @@ export class WaveBrowserWindow extends BaseWindow { } } + removeTabView(tabId: string, force: boolean) { + if (!force && this.activeTabView?.waveTabId == tabId) { + console.log("cannot remove active tab", tabId, this.waveWindowId); + return; + } + const tabView = this.allLoadedTabViews.get(tabId); + if (tabView == null) { + console.log("removeTabView -- tabView not found", tabId, this.waveWindowId); + // the tab was never loaded, so just return + return; + } + this.contentView.removeChildView(tabView); + this.allLoadedTabViews.delete(tabId); + tabView.destroy(); + } + destroy() { console.log("destroy win", this.waveWindowId); this.deleteAllowed = true; @@ -607,9 +641,7 @@ ipcMain.on("close-tab", async (event, workspaceId, tabId) => { console.log(`close-tab: no window found for workspace ws=${workspaceId} tab=${tabId}`); return; } - if (ww != null) { - await ww.closeTab(tabId); - } + await ww.queueCloseTab(tabId); event.returnValue = true; return null; }); @@ -685,9 +717,12 @@ export async function relaunchBrowserWindows() { console.log("relaunchBrowserWindows"); setGlobalIsRelaunching(true); const windows = getAllWaveWindows(); - for (const window of windows) { - console.log("relaunch -- closing window", window.waveWindowId); - window.close(); + if (windows.length > 0) { + for (const window of windows) { + console.log("relaunch -- closing window", window.waveWindowId); + window.close(); + } + await delay(1200); } setGlobalIsRelaunching(false);