Save group on window.onRemoved

Only works if browser has not exited. Not working if rootId === undefined

src/background/background.ts

@@ -7,8 +7,8 @@ export class BackgroundRemote {
 	listGroups(windowId: number) { return groupManager.listGroups(windowId) }
 	createGroup(name: string) { return groupManager.createGroup(name) }
 
-	switchGroup(windowId: number, groupId?: string, newGroupName?: string) {
-		const promise = groupManager.switchGroup(windowId, groupId, newGroupName)
+	switchGroup(windowId: number, groupId?: string, unsavedGroupName?: string) {
+		const promise = groupManager.switchGroup(windowId, groupId, unsavedGroupName)
 		promise.catch(console.error)
 		return promise
 	}

src/background/group-manager.ts

@@ -3,6 +3,7 @@ import { CriticalSection } from "../util/promise.js";
 import { GroupState } from "../common/types.js";
 import { getWindowTabsToSave } from "../common/common.js";
 import { S } from "./settings.js";
+import { PartialTab, WindowManager } from "./window-manager.js";
 
 const KEY_GROUP = 'group'
 
@@ -27,7 +28,7 @@ export class GroupManager {
 			{ key: 'pinned', prefix: '\uD83D\uDCCC\uFE0E' },
 		] as const
 
-		toBookmark(tab: browser.tabs.Tab): browser.bookmarks.CreateDetails {
+		toBookmark(tab: PartialTab): browser.bookmarks.CreateDetails {
 			let url = tab.url!
 			if (url.startsWith(this.restrictedURL)) {
 				try {
@@ -71,6 +72,7 @@ export class GroupManager {
 	private readonly criticalSection = new CriticalSection()
 	private rootId?: string
 	private readonly windowGroupMap = new Map<number, string | undefined>()
+	private readonly windowManager = new WindowManager()
 
 	protected readonly initialization = this.criticalSection.sync(async () => {
 		const windows = await browser.windows.getAll()
@@ -79,8 +81,17 @@ export class GroupManager {
 				group => { this.windowGroupMap.set(id!, group as string) })
 		}
 		browser.windows.onCreated.addListener(onCreated)
-		browser.windows.onRemoved.addListener(id => this.windowGroupMap.delete(id))
-		for (const w of windows) onCreated(w)
+		this.windowManager.onWindowRemoved.listen((windowId, tabs) => {
+			const groupId = this.windowGroupMap.get(windowId)
+			if (groupId === undefined) return
+			this.windowGroupMap.delete(windowId)
+			this.criticalSection.sync(async () => {
+				if (!await this.isValidGroupId(groupId)) return
+				await this.saveGroupImpl(groupId, tabs)
+			})
+		})
+
+		for (const w of windows) void onCreated(w)
 	})
 
 	private async loadSubtree() {
@@ -133,61 +144,60 @@ export class GroupManager {
 	}
 
 	private async createGroupImpl(name: string) {
-		if (this.rootId === undefined) await this.loadSubtree()
+		await this.loadSubtree() // ensure rootId
 		return browser.bookmarks.create({
 			parentId: this.rootId,
 			title: name,
 			index: Number.MAX_SAFE_INTEGER,
 		})
 	}
+
 	createGroup(name: string) {
 		return this.criticalSection.sync(() => this.createGroupImpl(name))
 	}
 
-	switchGroup(windowId: number, groupId?: string, newGroupName?: string) {
+	private async saveGroupImpl(groupId: string, tabs: PartialTab[]) {
+		const bookmarks = await browser.bookmarks.getChildren(groupId)
+		// TODO recovery
+		const transaction = (await browser.bookmarks.create({
+			parentId: groupId,
+			title: 'Transaction', url: GroupManager.transactionURL,
+			index: Number.MAX_SAFE_INTEGER,
+		}))!
+		for (const tab of tabs!)
+			await browser.bookmarks.create({
+				parentId: groupId,
+				index: Number.MAX_SAFE_INTEGER,
+				...GroupManager.converter.toBookmark(tab)
+			})
+		await browser.bookmarks.update(transaction.id, {
+			title: 'Transaction (committed)',
+			url: GroupManager.commitedTransactionURL,
+		})
+		for (const { id } of bookmarks)
+			await browser.bookmarks.remove(id)
+		await browser.bookmarks.remove(transaction.id)
+	}
+
+	switchGroup(windowId: number, groupId?: string, unsavedGroupName?: string) {
 		return this.criticalSection.sync(async () => {
 			let oldGroupId = this.windowGroupMap.get(windowId)
-			try {
-				if (oldGroupId && (await browser.bookmarks.get(oldGroupId))[0]
-					.type !== 'folder')
-					oldGroupId = undefined
-			} catch { oldGroupId = undefined }
+			if (!await this.isValidGroupId(oldGroupId)) oldGroupId = undefined
 
 			// do not create new group with single blank tab, if not saving directly
 			const oldTabs = await getWindowTabsToSave(windowId,
 				oldGroupId === undefined && groupId !== undefined)
 
-			if (oldGroupId === undefined && oldTabs.length) {
-				if (newGroupName === undefined) newGroupName = M.unnamed
-				oldGroupId = (await this.createGroupImpl(newGroupName))!.id
-			}
+			if (oldGroupId === undefined && oldTabs.length
+				&& unsavedGroupName !== undefined)
+				oldGroupId = (await this.createGroupImpl(unsavedGroupName))!.id
 
 			if (oldGroupId !== undefined) { // save old group
-				const bookmarks = await browser.bookmarks.getChildren(oldGroupId)
-
-				// TODO recovery
-				const transaction = (await browser.bookmarks.create({
-					parentId: oldGroupId,
-					title: 'Transaction', url: GroupManager.transactionURL,
-					index: Number.MAX_SAFE_INTEGER,
-				}))!
-				for (const tab of oldTabs!)
-					await browser.bookmarks.create({
-						parentId: oldGroupId,
-						index: Number.MAX_SAFE_INTEGER,
-						...GroupManager.converter.toBookmark(tab)
-					})
-				await browser.bookmarks.update(transaction.id, {
-					title: 'Transaction (committed)',
-					url: GroupManager.commitedTransactionURL,
-				})
-				for (const { id } of bookmarks)
-					await browser.bookmarks.remove(id)
-				await browser.bookmarks.remove(transaction.id)
+				await this.saveGroupImpl(oldGroupId, oldTabs)
 			}
 
 			if (groupId === undefined) {
-				if (oldGroupId === undefined) return // unreachable
+				if (oldGroupId === undefined) return // unsaved window is closed
 				groupId = oldGroupId
 			}
 
@@ -235,4 +245,13 @@ export class GroupManager {
 			await browser.sessions.setWindowValue(windowId, KEY_GROUP, groupId)
 		})
 	}
+
+	private async isValidGroupId(id?: string) {
+		if (id === undefined) return false
+		try {
+			const bookmark = (await browser.bookmarks.get([id]))[0]
+			return bookmark && bookmark.type === 'folder'
+				&& bookmark.parentId === this.rootId
+		} catch { return false }
+	}
 }

src/background/window-manager.ts

@@ -0,0 +1,87 @@
+import { mapInsert } from "../util/util.js"
+import { SimpleEventListener } from "../util/event.js";
+
+export interface PartialTab extends Pick<browser.tabs.Tab,
+	'id' | 'title' | 'url' | 'active' | 'pinned'> { }
+
+function arrayRemoveOne<T>(arr: T[], item: T) {
+	const i = arr.indexOf(item)
+	if (i > -1) { arr.splice(i, 1); return true }
+	return false
+}
+
+export class WindowManager {
+	private readonly tabs = new Map<number, PartialTab>()
+	private readonly windowTabIdMap = new Map<number, PartialTab[]>()
+
+	private window(id: number) {
+		return mapInsert(this.windowTabIdMap, id, () => [])
+	}
+
+	constructor() {
+		browser.tabs.onCreated.addListener(tab => this.createTab(tab))
+		browser.tabs.query({}).then(tabs => {
+			for (const tab of tabs) { this.createTab(tab) }
+		})
+
+		browser.tabs.onActivated.addListener(({ tabId, previousTabId }) => {
+			const tab = this.tabs.get(tabId)
+			if (tab) tab.active = true
+			if (previousTabId !== undefined) {
+				const previousTab = this.tabs.get(previousTabId)
+				if (previousTab) previousTab.active = false
+			}
+		})
+
+		browser.tabs.onMoved.addListener((tabId, { windowId, toIndex }) => {
+			const tabs = this.window(windowId)
+			const tab = this.tabs.get(tabId)
+			if (!tab || !arrayRemoveOne(tabs, tab)) return
+			tabs.splice(toIndex, 0, tab)
+		})
+
+		browser.tabs.onAttached.addListener((tabId, { newWindowId, newPosition }) => {
+			const tab = this.tabs.get(tabId)
+			if (!tab) return
+			this.window(newWindowId).splice(newPosition, 0, tab)
+		})
+
+		browser.tabs.onDetached.addListener((tabId, { oldWindowId }) => {
+			this.detachTab(tabId, oldWindowId)
+		})
+
+		browser.tabs.onRemoved.addListener((tabId, { windowId, isWindowClosing }) => {
+			if (!isWindowClosing) this.detachTab(tabId, windowId)
+			this.tabs.delete(tabId)
+		})
+
+		browser.windows.onRemoved.addListener(windowId => {
+			const tabs = this.windowTabIdMap.get(windowId)
+			if (!tabs) return
+			this.windowTabIdMap.delete(windowId)
+			this.onWindowRemoved.dispatch(windowId, tabs)
+		})
+
+		browser.tabs.onUpdated.addListener((tabId, changeInfo) => {
+			const tab = this.tabs.get(tabId)
+			if (!tab) return
+			for (const key of ['title', 'url', 'pinned'] as const)
+				if (changeInfo[key] !== undefined)
+					(tab[key] as any) = changeInfo[key]
+		})
+	}
+
+	private createTab(tab: browser.tabs.Tab): void {
+		if (this.tabs.has(tab.id!)) return // may be called twice on startup
+		this.tabs.set(tab.id!, tab)
+		this.window(tab.windowId!).splice(tab.index, 0, tab)
+	}
+
+	private detachTab(tabId: number, windowId: number) {
+		const tab = this.tabs.get(tabId)
+		if (!tab) return
+		arrayRemoveOne(this.window(windowId), tab)
+	}
+
+	readonly onWindowRemoved = new SimpleEventListener<[number, PartialTab[]]>()
+}

src/panel/panel.ts

@@ -33,14 +33,14 @@ class XGroupElement extends HTMLElement {
 				return
 			}
 
-			let newGroupName: string | null | undefined = undefined
+			let unsavedGroupName: string | null = M.unnamed
 			if (this.state === 'unsaved' ||
 				XGroupElement.parent.querySelector('.group[state="unsaved"]') &&
 				(await getWindowTabsToSave(windowId, true)).length) {
-				newGroupName = prompt(M.saveCurrentWindowAs, M.unnamed)
-				if (newGroupName == null) return
+				unsavedGroupName = prompt(M.saveCurrentWindowAs, M.unnamed)
+				if (unsavedGroupName == null) return
 			}
-			backgroundRemote.switchGroup(windowId, this.groupId, newGroupName)
+			backgroundRemote.switchGroup(windowId, this.groupId, unsavedGroupName)
 			location.reload()
 		})