diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt index 3544579c..95c79f2b 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -571,6 +571,7 @@ class MainActivity: FlutterFragmentActivity() { file.lastModified() < cutoff && (file.name.startsWith("bridge_json_") || file.name.startsWith("saf_") || + file.name.startsWith("native_saf_") || file.name.startsWith("ms_")) if (stale) file.delete() } diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt index 433107f7..1f6209be 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/SafDownloadHandler.kt @@ -303,6 +303,7 @@ object SafDownloadHandler { } fun copyContentUriToTemp(context: Context, uriStr: String): String? { + var temp: File? = null return try { val uri = Uri.parse(uriStr) val extension = DocumentFile.fromSingleUri(context, uri) @@ -311,14 +312,19 @@ object SafDownloadHandler { ?.takeIf { it.isNotBlank() } ?.let { ".$it" } ?: ".tmp" - val temp = File.createTempFile("native_saf_", extension, context.cacheDir) + val createdTemp = File.createTempFile("native_saf_", extension, context.cacheDir) + temp = createdTemp context.contentResolver.openInputStream(uri)?.use { input -> - temp.outputStream().use { output -> + createdTemp.outputStream().use { output -> input.copyTo(output) } - } ?: return null - temp.absolutePath + } ?: run { + createdTemp.delete() + return null + } + createdTemp.absolutePath } catch (e: Exception) { + try { temp?.delete() } catch (_: Exception) {} android.util.Log.w("SpotiFLAC", "Failed to copy SAF URI to temp: ${e.message}") null } diff --git a/go_backend/extension_runtime_file.go b/go_backend/extension_runtime_file.go index 2067e3db..45b75775 100644 --- a/go_backend/extension_runtime_file.go +++ b/go_backend/extension_runtime_file.go @@ -17,12 +17,20 @@ var ( ) func AddAllowedDownloadDir(dir string) { + absDir, err := filepath.Abs(dir) + if err != nil { + return + } + absDir = filepath.Clean(absDir) + allowedDownloadDirsMu.Lock() defer allowedDownloadDirsMu.Unlock() - absDir, err := filepath.Abs(dir) - if err == nil { - allowedDownloadDirs = append(allowedDownloadDirs, absDir) + for _, existing := range allowedDownloadDirs { + if existing == absDir { + return + } } + allowedDownloadDirs = append(allowedDownloadDirs, absDir) } // SetAllowedDownloadDirs replaces the whole allow-list in one call (passing nil @@ -31,7 +39,20 @@ func AddAllowedDownloadDir(dir string) { func SetAllowedDownloadDirs(dirs []string) { allowedDownloadDirsMu.Lock() defer allowedDownloadDirsMu.Unlock() - allowedDownloadDirs = dirs + allowedDownloadDirs = nil + seen := make(map[string]struct{}, len(dirs)) + for _, dir := range dirs { + absDir, err := filepath.Abs(dir) + if err != nil { + continue + } + absDir = filepath.Clean(absDir) + if _, duplicate := seen[absDir]; duplicate { + continue + } + seen[absDir] = struct{}{} + allowedDownloadDirs = append(allowedDownloadDirs, absDir) + } } func isPathInAllowedDirs(absPath string) bool { diff --git a/go_backend/extension_runtime_polyfills.go b/go_backend/extension_runtime_polyfills.go index 82c80ce6..404a1e03 100644 --- a/go_backend/extension_runtime_polyfills.go +++ b/go_backend/extension_runtime_polyfills.go @@ -67,7 +67,7 @@ func (r *extensionRuntime) fetchPolyfill(call goja.FunctionCall) goja.Value { } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + body, err := readExtensionHTTPResponseBody(resp) if err != nil { return r.createFetchError(err.Error()) } @@ -81,9 +81,15 @@ func (r *extensionRuntime) fetchPolyfill(call goja.FunctionCall) goja.Value { responseObj.Set("headers", respHeaders) responseObj.Set("url", resp.Request.URL.String()) - bodyString := string(body) - + var bodyString string + var bodyStringReady bool responseObj.Set("text", func(call goja.FunctionCall) goja.Value { + // Avoid allocating a second full response copy when callers only use + // json() or arrayBuffer(). + if !bodyStringReady { + bodyString = string(body) + bodyStringReady = true + } return r.vm.ToValue(bodyString) }) @@ -97,11 +103,10 @@ func (r *extensionRuntime) fetchPolyfill(call goja.FunctionCall) goja.Value { }) responseObj.Set("arrayBuffer", func(call goja.FunctionCall) goja.Value { - byteArray := make([]any, len(body)) - for i, b := range body { - byteArray[i] = int(b) - } - return r.vm.ToValue(byteArray) + // A Go-backed byte slice preserves the existing synchronous, array-like + // contract (length and numeric indexes) without one interface allocation + // per byte. + return r.vm.ToValue(body) }) return responseObj diff --git a/go_backend/extension_runtime_supplement_test.go b/go_backend/extension_runtime_supplement_test.go index 72008c13..3d0ed1f7 100644 --- a/go_backend/extension_runtime_supplement_test.go +++ b/go_backend/extension_runtime_supplement_test.go @@ -37,6 +37,17 @@ func TestExtensionRuntimeAuthAndPolyfills(t *testing.T) { Request: req, }, nil case "api.example.com": + if req.URL.Path == "/huge" { + return &http.Response{ + StatusCode: 200, + Header: make(http.Header), + Body: io.NopCloser(io.LimitReader( + strings.NewReader(strings.Repeat("x", maxExtensionHTTPResponseBytes+1)), + maxExtensionHTTPResponseBytes+1, + )), + Request: req, + }, nil + } return &http.Response{ StatusCode: 200, Header: http.Header{"X-Test": []string{"yes"}}, @@ -154,7 +165,8 @@ func TestExtensionRuntimeAuthAndPolyfills(t *testing.T) { ok: response.ok, status: response.status, jsonOk: response.json().ok, - bufferLen: response.arrayBuffer().length + bufferLen: response.arrayBuffer().length, + bufferFirst: response.arrayBuffer()[0] }); `) if err != nil { @@ -164,7 +176,8 @@ func TestExtensionRuntimeAuthAndPolyfills(t *testing.T) { if err := json.Unmarshal([]byte(value.String()), &result); err != nil { t.Fatalf("decode polyfill result: %v", err) } - if result["decoded"] != "hello" || result["host"] != "api.example.com" || result["ok"] != true { + if result["decoded"] != "hello" || result["host"] != "api.example.com" || result["ok"] != true || + result["bufferLen"] != float64(len(`{"ok":true,"items":[1,2]}`)) || result["bufferFirst"] != float64('{') { t.Fatalf("polyfill result = %#v", result) } @@ -172,6 +185,10 @@ func TestExtensionRuntimeAuthAndPolyfills(t *testing.T) { if blocked.Get("ok").ToBoolean() { t.Fatal("expected blocked fetch") } + huge := runtime.fetchPolyfill(goja.FunctionCall{Arguments: []goja.Value{vm.ToValue("https://api.example.com/huge")}}).ToObject(vm) + if huge.Get("ok").ToBoolean() || !strings.Contains(huge.Get("error").String(), "exceeds") { + t.Fatalf("expected bounded fetch response, got %s", huge.String()) + } runtime.authClear(goja.FunctionCall{}) if runtime.authIsAuthenticated(goja.FunctionCall{}).ToBoolean() { t.Fatal("expected auth cleared") @@ -887,6 +904,13 @@ func TestExtensionRuntimeFileAPIs(t *testing.T) { t.Fatal("expected sandbox escape error") } AddAllowedDownloadDir(dir) + AddAllowedDownloadDir(filepath.Clean(dir)) + allowedDownloadDirsMu.RLock() + allowedDirCount := len(allowedDownloadDirs) + allowedDownloadDirsMu.RUnlock() + if allowedDirCount != 1 { + t.Fatalf("duplicate allowed directories retained: %d", allowedDirCount) + } absolutePath := filepath.Join(dir, "allowed.txt") if got, err := runtime.validatePath(absolutePath); err != nil || got != absolutePath { t.Fatalf("absolute validatePath = %q/%v", got, err) @@ -1027,6 +1051,12 @@ func TestExtensionRuntimeUtilityAPIs(t *testing.T) { if key["success"] != true || key["key"] == "" || key["hex"] == "" { t.Fatalf("cryptoGenerateKey = %#v", key) } + for _, invalidLength := range []float64{-1, 0, 1.5, 4097} { + invalidKey := runtime.cryptoGenerateKey(goja.FunctionCall{Arguments: []goja.Value{vm.ToValue(invalidLength)}}).Export().(map[string]any) + if invalidKey["success"] != false { + t.Fatalf("cryptoGenerateKey(%v) should fail: %#v", invalidLength, invalidKey) + } + } if runtime.randomUserAgent(goja.FunctionCall{}).String() == "" || runtime.appUserAgent(goja.FunctionCall{}).String() == "" { t.Fatal("expected user agents") } diff --git a/go_backend/extension_runtime_utils.go b/go_backend/extension_runtime_utils.go index 325133fe..b8c00297 100644 --- a/go_backend/extension_runtime_utils.go +++ b/go_backend/extension_runtime_utils.go @@ -10,6 +10,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "math" "strings" "time" @@ -226,9 +227,12 @@ func (r *extensionRuntime) cryptoDecrypt(call goja.FunctionCall) goja.Value { func (r *extensionRuntime) cryptoGenerateKey(call goja.FunctionCall) goja.Value { length := 32 if len(call.Arguments) > 0 && !goja.IsUndefined(call.Arguments[0]) { - if l, ok := call.Arguments[0].Export().(float64); ok { - length = int(l) + requested := call.Arguments[0].ToFloat() + if math.IsNaN(requested) || math.IsInf(requested, 0) || + requested != math.Trunc(requested) || requested < 1 || requested > 4096 { + return r.jsError("key length must be an integer between 1 and 4096 bytes") } + length = int(requested) } key := make([]byte, length) diff --git a/go_backend/performance_benchmark_test.go b/go_backend/performance_benchmark_test.go index e4b852e9..b45d273f 100644 --- a/go_backend/performance_benchmark_test.go +++ b/go_backend/performance_benchmark_test.go @@ -14,8 +14,35 @@ var ( benchmarkStringSink string benchmarkIntSink int64 benchmarkScanSink *LibraryScanResult + benchmarkValueSink goja.Value ) +func BenchmarkGojaByteArrayConversion(b *testing.B) { + const payloadSize = 64 << 10 + payload := make([]byte, payloadSize) + vm := goja.New() + + b.Run("go_backed_bytes", func(b *testing.B) { + b.ReportAllocs() + b.SetBytes(payloadSize) + for b.Loop() { + benchmarkValueSink = vm.ToValue(payload) + } + }) + + b.Run("boxed_interfaces", func(b *testing.B) { + b.ReportAllocs() + b.SetBytes(payloadSize) + for b.Loop() { + boxed := make([]any, len(payload)) + for index, value := range payload { + boxed[index] = int(value) + } + benchmarkValueSink = vm.ToValue(boxed) + } + }) +} + func BenchmarkGojaProviderInvocation(b *testing.B) { vm := goja.New() if _, err := vm.RunString(`var extension = { searchTracks: function(query, limit) { return query.length + limit; } };`); err != nil { diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 38d69d55..a288b94d 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -1861,27 +1861,37 @@ class DownloadQueueNotifier extends Notifier { continue; } + final maxConcurrent = ref + .read(settingsProvider) + .concurrentDownloads + .clamp(1, 3); + if (activeDownloads.length >= maxConcurrent) { + // Keep pause/settings changes responsive without rescanning the full + // queue while every worker slot is already occupied. + await Future.any([ + Future.any(activeDownloads.values), + Future.delayed(_queueSchedulingInterval), + ]); + continue; + } + + final availableSlots = maxConcurrent - activeDownloads.length; final queuedItems = state.items .where( (item) => item.status == DownloadStatus.queued && !_pausePendingItemIds.contains(item.id), ) - .toList(); + .take(availableSlots) + .toList(growable: false); if (queuedItems.isEmpty && activeDownloads.isEmpty) { _log.d('No more items to process'); break; } - final maxConcurrent = ref - .read(settingsProvider) - .concurrentDownloads - .clamp(1, 3); - while (activeDownloads.length < maxConcurrent && - queuedItems.isNotEmpty && - !state.isPaused) { - final item = queuedItems.removeAt(0); + for (final item in queuedItems) { + if (state.isPaused) break; updateItemStatus(item.id, DownloadStatus.downloading); diff --git a/lib/providers/download_queue_state.dart b/lib/providers/download_queue_state.dart index 1a1b56df..14b6c51a 100644 --- a/lib/providers/download_queue_state.dart +++ b/lib/providers/download_queue_state.dart @@ -80,6 +80,7 @@ class DownloadQueueLookup { final Map byItemId; final Map indexByItemId; final List itemIds; + final List notCompletedItemIds; final int queuedCount; final int completedCount; final int failedCount; @@ -91,6 +92,7 @@ class DownloadQueueLookup { byItemId = const {}, indexByItemId = const {}, itemIds = const [], + notCompletedItemIds = const [], queuedCount = 0, completedCount = 0, failedCount = 0, @@ -102,6 +104,7 @@ class DownloadQueueLookup { required this.byItemId, required this.indexByItemId, required this.itemIds, + required this.notCompletedItemIds, required this.queuedCount, required this.completedCount, required this.failedCount, @@ -114,6 +117,7 @@ class DownloadQueueLookup { final byItemId = {}; final indexByItemId = {}; final itemIds = []; + final notCompletedItemIds = []; var queuedCount = 0; var completedCount = 0; var failedCount = 0; @@ -125,6 +129,9 @@ class DownloadQueueLookup { byItemId[item.id] = item; indexByItemId[item.id] = index; itemIds.add(item.id); + if (item.status != DownloadStatus.completed) { + notCompletedItemIds.add(item.id); + } if (_countsAsQueued(item.status)) queuedCount++; if (item.status == DownloadStatus.completed) completedCount++; if (item.status == DownloadStatus.failed) failedCount++; @@ -136,6 +143,7 @@ class DownloadQueueLookup { byItemId: Map.unmodifiable(byItemId), indexByItemId: Map.unmodifiable(indexByItemId), itemIds: List.unmodifiable(itemIds), + notCompletedItemIds: List.unmodifiable(notCompletedItemIds), queuedCount: queuedCount, completedCount: completedCount, failedCount: failedCount, @@ -185,6 +193,7 @@ class DownloadQueueLookup { var nextFailedCount = failedCount; var nextActiveDownloadsCount = activeDownloadsCount; var nextFinalizingCount = finalizingCount; + var notCompletedMembershipChanged = false; Map? nextByItemId; Map? nextByTrackId; @@ -195,6 +204,11 @@ class DownloadQueueLookup { return DownloadQueueLookup.fromItems(nextItems); } + if ((previous.status == DownloadStatus.completed) != + (next.status == DownloadStatus.completed)) { + notCompletedMembershipChanged = true; + } + nextByItemId ??= Map.from(byItemId); nextByItemId[next.id] = next; if (byTrackId[next.track.id]?.id == previous.id) { @@ -237,6 +251,12 @@ class DownloadQueueLookup { : Map.unmodifiable(nextByItemId), indexByItemId: indexByItemId, itemIds: itemIds, + notCompletedItemIds: notCompletedMembershipChanged + ? List.unmodifiable([ + for (final item in nextItems) + if (item.status != DownloadStatus.completed) item.id, + ]) + : notCompletedItemIds, queuedCount: nextQueuedCount, completedCount: nextCompletedCount, failedCount: nextFailedCount, diff --git a/lib/screens/now_playing_screen.dart b/lib/screens/now_playing_screen.dart index 57c15774..90ef1ff9 100644 --- a/lib/screens/now_playing_screen.dart +++ b/lib/screens/now_playing_screen.dart @@ -1314,6 +1314,7 @@ class _SyncedLyricsViewState extends ConsumerState<_SyncedLyricsView> { ProviderSubscription? _playingSubscription; ProviderSubscription? _loadingSubscription; Timer? _lineBoundaryTimer; + Timer? _userScrollIdleTimer; late List _lineKeys; int _active = -1; Duration _activeTransitionPosition = Duration.zero; @@ -1432,6 +1433,7 @@ class _SyncedLyricsViewState extends ConsumerState<_SyncedLyricsView> { _playingSubscription?.close(); _loadingSubscription?.close(); _lineBoundaryTimer?.cancel(); + _userScrollIdleTimer?.cancel(); _scroll.dispose(); super.dispose(); } @@ -1488,7 +1490,8 @@ class _SyncedLyricsViewState extends ConsumerState<_SyncedLyricsView> { onNotification: (notification) { if (notification.direction != ScrollDirection.idle) { _userScrolling = true; - Future.delayed(const Duration(seconds: 4), () { + _userScrollIdleTimer?.cancel(); + _userScrollIdleTimer = Timer(const Duration(seconds: 4), () { if (mounted) _userScrolling = false; }); } @@ -1883,13 +1886,27 @@ class _SweepingTimedLyricTextState extends State<_SweepingTimedLyricText> { ? constraints.maxWidth : pendingPainter.width; final height = pendingPainter.height; + final segmentBoxes = >[]; + var segmentOffset = 0; + for (final segment in widget.segments) { + final segmentEnd = segmentOffset + segment.length; + segmentBoxes.add( + highlightedPainter.getBoxesForSelection( + TextSelection( + baseOffset: segmentOffset, + extentOffset: segmentEnd, + ), + ), + ); + segmentOffset = segmentEnd; + } return Semantics( label: widget.semanticsLabel, child: CustomPaint( size: Size(width, height), painter: _TimedLyricSweepPainter( - segments: widget.segments, + segmentBoxes: segmentBoxes, starts: widget.starts, ends: widget.ends, currentPosition: widget.currentPosition, @@ -1905,7 +1922,7 @@ class _SweepingTimedLyricTextState extends State<_SweepingTimedLyricText> { } class _TimedLyricSweepPainter extends CustomPainter { - final List segments; + final List> segmentBoxes; final List starts; final List ends; final Duration Function() currentPosition; @@ -1913,7 +1930,7 @@ class _TimedLyricSweepPainter extends CustomPainter { final TextPainter highlightedPainter; _TimedLyricSweepPainter({ - required this.segments, + required this.segmentBoxes, required this.starts, required this.ends, required this.currentPosition, @@ -1929,9 +1946,7 @@ class _TimedLyricSweepPainter extends CustomPainter { final completedPath = Path(); final partialBoxes = <(Rect, double)>[]; final position = currentPosition(); - var offset = 0; - for (var index = 0; index < segments.length; index++) { - final end = offset + segments[index].length; + for (var index = 0; index < segmentBoxes.length; index++) { final value = index < starts.length && index < ends.length ? syncedLyricSegmentProgress( position: position, @@ -1939,11 +1954,8 @@ class _TimedLyricSweepPainter extends CustomPainter { end: ends[index], ) : 0.0; - if (value > 0 && end > offset) { - final boxes = highlightedPainter.getBoxesForSelection( - TextSelection(baseOffset: offset, extentOffset: end), - ); - for (final box in boxes) { + if (value > 0) { + for (final box in segmentBoxes[index]) { final rect = box.toRect(); if (value >= 1) { completedPath.addRect(rect); @@ -1952,7 +1964,6 @@ class _TimedLyricSweepPainter extends CustomPainter { } } } - offset = end; } if (!completedPath.getBounds().isEmpty) { @@ -1998,7 +2009,7 @@ class _TimedLyricSweepPainter extends CustomPainter { @override bool shouldRepaint(covariant _TimedLyricSweepPainter oldDelegate) { - return oldDelegate.segments != segments || + return oldDelegate.segmentBoxes != segmentBoxes || oldDelegate.starts != starts || oldDelegate.ends != ends || oldDelegate.currentPosition != currentPosition || diff --git a/lib/screens/queue_tab.dart b/lib/screens/queue_tab.dart index 8738f0e7..cfdffd21 100644 --- a/lib/screens/queue_tab.dart +++ b/lib/screens/queue_tab.dart @@ -1,6 +1,6 @@ import 'dart:async'; import 'dart:io'; -import 'package:flutter/foundation.dart'; +import 'package:flutter/foundation.dart' show ValueListenable; import 'package:flutter/material.dart'; import 'package:spotiflac_android/services/shell_navigation_service.dart'; import 'package:spotiflac_android/widgets/error_card.dart'; @@ -1223,7 +1223,10 @@ class _QueueTabState extends ConsumerState { ref.listen(downloadQueueLookupProvider, (previous, next) { if (previous == null) return; - for (final id in previous.itemIds) { + if (identical(previous.notCompletedItemIds, next.notCompletedItemIds)) { + return; + } + for (final id in previous.notCompletedItemIds) { final prevItem = previous.byItemId[id]; final nextItem = next.byItemId[id]; if (prevItem == null) continue; @@ -1656,16 +1659,9 @@ class _QueueTabState extends ConsumerState { return Consumer( builder: (context, ref, child) { final queueCount = ref.watch( - downloadQueueLookupProvider.select((lookup) { - var count = 0; - for (final id in lookup.itemIds) { - final entry = lookup.byItemId[id]; - if (entry != null && entry.status != DownloadStatus.completed) { - count++; - } - } - return count; - }), + downloadQueueLookupProvider.select( + (lookup) => lookup.notCompletedItemIds.length, + ), ); final failedCount = ref.watch( downloadQueueProvider.select((state) => state.failedCount), diff --git a/lib/screens/queue_tab_filter_widgets.dart b/lib/screens/queue_tab_filter_widgets.dart index c4d4dedf..32d566e2 100644 --- a/lib/screens/queue_tab_filter_widgets.dart +++ b/lib/screens/queue_tab_filter_widgets.dart @@ -57,19 +57,10 @@ extension _QueueTabFilterWidgets on _QueueTabState { ? const [] : ref .watch( - downloadQueueLookupProvider.select((lookup) { - final ids = []; - for (final id in lookup.itemIds) { - final entry = lookup.byItemId[id]; - if (entry != null && - entry.status != DownloadStatus.completed) { - ids.add(id); - } - } - return _QueueItemIdsSnapshot(ids); - }), + downloadQueueLookupProvider.select( + (lookup) => lookup.notCompletedItemIds, + ), ) - .ids .reversed .toList(growable: false); diff --git a/lib/screens/queue_tab_helpers.dart b/lib/screens/queue_tab_helpers.dart index d141d5f3..8a2a2096 100644 --- a/lib/screens/queue_tab_helpers.dart +++ b/lib/screens/queue_tab_helpers.dart @@ -495,17 +495,3 @@ final _queueLibraryCountsProvider = FutureProvider.autoDispose ); return LibraryDatabase.instance.getQueueCounts(request.toDbQuery()); }); - -class _QueueItemIdsSnapshot { - final List ids; - - const _QueueItemIdsSnapshot(this.ids); - - @override - bool operator ==(Object other) => - identical(this, other) || - other is _QueueItemIdsSnapshot && listEquals(ids, other.ids); - - @override - int get hashCode => Object.hashAll(ids); -} diff --git a/lib/services/music_player_service.dart b/lib/services/music_player_service.dart index 80ca2f43..becd8aa0 100644 --- a/lib/services/music_player_service.dart +++ b/lib/services/music_player_service.dart @@ -36,6 +36,24 @@ void setPlaybackNormalizationEnabled(bool enabled) { _activeMusicPlayerHandler?.reapplyNormalization(); } +List buildShuffleCandidatePool({ + required int mediaCount, + required int currentIndex, + required Iterable recentIndices, +}) { + final recent = recentIndices.toSet(); + final pool = []; + for (var index = 0; index < mediaCount; index++) { + if (index != currentIndex && !recent.contains(index)) pool.add(index); + } + if (pool.isEmpty) { + for (var index = 0; index < mediaCount; index++) { + if (index != currentIndex) pool.add(index); + } + } + return pool; +} + final AudioContext _musicAudioContext = AudioContext( android: const AudioContextAndroid( audioFocus: AndroidAudioFocus.none, @@ -967,15 +985,11 @@ class MusicPlayerHandler extends BaseAudioHandler int _pickNextShuffle() { if (_media.length <= 1) return _index; - final pool = []; - for (var i = 0; i < _media.length; i++) { - if (i != _index && !_recent.contains(i)) pool.add(i); - } - if (pool.isEmpty) { - for (var i = 0; i < _media.length; i++) { - if (i != _index) pool.add(i); - } - } + final pool = buildShuffleCandidatePool( + mediaCount: _media.length, + currentIndex: _index, + recentIndices: _recent, + ); return pool[_random.nextInt(pool.length)]; } diff --git a/test/models_and_utils_test.dart b/test/models_and_utils_test.dart index e18f2e92..080efa88 100644 --- a/test/models_and_utils_test.dart +++ b/test/models_and_utils_test.dart @@ -755,6 +755,7 @@ void main() { expect(lookup.queuedCount, 3); expect(lookup.activeDownloadsCount, 1); expect(lookup.finalizingCount, 1); + expect(lookup.notCompletedItemIds, ['queued', 'active', 'finalizing']); final next = List.from(previous); next[2] = previous[2].copyWith(status: DownloadStatus.completed); @@ -769,6 +770,22 @@ void main() { expect(updated.finalizingCount, 0); expect(identical(updated.indexByItemId, lookup.indexByItemId), isTrue); expect(identical(updated.itemIds, lookup.itemIds), isTrue); + expect(updated.notCompletedItemIds, ['queued', 'active']); + + final progressOnly = List.from(next); + progressOnly[1] = next[1].copyWith(progress: 0.5); + final progressUpdated = updated.updatedForIndices( + previousItems: next, + nextItems: progressOnly, + changedIndices: const [1], + ); + expect( + identical( + progressUpdated.notCompletedItemIds, + updated.notCompletedItemIds, + ), + isTrue, + ); }); }); diff --git a/test/music_player_media_metadata_test.dart b/test/music_player_media_metadata_test.dart index ceb95901..f139eba3 100644 --- a/test/music_player_media_metadata_test.dart +++ b/test/music_player_media_metadata_test.dart @@ -104,4 +104,26 @@ void main() { expect(calls, 1); expect(metadata['title'], 'Instrumental'); }); + + test('shuffle candidate selection excludes recent tracks', () { + expect( + buildShuffleCandidatePool( + mediaCount: 6, + currentIndex: 2, + recentIndices: const [0, 1, 3], + ), + [4, 5], + ); + }); + + test('shuffle candidate selection resets after exhausting the pool', () { + expect( + buildShuffleCandidatePool( + mediaCount: 4, + currentIndex: 2, + recentIndices: const [0, 1, 3], + ), + [0, 1, 3], + ); + }); }