Skip to content

Commit afbe275

Browse files
coroiutrmartin4
authored andcommitted
fix(badge): [PM-24661] Improve performance of badge state calculation for large number of tabs
* refactor: rewrite the badge service to only calculate states for active tab This also fixes an issue where the `difference` function didn't work and caused all tabs to update for every single state update. * fix: compilation issue * feat: add error logging * fix: linting * fix: badge clearing on reload on firefox * feat: optimize observable * feat(wip): update all active tabs tests are broken * fix: existing tests * feat: add new tests (cherry picked from commit 1a57ad3)
1 parent e7059b7 commit afbe275

File tree

5 files changed

+266
-61
lines changed

5 files changed

+266
-61
lines changed

apps/browser/src/platform/badge/badge-browser-api.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { map, Observable } from "rxjs";
1+
import { concat, defer, filter, map, merge, Observable, shareReplay, switchMap } from "rxjs";
22

33
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
44
import { Utils } from "@bitwarden/common/platform/misc/utils";
@@ -17,19 +17,48 @@ export interface RawBadgeState {
1717

1818
export interface BadgeBrowserApi {
1919
activeTab$: Observable<chrome.tabs.TabActiveInfo | undefined>;
20+
// activeTabs$: Observable<chrome.tabs.Tab[]>;
2021

2122
setState(state: RawBadgeState, tabId?: number): Promise<void>;
2223
getTabs(): Promise<number[]>;
24+
getActiveTabs(): Promise<chrome.tabs.Tab[]>;
2325
}
2426

2527
export class DefaultBadgeBrowserApi implements BadgeBrowserApi {
2628
private badgeAction = BrowserApi.getBrowserAction();
2729
private sidebarAction = BrowserApi.getSidebarAction(self);
2830

29-
activeTab$ = fromChromeEvent(chrome.tabs.onActivated).pipe(
30-
map(([tabActiveInfo]) => tabActiveInfo),
31+
private onTabActivated$ = fromChromeEvent(chrome.tabs.onActivated).pipe(
32+
switchMap(async ([activeInfo]) => activeInfo),
33+
shareReplay({ bufferSize: 1, refCount: true }),
3134
);
3235

36+
activeTab$ = concat(
37+
defer(async () => {
38+
const currentTab = await BrowserApi.getTabFromCurrentWindow();
39+
if (currentTab == null || currentTab.id === undefined) {
40+
return undefined;
41+
}
42+
43+
return { tabId: currentTab.id, windowId: currentTab.windowId };
44+
}),
45+
merge(
46+
this.onTabActivated$,
47+
fromChromeEvent(chrome.tabs.onUpdated).pipe(
48+
filter(
49+
([_, changeInfo]) =>
50+
// Only emit if the url was updated
51+
changeInfo.url != undefined,
52+
),
53+
map(([tabId, _changeInfo, tab]) => ({ tabId, windowId: tab.windowId })),
54+
),
55+
),
56+
).pipe(shareReplay({ bufferSize: 1, refCount: true }));
57+
58+
getActiveTabs(): Promise<chrome.tabs.Tab[]> {
59+
return BrowserApi.getActiveTabs();
60+
}
61+
3362
constructor(private platformUtilsService: PlatformUtilsService) {}
3463

3564
async setState(state: RawBadgeState, tabId?: number): Promise<void> {

apps/browser/src/platform/badge/badge.service.spec.ts

Lines changed: 130 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ describe("BadgeService", () => {
3737

3838
describe("given a single tab is open", () => {
3939
beforeEach(() => {
40-
badgeApi.tabs = [1];
41-
badgeApi.setActiveTab(tabId);
40+
badgeApi.tabs = [tabId];
41+
badgeApi.setActiveTabs([tabId]);
42+
badgeApi.setLastActivatedTab(tabId);
4243
badgeServiceSubscription = badgeService.startListening();
4344
});
4445

@@ -187,17 +188,18 @@ describe("BadgeService", () => {
187188
});
188189
});
189190

190-
describe("given multiple tabs are open", () => {
191+
describe("given multiple tabs are open, only one active", () => {
191192
const tabId = 1;
192193
const tabIds = [1, 2, 3];
193194

194195
beforeEach(() => {
195196
badgeApi.tabs = tabIds;
196-
badgeApi.setActiveTab(tabId);
197+
badgeApi.setActiveTabs([tabId]);
198+
badgeApi.setLastActivatedTab(tabId);
197199
badgeServiceSubscription = badgeService.startListening();
198200
});
199201

200-
it("sets state for each tab when no other state has been set", async () => {
202+
it("sets general state for active tab when no other state has been set", async () => {
201203
const state: BadgeState = {
202204
text: "text",
203205
backgroundColor: "color",
@@ -213,6 +215,67 @@ describe("BadgeService", () => {
213215
3: undefined,
214216
});
215217
});
218+
219+
it("only updates the active tab when setting state", async () => {
220+
const state: BadgeState = {
221+
text: "text",
222+
backgroundColor: "color",
223+
icon: BadgeIcon.Locked,
224+
};
225+
badgeApi.setState.mockReset();
226+
227+
await badgeService.setState("state-1", BadgeStatePriority.Default, state, tabId);
228+
await badgeService.setState("state-2", BadgeStatePriority.Default, state, 2);
229+
await badgeService.setState("state-2", BadgeStatePriority.Default, state, 2);
230+
231+
await new Promise((resolve) => setTimeout(resolve, 0));
232+
expect(badgeApi.setState).toHaveBeenCalledTimes(1);
233+
});
234+
});
235+
236+
describe("given multiple tabs are open and multiple are active", () => {
237+
const activeTabIds = [1, 2];
238+
const tabIds = [1, 2, 3];
239+
240+
beforeEach(() => {
241+
badgeApi.tabs = tabIds;
242+
badgeApi.setActiveTabs(activeTabIds);
243+
badgeApi.setLastActivatedTab(1);
244+
badgeServiceSubscription = badgeService.startListening();
245+
});
246+
247+
it("sets general state for active tabs when no other state has been set", async () => {
248+
const state: BadgeState = {
249+
text: "text",
250+
backgroundColor: "color",
251+
icon: BadgeIcon.Locked,
252+
};
253+
254+
await badgeService.setState("state-name", BadgeStatePriority.Default, state);
255+
256+
await new Promise((resolve) => setTimeout(resolve, 0));
257+
expect(badgeApi.specificStates).toEqual({
258+
1: state,
259+
2: state,
260+
3: undefined,
261+
});
262+
});
263+
264+
it("only updates the active tabs when setting general state", async () => {
265+
const state: BadgeState = {
266+
text: "text",
267+
backgroundColor: "color",
268+
icon: BadgeIcon.Locked,
269+
};
270+
badgeApi.setState.mockReset();
271+
272+
await badgeService.setState("state-1", BadgeStatePriority.Default, state, 1);
273+
await badgeService.setState("state-2", BadgeStatePriority.Default, state, 2);
274+
await badgeService.setState("state-3", BadgeStatePriority.Default, state, 3);
275+
276+
await new Promise((resolve) => setTimeout(resolve, 0));
277+
expect(badgeApi.setState).toHaveBeenCalledTimes(2);
278+
});
216279
});
217280
});
218281

@@ -222,7 +285,8 @@ describe("BadgeService", () => {
222285

223286
beforeEach(() => {
224287
badgeApi.tabs = [tabId];
225-
badgeApi.setActiveTab(tabId);
288+
badgeApi.setActiveTabs([tabId]);
289+
badgeApi.setLastActivatedTab(tabId);
226290
badgeServiceSubscription = badgeService.startListening();
227291
});
228292

@@ -491,13 +555,14 @@ describe("BadgeService", () => {
491555
});
492556
});
493557

494-
describe("given multiple tabs are open", () => {
558+
describe("given multiple tabs are open, only one active", () => {
495559
const tabId = 1;
496560
const tabIds = [1, 2, 3];
497561

498562
beforeEach(() => {
499563
badgeApi.tabs = tabIds;
500-
badgeApi.setActiveTab(tabId);
564+
badgeApi.setActiveTabs([tabId]);
565+
badgeApi.setLastActivatedTab(tabId);
501566
badgeServiceSubscription = badgeService.startListening();
502567
});
503568

@@ -528,5 +593,62 @@ describe("BadgeService", () => {
528593
});
529594
});
530595
});
596+
597+
describe("given multiple tabs are open and multiple are active", () => {
598+
const tabId = 1;
599+
const activeTabIds = [1, 2];
600+
const tabIds = [1, 2, 3];
601+
602+
beforeEach(() => {
603+
badgeApi.tabs = tabIds;
604+
badgeApi.setActiveTabs(activeTabIds);
605+
badgeApi.setLastActivatedTab(tabId);
606+
badgeServiceSubscription = badgeService.startListening();
607+
});
608+
609+
it("sets general state for all active tabs when no other state has been set", async () => {
610+
const generalState: BadgeState = {
611+
text: "general-text",
612+
backgroundColor: "general-color",
613+
icon: BadgeIcon.Unlocked,
614+
};
615+
616+
await badgeService.setState("general-state", BadgeStatePriority.Default, generalState);
617+
618+
await new Promise((resolve) => setTimeout(resolve, 0));
619+
expect(badgeApi.specificStates).toEqual({
620+
[tabIds[0]]: generalState,
621+
[tabIds[1]]: generalState,
622+
[tabIds[2]]: undefined,
623+
});
624+
});
625+
626+
it("sets tab-specific state for provided tab", async () => {
627+
const generalState: BadgeState = {
628+
text: "general-text",
629+
backgroundColor: "general-color",
630+
icon: BadgeIcon.Unlocked,
631+
};
632+
const specificState: BadgeState = {
633+
text: "tab-text",
634+
icon: BadgeIcon.Locked,
635+
};
636+
637+
await badgeService.setState("general-state", BadgeStatePriority.Default, generalState);
638+
await badgeService.setState(
639+
"tab-state",
640+
BadgeStatePriority.Default,
641+
specificState,
642+
tabIds[0],
643+
);
644+
645+
await new Promise((resolve) => setTimeout(resolve, 0));
646+
expect(badgeApi.specificStates).toEqual({
647+
[tabIds[0]]: { ...specificState, backgroundColor: "general-color" },
648+
[tabIds[1]]: generalState,
649+
[tabIds[2]]: undefined,
650+
});
651+
});
652+
});
531653
});
532654
});

0 commit comments

Comments
 (0)