diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt index 08c56f66..87a13020 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinator.kt @@ -143,12 +143,9 @@ internal class BackupCoordinator( @Suppress("UNUSED_PARAMETER") isFullBackup: Boolean, ): Boolean { val packageName = targetPackage.packageName - // Check that the app is not blacklisted by the user - val enabled = settingsManager.isBackupEnabled(packageName) - if (!enabled) Log.w(TAG, "Backup of $packageName disabled by user.") - // We need to exclude the DocumentsProvider used to store backup data. - // Otherwise, it gets killed when we back it up, terminating our backup. - return enabled && targetPackage.packageName != plugin.providerPackageName + val shouldInclude = packageService.shouldIncludeAppInBackup(packageName) + if (!shouldInclude) Log.i(TAG, "Excluding $packageName from backup.") + return shouldInclude } /** diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt index 8be00ac6..14fd6261 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/BackupModule.kt @@ -8,7 +8,8 @@ val backupModule = module { single { PackageService( context = androidContext(), - backupManager = get() + settingsManager = get(), + plugin = get() ) } single { diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt index fbadc1a4..d298856e 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/backup/PackageService.kt @@ -1,6 +1,5 @@ package com.stevesoltys.seedvault.transport.backup -import android.app.backup.IBackupManager import android.content.Context import android.content.pm.ApplicationInfo.FLAG_ALLOW_BACKUP import android.content.pm.ApplicationInfo.FLAG_STOPPED @@ -12,11 +11,12 @@ import android.content.pm.PackageManager import android.content.pm.PackageManager.GET_INSTRUMENTATION import android.content.pm.PackageManager.GET_SIGNING_CERTIFICATES import android.os.RemoteException -import android.os.UserHandle import android.util.Log import android.util.Log.INFO import androidx.annotation.WorkerThread import com.stevesoltys.seedvault.MAGIC_PACKAGE_MANAGER +import com.stevesoltys.seedvault.plugins.StoragePlugin +import com.stevesoltys.seedvault.settings.SettingsManager private val TAG = PackageService::class.java.simpleName @@ -28,11 +28,11 @@ private const val LOG_MAX_PACKAGES = 100 */ internal class PackageService( private val context: Context, - private val backupManager: IBackupManager, + private val settingsManager: SettingsManager, + private val plugin: StoragePlugin, ) { private val packageManager: PackageManager = context.packageManager - private val myUserId = UserHandle.myUserId() val eligiblePackages: Array @WorkerThread @@ -45,13 +45,16 @@ internal class PackageService( // log packages if (Log.isLoggable(TAG, INFO)) { Log.i(TAG, "Got ${packages.size} packages:") - packages.chunked(LOG_MAX_PACKAGES).forEach { - Log.i(TAG, it.toString()) - } + logPackages(packages) } - val eligibleApps = - backupManager.filterAppsEligibleForBackupForUser(myUserId, packages.toTypedArray()) + // We do not use BackupManager.filterAppsEligibleForBackupForUser because it + // always makes its determinations based on OperationType.BACKUP, never based on + // OperationType.MIGRATION, and there are no alternative publicly-available APIs. + // We don't need to use it, here, either; during a backup or migration, the system + // will perform its own eligibility checks regardless. We merely need to filter out + // apps that we, or the user, want to exclude. + val eligibleApps = packages.filter(::shouldIncludeAppInBackup) // log eligible packages if (Log.isLoggable(TAG, INFO)) { @@ -128,6 +131,14 @@ internal class PackageService( null } + fun shouldIncludeAppInBackup(packageName: String): Boolean { + // Check that the app is not excluded by user preference + val enabled = settingsManager.isBackupEnabled(packageName) + // We also need to exclude the DocumentsProvider used to store backup data. + // Otherwise, it gets killed when we back it up, terminating our backup. + return enabled && packageName != plugin.providerPackageName + } + private fun logPackages(packages: List) { packages.chunked(LOG_MAX_PACKAGES).forEach { Log.i(TAG, it.toString()) @@ -159,7 +170,16 @@ internal fun PackageInfo.isSystemApp(): Boolean { internal fun PackageInfo.allowsBackup(): Boolean { if (packageName == MAGIC_PACKAGE_MANAGER || applicationInfo == null) return false - return applicationInfo.flags and FLAG_ALLOW_BACKUP != 0 + + // TODO: Consider ways of replicating the system's logic so that the user can have advance + // knowledge of apps that the system will exclude, particularly apps targeting SDK 30 or below. + + // At backup time, the system will filter out any apps that *it* does not want to be backed up. + // Now that we have switched to D2D, *we* generally want to back up as much as possible; + // part of the point of D2D is to ignore FLAG_ALLOW_BACKUP (allowsBackup). So, we return true. + // See frameworks/base/services/backup/java/com/android/server/backup/utils/ + // BackupEligibilityRules.java lines 74-81 and 163-167 (tag: android-13.0.0_r8). + return true } /** diff --git a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt index 23d8b6a6..f6a00ecc 100644 --- a/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt +++ b/app/src/main/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinator.kt @@ -26,6 +26,14 @@ import com.stevesoltys.seedvault.transport.TRANSPORT_FLAGS import com.stevesoltys.seedvault.ui.notification.BackupNotificationManager import java.io.IOException +/** + * Device name used in AOSP to indicate that a restore set is part of a device-to-device migration. + * See getBackupEligibilityRules in frameworks/base/services/backup/java/com/android/server/ + * backup/restore/ActiveRestoreSession.java. AOSP currently relies on this constant, and it is not + * publicly exposed. Framework code indicates they intend to use a flag, instead, in the future. + */ +internal const val DEVICE_NAME_FOR_D2D_SET = "D2D" + private data class RestoreCoordinatorState( val token: Long, val packages: Iterator, @@ -92,7 +100,8 @@ internal class RestoreCoordinator( **/ suspend fun getAvailableRestoreSets(): Array? { return getAvailableMetadata()?.map { (_, metadata) -> - RestoreSet(metadata.deviceName, metadata.deviceName, metadata.token, TRANSPORT_FLAGS) + RestoreSet(metadata.deviceName /* name */, DEVICE_NAME_FOR_D2D_SET /* device */, + metadata.token, TRANSPORT_FLAGS) }?.toTypedArray() } diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt index 5a7ece97..bb9de194 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/backup/BackupCoordinatorTest.kt @@ -32,8 +32,6 @@ import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.runBlocking import org.junit.jupiter.api.Assertions.assertEquals -import org.junit.jupiter.api.Assertions.assertFalse -import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import java.io.IOException import java.io.OutputStream @@ -170,20 +168,6 @@ internal class BackupCoordinatorTest : BackupTest() { verify { metadataOutputStream.close() } } - @Test - fun `isAppEligibleForBackup() exempts plugin provider and blacklisted apps`() { - every { - settingsManager.isBackupEnabled(packageInfo.packageName) - } returns true andThen false andThen true - every { - plugin.providerPackageName - } returns packageInfo.packageName andThen "new.package" andThen "new.package" - - assertFalse(backup.isAppEligibleForBackup(packageInfo, true)) - assertFalse(backup.isAppEligibleForBackup(packageInfo, true)) - assertTrue(backup.isAppEligibleForBackup(packageInfo, true)) - } - @Test fun `clearing KV backup data throws`() = runBlocking { every { settingsManager.getToken() } returns token diff --git a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt index c7c11dd8..4f704dee 100644 --- a/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt +++ b/app/src/test/java/com/stevesoltys/seedvault/transport/restore/RestoreCoordinatorTest.kt @@ -87,7 +87,7 @@ internal class RestoreCoordinatorTest : TransportTest() { val sets = restore.getAvailableRestoreSets() ?: fail() assertEquals(2, sets.size) - assertEquals(metadata.deviceName, sets[0].device) + assertEquals(DEVICE_NAME_FOR_D2D_SET, sets[0].device) assertEquals(metadata.deviceName, sets[0].name) assertEquals(metadata.token, sets[0].token) }