fix: stabilize media progress and cancel controls

This commit is contained in:
NimBold
2026-07-17 17:12:56 +03:30
parent f4b830414d
commit db39cd2153
11 changed files with 372 additions and 44 deletions
+119 -9
View File
@@ -1320,10 +1320,12 @@ fn aggregate_media_byte_progress(
) -> Option<(u64, u64, bool)> {
if track_changed {
if let Some(total_bytes) = state.current_track_total_bytes {
let completed_downloaded = state
.current_track_downloaded_bytes
.unwrap_or(total_bytes)
.clamp(0.0, total_bytes.max(0.0));
let completed_downloaded = state.current_track_downloaded_bytes.unwrap_or(total_bytes);
let completed_downloaded = if state.current_track_total_is_estimate {
completed_downloaded.max(0.0)
} else {
completed_downloaded.clamp(0.0, total_bytes.max(0.0))
};
*state
.completed_tracks_total_bytes
.get_or_insert(0.0) += total_bytes;
@@ -1339,14 +1341,38 @@ fn aggregate_media_byte_progress(
state.current_track_total_bytes = None;
state.current_track_total_is_estimate = false;
}
state.current_track_total_bytes = progress.total_bytes;
// yt-dlp's estimated total is a moving bitrate-based guess. Keep the
// first total observed for a stream stable so the UI does not make the
// download's size appear to change on every progress update. A new media
// track gets its own stable total when split audio/video downloads switch
// tracks.
let next_total = progress
.total_bytes
.filter(|total_bytes| total_bytes.is_finite() && *total_bytes > 0.0);
if state.current_track_total_bytes.is_none()
|| (state.current_track_total_is_estimate && !progress.total_is_estimate)
{
if let Some(total_bytes) = next_total {
state.current_track_total_bytes = Some(total_bytes);
state.current_track_total_is_estimate = progress.total_is_estimate;
}
}
let stable_total_bytes = state.current_track_total_bytes;
state.current_track_downloaded_bytes = progress
.downloaded_bytes
.or_else(|| progress.total_bytes.map(|total| total * progress.fraction))
.zip(progress.total_bytes)
.map(|(downloaded, total)| downloaded.clamp(0.0, total.max(0.0)))
.zip(stable_total_bytes)
.map(|(downloaded, total)| {
// An estimated total is only a display anchor. yt-dlp can
// legitimately download more bytes than that first estimate,
// so never turn a moving estimate into a false 100% byte count.
if state.current_track_total_is_estimate {
downloaded.max(0.0)
} else {
downloaded.clamp(0.0, total.max(0.0))
}
})
.or(progress.downloaded_bytes);
state.current_track_total_is_estimate = progress.total_is_estimate;
match (
state.completed_tracks_downloaded_bytes,
@@ -1388,6 +1414,12 @@ fn emit_media_progress(
}
let byte_progress = aggregate_media_byte_progress(&progress, track_changed, state);
let (speed, eta) = media_progress_speed(&progress, Instant::now(), &mut state.speed_sampler);
let size = byte_progress
.map(|(_, total, total_is_estimate)| {
let prefix = if total_is_estimate { "~" } else { "" };
format!("{prefix}{}", crate::download::format_size(total as f64))
})
.or(progress.size);
let now = Instant::now();
if now.duration_since(state.last_progress_at) >= MEDIA_PROGRESS_EMIT_INTERVAL {
@@ -1398,7 +1430,7 @@ fn emit_media_progress(
fraction: overall_fraction,
speed,
eta,
size: progress.size,
size,
size_is_final: false,
downloaded_bytes: byte_progress.map(|value| value.0 as f64),
total_bytes: byte_progress.map(|value| value.1 as f64),
@@ -7641,6 +7673,84 @@ mod tests {
);
}
#[test]
fn freezes_a_media_track_estimate_across_progress_updates() {
let mut state = MediaProgressEmitterState::new();
let first = MediaProgress {
fraction: 0.25,
speed: "-".to_string(),
eta: "-".to_string(),
size: Some("~100B".to_string()),
downloaded_bytes: Some(25.0),
total_bytes: Some(100.0),
total_is_estimate: true,
};
let later = MediaProgress {
fraction: 0.75,
speed: "-".to_string(),
eta: "-".to_string(),
size: Some("~200B".to_string()),
downloaded_bytes: Some(150.0),
total_bytes: Some(200.0),
total_is_estimate: true,
};
assert_eq!(
aggregate_media_byte_progress(&first, false, &mut state),
Some((25, 100, true))
);
assert_eq!(
aggregate_media_byte_progress(&later, false, &mut state),
Some((150, 100, true))
);
let exact = MediaProgress {
fraction: 0.9,
speed: "-".to_string(),
eta: "-".to_string(),
size: Some("200B".to_string()),
downloaded_bytes: Some(180.0),
total_bytes: Some(200.0),
total_is_estimate: false,
};
assert_eq!(
aggregate_media_byte_progress(&exact, false, &mut state),
Some((180, 200, false))
);
}
#[test]
fn preserves_estimated_bytes_when_a_split_track_exceeds_its_estimate() {
let mut state = MediaProgressEmitterState::new();
let first = MediaProgress {
fraction: 0.75,
speed: "-".to_string(),
eta: "-".to_string(),
size: Some("~100B".to_string()),
downloaded_bytes: Some(150.0),
total_bytes: Some(100.0),
total_is_estimate: true,
};
let second = MediaProgress {
fraction: 0.01,
speed: "-".to_string(),
eta: "-".to_string(),
size: Some("50B".to_string()),
downloaded_bytes: Some(1.0),
total_bytes: Some(50.0),
total_is_estimate: false,
};
assert_eq!(
aggregate_media_byte_progress(&first, false, &mut state),
Some((150, 100, true))
);
assert_eq!(
aggregate_media_byte_progress(&second, true, &mut state),
Some((151, 150, true))
);
}
#[test]
fn derives_main_window_speed_from_downloaded_byte_delta() {
let first = MediaProgress {
+1 -1
View File
@@ -198,7 +198,7 @@ function App() {
<span>{actionLabel} in 10 seconds.</span>
<button
type="button"
className="app-button px-2 py-1"
className="app-button app-button-cancel px-2 py-1"
onClick={cancel}
>
Cancel
+1 -1
View File
@@ -90,7 +90,7 @@ export const DeleteConfirmationModal: React.FC = () => {
<button
onClick={handleCancel}
disabled={isRemoving}
className="px-4 py-2 rounded-lg text-sm font-medium transition-colors text-text-secondary hover:bg-item-hover hover:text-text-primary disabled:opacity-50"
className="app-button app-button-cancel px-4 py-2 rounded-lg text-sm font-medium disabled:opacity-50"
>
Cancel
</button>
+18 -18
View File
@@ -85,25 +85,25 @@ export const DownloadItem = React.memo<DownloadItemProps>(({
</span>
</div>
<div className="download-cell-truncate">
<span
className="tabular-nums"
title={hasDownloadedAmount
? `${sizeDisplay.downloaded} downloaded of ${sizeDisplay.totalIsEstimate ? '~' : ''}${sizeDisplay.total} ${sizeDisplay.unit}`
<div
className="download-cell-truncate download-size-cell tabular-nums"
title={hasDownloadedAmount
? `${sizeDisplay.downloaded} downloaded of ${sizeDisplay.totalIsEstimate ? '~' : ''}${sizeDisplay.total} ${sizeDisplay.unit}`
: completedSizeLabel}
aria-label={hasDownloadedAmount
? `${sizeDisplay.downloaded} downloaded of ${sizeDisplay.totalIsEstimate ? 'approximately ' : ''}${sizeDisplay.total} ${sizeDisplay.unit}`
: completedSizeLabel}
>
{hasDownloadedAmount ? (
<span className="download-size-progress">
<span className={downloadProgressColorClass(download.status)}>{sizeDisplay.downloaded}</span>
<span className="text-text-muted"> / </span>
</span>
) : null}
<span className="download-size-total">
{hasDownloadedAmount
? `${sizeDisplay.totalIsEstimate ? '~' : ''}${sizeDisplay.total} ${sizeDisplay.unit}`
: completedSizeLabel}
aria-label={hasDownloadedAmount
? `${sizeDisplay.downloaded} downloaded of ${sizeDisplay.totalIsEstimate ? 'approximately ' : ''}${sizeDisplay.total} ${sizeDisplay.unit}`
: completedSizeLabel}
>
{hasDownloadedAmount ? (
<>
<span className={downloadProgressColorClass(download.status)}>{sizeDisplay.downloaded}</span>
<span className="text-text-muted"> / </span>
<span>
{sizeDisplay.totalIsEstimate ? '~' : ''}{sizeDisplay.total} {sizeDisplay.unit}
</span>
</>
) : completedSizeLabel}
</span>
</div>
+1 -1
View File
@@ -538,7 +538,7 @@ export const DownloadTable: React.FC<DownloadTableProps> = ({ filter }) => {
className={`${index === 5 ? 'download-cell-right' : ''} cursor-pointer hover:text-text-primary transition-colors flex items-center justify-between`}
onClick={() => handleSort(label as DownloadSortColumn)}
>
<div className="flex items-center gap-1 w-full h-full select-none">
<div className={`flex items-center gap-1 w-full h-full select-none ${index === 1 ? 'justify-end' : ''}`}>
<span>{label}</span>
{(isQueueFilter ? queueSortConfig : sortConfig)?.column === label && (
(isQueueFilter ? queueSortConfig : sortConfig)?.direction === 'asc'
+1 -1
View File
@@ -68,7 +68,7 @@ export const DuplicateResolutionModal = ({ conflicts: initialConflicts, onConfir
</div>
<div className="p-4 border-t border-border-modal flex items-center justify-between bg-sidebar-bg/50">
<button onClick={onCancel} className="app-button border-transparent bg-transparent px-4 text-xs text-text-secondary">
<button onClick={onCancel} className="app-button app-button-cancel px-4 text-xs">
Cancel
</button>
<button
+1 -1
View File
@@ -454,7 +454,7 @@ export const PropertiesModal = () => {
<div className="flex gap-2">
<button
onClick={() => setSelectedPropertiesDownloadId(null)}
className="app-button border-transparent bg-transparent px-4 text-xs text-text-secondary"
className="app-button app-button-cancel px-4 text-xs"
>
Cancel
</button>
+86 -2
View File
@@ -441,7 +441,7 @@ html[data-list-density="relaxed"] {
color: hsl(var(--text-primary));
font-weight: 500;
font-size: 13px;
transition: background-color 100ms ease, transform 100ms ease;
transition: background-color 100ms ease, border-color 100ms ease, box-shadow 100ms ease, transform 100ms ease;
box-shadow: 0 1px 1px hsl(var(--shadow-color));
}
@@ -464,6 +464,28 @@ html[data-list-density="relaxed"] {
background: hsl(var(--accent-color) / 0.9);
}
.app-button-cancel {
border-color: hsl(var(--border-modal));
background:
linear-gradient(180deg, hsl(0 0% 100% / 0.055), transparent),
hsl(var(--bg-input) / 0.88);
color: hsl(var(--text-secondary));
box-shadow:
inset 0 1px 0 hsl(0 0% 100% / 0.055),
0 1px 2px hsl(var(--shadow-color));
}
.app-button-cancel:hover:not(:disabled) {
border-color: hsl(var(--text-muted) / 0.65);
background:
linear-gradient(hsl(var(--item-hover)), hsl(var(--item-hover))),
hsl(var(--bg-input));
color: hsl(var(--text-primary));
box-shadow:
inset 0 1px 0 hsl(0 0% 100% / 0.07),
0 2px 5px hsl(var(--shadow-color));
}
.app-icon-button {
display: inline-flex;
width: 28px;
@@ -844,12 +866,25 @@ html[data-list-density="relaxed"] {
}
.add-download-button-cancel {
border-color: hsl(var(--border-modal));
background:
linear-gradient(180deg, hsl(0 0% 100% / 0.055), transparent),
hsl(var(--bg-input) / 0.88);
color: hsl(var(--text-secondary));
box-shadow:
inset 0 1px 0 hsl(0 0% 100% / 0.055),
0 1px 2px hsl(var(--shadow-color));
}
.add-download-button-cancel:hover:not(:disabled) {
background: hsl(var(--item-hover));
border-color: hsl(var(--text-muted) / 0.65);
background:
linear-gradient(hsl(var(--item-hover)), hsl(var(--item-hover))),
hsl(var(--bg-input));
color: hsl(var(--text-primary));
box-shadow:
inset 0 1px 0 hsl(0 0% 100% / 0.07),
0 2px 5px hsl(var(--shadow-color));
}
.add-download-button-primary {
@@ -982,11 +1017,27 @@ html[data-list-density="relaxed"] {
box-shadow: none;
}
:is(.theme-dark, .theme-dracula, .theme-nord) .add-download-button-cancel {
border-color: hsl(var(--add-keyline));
background: hsl(var(--add-section-bg));
box-shadow:
inset 0 1px 0 hsl(0 0% 100% / 0.055),
0 1px 2px hsl(var(--add-shadow));
}
:is(.theme-dark, .theme-dracula, .theme-nord) .add-download-button-secondary:hover:not(:disabled) {
border-color: hsl(var(--add-keyline-strong));
background: hsl(var(--item-hover));
}
:is(.theme-dark, .theme-dracula, .theme-nord) .add-download-button-cancel:hover:not(:disabled) {
border-color: hsl(var(--add-keyline-strong));
background: hsl(var(--item-hover));
box-shadow:
inset 0 1px 0 hsl(0 0% 100% / 0.07),
0 2px 5px hsl(var(--add-shadow));
}
:is(.theme-dark, .theme-dracula, .theme-nord) .add-download-button-primary {
border-color: hsl(var(--accent-color));
background: hsl(var(--accent-color));
@@ -1980,6 +2031,39 @@ html[data-list-density="relaxed"] .download-ghost-row {
overflow: hidden;
}
.download-size-cell {
justify-content: flex-end;
text-align: right;
}
.download-size-cell > .download-size-progress {
flex: 1 1 auto;
text-align: right;
}
.download-size-cell > .download-size-total {
flex: 0 0 auto;
text-align: left;
white-space: nowrap;
}
.download-size-cell > .download-size-progress,
.download-size-cell > .download-size-total {
max-width: 100%;
min-width: 0;
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
}
.download-size-cell > .download-size-progress {
text-align: right;
}
.download-row > .download-size-cell {
padding-right: calc(var(--download-column-padding-x) + 6px);
}
.download-cell-truncate > span,
.download-cell-right > span {
display: block;
+72 -1
View File
@@ -1,6 +1,10 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { initDownloadListener, useDownloadProgressStore } from './downloadStore';
import { useDownloadStore } from './useDownloadStore';
import {
clearDownloadControlIntents,
setDownloadControlIntent,
useDownloadStore
} from './useDownloadStore';
import * as ipc from '../ipc';
vi.mock('../ipc', () => ({
@@ -13,6 +17,7 @@ describe('useDownloadProgressStore', () => {
vi.clearAllMocks();
vi.mocked(ipc.invokeCommand).mockResolvedValue(undefined);
useDownloadProgressStore.setState({ progressMap: {} });
clearDownloadControlIntents();
});
it('prunes terminal progress entries', () => {
@@ -233,4 +238,70 @@ describe('useDownloadProgressStore', () => {
expect(useDownloadStore.getState().downloads[0].status).toBe('completed');
release();
});
it('ignores a stale paused event during resume and accepts the new active state', async () => {
const handlers: Record<string, (event: any) => void> = {};
vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => {
handlers[event] = handler as (event: any) => void;
return Promise.resolve(vi.fn());
});
useDownloadStore.setState({
downloads: [{
id: 'resume-race',
url: 'https://example.com/file',
fileName: 'file.bin',
status: 'queued',
category: 'Other',
dateAdded: ''
}]
});
setDownloadControlIntent('resume-race', 'resume');
const release = await initDownloadListener();
handlers['download-state']({ payload: {
id: 'resume-race',
status: 'paused'
} });
expect(useDownloadStore.getState().downloads[0].status).toBe('queued');
handlers['download-state']({ payload: {
id: 'resume-race',
status: 'downloading'
} });
expect(useDownloadStore.getState().downloads[0].status).toBe('downloading');
release();
});
it('allows a later genuine pause after consuming the stale resume event', async () => {
const handlers: Record<string, (event: any) => void> = {};
vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => {
handlers[event] = handler as (event: any) => void;
return Promise.resolve(vi.fn());
});
useDownloadStore.setState({
downloads: [{
id: 'resume-pause-race',
url: 'https://example.com/file',
fileName: 'file.bin',
status: 'queued',
category: 'Other',
dateAdded: ''
}]
});
setDownloadControlIntent('resume-pause-race', 'resume');
const release = await initDownloadListener();
handlers['download-state']({ payload: {
id: 'resume-pause-race',
status: 'paused'
} });
expect(useDownloadStore.getState().downloads[0].status).toBe('queued');
handlers['download-state']({ payload: {
id: 'resume-pause-race',
status: 'paused'
} });
expect(useDownloadStore.getState().downloads[0].status).toBe('paused');
release();
});
});
+27 -1
View File
@@ -5,7 +5,11 @@ import type { DownloadItem } from '../bindings/DownloadItem';
import { categoryForFileName } from '../utils/downloads';
import { useDownloadProgressStore } from './downloadProgressStore';
import { useDownloadStore } from './useDownloadStore';
import {
clearDownloadControlIntent,
downloadControlIntentFor,
useDownloadStore
} from './useDownloadStore';
export { useDownloadProgressStore } from './downloadProgressStore';
@@ -78,6 +82,28 @@ const startDownloadListeners = async () => {
}
const status = payload.status as DownloadStatus;
// resume_download queues the row before the backend can emit its new
// active state. A paused event already emitted by the old lifecycle may
// arrive in that gap. Do not let it overwrite the queued transition;
// otherwise the guard below would reject the legitimate downloading
// event and leave the row visibly paused forever.
if (status === 'paused' &&
current.status === 'queued' &&
downloadControlIntentFor(payload.id) === 'resume') {
// Consume only the stale pause event that caused the resume race.
// A later real pause must be allowed through even if the backend has
// not emitted a new active state yet.
clearDownloadControlIntent(payload.id, 'resume');
return;
}
if (status === 'downloading' || status === 'processing' ||
status === 'completed' || status === 'failed') {
clearDownloadControlIntent(payload.id, 'resume');
}
if (status === 'paused') {
clearDownloadControlIntent(payload.id, 'pause');
}
// Prevent stale lifecycle events from moving a paused row back into an
// active state. A pause request can finish before one already-emitted
// worker event reaches the frontend. Resume paths set the row to queued
+45 -8
View File
@@ -24,6 +24,29 @@ const queueStartPromises = new Map<string, Promise<string[]>>();
const queueControlGenerations = new Map<string, number>();
let pendingStartupResume: Promise<void> | null = null;
type DownloadControlIntent = 'pause' | 'resume';
const downloadControlIntents = new Map<string, DownloadControlIntent>();
// State events do not carry a lifecycle generation. Keep the intent that
// initiated a control transition long enough for the listener to discard an
// already-emitted event from the previous transition.
export const setDownloadControlIntent = (id: string, intent: DownloadControlIntent): void => {
downloadControlIntents.set(id, intent);
};
export const clearDownloadControlIntent = (id: string, intent?: DownloadControlIntent): void => {
if (intent === undefined || downloadControlIntents.get(id) === intent) {
downloadControlIntents.delete(id);
}
};
export const downloadControlIntentFor = (id: string): DownloadControlIntent | undefined =>
downloadControlIntents.get(id);
export const clearDownloadControlIntents = (): void => {
downloadControlIntents.clear();
};
const waitForPendingStartupResume = async (): Promise<void> => {
const pending = pendingStartupResume;
if (pending) await pending.catch(() => undefined);
@@ -871,6 +894,7 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
},
removeDownload: async (id, deleteFile = false, preserveResumable = false) => {
await waitForPendingStartupResume();
clearDownloadControlIntent(id);
const { pendingDispatch } = await invalidateDispatch(id);
if (pendingDispatch) {
await pendingDispatch;
@@ -898,17 +922,23 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
},
pauseDownload: async (id) => {
await waitForPendingStartupResume();
if (!get().downloads.some(download => download.id === id)) return;
setDownloadControlIntent(id, 'pause');
const { generation, pendingDispatch } = await invalidateDispatch(id);
if (pendingDispatch) {
await pendingDispatch;
}
try {
if (pendingDispatch) {
await pendingDispatch;
}
await invoke('pause_download', { id });
await invoke('pause_download', { id });
if (!isCurrentDownloadLifecycle(id, generation)) return;
const current = get().downloads.find(download => download.id === id);
if (current && current.status !== 'completed' && current.status !== 'failed') {
get().updateDownload(id, { status: 'paused', speed: '-', eta: '-' });
if (!isCurrentDownloadLifecycle(id, generation)) return;
const current = get().downloads.find(download => download.id === id);
if (current && current.status !== 'completed' && current.status !== 'failed') {
get().updateDownload(id, { status: 'paused', speed: '-', eta: '-' });
}
} finally {
clearDownloadControlIntent(id, 'pause');
}
},
redownload: async (id) => {
@@ -925,6 +955,7 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
const url = targetItem.url?.trim();
if (!url) throw new Error('Cannot redownload: original URL is missing.');
setDownloadControlIntent(id, 'resume');
await invalidateAndWaitForDispatch(id);
// Remove from backend to clear its state and delete the existing file so we can overwrite
@@ -933,6 +964,7 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
get().unregisterBackendIds([id]);
} catch (e) {
console.warn("Could not remove old download from backend", e);
clearDownloadControlIntent(id, 'resume');
throw e;
}
@@ -951,6 +983,7 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
if (!await dispatchItem(id)) {
console.error("Failed to enqueue redownload");
get().updateDownload(id, { status: 'failed' });
clearDownloadControlIntent(id, 'resume');
} else {
get().updateDownload(id, { hasBeenDispatched: true });
info(`Download ${id} redownloaded (queued)`);
@@ -961,6 +994,7 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
const targetItem = get().downloads.find(d => d.id === id);
if (!targetItem) return false;
setDownloadControlIntent(id, 'resume');
try {
if (targetItem.status === 'ready' || targetItem.status === 'staged') {
get().updateDownload(id, { status: 'queued', hasBeenDispatched: true });
@@ -968,6 +1002,7 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
return true;
}
get().updateDownload(id, { status: targetItem.status });
clearDownloadControlIntent(id, 'resume');
return false;
}
@@ -1002,11 +1037,13 @@ export const useDownloadStore = create<DownloadState>((set, get) => ({
} else {
console.error("Failed to re-enqueue for resume");
get().updateDownload(id, { status: prevStatus });
clearDownloadControlIntent(id, 'resume');
return false;
}
} catch (e) {
console.error("Failed to resume download:", e);
get().updateDownload(id, { status: targetItem.status });
clearDownloadControlIntent(id, 'resume');
return false;
}
},