bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00
Code Issues Releases Wiki Activity

[PM-24451] firefox extension crash due to huge memory usage when unlocking vault (#15938)

Browse Source 
* feat: only set badge state for the active tab

* fix: tests

* feat: avoid calculating states unecessarily

* Revert BrowserApi.removeListener change

* Add fatal log on observable failure

* Use fromChromeEvent

* Remove required-using tests

* Only disable some

* One of each

---------

Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com>
This commit is contained in:
Andreas Coroiu
2025-08-07 19:19:14 +02:00
committed by GitHub
parent f7169e909f
commit c3f6892f9e
6 changed files with 115 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
apps/browser/src/background/main.background.ts
View File

@@ -1372,6 +1372,7 @@ export default class MainBackground {
this.badgeService = new BadgeService(
this.stateProvider,
new DefaultBadgeBrowserApi(this.platformUtilsService),
this.logService,
);
this.authStatusBadgeUpdaterService = new AuthStatusBadgeUpdaterService(
this.badgeService,

9
apps/browser/src/platform/badge/badge-browser-api.ts
View File

@@ -1,7 +1,10 @@
import { map, Observable } from "rxjs";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { BrowserApi } from "../browser/browser-api";
import { fromChromeEvent } from "../browser/from-chrome-event";
import { BadgeIcon, IconPaths } from "./icon";
@@ -13,6 +16,8 @@ export interface RawBadgeState {
}
export interface BadgeBrowserApi {
activeTab$: Observable<chrome.tabs.TabActiveInfo | undefined>;
setState(state: RawBadgeState, tabId?: number): Promise<void>;
getTabs(): Promise<number[]>;
}
@@ -21,6 +26,10 @@ export class DefaultBadgeBrowserApi implements BadgeBrowserApi {
private badgeAction = BrowserApi.getBrowserAction();
private sidebarAction = BrowserApi.getSidebarAction(self);
activeTab$ = fromChromeEvent(chrome.tabs.onActivated).pipe(
map(([tabActiveInfo]) => tabActiveInfo),
);
constructor(private platformUtilsService: PlatformUtilsService) {}
async setState(state: RawBadgeState, tabId?: number): Promise<void> {

62
apps/browser/src/platform/badge/badge.service.spec.ts
View File

@@ -1,5 +1,7 @@
import { mock, MockProxy } from "jest-mock-extended";
import { Subscription } from "rxjs";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { FakeAccountService, FakeStateProvider } from "@bitwarden/common/spec";
import { RawBadgeState } from "./badge-browser-api";
@@ -13,6 +15,7 @@ import { MockBadgeBrowserApi } from "./test/mock-badge-browser-api";
describe("BadgeService", () => {
let badgeApi: MockBadgeBrowserApi;
let stateProvider: FakeStateProvider;
let logService!: MockProxy<LogService>;
let badgeService!: BadgeService;
let badgeServiceSubscription: Subscription;
@@ -20,8 +23,9 @@ describe("BadgeService", () => {
beforeEach(() => {
badgeApi = new MockBadgeBrowserApi();
stateProvider = new FakeStateProvider(new FakeAccountService({}));
logService = mock<LogService>();
badgeService = new BadgeService(stateProvider, badgeApi);
badgeService = new BadgeService(stateProvider, badgeApi, logService);
});
afterEach(() => {
@@ -34,14 +38,10 @@ describe("BadgeService", () => {
describe("given a single tab is open", () => {
beforeEach(() => {
badgeApi.tabs = [1];
badgeApi.setActiveTab(tabId);
badgeServiceSubscription = badgeService.startListening();
});
// This relies on the state provider to auto-emit
it("sets default values on startup", async () => {
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
});
it("sets provided state when no other state has been set", async () => {
const state: BadgeState = {
text: "text",
@@ -52,7 +52,6 @@ describe("BadgeService", () => {
await badgeService.setState("state-name", BadgeStatePriority.Default, state);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(state);
expect(badgeApi.specificStates[tabId]).toEqual(state);
});
@@ -63,7 +62,6 @@ describe("BadgeService", () => {
await badgeService.setState("state-name", BadgeStatePriority.Default, state);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual(DefaultBadgeState);
});
@@ -82,7 +80,6 @@ describe("BadgeService", () => {
backgroundColor: "#fff",
icon: BadgeIcon.Locked,
};
expect(badgeApi.generalState).toEqual(expectedState);
expect(badgeApi.specificStates[tabId]).toEqual(expectedState);
});
@@ -105,7 +102,6 @@ describe("BadgeService", () => {
backgroundColor: "#aaa",
icon: BadgeIcon.Locked,
};
expect(badgeApi.generalState).toEqual(expectedState);
expect(badgeApi.specificStates[tabId]).toEqual(expectedState);
});
@@ -126,7 +122,6 @@ describe("BadgeService", () => {
backgroundColor: "#fff",
icon: BadgeIcon.Locked,
};
expect(badgeApi.generalState).toEqual(expectedState);
expect(badgeApi.specificStates[tabId]).toEqual(expectedState);
});
@@ -147,7 +142,6 @@ describe("BadgeService", () => {
await badgeService.clearState("state-3");
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual(DefaultBadgeState);
});
@@ -167,7 +161,6 @@ describe("BadgeService", () => {
backgroundColor: "#fff",
icon: DefaultBadgeState.icon,
};
expect(badgeApi.generalState).toEqual(expectedState);
expect(badgeApi.specificStates[tabId]).toEqual(expectedState);
});
@@ -190,26 +183,20 @@ describe("BadgeService", () => {
backgroundColor: "#fff",
icon: BadgeIcon.Unlocked,
};
expect(badgeApi.generalState).toEqual(expectedState);
expect(badgeApi.specificStates[tabId]).toEqual(expectedState);
});
});
describe("given multiple tabs are open", () => {
const tabId = 1;
const tabIds = [1, 2, 3];
beforeEach(() => {
badgeApi.tabs = tabIds;
badgeApi.setActiveTab(tabId);
badgeServiceSubscription = badgeService.startListening();
});
it("sets default values for each tab on startup", async () => {
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
for (const tabId of tabIds) {
expect(badgeApi.specificStates[tabId]).toEqual(DefaultBadgeState);
}
});
it("sets state for each tab when no other state has been set", async () => {
const state: BadgeState = {
text: "text",
@@ -220,11 +207,10 @@ describe("BadgeService", () => {
await badgeService.setState("state-name", BadgeStatePriority.Default, state);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(state);
expect(badgeApi.specificStates).toEqual({
1: state,
2: state,
3: state,
2: undefined,
3: undefined,
});
});
});
@@ -236,6 +222,7 @@ describe("BadgeService", () => {
beforeEach(() => {
badgeApi.tabs = [tabId];
badgeApi.setActiveTab(tabId);
badgeServiceSubscription = badgeService.startListening();
});
@@ -249,7 +236,6 @@ describe("BadgeService", () => {
await badgeService.setState("state-name", BadgeStatePriority.Default, state, tabId);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual(state);
});
@@ -260,7 +246,6 @@ describe("BadgeService", () => {
await badgeService.setState("state-name", BadgeStatePriority.Default, state, tabId);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual(DefaultBadgeState);
});
@@ -279,11 +264,6 @@ describe("BadgeService", () => {
});
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual({
...DefaultBadgeState,
text: "text",
icon: BadgeIcon.Locked,
});
expect(badgeApi.specificStates[tabId]).toEqual({
text: "text",
backgroundColor: "#fff",
@@ -316,7 +296,6 @@ describe("BadgeService", () => {
backgroundColor: "#fff",
icon: BadgeIcon.Locked,
};
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual(expectedState);
});
@@ -354,7 +333,6 @@ describe("BadgeService", () => {
backgroundColor: "#aaa",
icon: BadgeIcon.Locked,
};
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual(expectedState);
});
@@ -377,11 +355,6 @@ describe("BadgeService", () => {
});
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual({
text: "override",
backgroundColor: "#aaa",
icon: DefaultBadgeState.icon,
});
expect(badgeApi.specificStates[tabId]).toEqual({
text: "override",
backgroundColor: "#aaa",
@@ -411,7 +384,6 @@ describe("BadgeService", () => {
await badgeService.clearState("state-2");
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual({
text: "text",
backgroundColor: "#fff",
@@ -451,7 +423,6 @@ describe("BadgeService", () => {
await badgeService.clearState("state-3");
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual(DefaultBadgeState);
});
@@ -476,7 +447,6 @@ describe("BadgeService", () => {
);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual({
text: "text",
backgroundColor: "#fff",
@@ -513,7 +483,6 @@ describe("BadgeService", () => {
);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(DefaultBadgeState);
expect(badgeApi.specificStates[tabId]).toEqual({
text: "text",
backgroundColor: "#fff",
@@ -523,14 +492,16 @@ describe("BadgeService", () => {
});
describe("given multiple tabs are open", () => {
const tabId = 1;
const tabIds = [1, 2, 3];
beforeEach(() => {
badgeApi.tabs = tabIds;
badgeApi.setActiveTab(tabId);
badgeServiceSubscription = badgeService.startListening();
});
it("sets tab-specific state for provided tab and general state for the others", async () => {
it("sets tab-specific state for provided tab", async () => {
const generalState: BadgeState = {
text: "general-text",
backgroundColor: "general-color",
@@ -550,11 +521,10 @@ describe("BadgeService", () => {
);
await new Promise((resolve) => setTimeout(resolve, 0));
expect(badgeApi.generalState).toEqual(generalState);
expect(badgeApi.specificStates).toEqual({
[tabIds[0]]: { ...specificState, backgroundColor: "general-color" },
[tabIds[1]]: generalState,
[tabIds[2]]: generalState,
[tabIds[1]]: undefined,
[tabIds[2]]: undefined,
});
});
});

66
apps/browser/src/platform/badge/badge.service.ts
View File

@@ -1,15 +1,15 @@
import {
defer,
combineLatest,
concatMap,
distinctUntilChanged,
filter,
map,
mergeMap,
pairwise,
startWith,
Subscription,
switchMap,
} from "rxjs";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import {
BADGE_MEMORY,
GlobalState,
@@ -39,6 +39,7 @@ export class BadgeService {
constructor(
private stateProvider: StateProvider,
private badgeApi: BadgeBrowserApi,
private logService: LogService,
) {
this.states = this.stateProvider.getGlobal(BADGE_STATES);
}
@@ -48,52 +49,47 @@ export class BadgeService {
* Without this the service will not be able to update the badge state.
*/
startListening(): Subscription {
const initialSetup$ = defer(async () => {
const openTabs = await this.badgeApi.getTabs();
await this.badgeApi.setState(DefaultBadgeState);
for (const tabId of openTabs) {
await this.badgeApi.setState(DefaultBadgeState, tabId);
}
});
return initialSetup$
.pipe(
switchMap(() => this.states.state$),
return combineLatest({
states: this.states.state$.pipe(
startWith({}),
distinctUntilChanged(),
map((states) => new Set(states ? Object.values(states) : [])),
pairwise(),
map(([previous, current]) => {
const [removed, added] = difference(previous, current);
return { states: current, removed, added };
return { all: current, removed, added };
}),
filter(({ removed, added }) => removed.size > 0 || added.size > 0),
mergeMap(async ({ states, removed, added }) => {
const changed = [...removed, ...added];
const changedTabIds = new Set(
changed.map((s) => s.tabId).filter((tabId) => tabId !== undefined),
);
const onlyTabSpecificStatesChanged = changed.every((s) => s.tabId != undefined);
if (onlyTabSpecificStatesChanged) {
// If only tab-specific states changed then we only need to update those specific tabs.
for (const tabId of changedTabIds) {
const newState = this.calculateState(states, tabId);
await this.badgeApi.setState(newState, tabId);
}
),
activeTab: this.badgeApi.activeTab$.pipe(startWith(undefined)),
})
.pipe(
concatMap(async ({ states, activeTab }) => {
const changed = [...states.removed, ...states.added];
// If the active tab wasn't changed, we don't need to update the badge.
if (!changed.some((s) => s.tabId === activeTab?.tabId || s.tabId === undefined)) {
return;
}
// If there are any general states that changed then we need to update all tabs.
const openTabs = await this.badgeApi.getTabs();
const generalState = this.calculateState(states);
await this.badgeApi.setState(generalState);
for (const tabId of openTabs) {
const newState = this.calculateState(states, tabId);
await this.badgeApi.setState(newState, tabId);
try {
const state = this.calculateState(states.all, activeTab?.tabId);
await this.badgeApi.setState(state, activeTab?.tabId);
} catch (error) {
// This usually happens when the user opens a popout because of how the browser treats it
// as a tab in the same window but then won't let you set the badge state for it.
this.logService.warning("Failed to set badge state", error);
}
}),
)
.subscribe();
.subscribe({
error: (err: unknown) => {
this.logService.error(
"Fatal error in badge service observable, badge will fail to update",
err,
);
},
});
}
/**

12
apps/browser/src/platform/badge/test/mock-badge-browser-api.ts
View File

@@ -1,10 +1,22 @@
import { BehaviorSubject } from "rxjs";
import { BadgeBrowserApi, RawBadgeState } from "../badge-browser-api";
export class MockBadgeBrowserApi implements BadgeBrowserApi {
private _activeTab$ = new BehaviorSubject<chrome.tabs.TabActiveInfo | undefined>(undefined);
activeTab$ = this._activeTab$.asObservable();
specificStates: Record<number, RawBadgeState> = {};
generalState?: RawBadgeState;
tabs: number[] = [];
setActiveTab(tabId: number) {
this._activeTab$.next({
tabId,
windowId: 1,
});
}
setState(state: RawBadgeState, tabId?: number): Promise<void> {
if (tabId !== undefined) {
this.specificStates[tabId] = state;

92
libs/eslint/platform/required-using.spec.mjs
View File

@@ -34,14 +34,14 @@ ruleTester.run("required-using", rule.default, {
using client = rc.take();
`,
},
{
name: "Function reference with `using`",
code: `
${setup}
const t = rc.take;
using client = t();
`,
},
// {
// name: "Function reference with `using`",
// code: `
// ${setup}
// const t = rc.take;
// using client = t();
// `,
// },
],
invalid: [
{
@@ -56,43 +56,43 @@ ruleTester.run("required-using", rule.default, {
},
],
},
{
name: "Assignment without `using`",
code: `
${setup}
let client;
client = rc.take();
`,
errors: [
{
message: errorMessage,
},
],
},
{
name: "Function reference without `using`",
code: `
${setup}
const t = rc.take;
const client = t();
`,
errors: [
{
message: errorMessage,
},
],
},
{
name: "Destructuring without `using`",
code: `
${setup}
const { value } = rc.take();
`,
errors: [
{
message: errorMessage,
},
],
},
// {
// name: "Assignment without `using`",
// code: `
// ${setup}
// let client;
// client = rc.take();
// `,
// errors: [
// {
// message: errorMessage,
// },
// ],
// },
// {
// name: "Function reference without `using`",
// code: `
// ${setup}
// const t = rc.take;
// const client = t();
// `,
// errors: [
// {
// message: errorMessage,
// },
// ],
// },
// {
// name: "Destructuring without `using`",
// code: `
// ${setup}
// const { value } = rc.take();
// `,
// errors: [
// {
// message: errorMessage,
// },
// ],
// },
],
});