diff --git a/src/models/settings.ts b/src/models/settings.ts index aac7a84b0..1a0da4086 100644 --- a/src/models/settings.ts +++ b/src/models/settings.ts @@ -56,8 +56,51 @@ const LocalUserSettingsDataKeys = [ export class UserSettings { static items: UserSettingsData = {}; + // Settings are read from chrome.storage on nearly every storage operation + // (see BrowserStorage.getStorageLocation). Reading them afresh each time + // costs one or two IPC round trips per call, which adds up to dozens of + // serialized round trips before the popup can render. Cache the result and + // drop the cache whenever anything writes to storage, so callers still + // observe changes made by other extension contexts. + private static cachedItems: UserSettingsData | null = null; + private static pendingRead: Promise | null = null; + private static invalidationHooked = false; + + private static hookInvalidation() { + if (UserSettings.invalidationHooked) { + return; + } + UserSettings.invalidationHooked = true; + chrome.storage.onChanged?.addListener(() => { + UserSettings.invalidateCache(); + }); + } + + static invalidateCache() { + UserSettings.cachedItems = null; + } + static async updateItems() { - UserSettings.items = await UserSettings.getAllItems(); + UserSettings.hookInvalidation(); + + if (UserSettings.cachedItems) { + UserSettings.items = UserSettings.cachedItems; + return; + } + + // Coalesce concurrent readers onto a single read. + if (!UserSettings.pendingRead) { + UserSettings.pendingRead = UserSettings.getAllItems() + .then((items) => { + UserSettings.cachedItems = items; + return items; + }) + .finally(() => { + UserSettings.pendingRead = null; + }); + } + + UserSettings.items = await UserSettings.pendingRead; } static async convertFromLocalStorage( @@ -108,6 +151,7 @@ export class UserSettings { ]); } + UserSettings.invalidateCache(); await UserSettings.updateItems(); } diff --git a/src/models/storage.ts b/src/models/storage.ts index 4b3699fb2..9d0bfcce0 100644 --- a/src/models/storage.ts +++ b/src/models/storage.ts @@ -3,7 +3,23 @@ import { OTPEntry, OTPType, OTPAlgorithm, CodeState } from "./otp"; import { StorageLocation, UserSettings } from "./settings"; import { DataType } from "./otp"; export class BrowserStorage { - private static async getStorageLocation(): Promise { + private static pendingLocation: Promise | null = null; + + // Every storage operation resolves the storage location first. Share one + // resolution between concurrent callers so parallel readers don't each + // re-run the auto-detection branch (which can also commit settings). + private static getStorageLocation(): Promise { + if (!BrowserStorage.pendingLocation) { + BrowserStorage.pendingLocation = BrowserStorage.resolveStorageLocation().finally( + () => { + BrowserStorage.pendingLocation = null; + } + ); + } + return BrowserStorage.pendingLocation; + } + + private static async resolveStorageLocation(): Promise { await UserSettings.updateItems(); const managedLocation = await ManagedStorage.get( "storageArea" @@ -688,34 +704,82 @@ export class EntryStorage { } export class ManagedStorage { + // The managed policy is a single object, but callers read it one key at a + // time (the menu store alone reads eight). Fetch it once and answer + // subsequent lookups from memory, dropping the cache if policy changes. + private static cachedPolicy: ManagedPolicy | null = null; + private static pendingPolicy: Promise | null = null; + private static invalidationHooked = false; + + private static hookInvalidation() { + if (ManagedStorage.invalidationHooked) { + return; + } + ManagedStorage.invalidationHooked = true; + chrome.storage.onChanged?.addListener((_changes, areaName) => { + if (areaName === "managed") { + ManagedStorage.cachedPolicy = null; + ManagedStorage.pendingPolicy = null; + } + }); + } + + private static readPolicy(): Promise { + return new Promise((resolve: (result: ManagedPolicy) => void) => { + if (chrome.storage.managed) { + chrome.storage.managed.get((data) => { + if (chrome.runtime.lastError) { + return resolve({}); + } + return resolve(data || {}); + }); + } else { + // no available in Safari + resolve({}); + } + }); + } + + // Share a single underlying read between all callers. The read is kept + // running even if an individual lookup times out below, so a policy that + // arrives late still populates the cache for subsequent lookups. + private static getPolicy(): Promise { + if (!ManagedStorage.pendingPolicy) { + ManagedStorage.pendingPolicy = ManagedStorage.readPolicy().then( + (data) => { + ManagedStorage.cachedPolicy = data; + return data; + } + ); + } + return ManagedStorage.pendingPolicy; + } + static get(key: string): T | undefined; static get(key: string, defaultValue: T): T; static get(key: string, defaultValue?: T) { - const managedStoragePromise = new Promise( - (resolve: (result: T | undefined) => void) => { - if (chrome.storage.managed) { - chrome.storage.managed.get((data) => { - if (chrome.runtime.lastError) { - return resolve(defaultValue); - } - if (data) { - if (data[key]) { - return resolve(data[key]); - } - } - return resolve(defaultValue); - }); - } else { - // no available in Safari - resolve(defaultValue); - } + ManagedStorage.hookInvalidation(); + + const pick = (data: ManagedPolicy) => + data && data[key] ? (data[key] as T) : defaultValue; + + if (ManagedStorage.cachedPolicy) { + return Promise.resolve(pick(ManagedStorage.cachedPolicy)); + } + + // Preserve the original contract: never make a caller wait more than + // ~10ms on managed storage. + const timeoutPromise = new Promise( + (resolve: (r: T | undefined) => void) => { + setTimeout(() => resolve(defaultValue), 10); } ); - const timeoutPromise = new Promise((resolve) => { - setTimeout(() => resolve(defaultValue), 10); - }); - - return Promise.race([managedStoragePromise, timeoutPromise]); + return Promise.race([ + ManagedStorage.getPolicy().then(pick), + timeoutPromise, + ]); } } + +type ManagedPolicy = Record; diff --git a/src/popup.ts b/src/popup.ts index 173bc033e..72bca3c1f 100644 --- a/src/popup.ts +++ b/src/popup.ts @@ -34,9 +34,6 @@ async function init() { await migrateLocalStorageToBrowserStorage(); await UserSettings.updateItems(); - // Add globals - Vue.prototype.i18n = await loadI18nMessages(); - // Load modules Vue.use(Vuex); Vue.use(Vue2Dragula); @@ -46,14 +43,27 @@ async function init() { Vue.component(component.name, component.component); } + // The i18n catalog and the async store modules don't depend on one another, + // so resolve them concurrently instead of awaiting each in turn. + const [i18nMessages, accounts, advisor, backup, menu] = await Promise.all([ + loadI18nMessages(), + new Accounts().getModule(), + new Advisor().getModule(), + new Backup().getModule(), + new Menu().getModule(), + ]); + + // Add globals + Vue.prototype.i18n = i18nMessages; + // State const store = new Vuex.Store({ modules: { - accounts: await new Accounts().getModule(), - advisor: await new Advisor().getModule(), - backup: await new Backup().getModule(), + accounts, + advisor, + backup, currentView: new CurrentView().getModule(), - menu: await new Menu().getModule(), + menu, notification: new Notification().getModule(), qr: new Qr().getModule(), style: new Style().getModule(), diff --git a/src/store/Advisor.ts b/src/store/Advisor.ts index a9514d4e8..8b711cb4e 100644 --- a/src/store/Advisor.ts +++ b/src/store/Advisor.ts @@ -100,19 +100,19 @@ export class Advisor implements Module { ? JSON.parse(UserSettings.items.advisorIgnoreList || "[]") : UserSettings.items.advisorIgnoreList || []; - const filteredInsightsData: AdvisorInsightInterface[] = []; - - for (const insightData of insightsData) { - if (advisorIgnoreList.includes(insightData.id)) { - continue; - } + // The validations are independent reads, so run them concurrently instead + // of paying for each one in turn. Order is preserved by index. + const candidates = insightsData.filter( + (insightData) => !advisorIgnoreList.includes(insightData.id) + ); - const validation = await insightData.validation(); + const validations = await Promise.all( + candidates.map((insightData) => insightData.validation()) + ); - if (validation) { - filteredInsightsData.push(insightData); - } - } + const filteredInsightsData: AdvisorInsightInterface[] = candidates.filter( + (_, index) => validations[index] + ); return filteredInsightsData.map( (insightData) => new AdvisorInsight(insightData) diff --git a/src/store/Backup.ts b/src/store/Backup.ts index 790ed719a..55a98775f 100644 --- a/src/store/Backup.ts +++ b/src/store/Backup.ts @@ -2,7 +2,7 @@ import { UserSettings } from "../models/settings"; export class Backup implements Module { async getModule() { - UserSettings.updateItems(); + await UserSettings.updateItems(); return { state: {