From a59c749089b8b054f317b78874703eca87858cbb Mon Sep 17 00:00:00 2001 From: zarzet Date: Sun, 9 Aug 2026 05:23:47 +0700 Subject: [PATCH] fix(download): preserve SAF and defer background starts --- .../spotiflac/ForegroundServiceStartPolicy.kt | 18 ++++ .../kotlin/com/zarz/spotiflac/MainActivity.kt | 6 +- .../ForegroundServiceStartPolicyTest.kt | 29 ++++++ lib/main.dart | 5 + lib/providers/download_queue_provider.dart | 96 +++++++++++++++---- ...download_queue_provider_native_worker.dart | 56 +++++++++-- .../download_queue_provider_single_item.dart | 32 +++++-- lib/services/platform_bridge.dart | 8 ++ test/download_start_policy_test.dart | 54 +++++++++++ 9 files changed, 271 insertions(+), 33 deletions(-) create mode 100644 android/app/src/main/kotlin/com/zarz/spotiflac/ForegroundServiceStartPolicy.kt create mode 100644 android/app/src/test/kotlin/com/zarz/spotiflac/ForegroundServiceStartPolicyTest.kt create mode 100644 test/download_start_policy_test.dart diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/ForegroundServiceStartPolicy.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/ForegroundServiceStartPolicy.kt new file mode 100644 index 00000000..921ce580 --- /dev/null +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/ForegroundServiceStartPolicy.kt @@ -0,0 +1,18 @@ +package com.zarz.spotiflac + +/** Maps Android/OEM foreground-service launch denials to a stable Dart code. */ +object ForegroundServiceStartPolicy { + const val START_NOT_ALLOWED_CODE = "foreground_service_start_not_allowed" + + fun isStartNotAllowed(error: Throwable): Boolean { + if (error.javaClass.name == "android.app.ForegroundServiceStartNotAllowedException") { + return true + } + val message = error.message.orEmpty() + return message.contains("startForegroundService() not allowed", ignoreCase = true) || + message.contains("mAllowStartForeground false", ignoreCase = true) + } + + fun errorCode(error: Throwable): String = + if (isStartNotAllowed(error)) START_NOT_ALLOWED_CODE else "ERROR" +} 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 c707c801..155230a7 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -2326,7 +2326,11 @@ class MainActivity: FlutterFragmentActivity() { else -> result.notImplemented() } } catch (e: Exception) { - result.error("ERROR", e.message, null) + result.error( + ForegroundServiceStartPolicy.errorCode(e), + e.message, + null, + ) } } } diff --git a/android/app/src/test/kotlin/com/zarz/spotiflac/ForegroundServiceStartPolicyTest.kt b/android/app/src/test/kotlin/com/zarz/spotiflac/ForegroundServiceStartPolicyTest.kt new file mode 100644 index 00000000..41fdd6ef --- /dev/null +++ b/android/app/src/test/kotlin/com/zarz/spotiflac/ForegroundServiceStartPolicyTest.kt @@ -0,0 +1,29 @@ +package com.zarz.spotiflac + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ForegroundServiceStartPolicyTest { + @Test + fun mapsAndroidForegroundStartDenialsToStableCode() { + val error = IllegalStateException( + "startForegroundService() not allowed due to mAllowStartForeground false", + ) + + assertTrue(ForegroundServiceStartPolicy.isStartNotAllowed(error)) + assertEquals( + ForegroundServiceStartPolicy.START_NOT_ALLOWED_CODE, + ForegroundServiceStartPolicy.errorCode(error), + ) + } + + @Test + fun leavesUnrelatedPlatformFailuresGeneric() { + val error = IllegalStateException("network unavailable") + + assertFalse(ForegroundServiceStartPolicy.isStartNotAllowed(error)) + assertEquals("ERROR", ForegroundServiceStartPolicy.errorCode(error)) + } +} diff --git a/lib/main.dart b/lib/main.dart index 25e83812..aae344e2 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -261,6 +261,11 @@ class _EagerInitializationState extends ConsumerState<_EagerInitialization> void didChangeAppLifecycleState(AppLifecycleState state) { if (state == AppLifecycleState.resumed) { _maybeAutoScanLocalLibrary(); + if (ref.exists(downloadQueueProvider)) { + ref + .read(downloadQueueProvider.notifier) + .resumePendingDownloadsOnForeground(); + } } else if (state == AppLifecycleState.paused) { // Last reliable moment before the OS may kill the process: make sure // any debounced download-queue persistence reaches disk. diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 38a43676..caae88cf 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -62,6 +62,18 @@ const String safPermissionLostErrorMessage = const String downloadFolderAccessLostErrorMessage = 'Download folder access lost. Please re-select your download folder in Settings.'; +enum StorageWriteRecovery { requestSafAccess, useAppFolderFallback } + +StorageWriteRecovery storageWriteRecoveryFor({required bool useSaf}) { + return useSaf + ? StorageWriteRecovery.requestSafAccess + : StorageWriteRecovery.useAppFolderFallback; +} + +bool canStartForegroundDownloadForLifecycle(AppLifecycleState? lifecycleState) { + return lifecycleState == AppLifecycleState.resumed; +} + /// Keeps a download in its finalizing state until its durable Library record /// has been written. If persistence fails, completion is deliberately not /// published so the queue can surface the error instead of losing the file @@ -179,6 +191,7 @@ class DownloadQueueNotifier extends Notifier { int _failedInSession = 0; int _queueItemSequence = 0; bool _isLoaded = false; + bool _foregroundResumeScheduled = false; final Set _ensuredDirs = {}; final Map> _qualityVariantFileLocks = {}; Future? _appFolderStorageFallback; @@ -279,6 +292,26 @@ class DownloadQueueNotifier extends Notifier { await _queuePersistenceWrite; } + /// Restarts a queue that was deliberately left pending because Android did + /// not allow a foreground service to be launched while the app was hidden. + void resumePendingDownloadsOnForeground() { + if (_foregroundResumeScheduled || + state.isProcessing || + state.isPaused || + !state.items.any((item) => item.status == DownloadStatus.queued)) { + return; + } + _foregroundResumeScheduled = true; + Future.microtask(() async { + _foregroundResumeScheduled = false; + if (!state.isProcessing && + !state.isPaused && + state.items.any((item) => item.status == DownloadStatus.queued)) { + await _processQueue(); + } + }); + } + Future _loadQueueFromStorage() async { if (_isLoaded) return; _isLoaded = true; @@ -1239,14 +1272,25 @@ class DownloadQueueNotifier extends Notifier { Future _processQueue() async { if (state.isProcessing) return; + if (Platform.isAndroid && + state.items.any((item) => item.status == DownloadStatus.queued) && + !canStartForegroundDownloadForLifecycle( + WidgetsBinding.instance.lifecycleState, + )) { + _log.i( + 'Download queue is waiting for the app to return to the foreground', + ); + return; + } + var settings = ref.read(settingsProvider); updateSettings(settings); var isSafMode = _isSafMode(settings); var iosDownloadBookmarkActive = false; // Validate SAF before handing the batch to either queue implementation. - // A restored/missing tree URI and an OEM-revoked grant both fall back to a - // verified writable app folder instead of failing the whole queue. + // Never silently redirect a user-selected SAF destination into private app + // storage: keep the selection intact so the UI can request access again. if (Platform.isAndroid && settings.storageMode == 'saf') { var safAccessible = settings.downloadTreeUri.isNotEmpty; if (safAccessible) { @@ -1261,26 +1305,19 @@ class DownloadQueueNotifier extends Notifier { } if (!safAccessible) { _log.w( - 'SAF grant is missing or no longer writable; using app-folder fallback', + 'SAF grant is missing or no longer writable; download location must be reselected', ); - try { - await _activateAppFolderStorageFallback(); - settings = ref.read(settingsProvider); - isSafMode = false; - } catch (e) { - _log.e('Could not activate app-folder storage fallback: $e'); - for (final item in state.items) { - if (item.status == DownloadStatus.queued) { - updateItemStatus( - item.id, - DownloadStatus.failed, - error: safPermissionLostErrorMessage, - errorType: DownloadErrorType.permission, - ); - } + for (final item in state.items) { + if (item.status == DownloadStatus.queued) { + updateItemStatus( + item.id, + DownloadStatus.failed, + error: safPermissionLostErrorMessage, + errorType: DownloadErrorType.permission, + ); } - return; } + return; } } if (Platform.isAndroid && @@ -1320,6 +1357,19 @@ class DownloadQueueNotifier extends Notifier { return; } + // Native request construction can perform asynchronous metadata lookups. + // The app may have moved to the background while those were running. + if (Platform.isAndroid && + !canStartForegroundDownloadForLifecycle( + WidgetsBinding.instance.lifecycleState, + )) { + _log.i( + 'Download queue moved to the background during preparation; deferring start', + ); + _stopConnectivityMonitoring(); + return; + } + state = state.copyWith(isProcessing: true); _log.i('Starting queue processing...'); @@ -1343,6 +1393,14 @@ class DownloadQueueNotifier extends Notifier { ); _log.d('Foreground service started'); } catch (e) { + if (isForegroundServiceStartNotAllowed(e)) { + _log.w( + 'Android deferred the download service start until the app returns to the foreground', + ); + state = state.copyWith(isProcessing: false, currentDownload: null); + _stopConnectivityMonitoring(); + return; + } _log.e('Failed to start foreground service: $e'); } } diff --git a/lib/providers/download_queue_provider_native_worker.dart b/lib/providers/download_queue_provider_native_worker.dart index ffdc1721..891971b1 100644 --- a/lib/providers/download_queue_provider_native_worker.dart +++ b/lib/providers/download_queue_provider_native_worker.dart @@ -47,14 +47,40 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { return false; } - final failedOutputDir = context.storageMode == 'saf' - ? null - : context.outputDir; + if (storageWriteRecoveryFor(useSaf: context.storageMode == 'saf') == + StorageWriteRecovery.requestSafAccess) { + _log.w( + 'Native worker lost SAF access; cancelling the run without changing the selected destination', + ); + try { + await PlatformBridge.cancelNativeDownloadWorker(); + } catch (e) { + _log.w('Failed to cancel native worker after SAF access loss: $e'); + } + for (final pendingId in contexts.keys) { + if (reconciledIds.contains(pendingId)) continue; + final pending = _findItemById(pendingId); + if (pending == null || + pending.status == DownloadStatus.completed || + pending.status == DownloadStatus.skipped) { + continue; + } + reconciledIds.add(pendingId); + updateItemStatus( + pendingId, + DownloadStatus.failed, + error: safPermissionLostErrorMessage, + errorType: DownloadErrorType.permission, + ); + _failedInSession++; + } + return true; + } + final fallbackRoot = await _activateAppFolderStorageFallback( - failedOutputDir: failedOutputDir, + failedOutputDir: context.outputDir, ); - if (context.storageMode != 'saf' && - _pathIsInside(context.outputDir, fallbackRoot)) { + if (_pathIsInside(context.outputDir, fallbackRoot)) { // This request already used the verified fallback. Do not create an // infinite retry loop if the failure has another device-specific cause. return false; @@ -557,6 +583,16 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { }); } + if (!canStartForegroundDownloadForLifecycle( + WidgetsBinding.instance.lifecycleState, + )) { + _log.i( + 'Native worker preparation finished after the app entered the background; deferring start', + ); + _stopConnectivityMonitoring(); + return true; + } + state = state.copyWith(isProcessing: true, isPaused: false); _totalQueuedAtStart = queuedItems.length; _completedInSession = 0; @@ -608,6 +644,14 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { await Future.delayed(const Duration(seconds: 1)); } } catch (e, stack) { + if (isForegroundServiceStartNotAllowed(e)) { + _log.w( + 'Android rejected the native worker start while backgrounded; keeping the queue pending', + ); + await _clearNativeWorkerRunId(runId); + state = state.copyWith(isProcessing: false, currentDownload: null); + return true; + } if (e is _NativeWorkerStartupTimeout) { _log.w( 'Android native worker did not publish a matching snapshot; cancelling native worker and falling back to Dart queue', diff --git a/lib/providers/download_queue_provider_single_item.dart b/lib/providers/download_queue_provider_single_item.dart index d9d32a31..9a4d14ab 100644 --- a/lib/providers/download_queue_provider_single_item.dart +++ b/lib/providers/download_queue_provider_single_item.dart @@ -574,6 +574,19 @@ class _DownloadRun { _log.i('Download was cancelled before storage fallback, skipping'); return false; } + if (storageWriteRecoveryFor(useSaf: effectiveSafMode) == + StorageWriteRecovery.requestSafAccess) { + _log.w( + 'SAF write failed; preserving the selected destination for reauthorization', + ); + result = { + ...result, + 'success': false, + 'error': safPermissionLostErrorMessage, + 'error_type': 'permission', + }; + return true; + } _log.w('Storage write failed, retrying with a writable app folder'); try { await n._activateAppFolderStorageFallback( @@ -1679,14 +1692,17 @@ class _DownloadRun { String errorMsg = e.toString(); DownloadErrorType errorType = DownloadErrorType.unknown; - if (isStorageWriteFailure(errorMessage: errorMsg)) { + if (isStorageWriteFailure(errorMessage: errorMsg) && + storageWriteRecoveryFor(useSaf: n._isSafMode(settings)) == + StorageWriteRecovery.requestSafAccess) { + errorMsg = safPermissionLostErrorMessage; + errorType = DownloadErrorType.permission; + } else if (isStorageWriteFailure(errorMessage: errorMsg)) { try { await n._activateAppFolderStorageFallback( - failedOutputDir: n._isSafMode(settings) - ? null - : (effectiveOutputDir.isNotEmpty - ? effectiveOutputDir - : n.state.outputDir), + failedOutputDir: effectiveOutputDir.isNotEmpty + ? effectiveOutputDir + : n.state.outputDir, ); n.updateItemStatus(item.id, DownloadStatus.queued, progress: 0.0); _log.w( @@ -1701,7 +1717,9 @@ class _DownloadRun { } } - if (errorMsg.contains('could not find Deezer equivalent') || + if (errorType == DownloadErrorType.permission) { + // Keep the explicit SAF reauthorization error selected above. + } else if (errorMsg.contains('could not find Deezer equivalent') || errorMsg.contains('track not found on Deezer')) { errorMsg = 'Track not found on Deezer (Metadata Unavailable)'; errorType = DownloadErrorType.notFound; diff --git a/lib/services/platform_bridge.dart b/lib/services/platform_bridge.dart index 5c0825c6..196ee0ef 100644 --- a/lib/services/platform_bridge.dart +++ b/lib/services/platform_bridge.dart @@ -11,6 +11,14 @@ import 'package:spotiflac_android/utils/logger.dart'; final _log = AppLogger('PlatformBridge'); +const foregroundServiceStartNotAllowedCode = + 'foreground_service_start_not_allowed'; + +bool isForegroundServiceStartNotAllowed(Object error) { + return error is PlatformException && + error.code == foregroundServiceStartNotAllowedCode; +} + Object? _decodeJsonInBackground(String json) => jsonDecode(json); class ExtensionSessionGrantEvent { diff --git a/test/download_start_policy_test.dart b/test/download_start_policy_test.dart new file mode 100644 index 00000000..ba72dc82 --- /dev/null +++ b/test/download_start_policy_test.dart @@ -0,0 +1,54 @@ +import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/providers/download_queue_provider.dart'; +import 'package:spotiflac_android/services/platform_bridge.dart'; + +void main() { + group('download storage recovery', () { + test('requires reauthorization instead of redirecting SAF downloads', () { + expect( + storageWriteRecoveryFor(useSaf: true), + StorageWriteRecovery.requestSafAccess, + ); + }); + + test('keeps automatic fallback for app-managed folders', () { + expect( + storageWriteRecoveryFor(useSaf: false), + StorageWriteRecovery.useAppFolderFallback, + ); + }); + }); + + group('foreground download start policy', () { + test('allows service launch only while the app is resumed', () { + expect( + canStartForegroundDownloadForLifecycle(AppLifecycleState.resumed), + isTrue, + ); + expect( + canStartForegroundDownloadForLifecycle(AppLifecycleState.inactive), + isFalse, + ); + expect( + canStartForegroundDownloadForLifecycle(AppLifecycleState.paused), + isFalse, + ); + expect(canStartForegroundDownloadForLifecycle(null), isFalse); + }); + + test('recognizes the typed platform denial', () { + expect( + isForegroundServiceStartNotAllowed( + PlatformException(code: foregroundServiceStartNotAllowedCode), + ), + isTrue, + ); + expect( + isForegroundServiceStartNotAllowed(PlatformException(code: 'ERROR')), + isFalse, + ); + }); + }); +}