From 981776d374ccc0f6adde573243f877eb68861715 Mon Sep 17 00:00:00 2001 From: Timo <6156589+Shik3i@users.noreply.github.com> Date: Wed, 26 Aug 2026 23:56:48 +0200 Subject: [PATCH] fix(sync): close episode lobby without ready race --- extension/background.js | 38 +++++++++++---- extension/content.js | 15 +++--- extension/episode-lobby-background.test.mjs | 51 +++++++++++++++++++++ tests/e2e/extension.spec.mjs | 10 +++- 4 files changed, 97 insertions(+), 17 deletions(-) create mode 100644 extension/episode-lobby-background.test.mjs diff --git a/extension/background.js b/extension/background.js index f6cdcec..4ac260a 100644 --- a/extension/background.js +++ b/extension/background.js @@ -2534,18 +2534,23 @@ async function handleServerEvent(event, data, expectedConnectionGeneration = con } break; case EVENTS.EPISODE_READY: - if (episodeLobby && data.senderId) { - if (!episodeLobby.readyPeers.includes(data.senderId)) { - episodeLobby.readyPeers.push(data.senderId); + { + const lobby = episodeLobby; + if (!lobby || !data.senderId) break; + let readyAdded = false; + if (!lobby.readyPeers.includes(data.senderId)) { + lobby.readyPeers.push(data.senderId); + readyAdded = true; persistEpisodeLobby(); broadcastLobbyUpdate(); - addLog(`Episode ready from ${data.senderId} (${episodeLobby.readyPeers.length})`, 'info'); - checkEpisodeLobbyCompletion(); + addLog(`Episode ready from ${data.senderId} (${lobby.readyPeers.length})`, 'info'); } + const readyPeers = [...lobby.readyPeers]; if (currentRoom?.activeLobby) { - currentRoom.activeLobby.readyPeers = [...episodeLobby.readyPeers]; + currentRoom.activeLobby.readyPeers = readyPeers; if (storageInitialized) chrome.storage.session.set({ currentRoom }); } + if (readyAdded) checkEpisodeLobbyCompletion(); } break; case EVENTS.EPISODE_LOBBY_CANCEL: @@ -2698,6 +2703,10 @@ function cancelEpisodeLobby(reason) { function executeEpisodeLobby() { if (!episodeLobby) return; const title = episodeLobby.expectedTitle; + if (currentRoom) { + currentRoom.activeLobby = null; + if (storageInitialized) chrome.storage.session.set({ currentRoom }); + } clearEpisodeLobbyState(); addLog(`Episode lobby complete: Starting "${title}" via Force Sync`, 'success'); @@ -5259,15 +5268,24 @@ async function handleAsyncMessage(message, sender, sendResponse) { } } // Content script confirmed it loaded the lobby episode - if (episodeLobby && message.payload && sameEpisode(message.payload.title, episodeLobby.expectedTitle)) { - if (!episodeLobby.readyPeers.includes(peerId)) { + const lobby = episodeLobby; + if (lobby && message.payload && sameEpisode(message.payload.title, lobby.expectedTitle)) { + if (!lobby.readyPeers.includes(peerId)) { const settings = await getSettings(); + if (episodeLobby !== lobby) { + sendResponse({ status: 'ignored_stale_lobby' }); + return; + } + if (lobby.readyPeers.includes(peerId)) { + sendResponse({ status: 'ok' }); + return; + } const readyTitle = sanitizeSharedTitle(message.payload.title, settings.mediaTitlePrivacyMode); - episodeLobby.readyPeers.push(peerId); + lobby.readyPeers.push(peerId); persistEpisodeLobby(); broadcastLobbyUpdate(); emit(EVENTS.EPISODE_READY, { peerId, title: readyTitle }); - addLog(`Local episode ready: "${readyTitle || episodeLobby.expectedTitle}"`, 'success'); + addLog(`Local episode ready: "${readyTitle || lobby.expectedTitle}"`, 'success'); checkEpisodeLobbyCompletion(); } } diff --git a/extension/content.js b/extension/content.js index 58c443a..cb5f64d 100644 --- a/extension/content.js +++ b/extension/content.js @@ -1352,25 +1352,28 @@ // starting. Both paths reuse the same site/page-API abstractions and // native-event suppression as ordinary remote commands. if (mediaState.playbackState === 'paused' && !video.paused) { - if (!await tryMediaAction(EVENTS.PAUSE)) { + const pauseApplied = await tryMediaAction(EVENTS.PAUSE); + if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); + if (!pauseApplied) { return { status: 'apply_failed', reason: 'pause_action_failed' }; } - if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); } if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); if (shouldSeek) { _setSuppress('seek'); - if (!await tryMediaAction(EVENTS.SEEK, { targetTime: mediaState.currentTime })) { + const seekApplied = await tryMediaAction(EVENTS.SEEK, { targetTime: mediaState.currentTime }); + if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); + if (!seekApplied) { return { status: 'apply_failed', reason: 'seek_action_failed' }; } - if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); } if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); if (mediaState.playbackState === 'playing' && video.paused) { - if (!await tryMediaAction(EVENTS.PLAY)) { + const playApplied = await tryMediaAction(EVENTS.PLAY); + if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); + if (!playApplied) { return { status: 'apply_failed', reason: 'play_action_failed' }; } - if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); } const verified = await pollCanonicalMediaState(mediaState, startedAt, applyGeneration); if (!isCanonicalMediaApplyCurrent(applyGeneration)) return superseded(); diff --git a/extension/episode-lobby-background.test.mjs b/extension/episode-lobby-background.test.mjs new file mode 100644 index 0000000..c8e3035 --- /dev/null +++ b/extension/episode-lobby-background.test.mjs @@ -0,0 +1,51 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const extensionDir = path.dirname(fileURLToPath(import.meta.url)); +const backgroundSource = fs.readFileSync(path.join(extensionDir, 'background.js'), 'utf8'); + +function sourceBetween(startNeedle, endNeedle) { + const start = backgroundSource.indexOf(startNeedle); + const end = backgroundSource.indexOf(endNeedle, start + startNeedle.length); + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + return backgroundSource.slice(start, end); +} + +describe('episode lobby completion races', () => { + it('does not read the cleared lobby after a remote ready completes it', () => { + const handler = sourceBetween('case EVENTS.EPISODE_READY:', 'case EVENTS.EPISODE_LOBBY_CANCEL:'); + const snapshotIndex = handler.indexOf('const readyPeers = [...lobby.readyPeers]'); + const roomUpdateIndex = handler.indexOf('currentRoom.activeLobby.readyPeers = readyPeers'); + const completionIndex = handler.indexOf('checkEpisodeLobbyCompletion()'); + + expect(snapshotIndex).toBeGreaterThan(-1); + expect(roomUpdateIndex).toBeGreaterThan(snapshotIndex); + expect(completionIndex).toBeGreaterThan(roomUpdateIndex); + expect(handler.slice(completionIndex)).not.toContain('episodeLobby.readyPeers'); + }); + + it('revalidates the lobby after awaiting settings for a local ready', () => { + const handler = sourceBetween("message.type === 'EPISODE_READY_LOCAL'", "message.type === 'TITLE_PRIVACY_CHANGED'"); + const awaitIndex = handler.indexOf('const settings = await getSettings()'); + const guardIndex = handler.indexOf('if (episodeLobby !== lobby)'); + const mutationIndex = handler.indexOf('lobby.readyPeers.push(peerId)'); + + expect(handler).toContain('const lobby = episodeLobby'); + expect(awaitIndex).toBeGreaterThan(-1); + expect(guardIndex).toBeGreaterThan(awaitIndex); + expect(mutationIndex).toBeGreaterThan(guardIndex); + }); + + it('clears the persisted room lobby when completion starts Force Sync', () => { + const execute = sourceBetween('function executeEpisodeLobby()', 'function checkEpisodeLobbyCompletion()'); + const roomClearIndex = execute.indexOf('currentRoom.activeLobby = null'); + const lobbyClearIndex = execute.indexOf('clearEpisodeLobbyState()'); + + expect(roomClearIndex).toBeGreaterThan(-1); + expect(lobbyClearIndex).toBeGreaterThan(roomClearIndex); + expect(execute).toContain('chrome.storage.session.set({ currentRoom })'); + }); +}); diff --git a/tests/e2e/extension.spec.mjs b/tests/e2e/extension.spec.mjs index 4f916c7..6f0bd01 100644 --- a/tests/e2e/extension.spec.mjs +++ b/tests/e2e/extension.spec.mjs @@ -451,7 +451,15 @@ test('local media input cancels an in-flight canonical recovery', async ({ conte value: () => { const attempts = Number(video.dataset.koalaDelayedPlayAttempts || '0') + 1; video.dataset.koalaDelayedPlayAttempts = String(attempts); - return nativePlay().then(() => new Promise(resolve => setTimeout(resolve, 400))); + return nativePlay().then(() => new Promise((resolve, reject) => setTimeout(() => { + if (video.paused) { + const error = new Error('play interrupted by local pause'); + error.name = 'AbortError'; + reject(error); + } else { + resolve(); + } + }, 400))); } }); }