closetab / tab destroy fixes (#1424)

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;
-
+    isDestroyed: 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;
 
     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);
-
+        if (!this.isDestroyed) {
-        // TODO: circuitous
+            this.webContents?.close();
-        const waveWindow = waveWindowMap.get(this.waveWindowId);
-        if (waveWindow) {
-            waveWindow.allLoadedTabViews.delete(this.waveTabId);
         }
+        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) {
+        tryEvictEntry(sorted[i].waveTabId);
-            // don't evict WaveTabViews that are currently showing in a window
-            continue;
-        }
-        const tabView = sorted[i];
-        tabView?.destroy();
     }
 }
 
@@ -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) {
+        tryEvictEntry(tabView.waveTabId);
-            continue;
-        }
-        tabView?.destroy();
     }
 }
 
 // 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);
 }
 

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) {
+        this.removeAllChildViews();
-            for (const tab of this.allLoadedTabViews.values()) {
-                this.contentView.removeChildView(tab);
-                tab?.destroy();
-            }
-        }
         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}`);
+        await this.queueCloseTab(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;
-        }
-        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);
     }
 
     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.queueCloseTab(tabId);
-        await ww.closeTab(tabId);
-    }
     event.returnValue = true;
     return null;
 });
@@ -685,10 +717,13 @@ export async function relaunchBrowserWindows() {
     console.log("relaunchBrowserWindows");
     setGlobalIsRelaunching(true);
     const windows = getAllWaveWindows();
+    if (windows.length > 0) {
         for (const window of windows) {
             console.log("relaunch -- closing window", window.waveWindowId);
             window.close();
         }
+        await delay(1200);
+    }
     setGlobalIsRelaunching(false);
 
     const clientData = await ClientService.GetClientData();