summaryrefslogtreecommitdiff
path: root/services/backup
AgeCommit message (Collapse)Author
2025-03-07Merge "Log cancellation reason for full backups to BMM" into main Sarp Misoglu
2025-03-07Log cancellation reason for full backups to BMM Sarp Misoglu
Currently it's not possible to know from these logs if the backup was cancelled due to timeout or the app crashing or due to backups being explicitly cancelled by an external caller. This CL fixes that issue. Flag: EXEMPT log only update Test: manual // PFTBT is notoriously impossible to write unit tests for. Bug: 399645990 Change-Id: Ia18fdec1d9547130926bf62666ac055e320fb0b6
2025-03-06Merge "Don't cancel the entire backup if agent disconnects and log if the ↵ Sarp Misoglu
agent pipe is broken" into main
2025-03-05Merge "Set `mStatus` when we don't have a next package to restore" into main Joël Stemmer
2025-03-05Don't cancel the entire backup if agent disconnects and log if the agent ↵ Sarp Misoglu
pipe is broken Normally the bug fix and the extra logging should be in separate CLs. However as explained in the linked bug, there is a race condition between the method throwing the IOException returning and us getting a cancellation callback from ActivityManager. If we get the callback first, the session fails. The extra IPC from the logging in handling the exception means the callback gets a few additional milliseconds to win the race. So with the logging, the race condition is much more likely to happen and the logging should not be checked in without the bug fix. And if one gets rolled back, the other should as well. This is why I made them a single CL. Flag: EXEMPT bugfix Bug: 400669414 Test: manual // PFTBT is notoriously impossible to write unit tests for. Change-Id: Icbcb72a876f927a94675c9837b0206f1a631dff5
2025-03-05Set `mStatus` when we don't have a next package to restore Joël Stemmer
In `dispatchNextRestore` we ask the transport for the next package to restore. If this package is null, we immediately abort the restore. This can happen when the transport encounters some permanent failure and returns null, or when the transport disappears and the `BackupTransportClient` returns null. The problem with the current implementation is that it doesn't set `mStatus` to `TRANSPORT_ERROR`, which means the restore session is reported to have ended successfully. This means we won't attempt to retry the restore. See also ag/26064663. Bug: 397944266 Bug: 381408714 Flag: EXEMPT bugfix Test: manually tested Test: presubmits Change-Id: I40f4406e4180f3224ab3a5d7c32e4c7028d6484e
2025-03-04Pass in cancellation reason to B&R tasks Sarp Misoglu
This is to give more granular information to tasks about why they got cancelled. This will help them report why they failed. The cancelAll parameter was only set to false for cancellations due to timeouts, so I've removed it. This is a no-op refactor. Flag: EXEMPT refactor Bug: 399645990 Test: atest CtsBackupTestCases Change-Id: I1bd6d9eff42c5edeb0f4df453bae792cf994bb6a
2025-02-25Use correct package name for monitor events Sarp Misoglu
The currentPackage member variable in PFTBT is not necessary and is set to the last package that was passed in to the constructor. Then all events are sent with this package name even if they are for different packages. Bug: 396664485 Flag: EXEMPT Bugfix Test: presubmit Change-Id: Icd6c04399d6e5345a94dfdf4bb46e15928dcca7a
2025-01-27Synchronize calls to no restricted mode lists Sarp Misoglu
This is to avoid concurrent modification errors. Fixes: 390483439 Test: atest CtsBackupTestCases Flag: EXEMPT bug fix Change-Id: I8d75ea1fc15705ee847e911452b6ba0029d7a000
2025-01-13Use a variable to add userId to logs in UBMS Sarp Misoglu
And format it properly. The variable formats better compared to a method call. Test: n/a Flag: EXEMPT no-op refactor Change-Id: I617ee8d1b05a65e21b408186cf2c25cae76ed7bb
2025-01-09Move BackupWakeLock to its own file Sarp Misoglu
There's no reason for it to be under UserBackupManagerService and bloat it up. Test: n/a Flag: EXEMPT no-op refactor Change-Id: I595fe26e97e804e59033a5dc95e8fa8e16de895f
2025-01-09Fix debug logging in the backup service Sarp Misoglu
The existing DEBUG and DEBUG_SCHEDULING had been set to true (maybe accidentally?) for 8+ years. Whereas MORE_DEBUG is false as expected. In this CL we: 1) Remove existing DEBUG and DEBUG_SCHEDULING checks. This is a no-op. 2) Rename MORE_DEBUG to DEBUG. Also no-op. 3) Make any Log.v log that isn't guarded by if(DEBUG) a Log.d instead. Log.v logs should always be guarded by a default false constant so the compiler strips out the Strings from the binary. Test: n/a Flag: EXEMPT no-op refactor Change-Id: I117f7fe04da1a62c3ad42d71b080fed6454f73af
2025-01-09Use internal interface for AMS -> BMS calls Sarp Misoglu
Currently these go through the IBackupManager.aidl interface, which is unnecessary since they are local calls within the system server. Test: atest CtsBackupTestCases Flag: EXEMPT no-op refactor Change-Id: I260197d30184d10a92f356f729d28fd0ab22aec1
2024-12-23Acquire the backup wakelock with a timeout Sarp Misoglu
There have been a few reports of the backup service failing to call release() on the wakelock and draining battery for hours. These are rare failures and hard to debug individually but with the timeout we will limit the worst case significantly. I am setting the default timeout to 30 minutes. This should be more than enough time for a backup or restore operation. If in the future these operation get longer, the timeout is configurable by a setting. Test: atest CtsBackupTestCases Fixes: 364931501 Fixes: 357769443 Fixes: 289789401 Flag: EXEMPT bugfix Change-Id: Iaac0218d5258031b6f67c3c56c776ce098e8a8c4
2024-12-17Check backup wakelock is held before releasing Sarp Misoglu
This is to prevent a rare crash that can happen if something releases the wakelock beforehand. I'm not sure exactly what sequence of events leads to the crash scenario but this seems like a sensible check to have anyway since we have multiple operations that can acquire/release the wakelock. Flag: EXEMPT bugfix Fixes: 349280741 Test: CtsBackupTestCases, CtsBackupHostTestCases Change-Id: I4a5c7ea0cf381de051a4a4afdf2426f9715515c8
2024-12-03Merge "Delete Filipendo workaround code for R" into main Sarp Misoglu
2024-12-02Delete Filipendo workaround code for R Sarp Misoglu
It seems this was intended to be removed for S. I've also double checked this setting isn't used anywhere that I can find. Fixes: 154822946 Test: presubmit Change-Id: I974b30a54883280d16dacf9a2de876d87f1e2c13
2024-11-28Merge "Revert^2 "Mark @FlaggedApi flags as exported"" into main Treehugger Robot
2024-11-28Revert^2 "Mark @FlaggedApi flags as exported" Mårten Kongstad
322dd88f9dedd580115bd62377c2a21323540d2f Change-Id: I9d1b06d9bea270e22f364bbc4be24174d3ae53fe
2024-11-27Merge "Don't kill apps if they are not in restricted mode" into main Sarp Misoglu
2024-11-27Merge changes from topic "revert-30583310-fix-non-exported-flags-UJJZKWBRYV" ↵ Chaitanya Cheemala (xWF)
into main * changes: Revert "accessibility_flags.aconfig: remove leading whitespace" Revert "Mark @FlaggedApi flags as exported"
2024-11-27Don't kill apps if they are not in restricted mode Sarp Misoglu
Also respect the killAfterRestore flag for them. Before this change, the android:killAfterRestore attribute was only respected for apps that do Key/Value backups. This was because Full backup apps used to always be started in restricted mode for B&R operations and the framework needed to kill the app to reset the restricted mode state. Now, full backup apps may not be started in restricted mode (after ag/30155472) so there's no need to kill them unless if they set killAfterRestore. For this to work, we need to keep track of whether an app is in restricted mode. I've moved all connection/disconnection logic to the new connection manager class so this is now fairly easy. Note that apps will only not be in restricted mode if they are K/V or if enable_restricted_mode_changes is enabled and they opted out. So I think all the new behavior is guarded by the same flag. Flag: com.android.server.backup.enable_restricted_mode_changes Bug: 376661510 Test: atest BackupAgentConnectionManagerTest & atest CtsBackupHostTestCases Change-Id: I1e94fb3b794961904a6c603ac34b00f3878f4e3e
2024-11-27Revert "Mark @FlaggedApi flags as exported" Chaitanya Cheemala (xWF)
Revert submission 30583310-fix-non-exported-flags Reason for revert: Likely culprit for b/381233132 - verifying through ABTD before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted. Reverted changes: /q/submissionid:30583310-fix-non-exported-flags Change-Id: Iab8df31884c344069dfccde634623e0cc64ec13c
2024-11-27Merge changes from topic "fix-non-exported-flags" into main Mårten Kongstad
* changes: Mark @FlaggedApi flags as exported accessibility_flags.aconfig: remove leading whitespace
2024-11-27Add flag to enable SettingsBackupAgent to collect B&R agent metrics. Daniel Perez
Change-Id: Ibed0deef0a7ca67afe2df79fe447deae755ef49e Bug: 379861078 Flag: com.android.server.backup.enable_metrics_settings_backup_agents
2024-11-27Merge "Add flag for read_all_external_storage_files" into main XingHai Lu
2024-11-26Mark @FlaggedApi flags as exported Mårten Kongstad
An aconfig flag that is used together with @FlaggedApi must be marked as `is_exported: true` to guarantee that the auto-generated lookup code checks the actual flag value (instead of using a hard-coded value set at compile time). This is important when the API is called across aconfig container boundaries (e.g. a mainline module calling code on the system partition). Mark all non-exported flags used with @FlaggedApi as exported. The "all @FlaggedApi flags are exported" invariant should be checked at build time; this will be added in future CLs. Bug: 378061535 Test: treehugger Flag: EXEMPT can't flag changes to flag declarations Change-Id: If45930e4afdcc87e374679ea73502651ce2de445
2024-11-25Add flag for read_all_external_storage_files XingHaiLu
The flag is used in the long term solution of download files B&R project(go/dd-br-download-files). It will be used in - AppOpsManager which creates new appop - os/Environment which creates new public api to read external files The flag is added in the backup repo due to it's a B&R project and teamfood will be launched inside the backup team. Bug: 376598575 Test: FLAG_ONLY FLAG: com.android.server.backup.enable_read_all_external_storage_files Change-Id: I3c4e122fe6499a612b3986011bd021eb64c319af
2024-11-20Ensure we bind to the correct app's agent Sarp Misoglu
We have a 10s timeout in bindToAgentSynchronous and if we don't get an agentConnected callback by then the method returns null. There was a potential bug here because agentConnected doesn't check if the agent is the one we are currently expecting. It could happen with the following sequence of events: 1. bindToAgentSynchronous(A) 2. app A times out 3. bindToAgentSynchronous(B) 4. agentConnected(A) 5. the call from 3 returns the agent for app A instead of B. With this change we start tracking the state of the current connection better. Test: atest BackupAgentConnectionManagerTest & atest CtsBackupHostTestCases Bug: 328019933 Change-Id: I0556aff00fe8c2bbfe56fe10f15b7b15ff260590 Change-Id: I4c04e9279d9863716fc080c6ab218665e1591d51
2024-11-18Move BackupAgent connection to a helper class Sarp Misoglu
This is to isolate some scope of UserBackupManagerService. It's too big and unreadable. I've also added some tests. But since these are blocking and async operations, I couldn't see any way other than adding some thread sleeps to avoid flakiness. These tests could still end up being too flaky. If that's the case we might have to leave this code untested :( This is a no-op refactor. Bug: 376661510 Flag: EXEMPT refactor Test: atest BackupAgentConnectionManagerTest & atest CtsBackupHostTestCases Change-Id: I9893336fb224148ce5b2f3c6fa2fc26828d2a1e9
2024-11-14Don't use backup restricted mode in certain cases Sarp Misoglu
In this change we introduce a new ApplicationManifest property which allows apps to specify whether they want to be put in restricted mode for B&R operations. If the app explicitly set the property, it's always respected. If the app has not set the property then we use targetSdk gating: * For targetSdk < 36, we keep the status quo and put the app into restricted mode. * For targetSdk >= 36, we call a new API to the BackupTransport for it to make a decision on a per-package basis. Some implementation details explained: * In order to not block process creation in ActivityManager on an IPC to the BackupTransport, we call the transport earlier in PerformFullTransportBackupTask (for backup) and PerforUnifiedRestoreTask (for restore). We cache the list in memory in BackupManagerService. * When AMS#bindBackupAgent is called, the BackupManagerService tells it whether to use restricted mode. AMS stores this in the existing BackupRecord data class and uses it in attachApplication(). * PerformUnifiedRestoreTask is an extremely untestable state machine and testing this properly is difficult without a significant rewrite so I'm using @VisibleForTesting. * Seems like AMS#attachApplication also requires a lot of mocking so couldn't add tests there. Flag: com.android.server.backup.enable_restricted_mode_changes Bug: 376661510 Test: atest (see tests changed) and manually with a test app that defined its own Application subclass. API-Coverage-Bug: 379086316 Change-Id: Ie060890131ba526d58ec9134e52fd80acc23ef63
2024-11-05Allowlist TelephonyProvider HSUM backup&restore Adam Bookatz
Allow TelephonyProvider to be eligible for backup&restore on non-user-0 for HSUM devices, since on HSUM the provider really belongs to the MainUser. Test: manual backup and restore of messages on HSUM device Bug: 369560193 Flag: EXEMPT bugfix Change-Id: I4dfd4287a1f291b8c7ec3c2075a109696ffb8baa
2024-09-16Merge "Batch migration of frameworks/base TEST_MAPPING to test_module_config ↵ Ronald Braunstein
modules" into main am: cb650f8398 am: 0c9ffcdcf5 Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/3263804 Change-Id: I6d38c92169c7c58c4c8dec3d6c1caaeb70804038 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2024-09-15Batch migration of frameworks/base TEST_MAPPING to test_module_config modules Ronald Braunstein
As part of go/test-module-config we are moving test options from TEST_MAPPING -> Android.bp files. In previous Cls, we created the new `test_module_config` rules in Android.bp This is updating the TEST_MAPPING file to use those rules. It is also removing "FlakyTest and IgnoreTest" exclude annoations as they are now added in gcl files per run rather than ad-hoc per module. I have a script that looks at the generated tradefed config file for the new options added in Android.bp files, then it looks at TEST_MAPPING files and find the places to update where the options match for the test. I am also doing abtd runs of each TEST_MAPPING file before and after my change and ensuring the number of tests run is the same (or at least as many). There are cases where tradefed would comping include-filters across TEST_MAPPING entries for the same module, but now they will be purposefully split up, causing some tests to be run under two different modules. Flag: TEST_ONLY Test: Ran adbt on each TEST_MAPPING and compared before and after results. Verified we were still running all the tests we were before. i.e. after the adbt run, I would download the test artfiact for the tradefed detailed evenvt and compare test counts. You can see CtsAppTestCases became CtsAppTestCases_cts_requesttileserviceaddtest, etc. I'm not including results for all 100 TEST_MAPPING files, but I did verify with scripts and eyes. Minor differences (like 2011 vs 2009) on a test that didn't change are ignored, but in general there were more tests run, not fewer. % diff <(grep started frameworks_base_services_core_java_com_android_server_statusbar_TEST_MAPPING/BASE.details) <(grep started frameworks_base_services_core_java_com_android_server_statusbar_TEST_MAPPING/NEW.details) | grep run < [run x86_64 CtsAppTestCases (testCount: 6,attempt: 0) started] > [run x86_64 CtsAppTestCases_cts_requesttileserviceaddtest (testCount: 6,attempt: 0) started] < [run x86_64 CtsLocationFineTestCases (testCount: 96,attempt: 0) started] > [run x86_64 CtsLocationFineTestCases_android_server_location (testCount: 96,attempt: 0) started] < [run x86_64 FrameworksNetTests (testCount: 2009,attempt: 0) started] > [run x86_64 FrameworksNetTests (testCount: 2011,attempt: 0) started] Test-Mapping-Slo-Bypass-Bug: b/335015078 Change-Id: If6e3cd0624ac1c16f1cd088566d967769f47199c
2024-08-13Mention V_TO_U_RESTORE_ALLOWLIST in TarBackupReader's javadoc Beatrice Marchegiani
Bug: 358316882 Test: not needed, just adding a javadoc comment Flag: DOCS_ONLY Change-Id: Icf5fe938dd9cd3efdf03a781e019d602f9e3919b
2024-07-11Fix NPE when reading the allowlist Beatrice Marchegiani
Bug: 351750747 Test: atest -v TarBackupReaderTest, manual test Flag: EXEMPT bugfix (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a8cac82ccdd9efdd88a62133e4eec9570d50b7f0) Merged-In: Icf4fe058e7d261c197e04ccad940595413bd4a99 Change-Id: Icf4fe058e7d261c197e04ccad940595413bd4a99
2024-07-08Fix NPE when reading the allowlist Beatrice Marchegiani
Bug: 351750747 Test: atest -v TarBackupReaderTest, manual test Flag: EXEMPT bugfix Change-Id: Icf4fe058e7d261c197e04ccad940595413bd4a99
2024-05-29Merge "Remove PM#getPackageInfo call in restoreAtInstall flow" into main Piyush Mehrotra
2024-05-29Remove PM#getPackageInfo call in restoreAtInstall flow Piyush Mehrotra
A previous change (ag/26748034) added this call for BMM logging. getPackageInfo is a relatively heavy binder call, which triggered performance regression alerts (b/342082681 and b/342273383). The additional getPackageInfo call resulted in 5-10% increase in latency for package commit operation. For the purposes of BMM logging in restoreAtInstall flow, we just need the name of the package. Fields like version code are already logged in subsequent BMM logging during restore. Bug: 342082681 Test: Verified that logging is correct Change-Id: I8025789b2c696eac1b7738a1ae332c67aea8ce5f
2024-05-27Merge "Fix downgrade wallpaper restore bug" into 24D1-dev am: 0753af02fc Beatrice Marchegiani
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/27486895 Change-Id: I552392cf12b59ba0c931cbc246bd3791f799162b Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2024-05-26Fix downgrade wallpaper restore bug Beatrice Marchegiani
Add a check in TarBackupReader to enable the restore of packages in the V_To_U_Allowlist in the V to U downgrade scenario Bug: 341732443 Test: atest -v TarBackupReaderTest, manual test (details: https://b.corp.google.com/issues/341732443#comment18) Change-Id: I12a1f272b17a12abb532a98c0d93f0a9854fcb51
2024-05-14Catch and ignore `IllegalArgumentException` thrown by `unbindService` Joël Stemmer
When `TransportConnection` handles `onServiceConnected` or `onBindingDied` it calls `Context.unbindService`. In a very small number of cases this fails because the connection is not (or no longer?) registered. When this happens, the system server crashes causing the device to reboot. We don't really care if the service is not registered when we try to unbind it, so we just catch and ignore the exception to avoid crashing the system server. Bug: 335547110 Test: atest TransportConnectionTest.java Change-Id: Ic7f34eacd1f621c42a68128ef992043dba6808f1
2024-04-19Re-enable backup dumpsys in bugreports, and remove some behaviour change ↵ Andrea Zilio
introduced after ag/26833810 Change-Id: I501151d4d73490b3f6f3c0326e741ec046cad36b
2024-04-10Improve `dumpsys backup` Piyush Mehrotra
This is a re-submit of the previously reverted ag/22327091 by piee@google.com. Original description: This change refactors `dumpsys backup` code and adds some improvements - - Removes `isUserReadyForBackup()` check on calling user. This check is not needed, as it prevents a User with INTERACT_ACROSS_USERS_FULL permission, but not Backup activated, to get info about Backup service on other users. This is particularly relevant in case of headless mode, as Backup service isn't running for SYSTEM_USER. - Adds `--user <userId>` optional parameter for `dumpsys backup`, which will return info about Backup service for the user passed in argument. This is also helpul in case of multiple users on the device. Changes by sarpm@google.com over the original: - Some test improvements: remove @VisibleForTesting, increase coverage slightly. - Remove redundant robo tests that are now covered by the unit tests. Bug: 267755698 Test: atest BackupManagerServiceTest Manual test by running dumpsys backup with optional arguments. Change-Id: Id7790a501a8befb12a212f66adfc0cd8cc9f9e8b
2024-04-04Set more aconfig_declarations containers to system in frameworks/base Yu Liu
Bug: 330354107 Test: CI Flag: NONE Ignore-AOSP-First: It is easier to detect all the mismatch in internal master. Change-Id: If6d417b269e23f205c21686d147e9249d47a18b9
2024-04-02Merge "Increase BMM logging coverage for restore at install flow" into ↵ Piyush Mehrotra
24D1-dev am: 6afb3e754e Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/26748034 Change-Id: I5216ce32ba2c6b7db3e8cbe1dd8af816f958455d Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2024-04-02Increase BMM logging coverage for restore at install flow Piyush Mehrotra
This change adds BMM logging in restore at install flow right from the point when PackageManager triggers restore. The change also adds more detailed logging in Full Restore flow, around various failure points. This'll help with quantify different failures that happen in Full Restore flow, some of which are masked by the complexity in Framework code. Bug: 331749778 Test: Ensure that the change builds (m -j frameworks/base) Ensure CTS and GTS B&R tests pass Change-Id: Ieff6df433398ffff71c7eeda73174f9f1d509b58
2024-03-21Avoid entering an infinite loop in PackageManagerBackupAgent#onRestore Joël Stemmer
When the PackageManagerBackupAgent restores an old backup that does not have an ancestral record version and the backup contains a package that does not have any signatures it will enter an infinite loop. This happens because it calls `continue` without reading the next key/value pair in the backup. Test: atest PackageManagerBackupAgentTest Bug: 330708733 Change-Id: Ifb71293b75ccb68dc7e721593f04128417360491
2024-03-15Merge "Stop backing up the ancestral record version unless it has changed" ↵ Joël Stemmer
into main
2024-03-13Stop backing up the ancestral record version unless it has changed Joël Stemmer
The ancestral record version is a number that's contained in the backup created by the PackageManagerBackupAgent. It's used to indicate how to restore the backed up data. This allows the PackageManagerBackupAgent to change how it stores its data without worrying that it may break other versions of the backup agent during restore. Backup agents should only back up key/value pairs that haven't changed since the last time a backup was done. It should do this by maintaining a local state. The agent is given the state file from the previous backup run (if any) and is given a file to write the new state to. The ancestral record version that the PackageManagerBackupAgent writes to the backup almost never changes. However, this key/value pair was not stored in the local state. As a result, the backup agent backs up this key/value pair on every backup causing unnecessary backup traffic. This change fixes that by storing the ancestral record version in the local state. It then avoids backing it up again when it's present in the local state. It's possible for the ancestral record version in the state file to be different from the current value in the backup agent, for example after a system update. If this happens that means a change was made to the format of the data we back up, so we intentionally back up all key/value pairs to ensure they're all written in the new format. In addition, there may be key/value pairs that were backed up in the past but are no longer backed up (e.g. an app was uninstalled). We remove these key/value pairs since they were created with the older ancestral record version but we can't update them. Bug: 268341874 Test: atest PackageManagerBackupAgentTest.java Change-Id: Iaa805198801b38b0c4867536749095fff243e18e