diff --git a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts index 354fff67009..f2cbc042a20 100644 --- a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts +++ b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { Mock } from 'vitest' import { cancelScheduledHiddenOutputRestore, @@ -6,6 +7,11 @@ import { scheduleHiddenOutputRestore } from './hidden-output-restore-scheduler' +/** A pane that actually replays scrollback when its turn comes. */ +const replaying = (): Mock<() => boolean> => vi.fn(() => true) +/** A pane whose guards decline — hidden, disposed, or superseded restore. */ +const declining = (): Mock<() => boolean> => vi.fn(() => false) + describe('hidden output restore scheduler', () => { beforeEach(() => { vi.useFakeTimers() @@ -19,7 +25,7 @@ describe('hidden output restore scheduler', () => { it('runs active restores immediately', () => { const target = {} - const requestRestore = vi.fn() + const requestRestore = replaying() scheduleHiddenOutputRestore(target, requestRestore, 'active') @@ -27,8 +33,8 @@ describe('hidden output restore scheduler', () => { }) it('spreads inactive restores across timer ticks', () => { - const firstRestore = vi.fn() - const secondRestore = vi.fn() + const firstRestore = replaying() + const secondRestore = replaying() scheduleHiddenOutputRestore({}, firstRestore, 'inactive') scheduleHiddenOutputRestore({}, secondRestore, 'inactive') @@ -46,8 +52,8 @@ describe('hidden output restore scheduler', () => { it('cancels pending inactive restore when a target is promoted', () => { const target = {} - const inactiveRestore = vi.fn() - const activeRestore = vi.fn() + const inactiveRestore = replaying() + const activeRestore = replaying() scheduleHiddenOutputRestore(target, inactiveRestore, 'inactive') scheduleHiddenOutputRestore(target, activeRestore, 'active') @@ -59,7 +65,7 @@ describe('hidden output restore scheduler', () => { it('can cancel pending inactive restores', () => { const target = {} - const requestRestore = vi.fn() + const requestRestore = replaying() scheduleHiddenOutputRestore(target, requestRestore, 'inactive') cancelScheduledHiddenOutputRestore(target) @@ -67,4 +73,69 @@ describe('hidden output restore scheduler', () => { expect(requestRestore).not.toHaveBeenCalled() }) + + it('does not charge a frame to panes that replay nothing', () => { + const hiddenPanes = [declining(), declining(), declining()] + const visibleRestore = replaying() + + for (const hiddenPane of hiddenPanes) { + scheduleHiddenOutputRestore({}, hiddenPane, 'inactive') + } + scheduleHiddenOutputRestore({}, visibleRestore, 'inactive') + + vi.advanceTimersByTime(16) + + for (const hiddenPane of hiddenPanes) { + expect(hiddenPane).toHaveBeenCalledTimes(1) + } + expect(visibleRestore).toHaveBeenCalledTimes(1) + }) + + it('keeps one replay per frame once a queued pane replays', () => { + const firstRestore = replaying() + const declined = declining() + const secondRestore = replaying() + + scheduleHiddenOutputRestore({}, firstRestore, 'inactive') + scheduleHiddenOutputRestore({}, declined, 'inactive') + scheduleHiddenOutputRestore({}, secondRestore, 'inactive') + + vi.advanceTimersByTime(16) + expect(firstRestore).toHaveBeenCalledTimes(1) + expect(declined).not.toHaveBeenCalled() + expect(secondRestore).not.toHaveBeenCalled() + + vi.advanceTimersByTime(16) + expect(declined).toHaveBeenCalledTimes(1) + expect(secondRestore).toHaveBeenCalledTimes(1) + }) + + it('drops declining panes instead of retrying them', () => { + const declined = declining() + + scheduleHiddenOutputRestore({}, declined, 'inactive') + vi.advanceTimersByTime(16) + vi.advanceTimersByTime(160) + + expect(declined).toHaveBeenCalledTimes(1) + expect(vi.getTimerCount()).toBe(0) + }) + + it('does not re-enter a pane queued by a restore that ran in the same drain', () => { + const requeued = declining() + const target = {} + const reschedulingRestore = vi.fn(() => { + scheduleHiddenOutputRestore(target, requeued, 'inactive') + return false + }) + + scheduleHiddenOutputRestore({}, reschedulingRestore, 'inactive') + vi.advanceTimersByTime(16) + + expect(reschedulingRestore).toHaveBeenCalledTimes(1) + expect(requeued).not.toHaveBeenCalled() + + vi.advanceTimersByTime(16) + expect(requeued).toHaveBeenCalledTimes(1) + }) }) diff --git a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts index 909dfe29d59..df22453e59d 100644 --- a/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts +++ b/src/renderer/src/components/terminal-pane/hidden-output-restore-scheduler.ts @@ -1,6 +1,7 @@ type HiddenOutputRestorePriority = 'active' | 'inactive' -type HiddenOutputRestoreRequest = () => void +/** Returns whether the pane actually started a replay; a guard-only return is free. */ +type HiddenOutputRestoreRequest = () => boolean type HiddenOutputRestoreEntry = { requestRestore: HiddenOutputRestoreRequest @@ -30,13 +31,22 @@ function scheduleInactiveRestoreDrain(): void { function drainInactiveRestoreQueue(): void { inactiveRestoreTimer = null - const next = inactiveRestoreQueue.entries().next() - if (next.done) { - return + // Why the loop: an entry whose pane went hidden, was disposed, or had its restore + // superseded replays nothing, so charging it a whole frame only delays the next + // on-screen pane. Still at most one real replay per frame; the skips are guard reads. + let remaining = inactiveRestoreQueue.size + while (remaining > 0) { + remaining -= 1 + const next = inactiveRestoreQueue.entries().next() + if (next.done) { + break + } + const [target, entry] = next.value + inactiveRestoreQueue.delete(target) + if (entry.requestRestore()) { + break + } } - const [target, entry] = next.value - inactiveRestoreQueue.delete(target) - entry.requestRestore() scheduleInactiveRestoreDrain() } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-request.ts b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-request.ts index f245c9c16f2..08946421c5d 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-request.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/hidden-output-restore-request.ts @@ -68,7 +68,7 @@ export function bindHiddenOutputRestoreRequest(session: ConnectPanePtySession): // Why: resume can reveal many split panes at once; spread inactive replays across frames so xterm scrollback replay doesn't block return. scheduleHiddenOutputRestore( session.pane.terminal, - () => { + (): boolean => { session.hiddenOutputRestoreScheduled = false if ( session.disposed || @@ -80,9 +80,11 @@ export function bindHiddenOutputRestoreRequest(session: ConnectPanePtySession): session.hiddenOutputRestorePendingChunks.length === 0) || !shouldWritePtyOutputForeground(session.deps.isVisibleRef.current) ) { - return + // Why report false: nothing replayed here, so the scheduler can spend + // this frame on the next queued pane instead of on a hidden/stale one. + return false } - session.requestHiddenOutputRestoreIfNeeded({ bypassScheduler: true }) + return session.requestHiddenOutputRestoreIfNeeded({ bypassScheduler: true }) === true }, priority )