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 +}