From c2f737458cc61bfb6e090994f405722541c8cbf9 Mon Sep 17 00:00:00 2001 From: t-m-w <7275539+t-m-w@users.noreply.github.com> Date: Wed, 16 Nov 2022 13:08:51 -0500 Subject: [PATCH] Initial support for backup of D2D-only apps Allow backup of apps that would otherwise only support device-to-device migration. This is an initial-support patch to help determine the viability of this approach. Known issues / TODO: * System-scheduled backups will not handle D2D-only apps, unless accompanied by a framework change forcing OperationType.MIGRATION. Backups triggered by the connection of a USB device or by Seedvault's StorageBackupService (files) scheduling are not affected, so they *will* back up D2D-only apps as expected; otherwise, the user may need to perform a backup manually via Backup Now. * Apps with `allowBackup="false"` will appear in Backup Status under "Installed Apps" rather than "Apps that do not allow data backup", and their status will always be blank until they have been backed up. If they are not eligible for migration, it will never change. Other notes: * The unit test for excluding the Storage Plugin provider from backups was discussed, deemed unnecessary, and removed. Co-authored-by: Oliver Scott Change-Id: I5a23d68be66f7d8ed755f2bccb9570ab7be49356 --- .../transport/backup/BackupCoordinator.kt | 9 ++--- .../transport/backup/BackupModule.kt | 3 +- .../transport/backup/PackageService.kt | 40 ++++++++++++++----- .../transport/restore/RestoreCoordinator.kt | 11 ++++- .../transport/backup/BackupCoordinatorTest.kt | 16 -------- .../restore/RestoreCoordinatorTest.kt | 2 +- 6 files changed, 46 insertions(+), 35 deletions(-) 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) }