From 4f6ecc8cc1f83536fd54be64e0f191c4dee0a10e Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 12 Aug 2026 18:28:10 -0400 Subject: [PATCH] feat(background): walk the follow list in the worker, not in the popup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is one request per hundred follows, which makes it the only read here whose length is set by how sociable the user is rather than by what is on the page: an account following fifty thousand people is five hundred requests. That must not be something a window is waiting on. The popup now asks the worker for the set and draws whatever comes back whenever it comes back, and closing the popup no longer throws a walk in progress away. The worker holds one walk per account so two cards asking at once still cost one, and answers with the DID it read for — an account switch between ask and answer makes that different from the one the popup asked about, and the label is what the cards check before drawing. `send` moves out of popup.ts so a card can use it without importing the module that imports the cards. --- src/background.ts | 58 ++++++++++++++++++++- src/lib/types.ts | 12 +++++ src/popup/cards/follows.test.ts | 90 ++++++++++++++++----------------- src/popup/cards/follows.ts | 41 +++++++-------- src/popup/popup.ts | 11 +--- src/popup/send.ts | 19 +++++++ 6 files changed, 155 insertions(+), 76 deletions(-) create mode 100644 src/popup/send.ts diff --git a/src/background.ts b/src/background.ts index 8a3862d..50a11cb 100644 --- a/src/background.ts +++ b/src/background.ts @@ -8,8 +8,9 @@ import { listRecords, parseAtUri, resolveDid } from './lib/atproto' import { cached } from './lib/cache' import { detectPage } from './lib/detection' import { type IconState, badgeFor, iconStateFor, titleFor } from './lib/icon' +import { followingDids } from './lib/subscribers' import { startSignIn } from './signin' -import type { Msg, PageState, SessionInfo } from './lib/types' +import type { FollowSet, Msg, PageState, SessionInfo } from './lib/types' const SUB_COLLECTION = 'site.standard.graph.subscription' const BLOCK_COLLECTION = 'app.bsky.graph.block' @@ -136,6 +137,52 @@ async function getBlocks(refresh: boolean): Promise { ) } +// --- the viewer's follow set ------------------------------------------------- +// +// The one read in the extension whose size is set by how sociable the user is +// rather than by what is on the page: `listRecords` pages a hundred at a time, +// and an account following fifty thousand people is five hundred requests. It +// runs here rather than in the popup for that reason. The popup asks and draws +// whatever comes back whenever it comes back; a walk this long must never be +// something a window is waiting on, and the worker outliving the popup means +// closing it does not throw the walk away. +// +// Cached under `graph`, which is a day and on disk (src/lib/cache.ts), so the +// full walk is a once-a-day event and not a once-a-popup one. + +/** Walks in flight, keyed by account: two askers share one rather than starting two. */ +const followWalks = new Map>() + +async function getFollows(refresh: boolean): Promise { + const session = await getSession() + if (!session) return null + const did = session.did + + // A walk that bypassed the cache also satisfies an asker that would have + // accepted a cached answer; the reverse is not true. + const inFlight = followWalks.get(did) + if (inFlight && !refresh) return { did, dids: await inFlight } + + const work = cached( + { scope: 'graph', subject: did, name: 'follows' }, + async () => { + const set = await followingDids(did) + // The walk stops at listRecords' 2000-record cap, which is invisible + // from its result: a count here is what makes a heavy follower's + // missing answer explicable afterwards (see TODO.md). + console.debug(`[substandard] read ${set.size} follow records for ${did}`) + return [...set] + }, + { refresh }, + ) + followWalks.set(did, work) + try { + return { did, dids: await work } + } finally { + if (followWalks.get(did) === work) followWalks.delete(did) + } +} + // --- detection orchestration ------------------------------------------------- /** @@ -229,6 +276,15 @@ async function handle(msg: Msg, sender: chrome.runtime.MessageSender): Promise { + console.debug('[substandard] follow walk failed', err) + return null + }) + } } } diff --git a/src/lib/types.ts b/src/lib/types.ts index e479514..73382ee 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -89,11 +89,23 @@ export interface SessionInfo { avatarUrl?: string } +/** + * What the worker answers a `follows` message with. Null means the question + * has no answer — signed out, or the walk failed — which every caller draws as + * nothing rather than as "you follow nobody". + */ +export interface FollowSet { + /** Whose follows these are. The account can change between ask and answer. */ + did: string + dids: string[] +} + /** Messages between content script / popup and the background worker. */ export type Msg = | { type: 'page-hints'; pubHint?: string; docHint?: string } | { type: 'get-state'; tabId: number; refresh?: boolean } | { type: 'signin'; handle: string } + | { type: 'follows'; refresh?: boolean } /** * Messages from the worker to the offscreen document that hosts the OAuth diff --git a/src/popup/cards/follows.test.ts b/src/popup/cards/follows.test.ts index 2fdbe2d..ce4cf73 100644 --- a/src/popup/cards/follows.test.ts +++ b/src/popup/cards/follows.test.ts @@ -1,19 +1,18 @@ // The one thing this module exists for: two cards asking at once cost one -// walk of the follow list, not two. +// request to the worker, not two. The walk itself is the worker's, and these +// tests are the popup's half of that split — see src/background.ts. import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { SessionInfo } from '../../lib/types' +import type { FollowSet, SessionInfo } from '../../lib/types' import type { CardHost } from './card' import { resetFollows, viewerFollows } from './follows' -const followingDids = vi.fn() -vi.mock('../../lib/subscribers', () => ({ followingDids: (did: string) => followingDids(did) })) +const sendMessage = vi.fn() const ME = 'did:plc:thereader000000000000000' +const OTHER = 'did:plc:someoneelse0000000000000' const FRIEND = 'did:plc:afollowedaccount00000000' -let store = new Map() - function hostFor(did?: string): CardHost { return { state: () => undefined, @@ -22,10 +21,13 @@ function hostFor(did?: string): CardHost { } } -/** A walk that does not settle until the test says so. */ +/** What the worker answers with, for whoever it finds in the session mirror. */ +const answer = (did: string, ...dids: string[]): FollowSet => ({ did, dids }) + +/** A reply that does not settle until the test says so. */ function deferred() { - let release!: (dids: Set) => void - const work = new Promise>((resolve) => { + let release!: (set: FollowSet) => void + const work = new Promise((resolve) => { release = resolve }) return { work, release } @@ -33,26 +35,8 @@ function deferred() { beforeEach(() => { resetFollows() - followingDids.mockReset() - store = new Map() - vi.stubGlobal('chrome', { - storage: { - session: { - get: async (keys: string | null) => - keys === null - ? Object.fromEntries(store) - : store.has(keys) - ? { [keys]: store.get(keys) } - : {}, - set: async (items: Record) => { - for (const [k, v] of Object.entries(items)) store.set(k, v) - }, - remove: async (keys: string | string[]) => { - for (const k of [keys].flat()) store.delete(k) - }, - }, - }, - }) + sendMessage.mockReset() + vi.stubGlobal('chrome', { runtime: { sendMessage } }) }) afterEach(() => { @@ -60,55 +44,69 @@ afterEach(() => { }) describe('viewerFollows', () => { - it('walks once for two cards that ask before the first answer lands', async () => { + it('asks once for two cards that ask before the first answer lands', async () => { const { work, release } = deferred() - followingDids.mockReturnValue(work) + sendMessage.mockReturnValue(work) const host = hostFor(ME) const both = Promise.all([viewerFollows(host, false), viewerFollows(host, false)]) - release(new Set([FRIEND])) + release(answer(ME, FRIEND)) const [a, b] = await both - expect(followingDids).toHaveBeenCalledTimes(1) + expect(sendMessage).toHaveBeenCalledTimes(1) expect(a?.following.has(FRIEND)).toBe(true) expect(b).toBe(a) }) it('joins a refresh already in flight, which is the stronger read', async () => { - followingDids.mockResolvedValue(new Set([FRIEND])) + sendMessage.mockResolvedValue(answer(ME, FRIEND)) const host = hostFor(ME) await Promise.all([viewerFollows(host, true), viewerFollows(host, false)]) - expect(followingDids).toHaveBeenCalledTimes(1) + expect(sendMessage).toHaveBeenCalledTimes(1) }) it('does not let a cached read stand in for a refresh', async () => { - followingDids.mockResolvedValue(new Set([FRIEND])) + sendMessage.mockResolvedValue(answer(ME, FRIEND)) const host = hostFor(ME) await viewerFollows(host, false) await viewerFollows(host, true) - expect(followingDids).toHaveBeenCalledTimes(2) + expect(sendMessage).toHaveBeenCalledTimes(2) + // The worker owns the cache, so the refresh has to reach it to matter. + expect(sendMessage).toHaveBeenLastCalledWith({ type: 'follows', refresh: true }) }) - it('answers a signed-out popup without asking the network', async () => { + it('answers a signed-out popup without asking the worker', async () => { expect(await viewerFollows(hostFor(undefined), false)).toBeUndefined() - expect(followingDids).not.toHaveBeenCalled() + expect(sendMessage).not.toHaveBeenCalled() }) - it('gives no answer when the walk fails, and lets the next card try again', async () => { - followingDids.mockRejectedValueOnce(new Error('pds down')) + it('gives no answer when the worker has none, and lets the next card try again', async () => { + sendMessage.mockResolvedValueOnce(null) const host = hostFor(ME) expect(await viewerFollows(host, false)).toBeUndefined() - followingDids.mockResolvedValue(new Set([FRIEND])) + sendMessage.mockResolvedValue(answer(ME, FRIEND)) expect((await viewerFollows(host, false))?.following.has(FRIEND)).toBe(true) }) + it('gives no answer when the message itself fails', async () => { + sendMessage.mockRejectedValueOnce(new Error('worker asleep')) + expect(await viewerFollows(hostFor(ME), false)).toBeUndefined() + }) + it('never hands one account the follows of another', async () => { - followingDids.mockResolvedValue(new Set([FRIEND])) + sendMessage.mockResolvedValue(answer(ME, FRIEND)) await viewerFollows(hostFor(ME), false) - followingDids.mockResolvedValue(new Set()) - const other = await viewerFollows(hostFor('did:plc:someoneelse0000000000000'), false) + sendMessage.mockResolvedValue(answer(OTHER)) + const other = await viewerFollows(hostFor(OTHER), false) expect(other?.following.has(FRIEND)).toBe(false) - expect(followingDids).toHaveBeenCalledTimes(2) + expect(sendMessage).toHaveBeenCalledTimes(2) + }) + + it('labels the set with the account the worker answered for, not the one asked about', async () => { + // An account switch between ask and answer: the label is what every caller + // checks before drawing, so it has to be the worker's, not the popup's. + sendMessage.mockResolvedValue(answer(OTHER, FRIEND)) + expect((await viewerFollows(hostFor(ME), false))?.did).toBe(OTHER) }) }) diff --git a/src/popup/cards/follows.ts b/src/popup/cards/follows.ts index 203884c..d455399 100644 --- a/src/popup/cards/follows.ts +++ b/src/popup/cards/follows.ts @@ -2,22 +2,30 @@ // // Two cards want the same answer: the subscriber row intersects it with a // publication's subscribers, and the owner card asks whether one DID is in it. -// It is one request per hundred follows, so it is cached with the rest of what -// we read from that account's own repo (src/lib/cache.ts) — but the cards load -// in parallel and `cached` does not dedupe requests in flight, so a cold cache -// would otherwise walk the follow list twice before either write landed. +// +// The read itself is the worker's (src/background.ts): it is one request per +// hundred follows, so it is the one read here that a very sociable account can +// make very long, and a window must not be what it happens inside. This module +// is the popup's half — it asks once and hands the answer to whichever card +// asks next, because the cards load in parallel and would otherwise send two +// messages before either reply landed. -import { cached } from '../../lib/cache' -import { type Viewer, followingDids } from '../../lib/subscribers' +import { type Viewer } from '../../lib/subscribers' +import type { FollowSet } from '../../lib/types' +import { send } from '../send' import type { CardHost } from './card' -/** The read this popup already started, so the second card joins it. */ +/** The request this popup already sent, so the second card joins it. */ let pending: { did: string; refresh: boolean; work: Promise } | undefined /** * The signed-in account's follows. Undefined means the question has no answer * — signed out, or the walk failed — which every caller draws as nothing * rather than as "you follow nobody". + * + * On a cold cache this resolves when the walk does, which can be a while. + * Every caller is expected to have drawn without it by then and to redraw when + * it lands; none of them may hold a row open waiting. */ export function viewerFollows(host: CardHost, refresh: boolean): Promise { const did = host.session()?.did @@ -41,19 +49,12 @@ export function resetFollows(): void { async function read(did: string, refresh: boolean): Promise { try { - const dids = await cached( - { scope: 'own', subject: did, name: 'follows' }, - async () => { - const set = await followingDids(did) - // The walk stops at listRecords' 2000-record cap, which is invisible - // from its result: a count here is what makes a heavy follower's - // missing answer explicable afterwards (see TODO.md). - console.debug(`[substandard] read ${set.size} follow records for ${did}`) - return [...set] - }, - { refresh }, - ) - return { did, following: new Set(dids) } + const set = await send({ type: 'follows', refresh }) + if (!set) return undefined + // Labelled with the account the worker answered for, not the one this + // asked about: an account switch between the two makes them different, and + // every caller checks the label before drawing anything from the set. + return { did: set.did, following: new Set(set.dids) } } catch (err) { // The subscriber count survives without the faces, and the owner card // survives without the line; see summarizeSubscribers. diff --git a/src/popup/popup.ts b/src/popup/popup.ts index 77b63bf..cc76eae 100644 --- a/src/popup/popup.ts +++ b/src/popup/popup.ts @@ -27,8 +27,9 @@ import { shareOnBlueskyUrl } from '../lib/share' import { type StatusMessage, statusMessagesFor } from '../lib/status' import { CARDS, type CardHost } from './cards' import { $, TONE_ICONS, failedIconUrls } from './dom' +import { send } from './send' import { wireTypeaheadSubmit } from '../lib/typeahead' -import type { Msg, PageState, PubInfo, SessionInfo } from '../lib/types' +import type { PageState, PubInfo, SessionInfo } from '../lib/types' const SUB_COLLECTION = 'site.standard.graph.subscription' @@ -56,14 +57,6 @@ let subscribeArmed = false /** The feedback board, read once per popup open (see loadBoard). */ let board: BoardSpace | undefined -async function send(msg: Msg): Promise { - const res = await chrome.runtime.sendMessage(msg) - if (res && typeof res === 'object' && '__error' in res) { - throw new Error(String((res as { __error: unknown }).__error)) - } - return res as T -} - /** * What a write owes the cache: this one changed the subscription collection, * so the list read from it is no longer what the PDS holds. Nothing else the diff --git a/src/popup/send.ts b/src/popup/send.ts new file mode 100644 index 0000000..6364b3a --- /dev/null +++ b/src/popup/send.ts @@ -0,0 +1,19 @@ +// Asking the worker something, from anywhere in the popup. +// +// Its own module rather than a helper in popup.ts, because popup.ts imports +// the cards and a card that needed this would import it back. + +import type { Msg } from '../lib/types' + +/** + * A round trip to the background worker. The worker answers a thrown handler + * with `{__error}` rather than by rejecting (a rejected sendResponse does not + * survive the channel), so unwrap that back into a rejection here. + */ +export async function send(msg: Msg): Promise { + const res = await chrome.runtime.sendMessage(msg) + if (res && typeof res === 'object' && '__error' in res) { + throw new Error(String((res as { __error: unknown }).__error)) + } + return res as T +} -- 2.51.2