From 639947b87e220a5c7b96a31bacf6d86b09bef1d4 Mon Sep 17 00:00:00 2001 From: Torsten Grote 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 4a4351e60..18ca14fa8 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" /> - + + + - + ().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 000000000..017b154eb --- /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 dc3af8756..d62bb9dbc 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 89f006708..0d4fccef0 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 04de1dcf3..a8230c154 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -169,6 +169,9 @@ Insufficient backup space Your backup location is running out of space. Free up space, so backups can run. + Restore notification + Restore running + Auto restore flash drive error Could not restore data for %1$s Plug in your %1$s before installing the app to restore its data from backup. 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 562766b7b..389bfaf2a 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 8d2548c43..8fcaece16 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 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 0bf703a42..8d8bb01fb 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 d62bb9dbc..48aa99f9d 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 389bfaf2a..5fae19666 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 8fcaece16..201824f6c 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 }