Skip to content

Commit 16b02bc

Browse files
GreyCJeff Chenjackwener
authored
fix(extension): reuse existing adapter tab group (#1541)
* fix(extension): reuse existing adapter tab group * fix(extension): choose best existing adapter group --------- Co-authored-by: Jeff Chen <jeff@adtiming.com> Co-authored-by: jackwener <jakevingoo@gmail.com>
1 parent 5af2ff1 commit 16b02bc

3 files changed

Lines changed: 343 additions & 22 deletions

File tree

extension/dist/background.js

Lines changed: 77 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,7 @@ const CONTAINER_TAB_GROUP_TITLE = {
787787
interactive: "OpenCLI Browser",
788788
automation: "OpenCLI Adapter"
789789
};
790+
const LEGACY_AUTOMATION_TAB_GROUP_TITLE = "OpenCLI";
790791
const AUTOMATION_TAB_GROUP_COLOR = "orange";
791792
let leaseMutationQueue = Promise.resolve();
792793
const ownedContainers = {
@@ -1022,11 +1023,70 @@ async function getOwnedContainerGroupId(role, windowId) {
10221023
}
10231024
container.groupId = null;
10241025
}
1025-
const groups = await chrome.tabGroups.query({ windowId, title: CONTAINER_TAB_GROUP_TITLE[role] });
1026-
const existing = groups[0];
1027-
if (!existing) return null;
1028-
container.groupId = existing.id;
1029-
return existing.id;
1026+
for (const title of getOwnedContainerGroupTitles(role)) {
1027+
const groups = await chrome.tabGroups.query({ windowId, title });
1028+
const existing = groups[0];
1029+
if (existing) {
1030+
container.groupId = existing.id;
1031+
return existing.id;
1032+
}
1033+
}
1034+
return null;
1035+
}
1036+
function getOwnedContainerGroupTitles(role) {
1037+
return role === "automation" ? [CONTAINER_TAB_GROUP_TITLE.automation, LEGACY_AUTOMATION_TAB_GROUP_TITLE] : [CONTAINER_TAB_GROUP_TITLE.interactive];
1038+
}
1039+
async function focusOwnedWindowIfRequested(windowId, mode) {
1040+
if (mode !== "foreground") return;
1041+
const updateWindow = chrome.windows.update;
1042+
if (typeof updateWindow === "function") await updateWindow(windowId, { focused: true }).catch(() => {
1043+
});
1044+
}
1045+
async function toOwnedContainerDiscoveryCandidate(group) {
1046+
try {
1047+
const chromeWindow = await chrome.windows.get(group.windowId);
1048+
const reusableTabId = await findReusableOwnedContainerTab(group.windowId);
1049+
return {
1050+
windowId: group.windowId,
1051+
groupId: group.id,
1052+
focused: !!chromeWindow.focused,
1053+
hasReusableTab: reusableTabId !== void 0
1054+
};
1055+
} catch {
1056+
return null;
1057+
}
1058+
}
1059+
function selectOwnedContainerDiscoveryCandidate(candidates) {
1060+
if (candidates.length === 0) return null;
1061+
return [...candidates].sort((a, b) => {
1062+
if (a.focused !== b.focused) return a.focused ? -1 : 1;
1063+
if (a.hasReusableTab !== b.hasReusableTab) return a.hasReusableTab ? -1 : 1;
1064+
return a.groupId - b.groupId;
1065+
})[0];
1066+
}
1067+
async function discoverOwnedContainerFromTabGroup(role) {
1068+
const container = ownedContainers[role];
1069+
if (container.groupId !== null) {
1070+
try {
1071+
const group = await chrome.tabGroups.get(container.groupId);
1072+
await chrome.windows.get(group.windowId);
1073+
container.windowId = group.windowId;
1074+
return { windowId: group.windowId, groupId: group.id };
1075+
} catch {
1076+
container.windowId = null;
1077+
container.groupId = null;
1078+
}
1079+
}
1080+
for (const title of getOwnedContainerGroupTitles(role)) {
1081+
const groups = await chrome.tabGroups.query({ title });
1082+
const candidates = (await Promise.all(groups.map(toOwnedContainerDiscoveryCandidate))).filter((candidate) => candidate !== null);
1083+
const selected = selectOwnedContainerDiscoveryCandidate(candidates);
1084+
if (!selected) continue;
1085+
container.windowId = selected.windowId;
1086+
container.groupId = selected.groupId;
1087+
return { windowId: selected.windowId, groupId: selected.groupId };
1088+
}
1089+
return null;
10301090
}
10311091
async function ensureOwnedContainerTabGroup(role, windowId, tabIds) {
10321092
const ids = [...new Set(tabIds.filter((id) => id !== void 0))];
@@ -1066,11 +1126,7 @@ async function ensureOwnedContainerWindowUnlocked(role, initialUrl, mode = "back
10661126
if (container.windowId !== null) {
10671127
try {
10681128
await chrome.windows.get(container.windowId);
1069-
if (mode === "foreground") {
1070-
const updateWindow = chrome.windows.update;
1071-
if (typeof updateWindow === "function") await updateWindow(container.windowId, { focused: true }).catch(() => {
1072-
});
1073-
}
1129+
await focusOwnedWindowIfRequested(container.windowId, mode);
10741130
const initialTabId2 = await findReusableOwnedContainerTab(container.windowId);
10751131
await ensureOwnedContainerTabGroup(role, container.windowId, [initialTabId2]);
10761132
return {
@@ -1082,6 +1138,17 @@ async function ensureOwnedContainerWindowUnlocked(role, initialUrl, mode = "back
10821138
container.groupId = null;
10831139
}
10841140
}
1141+
const discovered = await discoverOwnedContainerFromTabGroup(role);
1142+
if (discovered) {
1143+
await focusOwnedWindowIfRequested(discovered.windowId, mode);
1144+
const initialTabId2 = await findReusableOwnedContainerTab(discovered.windowId);
1145+
await ensureOwnedContainerTabGroup(role, discovered.windowId, [initialTabId2]);
1146+
await persistRuntimeState();
1147+
return {
1148+
windowId: discovered.windowId,
1149+
initialTabId: initialTabId2
1150+
};
1151+
}
10851152
const startUrl = initialUrl && isSafeNavigationUrl(initialUrl) ? initialUrl : BLANK_PAGE;
10861153
const win = await chrome.windows.create({
10871154
url: startUrl,

extension/src/background.test.ts

Lines changed: 171 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ function createChromeMock() {
176176
onEvent: { addListener: vi.fn() } as Listener<(source: any, method: string, params: any) => void>,
177177
},
178178
windows: {
179-
get: vi.fn(async (windowId: number) => ({ id: windowId })),
179+
get: vi.fn(async (windowId: number) => ({ id: windowId, focused: windowId === lastFocusedWindowId })),
180180
create: vi.fn(async ({ url, focused, width, height, type }: any) => ({ id: 1, url, focused, width, height, type })),
181181
remove: vi.fn(async (_windowId: number) => {}),
182182
onRemoved: { addListener: vi.fn() } as Listener<(windowId: number) => void>,
@@ -205,7 +205,15 @@ function createChromeMock() {
205205
},
206206
};
207207

208-
return { chrome, tabs, groups, query, create, update };
208+
return {
209+
chrome,
210+
tabs,
211+
groups,
212+
query,
213+
create,
214+
update,
215+
setLastFocusedWindowId: (windowId: number) => { lastFocusedWindowId = windowId; },
216+
};
209217
}
210218

211219
describe('background tab isolation', () => {
@@ -1021,7 +1029,7 @@ describe('background tab isolation', () => {
10211029
const { chrome } = createChromeMock();
10221030
chrome.windows.get = vi.fn(async (windowId: number) => {
10231031
if (windowId === 90 || windowId === 91) throw new Error(`stale window ${windowId}`);
1024-
return { id: windowId };
1032+
return { id: windowId, focused: false };
10251033
});
10261034
vi.stubGlobal('chrome', chrome);
10271035

@@ -1163,6 +1171,166 @@ describe('background tab isolation', () => {
11631171
expect(chrome.tabGroups.update).not.toHaveBeenCalled();
11641172
});
11651173

1174+
it('discovers and reuses an existing OpenCLI Adapter group in another window before creating one', async () => {
1175+
const { chrome, tabs, groups } = createChromeMock();
1176+
tabs.push({
1177+
id: 77,
1178+
windowId: 7,
1179+
url: 'about:blank',
1180+
title: 'blank',
1181+
active: true,
1182+
status: 'complete',
1183+
groupId: -1,
1184+
});
1185+
groups.push({
1186+
id: 99,
1187+
windowId: 7,
1188+
title: 'OpenCLI Adapter',
1189+
color: 'orange',
1190+
collapsed: true,
1191+
});
1192+
vi.stubGlobal('chrome', chrome);
1193+
1194+
const mod = await import('./background');
1195+
const tabId = await mod.__test__.resolveTabId(undefined, adapterKey('twitter'));
1196+
1197+
expect(tabId).toBe(77);
1198+
expect(chrome.windows.create).not.toHaveBeenCalled();
1199+
expect(mod.__test__.getAutomationWindowId(adapterKey('twitter'))).toBe(7);
1200+
expect(tabs.find((tab) => tab.id === 77)?.groupId).toBe(99);
1201+
expect(chrome.tabs.group).toHaveBeenCalledWith({ groupId: 99, tabIds: [77] });
1202+
expect(chrome.tabGroups.update).not.toHaveBeenCalled();
1203+
});
1204+
1205+
it('prefers a focused OpenCLI Adapter group when multiple matching groups exist', async () => {
1206+
const { chrome, tabs, groups, setLastFocusedWindowId } = createChromeMock();
1207+
setLastFocusedWindowId(8);
1208+
tabs.push({
1209+
id: 77,
1210+
windowId: 7,
1211+
url: 'about:blank',
1212+
title: 'blank',
1213+
active: true,
1214+
status: 'complete',
1215+
groupId: -1,
1216+
});
1217+
tabs.push({
1218+
id: 78,
1219+
windowId: 8,
1220+
url: 'about:blank',
1221+
title: 'blank',
1222+
active: true,
1223+
status: 'complete',
1224+
groupId: -1,
1225+
});
1226+
groups.push(
1227+
{
1228+
id: 99,
1229+
windowId: 7,
1230+
title: 'OpenCLI Adapter',
1231+
color: 'orange',
1232+
collapsed: true,
1233+
},
1234+
{
1235+
id: 98,
1236+
windowId: 8,
1237+
title: 'OpenCLI Adapter',
1238+
color: 'orange',
1239+
collapsed: true,
1240+
},
1241+
);
1242+
vi.stubGlobal('chrome', chrome);
1243+
1244+
const mod = await import('./background');
1245+
const tabId = await mod.__test__.resolveTabId(undefined, adapterKey('twitter'));
1246+
1247+
expect(tabId).toBe(78);
1248+
expect(chrome.windows.create).not.toHaveBeenCalled();
1249+
expect(mod.__test__.getAutomationWindowId(adapterKey('twitter'))).toBe(8);
1250+
expect(tabs.find((tab) => tab.id === 78)?.groupId).toBe(98);
1251+
expect(chrome.tabs.group).toHaveBeenCalledWith({ groupId: 98, tabIds: [78] });
1252+
});
1253+
1254+
it('prefers an OpenCLI Adapter group with a reusable debuggable tab when none are focused', async () => {
1255+
const { chrome, tabs, groups, setLastFocusedWindowId } = createChromeMock();
1256+
setLastFocusedWindowId(2);
1257+
tabs.push({
1258+
id: 77,
1259+
windowId: 7,
1260+
url: 'chrome://settings',
1261+
title: 'settings',
1262+
active: true,
1263+
status: 'complete',
1264+
groupId: -1,
1265+
});
1266+
tabs.push({
1267+
id: 78,
1268+
windowId: 8,
1269+
url: 'about:blank',
1270+
title: 'blank',
1271+
active: true,
1272+
status: 'complete',
1273+
groupId: -1,
1274+
});
1275+
groups.push(
1276+
{
1277+
id: 97,
1278+
windowId: 7,
1279+
title: 'OpenCLI Adapter',
1280+
color: 'orange',
1281+
collapsed: true,
1282+
},
1283+
{
1284+
id: 98,
1285+
windowId: 8,
1286+
title: 'OpenCLI Adapter',
1287+
color: 'orange',
1288+
collapsed: true,
1289+
},
1290+
);
1291+
vi.stubGlobal('chrome', chrome);
1292+
1293+
const mod = await import('./background');
1294+
const tabId = await mod.__test__.resolveTabId(undefined, adapterKey('twitter'));
1295+
1296+
expect(tabId).toBe(78);
1297+
expect(chrome.windows.create).not.toHaveBeenCalled();
1298+
expect(mod.__test__.getAutomationWindowId(adapterKey('twitter'))).toBe(8);
1299+
expect(tabs.find((tab) => tab.id === 78)?.groupId).toBe(98);
1300+
expect(chrome.tabs.group).toHaveBeenCalledWith({ groupId: 98, tabIds: [78] });
1301+
});
1302+
1303+
it('discovers and reuses a legacy OpenCLI automation group before creating a duplicate', async () => {
1304+
const { chrome, tabs, groups } = createChromeMock();
1305+
tabs.push({
1306+
id: 78,
1307+
windowId: 8,
1308+
url: 'about:blank',
1309+
title: 'blank',
1310+
active: true,
1311+
status: 'complete',
1312+
groupId: -1,
1313+
});
1314+
groups.push({
1315+
id: 98,
1316+
windowId: 8,
1317+
title: 'OpenCLI',
1318+
color: 'orange',
1319+
collapsed: true,
1320+
});
1321+
vi.stubGlobal('chrome', chrome);
1322+
1323+
const mod = await import('./background');
1324+
const tabId = await mod.__test__.resolveTabId(undefined, adapterKey('twitter'));
1325+
1326+
expect(tabId).toBe(78);
1327+
expect(chrome.windows.create).not.toHaveBeenCalled();
1328+
expect(mod.__test__.getAutomationWindowId(adapterKey('twitter'))).toBe(8);
1329+
expect(tabs.find((tab) => tab.id === 78)?.groupId).toBe(98);
1330+
expect(chrome.tabs.group).toHaveBeenCalledWith({ groupId: 98, tabIds: [78] });
1331+
expect(chrome.tabGroups.update).not.toHaveBeenCalled();
1332+
});
1333+
11661334
it('reuses a persisted automation group id after service worker restart even if the user renamed it', async () => {
11671335
const { chrome, tabs, groups } = createChromeMock();
11681336
groups.push({

0 commit comments

Comments
 (0)