tab race condition fixes (#1407)

emain/emain-tabview.ts

@@ -1,6 +1,7 @@
 // Copyright 2024, Command Line Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+import { FileService } from "@/app/store/services";
 import { adaptFromElectronKeyEvent } from "@/util/keyutil";
 import { Rectangle, shell, WebContentsView } from "electron";
 import path from "path";
@@ -40,6 +41,8 @@ export class WaveTabView extends WebContentsView {
     savedInitOpts: WaveInitOpts;
     waveReadyPromise: Promise<void>;
     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;
@@ -58,6 +61,7 @@ export class WaveTabView extends WebContentsView {
             this.initResolve = resolve;
         });
         this.initPromise.then(() => {
+            this.isInitialized = true;
             console.log("tabview init", Date.now() - this.createdTs + "ms");
         });
         this.waveReadyPromise = new Promise((resolve, _) => {
@@ -67,6 +71,7 @@ export class WaveTabView extends WebContentsView {
         // 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);
         });
@@ -184,11 +189,12 @@ export function clearTabCache() {
 }
 
 // returns [tabview, initialized]
-export function getOrCreateWebViewForTab(fullConfig: FullConfigType, tabId: string): [WaveTabView, boolean] {
+export async function getOrCreateWebViewForTab(tabId: string): Promise<[WaveTabView, boolean]> {
     let tabView = getWaveTabView(tabId);
     if (tabView) {
         return [tabView, true];
     }
+    const fullConfig = await FileService.GetFullConfig();
     tabView = getSpareTab(fullConfig);
     tabView.lastUsedTs = Date.now();
     tabView.waveTabId = tabId;

emain/emain-window.ts

@@ -18,6 +18,17 @@ export type WindowOpts = {
 export const waveWindowMap = new Map<string, WaveBrowserWindow>(); // waveWindowId -> WaveBrowserWindow
 export let focusedWaveWindow = null; // on blur we do not set this to null (but on destroy we do)
 
+let cachedClientId: string = null;
+
+async function getClientId() {
+    if (cachedClientId != null) {
+        return cachedClientId;
+    }
+    const clientData = await ClientService.GetClientData();
+    cachedClientId = clientData?.oid;
+    return cachedClientId;
+}
+
 export class WaveBrowserWindow extends BaseWindow {
     waveWindowId: string;
     workspaceId: string;
@@ -26,7 +37,7 @@ export class WaveBrowserWindow extends BaseWindow {
     activeTabView: WaveTabView;
     private canClose: boolean;
     private deleteAllowed: boolean;
-    private tabSwitchQueue: { tabView: WaveTabView; tabInitialized: boolean }[];
+    private tabSwitchQueue: { tabId: string; setInBackend: boolean }[];
 
     constructor(waveWindow: WaveWindow, fullConfig: FullConfigType, opts: WindowOpts) {
         console.log("create win", waveWindow.oid);
@@ -292,15 +303,7 @@ export class WaveBrowserWindow extends BaseWindow {
 
     async setActiveTab(tabId: string, setInBackend: boolean) {
         console.log("setActiveTab", tabId, this.waveWindowId, this.workspaceId, setInBackend);
-        if (this.activeTabView?.waveTabId == tabId) {
+        await this.queueTabSwitch(tabId, setInBackend);
-            return;
-        }
-        if (setInBackend) {
-            await WorkspaceService.SetActiveTab(this.workspaceId, tabId);
-        }
-        const fullConfig = await FileService.GetFullConfig();
-        const [tabView, tabInitialized] = getOrCreateWebViewForTab(fullConfig, tabId);
-        await this.queueTabSwitch(tabView, tabInitialized);
     }
 
     async createTab(pinned = false) {
@@ -327,8 +330,26 @@ export class WaveBrowserWindow extends BaseWindow {
         this.allLoadedTabViews.delete(tabId);
     }
 
+    async initializeTab(tabView: WaveTabView) {
+        const clientId = await getClientId();
+        await tabView.initPromise;
+        this.contentView.addChildView(tabView);
+        const initOpts = {
+            tabId: tabView.waveTabId,
+            clientId: clientId,
+            windowId: this.waveWindowId,
+            activate: true,
+        };
+        tabView.savedInitOpts = { ...initOpts };
+        tabView.savedInitOpts.activate = false;
+        let startTime = Date.now();
+        console.log("before wave ready, init tab, sending wave-init", tabView.waveTabId);
+        tabView.webContents.send("wave-init", initOpts);
+        await tabView.waveReadyPromise;
+        console.log("wave-ready init time", Date.now() - startTime + "ms");
+    }
+
     async setTabViewIntoWindow(tabView: WaveTabView, tabInitialized: boolean) {
-        const clientData = await ClientService.GetClientData();
         if (this.activeTabView == tabView) {
             return;
         }
@@ -341,26 +362,11 @@ export class WaveBrowserWindow extends BaseWindow {
         this.allLoadedTabViews.set(tabView.waveTabId, tabView);
         if (!tabInitialized) {
             console.log("initializing a new tab");
-            await tabView.initPromise;
+            const p1 = this.initializeTab(tabView);
-            this.contentView.addChildView(tabView);
+            const p2 = this.repositionTabsSlowly(100);
-            const initOpts = {
+            await Promise.all([p1, p2]);
-                tabId: tabView.waveTabId,
-                clientId: clientData.oid,
-                windowId: this.waveWindowId,
-                activate: true,
-            };
-            tabView.savedInitOpts = { ...initOpts };
-            tabView.savedInitOpts.activate = false;
-            let startTime = Date.now();
-            tabView.webContents.send("wave-init", initOpts);
-            console.log("before wave ready");
-            await tabView.waveReadyPromise;
-            // positionTabOnScreen(tabView, this.getContentBounds());
-            console.log("wave-ready init time", Date.now() - startTime + "ms");
-            // positionTabOffScreen(oldActiveView, this.getContentBounds());
-            await this.repositionTabsSlowly(100);
         } else {
-            console.log("reusing an existing tab");
+            console.log("reusing an existing tab, calling wave-init", tabView.waveTabId);
             const p1 = this.repositionTabsSlowly(35);
             const p2 = tabView.webContents.send("wave-init", tabView.savedInitOpts); // reinit
             await Promise.all([p1, p2]);
@@ -423,28 +429,41 @@ export class WaveBrowserWindow extends BaseWindow {
         }
     }
 
-    async queueTabSwitch(tabView: WaveTabView, tabInitialized: boolean) {
+    async queueTabSwitch(tabId: string, setInBackend: boolean) {
-        if (this.tabSwitchQueue.length == 2) {
+        if (this.tabSwitchQueue.length >= 2) {
-            this.tabSwitchQueue[1] = { tabView, tabInitialized };
+            this.tabSwitchQueue[1] = { tabId, setInBackend };
             return;
         }
-        this.tabSwitchQueue.push({ tabView, tabInitialized });
+        const wasEmpty = this.tabSwitchQueue.length === 0;
-        if (this.tabSwitchQueue.length == 1) {
+        this.tabSwitchQueue.push({ tabId, setInBackend });
+        if (wasEmpty) {
             await this.processTabSwitchQueue();
         }
     }
 
+    // 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
+    // queueTabSwitch will replace [1] if it is already set
+    // we don't mess with [0] because it is "in process"
+    // we replace [1] because there is no point to switching to a tab that will be switched out of immediately
     async processTabSwitchQueue() {
-        if (this.tabSwitchQueue.length == 0) {
+        while (this.tabSwitchQueue.length > 0) {
-            this.tabSwitchQueue = [];
+            try {
-            return;
+                const { tabId, setInBackend } = this.tabSwitchQueue[0];
-        }
+                if (this.activeTabView?.waveTabId == tabId) {
-        try {
+                    continue;
-            const { tabView, tabInitialized } = this.tabSwitchQueue[0];
+                }
-            await this.setTabViewIntoWindow(tabView, tabInitialized);
+                if (setInBackend) {
-        } finally {
+                    await WorkspaceService.SetActiveTab(this.workspaceId, tabId);
-            this.tabSwitchQueue.shift();
+                }
-            await this.processTabSwitchQueue();
+                const [tabView, tabInitialized] = await getOrCreateWebViewForTab(tabId);
+                await this.setTabViewIntoWindow(tabView, tabInitialized);
+            } catch (e) {
+                console.log("error caught in processTabSwitchQueue", e);
+            } finally {
+                this.tabSwitchQueue.shift();
+            }
         }
     }
 

emain/emain.ts

@@ -403,7 +403,6 @@ async function createNewWaveWindow(): Promise<void> {
     newBrowserWindow.show();
 }
 
-// Here's where init is not getting fired
 electron.ipcMain.on("set-window-init-status", (event, status: "ready" | "wave-ready") => {
     const tabView = getWaveTabViewByWebContentsId(event.sender.id);
     if (tabView == null || tabView.initResolve == null) {
@@ -412,10 +411,9 @@ electron.ipcMain.on("set-window-init-status", (event, status: "ready" | "wave-re
     if (status === "ready") {
         tabView.initResolve();
         if (tabView.savedInitOpts) {
-            console.log("savedInitOpts");
+            // this handles the "reload" case.  we'll re-send the init opts to the frontend
+            console.log("savedInitOpts calling wave-init", tabView.waveTabId);
             tabView.webContents.send("wave-init", tabView.savedInitOpts);
-        } else {
-            console.log("no-savedInitOpts");
         }
     } else if (status === "wave-ready") {
         tabView.waveReadyResolve();

frontend/app/store/global.ts

@@ -103,8 +103,8 @@ function initGlobalAtoms(initOpts: GlobalInitOptions) {
     const tabAtom: Atom<Tab> = atom((get) => {
         return WOS.getObjectValue(WOS.makeORef("tab", initOpts.tabId), get);
     });
-    // This atom is used to determine the tab id to use for the static tab. It is set to the overrideStaticTabAtom value if it is not null, otherwise it is set to the initOpts.tabId value.
+    // this is *the* tab that this tabview represents.  it should never change.
-    const staticTabIdAtom: Atom<string> = atom((get) => get(overrideStaticTabAtom) ?? initOpts.tabId);
+    const staticTabIdAtom: Atom<string> = atom(initOpts.tabId);
     const controlShiftDelayAtom = atom(false);
     const updaterStatusAtom = atom<UpdaterStatus>("up-to-date") as PrimitiveAtom<UpdaterStatus>;
     try {