diff --git a/extension/background.js b/extension/background.js index bc75cec..14d279a 100644 --- a/extension/background.js +++ b/extension/background.js @@ -91,6 +91,7 @@ let activeTargetActivation = null; // through webNavigation; this registry rebuilds the same knowledge from // sender.frameId, which every content script hands us for free. const knownFrameIdsByTab = new Map(); +const MAX_KNOWN_FRAMES_PER_TAB = 24; function rememberFrameId(tabId, frameId) { const normalizedTabId = normalizeTabId(tabId); @@ -101,11 +102,14 @@ function rememberFrameId(tabId, frameId) { 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); + // Every id in here is probed individually when a sweep is rejected, so the + // set is also a cost ceiling. Keep it small: a page with more media-bearing + // frames than this is not a player page, and the sweep still covers the + // normal case. The top frame is never evicted. + while (frames.size > MAX_KNOWN_FRAMES_PER_TAB) { + const evictable = Array.from(frames).find(candidate => candidate !== 0); + if (evictable === undefined) break; + frames.delete(evictable); } } @@ -2262,7 +2266,14 @@ function setPageApiSeekEnabled(enabled) { } async function deactivateMediaFrameMonitors(tabId) { - const targets = listMediaFrameScriptTargets(tabId); + // Same reasoning as the injection: a rejected sweep must not leave a monitor + // running in a deep frame, or the old target keeps reporting after a switch. + const targets = [ + ...listMediaFrameScriptTargets(tabId), + ...listKnownFrameIds(tabId) + .filter(frameId => frameId !== 0) + .map(frameId => ({ tabId, frameIds: [frameId] })) + ]; await Promise.all(targets.map(async target => { const documentId = target.documentIds?.[0]; const frameId = target.frameIds?.[0]; @@ -2405,7 +2416,15 @@ function executeScriptWithTimeout(options, timeoutMs = SCRIPT_INJECTION_TIMEOUT_ } async function injectMediaFrameMonitors(tabId, contentTarget) { - const targets = listMediaFrameScriptTargets(tabId); + // The sweep is best effort; the known frames are addressed individually so a + // rejected sweep cannot leave the deep player frame without a monitor — and + // therefore without any way to report itself later. + const targets = [ + ...listMediaFrameScriptTargets(tabId), + ...listKnownFrameIds(tabId) + .filter(frameId => frameId !== 0) + .map(frameId => ({ tabId, frameIds: [frameId] })) + ]; let injectedCount = 0; await Promise.all(targets.map(async target => { try { @@ -2463,6 +2482,13 @@ async function injectContentScript(tabId, { contentTarget = await resolveMediaContentTarget(chrome, tabId, { knownFrameIds: listKnownFrameIds(tabId) }); + // The probe reached these frames; remember them before anything is + // injected. Waiting for a content script to message us first would leave + // the registry empty exactly when it is needed most — the first + // activation, before any script exists to report itself. + for (const frameId of contentTarget.discoveredFrameIds || []) { + rememberFrameId(tabId, frameId); + } if (!isTargetActivationSuperseded(tabId, activationGeneration) && activeTargetActivation?.tabId === tabId) { activeTargetActivation.frameId = contentTarget.frameId; @@ -3048,6 +3074,9 @@ async function selectedMediaTargetMoved(tabId) { attempts: 1, knownFrameIds: listKnownFrameIds(tabId) }); + for (const frameId of resolved.discoveredFrameIds || []) { + rememberFrameId(tabId, frameId); + } } catch { // An access-required error must reach the full activation path so the // popup can surface it. @@ -3079,7 +3108,9 @@ function refreshCurrentMediaTarget(tabId, { queueIfRunning = false, onlyIfTarget return mediaTargetRefreshTask; } if (activeTargetActivation?.tabId === selectedTabId) { - return Promise.resolve({ status: 'activation_in_progress' }); + if (!expireStuckActivation()) { + return Promise.resolve({ status: 'activation_in_progress' }); + } } const task = (async () => { @@ -3189,6 +3220,14 @@ async function retryPendingTarget({ expectedRequestId = null, requireGrantedAcce } } +chrome.tabs.onUpdated.addListener((tabId, changeInfo) => { + // A committed navigation replaces every frame in the tab. Keeping the old + // ids would mean probing dead frames on every resolve from then on. + if (changeInfo.status === 'loading' && typeof changeInfo.url === 'string') { + forgetFrameIds(tabId); + } +}); + if (chrome.permissions?.onAdded?.addListener) { chrome.permissions.onAdded.addListener((addedPermissions) => { ensureState().then(async () => { diff --git a/extension/media-frame-target.js b/extension/media-frame-target.js index d4871e8..3d7284a 100644 --- a/extension/media-frame-target.js +++ b/extension/media-frame-target.js @@ -167,10 +167,16 @@ export function inspectMediaFrame(expectedVisibilityToken = null) { const rect = frame.getBoundingClientRect(); const directVisible = elementIsVisible(frame, rect); const visible = ancestorVisible && directVisible; + // An iframe without src resolves to the *parent* document's URL, so + // record whether the element really carried one. Treating a srcless + // ad slot's href as its own would let a hidden slot mark the page + // that contains it as hidden. + const rawSrc = frame.getAttribute?.('src') || ''; let href = ''; try { href = new URL(frame.src || '', doc.location.href).href; } catch { href = ''; } embeddedFrames.push({ href, + explicitSrc: rawSrc.trim().length > 0, origin: (() => { try { return new URL(href).origin; } catch { return null; } })(), area: Math.max(0, rect.width) * Math.max(0, rect.height), width: Math.max(0, rect.width), @@ -347,6 +353,10 @@ function hiddenFrameHrefs(injectionResults) { for (const entry of Array.isArray(injectionResults) ? injectionResults : []) { for (const frame of entry?.result?.embeddedFrames || []) { if (typeof frame?.href !== 'string' || !frame.href) continue; + // Without an explicit src the href is the parent's, not this frame's. + if (frame.explicitSrc !== true) continue; + // A frame cannot testify about the document that reported it. + if (frame.href === entry?.result?.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); } @@ -442,11 +452,14 @@ function accessRequiredError(access) { return error; } -function contentTarget(tabId, selected) { +function contentTarget(tabId, selected, discoveredFrameIds = null) { const frameId = normalizeFrameId(selected?.frameId); const documentId = typeof selected?.documentId === 'string' ? selected.documentId : null; return { frameId, + // Every frame this probe reached, so the caller can remember them and + // recover directly next time the all-frames sweep is rejected. + discoveredFrameIds: Array.isArray(discoveredFrameIds) ? discoveredFrameIds : [], documentId, frameUrl: typeof selected?.result?.href === 'string' ? selected.result.href : null, hasVideo: !!selected?.result?.bestVideo, @@ -567,6 +580,7 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { let missingAccess = null; let ambiguous = false; let unresolvedGrantedHost = null; + const discovered = new Set(Array.isArray(knownFrameIds) ? knownFrameIds : []); for (let attempt = 0; attempt < attempts; attempt++) { const scriptTargets = listMediaFrameScriptTargets(tabId); @@ -664,6 +678,9 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { } } + for (const entry of results) { + if (Number.isInteger(entry?.frameId)) discovered.add(entry.frameId); + } const selected = selectMediaFrame(results); const videoCandidates = results.filter(entry => entry?.result?.bestVideo?.rendered === true && entry.result.parentFrameVisible !== false); @@ -688,7 +705,7 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { if (selected.result.bestVideo.hasSource && selected.result.bestVideo.rendered && !shouldPreferMissingAccess(currentMissingAccess, selected)) { - return contentTarget(tabId, selected); + return contentTarget(tabId, selected, Array.from(discovered)); } } @@ -701,17 +718,17 @@ export async function resolveMediaContentTarget(chromeApi, tabId, { } if (missingAccess) throw accessRequiredError(missingAccess); - if (fallback) return contentTarget(tabId, fallback); + if (fallback) return contentTarget(tabId, fallback, Array.from(discovered)); // A player whose origin is already granted but which never answered is a // timing problem, not a user decision. Keep the tab selected on its top // frame so the injected monitor can promote the real player once it loads, // instead of failing the activation or prompting for nothing. - if (unresolvedGrantedHost) return contentTarget(tabId, null); + if (unresolvedGrantedHost) return contentTarget(tabId, null, Array.from(discovered)); // Several equally-ranked players — anime mirrors, alternative dubs — are a // normal page layout, not an error. Refusing to activate made those pages // unusable, and flipping between candidates restarted the target forever. // Hold the top frame and let the monitor promote the one that starts // playing, which is the signal that breaks the tie. - if (ambiguous) return { ...contentTarget(tabId, null), ambiguous: true }; - return contentTarget(tabId, null); + if (ambiguous) return { ...contentTarget(tabId, null, Array.from(discovered)), ambiguous: true }; + return contentTarget(tabId, null, Array.from(discovered)); } diff --git a/extension/media-frame-target.test.mjs b/extension/media-frame-target.test.mjs index 7539456..759db2e 100644 --- a/extension/media-frame-target.test.mjs +++ b/extension/media-frame-target.test.mjs @@ -108,6 +108,63 @@ describe('cross-origin media-frame targeting', () => { expect(selected.frameId).toBe(0); }); + it('ignores a hidden srcless ad slot that resolves to another frame url', () => { + // An iframe without src resolves to its parent document's URL. A hidden + // ad slot must therefore never be able to declare a real player hidden. + const selected = selectMediaFrame([ + frame(0, { + bestVideo: null, + videoCount: 0, + embeddedFrames: [{ + href: 'https://player-5.example/embed', + explicitSrc: false, + visible: false, + area: 0, + width: 0, + height: 0, + depth: 1, + mediaHint: false + }] + }), + frame(5, { parentFrameVisible: null }) + ]); + expect(selected?.frameId).toBe(5); + }); + + it('drops a player parked inside a collapsed same-origin wrapper', () => { + const selected = selectMediaFrame([ + frame(0, { + bestVideo: null, + videoCount: 0, + embeddedFrames: [ + { + href: 'https://player-5.example/embed', + explicitSrc: true, + visible: true, + area: 830 * 498, + width: 830, + height: 498, + depth: 2, + mediaHint: true + }, + { + href: 'https://player-6.example/embed', + explicitSrc: true, + visible: false, + area: 0, + width: 0, + height: 0, + depth: 2, + mediaHint: true + } + ] + }), + frame(5, { parentFrameVisible: null }), + frame(6, { parentFrameVisible: null }) + ]); + expect(selected?.frameId).toBe(5); + }); + it('refuses to guess between equally-ranked frames without visibility evidence', () => { expect(selectMediaFrame([ frame(3, { parentFrameVisible: null }), @@ -127,6 +184,9 @@ describe('cross-origin media-frame targeting', () => { documentId: 'document-8', frameUrl: 'https://player-8.example/embed', hasVideo: true, + // Reported back so the caller can address these frames directly when + // a later all-frames sweep is rejected wholesale. + discoveredFrameIds: [0, 8], scriptTarget: { tabId: 42, documentIds: ['document-8'] } }); const visibilityDispatches = executeScript.mock.calls.filter(([options]) => ( @@ -153,6 +213,7 @@ describe('cross-origin media-frame targeting', () => { documentId: null, frameUrl: null, hasVideo: false, + discoveredFrameIds: [0, 6], scriptTarget: { tabId: 42 } }); }); @@ -492,6 +553,7 @@ describe('embedded player access diagnosis', () => { documentId: null, frameUrl: null, hasVideo: false, + discoveredFrameIds: [0], scriptTarget: { tabId: 42 } }); expect(contains).toHaveBeenCalledWith({ diff --git a/extension/target-tab-lifecycle.test.mjs b/extension/target-tab-lifecycle.test.mjs index 1d83745..c026fd4 100644 --- a/extension/target-tab-lifecycle.test.mjs +++ b/extension/target-tab-lifecycle.test.mjs @@ -59,6 +59,17 @@ describe('target tab lifecycle', () => { expect(monitorSource).toContain('element.readyState > 0 ? 1 : 0'); }); + it('keeps the frame registry bounded and free of a navigation permission', () => { + expect(backgroundSource).toContain('const MAX_KNOWN_FRAMES_PER_TAB = 24'); + // Frame ids are learned, never enumerated through a permission. + expect(backgroundSource).toContain('function rememberFrameId(tabId, frameId)'); + expect(backgroundSource).toContain('rememberFrameId(senderTabId, sender?.frameId)'); + expect(backgroundSource).toContain('contentTarget.discoveredFrameIds'); + // A committed navigation invalidates every id from the previous page. + expect(backgroundSource).toMatch(/changeInfo\.status === 'loading'[\s\S]{0,120}forgetFrameIds\(tabId\)/); + expect(backgroundSource).not.toMatch(/chrome\.webNavigation/); + }); + it('bounds every frame probe and verifies withheld origins', () => { const resolverSource = fs.readFileSync( path.join(extensionDir, 'media-frame-target.js'), @@ -90,7 +101,10 @@ describe('target tab lifecycle', () => { it('uses all-frame probing for cross-origin targets without navigation permissions', () => { expect(backgroundSource).toContain("files: ['media-frame-monitor.js']"); - expect(backgroundSource).toContain('const targets = listMediaFrameScriptTargets(tabId)'); + // Monitors must reach the frames we know about, not only whatever the + // all-frames sweep happens to accept — both on the way in and out. + expect(backgroundSource.match(/\.\.\.listMediaFrameScriptTargets\(tabId\),/g)?.length).toBe(2); + expect(backgroundSource.match(/\.\.\.listKnownFrameIds\(tabId\)/g)?.length).toBe(2); expect(backgroundSource).toContain('One denied widget frame must not block the selected player'); expect(backgroundSource).toContain("navigationError.code = 'media_target_navigated'"); expect(backgroundSource).toContain("{ type: 'MEDIA_MONITOR_DEACTIVATE' }");