From 68f2d9f27c1d76d9d96c5b3d82554941135deaa5 Mon Sep 17 00:00:00 2001 From: Timo <6156589+Shik3i@users.noreply.github.com> Date: Tue, 18 Aug 2026 15:56:17 +0200 Subject: [PATCH] fix(extension): find the player when the all-frames sweep comes back empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Diagnosed against a fixture rebuilt from the live yummyanime.tv page, with the ad churn the real site produces. Under that churn the resolver reported frame=0, hasVideo=false while the video demonstrably existed two frame levels down: one ad slot tearing down mid-call makes Chromium reject the whole allFrames sweep, and the resolver then silently fell back to the top frame and never looked again. v3.1.2 did not have this failure because webNavigation.getAllFrames() gave it an explicit frame list. That list is now rebuilt without the permission: every content script that messages the background carries sender.frameId, so the background keeps a per-tab registry of frames it has seen and the resolver asks any frame the sweep missed directly. One rejected probe now costs one frame instead of the whole page. Two supporting fixes fell out of the same investigation. Frames reported hidden by an ancestor that could inspect them directly — the 0x0 same-origin wrapper an anime host parks unwatched mirrors in — are now excluded without waiting for the postMessage visibility handshake, which was the tie the resolver kept failing to break. And leaf frames with no video and no nested frames are left out of that handshake entirely, so a churning ad slot can no longer make every phase wait on a frame that is already gone. Co-Authored-By: Claude Opus 5 --- extension/background.js | 64 +++++++++++++++++-- extension/media-frame-target.js | 56 +++++++++++++++- extension/target-tab-lifecycle.test.mjs | 12 +++- tests/e2e/extension.spec.mjs | 35 ++++++++++ .../fixtures/pages/yummy-churning-player.html | 43 +++++++++++++ 5 files changed, 197 insertions(+), 13 deletions(-) create mode 100644 tests/e2e/fixtures/pages/yummy-churning-player.html diff --git a/extension/background.js b/extension/background.js index 75b5ccc..bc75cec 100644 --- a/extension/background.js +++ b/extension/background.js @@ -84,6 +84,40 @@ let currentTargetDocumentId = null; let currentTargetHasVideo = false; let targetActivationGeneration = 0; let activeTargetActivation = null; +// Frame ids seen in this tab, learned from script results and from the frames +// that message us. An allFrames sweep is all-or-nothing — one ad frame tearing +// down while it runs makes Chromium reject the whole call, and the resolver +// then sees nothing but the top frame. v3.1.2 avoided that by listing frames +// through webNavigation; this registry rebuilds the same knowledge from +// sender.frameId, which every content script hands us for free. +const knownFrameIdsByTab = new Map(); + +function rememberFrameId(tabId, frameId) { + const normalizedTabId = normalizeTabId(tabId); + if (normalizedTabId === null || !Number.isInteger(frameId) || frameId < 0) return; + let frames = knownFrameIdsByTab.get(normalizedTabId); + if (!frames) { + frames = new Set(); + knownFrameIdsByTab.set(normalizedTabId, frames); + } + frames.add(frameId); + // A tab cannot plausibly hold this many media-bearing frames; cap the set so + // a page that recycles frames forever cannot grow it without bound. + if (frames.size > 64) { + const oldest = frames.values().next().value; + if (oldest !== 0) frames.delete(oldest); + } +} + +function listKnownFrameIds(tabId) { + const frames = knownFrameIdsByTab.get(normalizeTabId(tabId)); + return frames ? Array.from(frames) : []; +} + +function forgetFrameIds(tabId) { + knownFrameIdsByTab.delete(normalizeTabId(tabId)); +} + let mediaTargetRefreshTask = null; let mediaTargetRefreshTabId = null; let mediaTargetRefreshDirty = false; @@ -2426,7 +2460,9 @@ async function injectContentScript(tabId, { access = await inspectTabHostAccess(chrome, tabId); const url = access.url || ''; needsPageApiSeek = shouldUsePageApiSeek(url); - contentTarget = await resolveMediaContentTarget(chrome, tabId); + contentTarget = await resolveMediaContentTarget(chrome, tabId, { + knownFrameIds: listKnownFrameIds(tabId) + }); if (!isTargetActivationSuperseded(tabId, activationGeneration) && activeTargetActivation?.tabId === tabId) { activeTargetActivation.frameId = contentTarget.frameId; @@ -3008,7 +3044,10 @@ async function reactivateCurrentTarget(tabId, { expectedGeneration = targetActiv async function selectedMediaTargetMoved(tabId) { let resolved; try { - resolved = await resolveMediaContentTarget(chrome, tabId, { attempts: 1 }); + resolved = await resolveMediaContentTarget(chrome, tabId, { + attempts: 1, + knownFrameIds: listKnownFrameIds(tabId) + }); } catch { // An access-required error must reach the full activation path so the // popup can surface it. @@ -3027,7 +3066,10 @@ async function selectedMediaTargetMoved(tabId) { && resolved.documentId !== currentTargetDocumentId); } -function refreshCurrentMediaTarget(tabId, { queueIfRunning = false, onlyIfTargetMoved = false } = {}) { +// Reinjection is the exception, not the default. Every caller that merely +// wants the target confirmed gets the guarded path; only a genuinely +// unreachable content script or an explicit request forces a rebuild. +function refreshCurrentMediaTarget(tabId, { queueIfRunning = false, onlyIfTargetMoved = true } = {}) { const selectedTabId = normalizeTabId(tabId); if (selectedTabId === null || normalizeTabId(currentTabId) !== selectedTabId) { return Promise.resolve({ status: 'superseded' }); @@ -3072,7 +3114,10 @@ function refreshCurrentMediaTarget(tabId, { queueIfRunning = false, onlyIfTarget if (needsFollowup && mediaTargetRefreshFollowupTimer === null) { mediaTargetRefreshFollowupTimer = setTimeout(() => { mediaTargetRefreshFollowupTimer = null; - refreshCurrentMediaTarget(selectedTabId, { queueIfRunning: true }).catch(() => {}); + refreshCurrentMediaTarget(selectedTabId, { + queueIfRunning: true, + onlyIfTargetMoved + }).catch(() => {}); }, 250); } }); @@ -3172,6 +3217,7 @@ if (chrome.tabs?.onRemoved?.addListener) { const isCurrent = normalizeTabId(currentTabId) === tabId; const isPending = pending?.tabId === tabId; const isActivating = activeTargetActivation?.tabId === tabId; + forgetFrameIds(tabId); const isSelected = normalizeTabId(userSelectedTabId) === tabId; if (isSelected) await clearUserSelection(tabId); if (!isCurrent && !isPending && !isActivating) return; @@ -3284,7 +3330,7 @@ async function _routeToContentInternal(tabId, action, payload, actionTimestamp, || message.includes('No document with id') || message.includes('No document with ID')) { try { - const response = await refreshCurrentMediaTarget(tabId); + const response = await refreshCurrentMediaTarget(tabId, { onlyIfTargetMoved: false }); if (response?.status !== 'ok' && response?.status !== 'activation_in_progress') return; await new Promise(resolve => setTimeout(resolve, 150)); await _routeToContentInternal( @@ -3449,6 +3495,7 @@ async function handleAsyncMessage(message, sender, sendResponse) { await ensureState(); const senderTabId = normalizeTabId(sender?.tab?.id); + if (senderTabId !== null) rememberFrameId(senderTabId, sender?.frameId); const mediaLifecycleMessage = message.type === 'MEDIA_FRAME_CANDIDATE_CHANGED' || message.type === 'MEDIA_FRAME_VISIBILITY' || message.type === 'MEDIA_TARGET_REFRESH'; @@ -4218,7 +4265,7 @@ async function handleAsyncMessage(message, sender, sendResponse) { return true; } - refreshCurrentMediaTarget(tabId).then(response => { + refreshCurrentMediaTarget(tabId, { onlyIfTargetMoved: false }).then(response => { sendResponse(response); }).catch(err => { addLog(`Failed to inject into tab: ${err.message}`, 'warn'); @@ -4509,7 +4556,10 @@ async function handleAsyncMessage(message, sender, sendResponse) { initTabManager({ getCurrentTabId: () => currentTabId, - reactivateCurrentTarget: tabId => refreshCurrentMediaTarget(tabId, { queueIfRunning: true }), + reactivateCurrentTarget: tabId => refreshCurrentMediaTarget(tabId, { + queueIfRunning: true, + onlyIfTargetMoved: false + }), ensureState, sendToCurrentContent: sendMessageToCurrentContent }); diff --git a/extension/media-frame-target.js b/extension/media-frame-target.js index 0bdacde..d4871e8 100644 --- a/extension/media-frame-target.js +++ b/extension/media-frame-target.js @@ -335,11 +335,34 @@ function sameMeaningfulRank(left, right) { return leftRank.slice(0, 8).every((value, index) => value === rightRank[index]); } +/** + * Frames whose own element was seen as hidden by an ancestor that could inspect + * it directly. A same-origin wrapper collapsed to 0x0 — the usual way an anime + * host parks the mirrors you are not watching — is reported here by the top + * frame itself, so the hidden player can be ruled out without waiting for the + * postMessage visibility handshake to complete. + */ +function hiddenFrameHrefs(injectionResults) { + const visibility = new Map(); + for (const entry of Array.isArray(injectionResults) ? injectionResults : []) { + for (const frame of entry?.result?.embeddedFrames || []) { + if (typeof frame?.href !== 'string' || !frame.href) continue; + // Any ancestor reporting it visible wins over one reporting it hidden. + visibility.set(frame.href, (visibility.get(frame.href) === true) || frame.visible === true); + } + } + const hidden = new Set(); + for (const [href, visible] of visibility) if (!visible) hidden.add(href); + return hidden; +} + export function selectMediaFrame(injectionResults) { + const hidden = hiddenFrameHrefs(injectionResults); const candidates = (Array.isArray(injectionResults) ? injectionResults : []) .filter(entry => Number.isInteger(entry?.frameId) && entry?.result?.bestVideo?.rendered === true) .filter(entry => entry.result.parentFrameVisible !== false) + .filter(entry => !hidden.has(entry.result.href)) .sort(compareRanks); if (candidates.length === 0) return null; if (candidates.length > 1 @@ -532,6 +555,9 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { retryDelayMs = 200, probeDelayMs = 60, probeTimeoutMs = DEFAULT_PROBE_TIMEOUT_MS, + // Frame ids the background has seen in this tab. They rescue the probe when + // the all-frames sweep is rejected wholesale by one unrelated frame. + knownFrameIds = [], // ...but the budget is now wall-clock bounded, so a page whose frames all // time out cannot hold the activation open for minutes. deadlineMs = 12000 @@ -551,6 +577,21 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { [null], probeTimeoutMs ); + // Any frame the sweep missed but that we know exists gets asked directly. + // One rejected probe then costs one frame, not the whole page. + const missingFrameIds = knownFrameIds.filter(frameId => Number.isInteger(frameId) + && !results.some(entry => entry.frameId === frameId)); + if (missingFrameIds.length > 0) { + const { results: recovered } = await executeInAccessibleFrames( + chromeApi, + missingFrameIds.map(frameId => ({ tabId, frameIds: [frameId] })), + inspectMediaFrame, + [null], + probeTimeoutMs + ); + if (recovered.length > 0) results = mergeFrameResults(results, recovered); + } + if (results.length === 0) { // The all-frames sweep answered for nothing at all, so fall back to // the top document alone. Every probe is time-boxed: an unreachable @@ -580,14 +621,23 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { // that by listing frames through webNavigation — but the sweep // above already reports frameId and documentId for every frame it // reached, so the same isolation costs no permission at all. - const frameTargets = results.map(entry => frameScriptTarget(tabId, entry)); + // A leaf frame with no video and no nested frames can never be a + // candidate nor an ancestor of one. Ad slots are exactly that, and + // they churn constantly, so every phase below would otherwise wait + // on a frame that was already being torn down. + const relevant = results.filter(entry => (entry?.result?.videoCount || 0) > 0 + || (entry?.result?.embeddedFrames?.length || 0) > 0 + || entry?.result?.isTop === true); + const frameTargets = (relevant.length > 0 ? relevant : results) + .map(entry => frameScriptTarget(tabId, entry)); try { + const visibilityTimeoutMs = Math.min(probeTimeoutMs, 750); await executeInAccessibleFrames( chromeApi, frameTargets, installParentFrameVisibilityProbe, [token], - probeTimeoutMs + visibilityTimeoutMs ); // Four passes match the maximum same-origin recursion depth. for (let pass = 0; pass < 4; pass++) { @@ -596,7 +646,7 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { frameTargets, dispatchParentFrameVisibilityProbe, [token], - probeTimeoutMs + visibilityTimeoutMs ); await new Promise(resolve => setTimeout(resolve, probeDelayMs)); } diff --git a/extension/target-tab-lifecycle.test.mjs b/extension/target-tab-lifecycle.test.mjs index 546733b..1d83745 100644 --- a/extension/target-tab-lifecycle.test.mjs +++ b/extension/target-tab-lifecycle.test.mjs @@ -14,7 +14,10 @@ describe('target tab lifecycle', () => { it('injects playback and chat scripts only into the explicitly selected tab', () => { expect(backgroundSource).not.toContain('chrome.tabs.onActivated'); expect(backgroundSource).not.toContain('chrome.tabs.query({})'); - expect(backgroundSource).toContain('contentTarget = await resolveMediaContentTarget(chrome, tabId)'); + expect(backgroundSource).toMatch(/contentTarget = await resolveMediaContentTarget\(chrome, tabId, \{\s*knownFrameIds/); + // Frame ids must come from observed senders, never from a navigation permission. + expect(backgroundSource).toContain('function rememberFrameId(tabId, frameId)'); + expect(backgroundSource).toContain('rememberFrameId(senderTabId, sender?.frameId)'); expect(backgroundSource).toContain('target: scriptTarget'); expect(backgroundSource).toContain("files: ['chat-format.js', 'chat-overlay.js', 'content.js']"); expect(backgroundSource).toContain("chrome.tabs.query({ url: 'https://sync.koalastuff.net/*' })"); @@ -45,8 +48,11 @@ describe('target tab lifecycle', () => { it('does not reactivate the target for ordinary playback churn', () => { expect(backgroundSource).toContain('async function selectedMediaTargetMoved(tabId)'); - expect(backgroundSource).toContain('onlyIfTargetMoved = false'); - expect(backgroundSource.match(/onlyIfTargetMoved: true/g)?.length).toBe(5); + expect(backgroundSource).toContain('onlyIfTargetMoved = true'); + // Forcing a rebuild must stay rare and deliberate: an unreachable content + // script, an explicit request, and a completed navigation. Everything else + // takes the guarded path by default. + expect(backgroundSource.match(/onlyIfTargetMoved: false/g)?.length).toBe(3); // Playback state must stay out of the candidate signature, otherwise // every play/pause looks like a frame layout change. expect(monitorSource).not.toContain('element.paused ? 0 : 1'); diff --git a/tests/e2e/extension.spec.mjs b/tests/e2e/extension.spec.mjs index d2ba6d9..34610ac 100644 --- a/tests/e2e/extension.spec.mjs +++ b/tests/e2e/extension.spec.mjs @@ -706,6 +706,41 @@ test('polling video state on a page with no video does not restart the target', .toBe('true'); }); +test('stays ready on a page whose ad frames keep mutating', async ({ context, extensionId, baseURL }) => { + test.setTimeout(90000); + // Live ad churn wakes the media-frame monitor several times a second. Each + // wake used to schedule a trailing refresh that rebuilt the target + // unconditionally, and rebuilding produced more churn — a loop that never + // let the activation settle and pinned the popup on "activating". + const url = `${baseURL}/pages/yummy-churning-player.html`; + const page = await context.newPage(); + await page.goto(url); + await page.waitForFunction(() => window.__fixtureReady === true); + + const { tabId } = await selectTargetTab(context, extensionId, url); + + for (let sample = 0; sample < 8; sample++) { + await page.waitForTimeout(700); + const status = await getExtensionState(context, extensionId, { type: 'GET_STATUS' }); + expect(status, `sample ${sample} must not be stuck activating`).toMatchObject({ + targetTabId: tabId, + targetReady: true, + targetActivationState: 'ready' + }); + } + + // The player still has to be picked up while the churn continues. + const deferred = page.frames().find(frame => frame.url().endsWith('/frames/deferred-player-frame.html')); + await deferred.locator('#poster').click(); + await expect + .poll(() => deferred.locator('video').getAttribute('data-koala-attached'), { timeout: 20000 }) + .toBe('true'); + await expect + .poll(() => getExtensionState(context, extensionId, { type: 'GET_STATUS' }) + .then(state => state.targetActivationState), { timeout: 20000 }) + .toBe('ready'); +}); + test('keeps the tab selected when its activation fails', async ({ context, extensionId }) => { // A page the extension is not allowed to script stands in for any activation // failure the user can act on. Losing the selection here is what made the diff --git a/tests/e2e/fixtures/pages/yummy-churning-player.html b/tests/e2e/fixtures/pages/yummy-churning-player.html new file mode 100644 index 0000000..9500954 --- /dev/null +++ b/tests/e2e/fixtures/pages/yummy-churning-player.html @@ -0,0 +1,43 @@ + + +Anime page with live ad churn + + +
+

Series page

+
+
+ + +