From 639947b87e220a5c7b96a31bacf6d86b09bef1d4 Mon Sep 17 00:00:00 2001 From: Torsten Grote <t@grobox.de> Date: Tue, 20 Aug 2024 17:37:02 -0300 Subject: [PATCH 1/2] Start a foreground service during restore so the system won't kill us, even if the user navigates away. --- app/src/main/AndroidManifest.xml | 10 +++- .../restore/AppDataRestoreManager.kt | 6 +++ .../seedvault/restore/RestoreService.kt | 46 +++++++++++++++++++ .../seedvault/restore/install/ApkRestore.kt | 5 ++ .../notification/BackupNotificationManager.kt | 26 +++++++++-- app/src/main/res/values/strings.xml | 3 ++ .../restore/install/ApkBackupRestoreTest.kt | 8 ++++ .../restore/install/ApkRestoreTest.kt | 8 ++++ 8 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 app/src/main/java/com/stevesoltys/seedvault/restore/RestoreService.kt diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 4a4351e6..18ca14fa 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -177,13 +177,19 @@ android:exported="false" android:label="BackupJobService" android:permission="android.permission.BIND_JOB_SERVICE" /> - <!-- Does the actual backup work as a foreground service --> + <!-- Does app restore as a foreground service --> + <service + android:name=".restore.RestoreService" + android:exported="false" + android:foregroundServiceType="dataSync" + android:label="RestoreService" /> + <!-- Does the actual file backup work as a foreground service --> <service android:name=".storage.StorageBackupService" android:exported="false" android:foregroundServiceType="dataSync" android:label="BackupService" /> - <!-- Does restore as a foreground service --> + <!-- Does file restore as a foreground service --> <service android:name=".storage.StorageRestoreService" android:exported="false" diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt index ccd7dd3f..0bf703a4 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt @@ -12,6 +12,7 @@ import android.app.backup.IRestoreObserver import android.app.backup.IRestoreSession import android.app.backup.RestoreSet import android.content.Context +import android.content.Intent import android.os.RemoteException import android.os.UserHandle import android.util.Log @@ -60,6 +61,7 @@ internal class AppDataRestoreManager( private var session: IRestoreSession? = null private val monitor = BackupMonitor() + private val foregroundServiceIntent = Intent(context, RestoreService::class.java) private val mRestoreProgress = MutableLiveData( LinkedList<AppRestoreResult>().apply { @@ -120,6 +122,8 @@ internal class AppDataRestoreManager( mRestoreBackupResult.postValue( RestoreBackupResult(context.getString(R.string.restore_set_error)) ) + } else { + context.startForegroundService(foregroundServiceIntent) } } @@ -208,6 +212,8 @@ internal class AppDataRestoreManager( mRestoreProgress.postValue(list) mRestoreBackupResult.postValue(result) + + context.stopService(foregroundServiceIntent) } fun closeSession() { diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreService.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreService.kt new file mode 100644 index 00000000..017b154e --- /dev/null +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/RestoreService.kt @@ -0,0 +1,46 @@ +/* + * SPDX-FileCopyrightText: 2024 The Calyx Institute + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.stevesoltys.seedvault.restore + +import android.app.Service +import android.content.Intent +import android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_MANIFEST +import android.os.IBinder +import android.util.Log +import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager +import com.stevesoltys.seedvault.ui.notification.NOTIFICATION_ID_RESTORE +import org.koin.android.ext.android.inject + +class RestoreService : Service() { + + companion object { + private const val TAG = "RestoreService" + } + + private val nm: BackupNotificationManager by inject() + + override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { + Log.i(TAG, "onStartCommand $intent $flags $startId") + + startForeground( + NOTIFICATION_ID_RESTORE, + nm.getRestoreNotification(), + FOREGROUND_SERVICE_TYPE_MANIFEST, + ) + return START_STICKY_COMPATIBILITY + } + + override fun onBind(intent: Intent?): IBinder? { + return null + } + + override fun onDestroy() { + Log.i(TAG, "onDestroy") + super.onDestroy() + nm.cancelRestoreNotification() + } + +} diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt index dc3af875..d62bb9db 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt @@ -7,6 +7,7 @@ package com.stevesoltys.seedvault.restore.install import android.app.backup.IBackupManager import android.content.Context +import android.content.Intent import android.content.pm.PackageManager import android.content.pm.PackageManager.GET_SIGNATURES import android.content.pm.PackageManager.GET_SIGNING_CERTIFICATES @@ -20,6 +21,7 @@ import com.stevesoltys.seedvault.plugins.LegacyStoragePlugin import com.stevesoltys.seedvault.plugins.StoragePlugin import com.stevesoltys.seedvault.plugins.StoragePluginManager import com.stevesoltys.seedvault.restore.RestorableBackup +import com.stevesoltys.seedvault.restore.RestoreService import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED import com.stevesoltys.seedvault.restore.install.ApkInstallState.FAILED_SYSTEM_APP import com.stevesoltys.seedvault.restore.install.ApkInstallState.IN_PROGRESS @@ -85,14 +87,17 @@ internal class ApkRestore( return } mInstallResult.value = InstallResult(packages) + val i = Intent(context, RestoreService::class.java) val autoRestore = backupStateManager.isAutoRestoreEnabled try { + context.startForegroundService(i) // disable auto-restore before installing apps, if it was enabled before if (autoRestore) backupManager.setAutoRestore(false) reInstallApps(backup, packages.asIterable().reversed()) } finally { // re-enable auto-restore, if it was enabled before if (autoRestore) backupManager.setAutoRestore(true) + context.stopService(i) } mInstallResult.update { it.copy(isFinished = true) } } diff --git a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt index 89f00670..0d4fccef 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/ui/notification/BackupNotificationManager.kt @@ -38,14 +38,16 @@ import kotlin.math.min private const val CHANNEL_ID_OBSERVER = "NotificationBackupObserver" private const val CHANNEL_ID_SUCCESS = "NotificationBackupSuccess" private const val CHANNEL_ID_ERROR = "NotificationError" +private const val CHANNEL_ID_RESTORE = "NotificationRestore" private const val CHANNEL_ID_RESTORE_ERROR = "NotificationRestoreError" internal const val NOTIFICATION_ID_OBSERVER = 1 private const val NOTIFICATION_ID_SUCCESS = 2 private const val NOTIFICATION_ID_ERROR = 3 private const val NOTIFICATION_ID_SPACE_ERROR = 4 -private const val NOTIFICATION_ID_RESTORE_ERROR = 5 -private const val NOTIFICATION_ID_BACKGROUND = 6 -private const val NOTIFICATION_ID_NO_MAIN_KEY_ERROR = 7 +internal const val NOTIFICATION_ID_RESTORE = 5 +private const val NOTIFICATION_ID_RESTORE_ERROR = 6 +private const val NOTIFICATION_ID_BACKGROUND = 7 +private const val NOTIFICATION_ID_NO_MAIN_KEY_ERROR = 8 private val TAG = BackupNotificationManager::class.java.simpleName @@ -55,6 +57,7 @@ internal class BackupNotificationManager(private val context: Context) { createNotificationChannel(getObserverChannel()) createNotificationChannel(getSuccessChannel()) createNotificationChannel(getErrorChannel()) + createNotificationChannel(getRestoreChannel()) createNotificationChannel(getRestoreErrorChannel()) } @@ -77,6 +80,11 @@ internal class BackupNotificationManager(private val context: Context) { return NotificationChannel(CHANNEL_ID_ERROR, title, IMPORTANCE_DEFAULT) } + private fun getRestoreChannel(): NotificationChannel { + val title = context.getString(R.string.notification_restore_error_channel_title) + return NotificationChannel(CHANNEL_ID_RESTORE, title, IMPORTANCE_LOW) + } + private fun getRestoreErrorChannel(): NotificationChannel { val title = context.getString(R.string.notification_restore_error_channel_title) return NotificationChannel(CHANNEL_ID_RESTORE_ERROR, title, IMPORTANCE_HIGH) @@ -235,6 +243,18 @@ internal class BackupNotificationManager(private val context: Context) { nm.notify(NOTIFICATION_ID_SPACE_ERROR, notification) } + fun getRestoreNotification() = Notification.Builder(context, CHANNEL_ID_RESTORE).apply { + setSmallIcon(R.drawable.ic_cloud_restore) + setContentTitle(context.getString(R.string.notification_restore_title)) + setOngoing(true) + setShowWhen(false) + setWhen(System.currentTimeMillis()) + }.build() + + fun cancelRestoreNotification() { + nm.cancel(NOTIFICATION_ID_RESTORE) + } + @SuppressLint("RestrictedApi") fun onRemovableStorageNotAvailableForRestore(packageName: String, storageName: String) { val appName = try { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 04de1dcf..a8230c15 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -169,6 +169,9 @@ <string name="notification_space_error_title">Insufficient backup space</string> <string name="notification_space_error_text">Your backup location is running out of space. Free up space, so backups can run.</string> + <string name="notification_restore_channel_title">Restore notification</string> + <string name="notification_restore_title">Restore running</string> + <string name="notification_restore_error_channel_title">Auto restore flash drive error</string> <string name="notification_restore_error_title">Could not restore data for %1$s</string> <string name="notification_restore_error_text">Plug in your %1$s before installing the app to restore its data from backup.</string> diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt index 562766b7..389bfaf2 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt @@ -6,6 +6,7 @@ package com.stevesoltys.seedvault.restore.install import android.app.backup.IBackupManager +import android.content.ComponentName import android.content.Context import android.content.pm.PackageManager import android.content.pm.PackageManager.NameNotFoundException @@ -127,6 +128,13 @@ internal class ApkBackupRestoreTest : TransportTest() { writeBytes(splitBytes) }.absolutePath) + // related to starting/stopping service + every { strictContext.packageName } returns "org.foo.bar" + every { + strictContext.startForegroundService(any()) + } returns ComponentName(strictContext, "org.foo.bar.Class") + every { strictContext.stopService(any()) } returns true + every { settingsManager.isBackupEnabled(any()) } returns true every { settingsManager.backupApks() } returns true every { sigInfo.hasMultipleSigners() } returns false diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt index 8d2548c4..8fcaece1 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt @@ -6,6 +6,7 @@ package com.stevesoltys.seedvault.restore.install import android.app.backup.IBackupManager +import android.content.ComponentName import android.content.Context import android.content.pm.ApplicationInfo.FLAG_INSTALLED import android.content.pm.ApplicationInfo.FLAG_SYSTEM @@ -107,6 +108,13 @@ internal class ApkRestoreTest : TransportTest() { packageInfo.signingInfo = mockk(relaxed = true) every { storagePluginManager.appPlugin } returns storagePlugin + + // related to starting/stopping service + every { strictContext.packageName } returns "org.foo.bar" + every { + strictContext.startForegroundService(any()) + } returns ComponentName(strictContext, "org.foo.bar.Class") + every { strictContext.stopService(any()) } returns true } @Test From d266c36c91f98359059ae6f7a68e58e8026451d3 Mon Sep 17 00:00:00 2001 From: Torsten Grote <t@grobox.de> Date: Thu, 22 Aug 2024 17:36:26 -0300 Subject: [PATCH 2/2] Don't use Context#startForegroundService() because we may get killed If the foreground service doesn't have anything to do and terminates quickly, the system will kill us, even though the service had called startForeground(). To prevent this, we don't promise that our service will be a foreground service. We can still be a foreground service, but escape the punishment if we are too quick. --- .../com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt | 3 ++- .../com/stevesoltys/seedvault/restore/install/ApkRestore.kt | 3 ++- .../seedvault/restore/install/ApkBackupRestoreTest.kt | 2 +- .../stevesoltys/seedvault/restore/install/ApkRestoreTest.kt | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt index 0bf703a4..8d8bb01f 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/AppDataRestoreManager.kt @@ -123,7 +123,8 @@ internal class AppDataRestoreManager( RestoreBackupResult(context.getString(R.string.restore_set_error)) ) } else { - context.startForegroundService(foregroundServiceIntent) + // don't use startForeground(), because we may stop it sooner than the system likes + context.startService(foregroundServiceIntent) } } diff --git a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt index d62bb9db..48aa99f9 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/restore/install/ApkRestore.kt @@ -90,7 +90,8 @@ internal class ApkRestore( val i = Intent(context, RestoreService::class.java) val autoRestore = backupStateManager.isAutoRestoreEnabled try { - context.startForegroundService(i) + // don't use startForeground(), because we may stop it sooner than the system likes + context.startService(i) // disable auto-restore before installing apps, if it was enabled before if (autoRestore) backupManager.setAutoRestore(false) reInstallApps(backup, packages.asIterable().reversed()) diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt index 389bfaf2..5fae1966 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkBackupRestoreTest.kt @@ -131,7 +131,7 @@ internal class ApkBackupRestoreTest : TransportTest() { // related to starting/stopping service every { strictContext.packageName } returns "org.foo.bar" every { - strictContext.startForegroundService(any()) + strictContext.startService(any()) } returns ComponentName(strictContext, "org.foo.bar.Class") every { strictContext.stopService(any()) } returns true diff --git a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt index 8fcaece1..201824f6 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/restore/install/ApkRestoreTest.kt @@ -112,7 +112,7 @@ internal class ApkRestoreTest : TransportTest() { // related to starting/stopping service every { strictContext.packageName } returns "org.foo.bar" every { - strictContext.startForegroundService(any()) + strictContext.startService(any()) } returns ComponentName(strictContext, "org.foo.bar.Class") every { strictContext.stopService(any()) } returns true }