| Age | Commit message (Collapse) | Author |
|
|
|
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
|
|
agent pipe is broken" into main
|
|
|
|
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
|
|
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
|
|
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
|
|
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
|
|
This is to avoid concurrent modification errors.
Fixes: 390483439
Test: atest CtsBackupTestCases
Flag: EXEMPT bug fix
Change-Id: I8d75ea1fc15705ee847e911452b6ba0029d7a000
|
|
And format it properly. The variable formats better compared to a method
call.
Test: n/a
Flag: EXEMPT no-op refactor
Change-Id: I617ee8d1b05a65e21b408186cf2c25cae76ed7bb
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
|
|
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
|
|
|
|
322dd88f9dedd580115bd62377c2a21323540d2f
Change-Id: I9d1b06d9bea270e22f364bbc4be24174d3ae53fe
|
|
|
|
into main
* changes:
Revert "accessibility_flags.aconfig: remove leading whitespace"
Revert "Mark @FlaggedApi flags as exported"
|
|
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
|
|
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
|
|
* changes:
Mark @FlaggedApi flags as exported
accessibility_flags.aconfig: remove leading whitespace
|
|
Change-Id: Ibed0deef0a7ca67afe2df79fe447deae755ef49e
Bug: 379861078
Flag: com.android.server.backup.enable_metrics_settings_backup_agents
|
|
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
Bug: 358316882
Test: not needed, just adding a javadoc comment
Flag: DOCS_ONLY
Change-Id: Icf5fe938dd9cd3efdf03a781e019d602f9e3919b
|
|
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
|
|
Bug: 351750747
Test: atest -v TarBackupReaderTest, manual test
Flag: EXEMPT bugfix
Change-Id: Icf4fe058e7d261c197e04ccad940595413bd4a99
|
|
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
introduced after ag/26833810
Change-Id: I501151d4d73490b3f6f3c0326e741ec046cad36b
|
|
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
|
|
Bug: 330354107
Test: CI
Flag: NONE
Ignore-AOSP-First: It is easier to detect all the mismatch in internal
master.
Change-Id: If6d417b269e23f205c21686d147e9249d47a18b9
|
|
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>
|
|
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
|
|
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
|
|
into main
|
|
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
|