From 8fc533d767441fdaab06ea7310462dece0db1eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E4=B8=80=E4=BA=8C?= Date: Sun, 12 Dec 2021 20:51:04 +0800 Subject: [PATCH] refactor: add more log and assertion --- src/services/view/index.ts | 38 +++++++++++++----- src/services/workspaces/interface.ts | 4 ++ src/services/workspacesView/index.ts | 58 +++++++++++++++++++--------- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/services/view/index.ts b/src/services/view/index.ts index 60412859..ecde8970 100644 --- a/src/services/view/index.ts +++ b/src/services/view/index.ts @@ -223,7 +223,12 @@ export class View implements IViewService { // we assume each window will only have one view, so get view by window name + workspace const existedView = this.getView(workspace.id, windowName); const browserWindow = this.windowService.get(windowName); - if (existedView !== undefined || browserWindow === undefined) { + if (existedView !== undefined) { + logger.warn(`BrowserViewService.addView: ${workspace.id} 's view already exists`); + return; + } + if (browserWindow === undefined) { + logger.warn(`BrowserViewService.addView: ${workspace.id} 's browser window is not ready`); return; } // create a new BrowserView @@ -294,6 +299,11 @@ export class View implements IViewService { */ const loadInitialUrlWithCatch = async (): Promise => { try { + logger.debug( + `loadInitialUrlWithCatch(): view.webContents: ${String(view.webContents)} ${hostReplacedUrl} for windowName ${windowName} for workspace ${ + workspace.name + }`, + ); await view.webContents.loadURL(hostReplacedUrl); const unregisterContextMenu = await this.menuService.initContextMenuForWindowWebContents(view.webContents); view.webContents.on('destroyed', () => { @@ -337,18 +347,14 @@ export class View implements IViewService { public async setActiveView(workspaceID: string, windowName: WindowNames): Promise { const browserWindow = this.windowService.get(windowName); + logger.debug(`setActiveView(): ${workspaceID} ${windowName} browserWindow: ${String(browserWindow !== undefined)}`); if (browserWindow === undefined) { return; } - // stop find in page when switching workspaces - const currentView = browserWindow.getBrowserView(); - if (currentView !== null) { - currentView.webContents.stopFindInPage('clearSelection'); - currentView.webContents.send(WindowChannel.closeFindInPage); - } const workspace = await this.workspaceService.get(workspaceID); const view = this.getView(workspaceID, windowName); - if (view === undefined) { + logger.debug(`setActiveView(): view: ${String(view !== undefined && view !== null)} workspace: ${String(workspace !== undefined)}`); + if (view === undefined || view === null) { if (workspace !== undefined) { return await this.addView(workspace, windowName); } else { @@ -377,6 +383,9 @@ export class View implements IViewService { const view = this.getView(workspaceID, windowName); void session.fromPartition(`persist:${workspaceID}`).clearStorageData(); if (view !== undefined) { + // stop find in page when switching workspaces + view.webContents.stopFindInPage('clearSelection'); + view.webContents.send(WindowChannel.closeFindInPage); // currently use workaround https://github.com/electron/electron/issues/10096 // @ts-expect-error Property 'destroy' does not exist on type 'WebContents'.ts(2339) // eslint-disable-next-line @typescript-eslint/no-unsafe-call @@ -386,7 +395,15 @@ export class View implements IViewService { delete this.views[workspaceID]![windowName]; }; - public removeAllViewOfWorkspace = (workspaceID: string): void => {}; + public removeAllViewOfWorkspace = (workspaceID: string): void => { + const views = this.views[workspaceID]; + if (views !== undefined) { + Object.keys(views).forEach((name) => { + this.removeView(workspaceID, name as WindowNames); + }); + } + this.views[workspaceID] = undefined; + }; public setViewsAudioPref = (_shouldMuteAudio?: boolean): void => { if (_shouldMuteAudio !== undefined) { @@ -461,6 +478,7 @@ export class View implements IViewService { const contentSize = browserWindow.getContentSize(); const didFailLoadErrorMessage = (await this.workspaceService.getMetaData(activeId)).didFailLoadErrorMessage; if (typeof didFailLoadErrorMessage === 'string' && didFailLoadErrorMessage.length > 0) { + logger.warn(`realignActiveView() hide because didFailLoadErrorMessage: ${didFailLoadErrorMessage}`); view?.setBounds(await getViewBounds(contentSize as [number, number], false, 0, 0)); // hide browserView to show error message } else { view?.setBounds(await getViewBounds(contentSize as [number, number])); @@ -468,6 +486,8 @@ export class View implements IViewService { } else if (isRetry !== true) { // retry one time later if webContent is not ready yet setTimeout(() => void this.realignActiveView(browserWindow, activeId, true), 1000); + } else { + logger.error(`realignActiveView() failed view?.webContents is ${String(view?.webContents)} and isRetry is ${String(isRetry)}`); } }; } diff --git a/src/services/workspaces/interface.ts b/src/services/workspaces/interface.ts index dbd95ee6..107d23dc 100644 --- a/src/services/workspaces/interface.ts +++ b/src/services/workspaces/interface.ts @@ -133,6 +133,10 @@ export interface IWorkspaceService { remove(id: string): Promise; removeWorkspacePicture(id: string): Promise; set(id: string, workspace: IWorkspace, immediate?: boolean): Promise; + /** + * Set new workspace to active, and make the old active workspace inactive + * @param id id to active + */ setActiveWorkspace(id: string): Promise; setWorkspacePicture(id: string, sourcePicturePath: string): Promise; setWorkspaces(newWorkspaces: Record): Promise; diff --git a/src/services/workspacesView/index.ts b/src/services/workspacesView/index.ts index 0c09f5f3..ea52cf23 100644 --- a/src/services/workspacesView/index.ts +++ b/src/services/workspacesView/index.ts @@ -56,6 +56,7 @@ export class WorkspaceView implements IWorkspaceViewService { if ((await this.wikiService.checkWikiExist(workspace, { shouldBeMainWiki: !workspace.isSubWiki, showDialog: true })) !== true) { return; } + logger.debug(`initializeWorkspaceView() Initializing workspace ${workspace.id}, ${JSON.stringify(options)}`); if ( followHibernateSettingWhenInit && ((await this.preferenceService.get('hibernateUnusedWorkspacesAtLaunch')) || workspace.hibernateWhenUnused) && @@ -262,6 +263,7 @@ export class WorkspaceView implements IWorkspaceViewService { public async hibernateWorkspaceView(workspaceID: string): Promise { const workspace = await this.workspaceService.get(workspaceID); + logger.debug(`Hibernating workspace ${workspaceID}, workspace.active: ${String(workspace?.active)}`); if (workspace !== undefined && !workspace.active) { await Promise.all([ this.wikiService.stopWiki(workspace.wikiFolderLocation), @@ -279,24 +281,33 @@ export class WorkspaceView implements IWorkspaceViewService { if (newWorkspace === undefined) { throw new Error(`Workspace id ${nextWorkspaceID} does not exist. When setActiveWorkspaceView().`); } - if (oldActiveWorkspace !== undefined) { - const asyncTasks: Array> = []; - // switch view first, so user fell app is responding to the click, this will failed loading for the first time, just let it retry loading view - asyncTasks.push(this.workspaceService.setActiveWorkspace(nextWorkspaceID), this.viewService.setActiveViewForAllBrowserViews(nextWorkspaceID)); - // if we are switching to a new workspace, we hibernate old view, and activate new view - if (oldActiveWorkspace.id !== nextWorkspaceID && oldActiveWorkspace.hibernateWhenUnused) { - asyncTasks.push(this.hibernateWorkspaceView(oldActiveWorkspace.id)); - } - if (newWorkspace.hibernated) { - asyncTasks.push( - this.initializeWorkspaceView(newWorkspace, { followHibernateSettingWhenInit: false, syncImmediately: false }), - this.workspaceService.update(nextWorkspaceID, { - hibernated: false, - }), - ); - } - await Promise.all(asyncTasks); + logger.debug(`Set active workspace oldActiveWorkspace.id: ${oldActiveWorkspace?.id ?? 'undefined'} nextWorkspaceID: ${nextWorkspaceID}`); + // later process will use the current active workspace + await this.workspaceService.setActiveWorkspace(nextWorkspaceID); + const asyncTasks: Array> = []; + // if we are switching to a new workspace, we hibernate old view, and activate new view + if ( + oldActiveWorkspace !== undefined && + oldActiveWorkspace.id !== nextWorkspaceID && + (oldActiveWorkspace.hibernateWhenUnused || (await this.preferenceService.get('hibernateUnusedWorkspacesAtLaunch'))) + ) { + asyncTasks.push(this.hibernateWorkspaceView(oldActiveWorkspace.id)); + } + if (newWorkspace.hibernated) { + asyncTasks.push( + this.initializeWorkspaceView(newWorkspace, { followHibernateSettingWhenInit: false, syncImmediately: false }), + this.workspaceService.update(nextWorkspaceID, { + hibernated: false, + }), + ); + } + await Promise.all(asyncTasks); + try { + await this.viewService.setActiveViewForAllBrowserViews(nextWorkspaceID); await this.realignActiveWorkspace(); + } catch (error) { + logger.error(`Error while setActiveWorkspaceView(): ${(error as Error).message}`, error); + throw error; } } @@ -384,16 +395,27 @@ export class WorkspaceView implements IWorkspaceViewService { // do not attempt to recall browserView.webContents.focus() // as it breaks page focus (cursor, scroll bar not visible) await this.realignActiveWorkspaceView(); - await this.menuService.buildMenu(); + try { + await this.menuService.buildMenu(); + } catch (error) { + logger.error(`Error buildMenu() while realignActiveWorkspace(): ${(error as Error).message}`, error); + throw error; + } } private async realignActiveWorkspaceView(): Promise { const activeWorkspace = await this.workspaceService.getActiveWorkspace(); + logger.debug(`realignActiveWorkspaceView() activeWorkspace.id: ${activeWorkspace?.id ?? 'undefined'}`); const mainWindow = this.windowService.get(WindowNames.main); const menuBarWindow = this.windowService.get(WindowNames.menuBar); if (activeWorkspace !== undefined) { + if (mainWindow === undefined && menuBarWindow === undefined) { + logger.warn('realignActiveWorkspaceView: no active window'); + } mainWindow !== undefined && void this.viewService.realignActiveView(mainWindow, activeWorkspace.id); menuBarWindow !== undefined && void this.viewService.realignActiveView(menuBarWindow, activeWorkspace.id); + } else { + logger.warn('realignActiveWorkspaceView: no active workspace'); } } }