From 104604674f01be4cf4b9d35d38ed14bd1046e987 Mon Sep 17 00:00:00 2001 From: Sarp Misoglu Date: Mon, 31 Oct 2022 13:54:42 +0000 Subject: [PATCH 1/3] Pass agent logs to monitor after package B&R In this change B&R tasks ask for logs from each BackupAgent after it is backed up or restored. These logs are then immediately passed on to the BackupManagerMonitor. Bug: 252760247 Change-Id: Iaf43ce61e98bb4b42c05c18a46031a06d41abb12 Test: $ atest BackupFrameworksServicesRoboTests:KeyValueBackupTaskTest $ atest BackupManagerMonitorUtilsTest Unfortunately, the other classes are not unit testable. For manual testing, I implemented some fake loggers for various BackupAgents, passed in a fake BackupManagerMonitor, ran B&R with the following commands, and observed that the monitor received the logs. 1. $ adb shell bmgr backupnow --all 2. $ adb shell bmgr restore ... (for one KV and one FullBackup package) --- core/api/system-current.txt | 2 + .../app/backup/BackupManagerMonitor.java | 18 +++--- .../backup/fullbackup/FullBackupEngine.java | 12 +++- .../fullbackup/PerformAdbBackupTask.java | 3 +- .../PerformFullTransportBackupTask.java | 3 +- .../backup/keyvalue/KeyValueBackupTask.java | 3 + .../restore/PerformUnifiedRestoreTask.java | 4 ++ .../utils/BackupManagerMonitorUtils.java | 61 +++++++++++++++++++ .../keyvalue/KeyValueBackupTaskTest.java | 35 +++++++++++ .../utils/BackupManagerMonitorUtilsTest.java | 51 +++++++++++++++- 10 files changed, 179 insertions(+), 13 deletions(-) diff --git a/core/api/system-current.txt b/core/api/system-current.txt index baf1f3107e2c0..2a6383498396a 100644 --- a/core/api/system-current.txt +++ b/core/api/system-current.txt @@ -1470,6 +1470,7 @@ package android.app.backup { public class BackupManagerMonitor { ctor public BackupManagerMonitor(); method public void onEvent(android.os.Bundle); + field public static final String EXTRA_LOG_AGENT_LOGGING_RESULTS = "android.app.backup.extra.LOG_AGENT_LOGGING_RESULTS"; field public static final String EXTRA_LOG_CANCEL_ALL = "android.app.backup.extra.LOG_CANCEL_ALL"; field public static final String EXTRA_LOG_EVENT_CATEGORY = "android.app.backup.extra.LOG_EVENT_CATEGORY"; field public static final String EXTRA_LOG_EVENT_ID = "android.app.backup.extra.LOG_EVENT_ID"; @@ -1488,6 +1489,7 @@ package android.app.backup { field public static final int LOG_EVENT_CATEGORY_AGENT = 2; // 0x2 field public static final int LOG_EVENT_CATEGORY_BACKUP_MANAGER_POLICY = 3; // 0x3 field public static final int LOG_EVENT_CATEGORY_TRANSPORT = 1; // 0x1 + field public static final int LOG_EVENT_ID_AGENT_LOGGING_RESULTS = 52; // 0x34 field public static final int LOG_EVENT_ID_APK_NOT_INSTALLED = 40; // 0x28 field public static final int LOG_EVENT_ID_APP_HAS_NO_AGENT = 28; // 0x1c field public static final int LOG_EVENT_ID_BACKUP_DISABLED = 13; // 0xd diff --git a/core/java/android/app/backup/BackupManagerMonitor.java b/core/java/android/app/backup/BackupManagerMonitor.java index 07e7688a48aef..d134ca27b3543 100644 --- a/core/java/android/app/backup/BackupManagerMonitor.java +++ b/core/java/android/app/backup/BackupManagerMonitor.java @@ -129,6 +129,13 @@ public class BackupManagerMonitor { */ public static final String EXTRA_LOG_OLD_VERSION = "android.app.backup.extra.LOG_OLD_VERSION"; + /** + * ParcelableList: when we have an event of id LOG_EVENT_ID_AGENT_LOGGING_RESULTS we send a list + * of {@link android.app.backup.BackupRestoreEventLogger.DataTypeResult}. + */ + public static final String EXTRA_LOG_AGENT_LOGGING_RESULTS = + "android.app.backup.extra.LOG_AGENT_LOGGING_RESULTS"; + // TODO complete this list with all log messages. And document properly. public static final int LOG_EVENT_ID_FULL_BACKUP_CANCEL = 4; public static final int LOG_EVENT_ID_ILLEGAL_KEY = 5; @@ -171,15 +178,10 @@ public class BackupManagerMonitor { public static final int LOG_EVENT_ID_WIDGET_UNKNOWN_VERSION = 48; public static final int LOG_EVENT_ID_NO_PACKAGES = 49; public static final int LOG_EVENT_ID_TRANSPORT_IS_NULL = 50; + /** The transport returned {@link BackupTransport#TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED}. */ + public static final int LOG_EVENT_ID_TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED = 51; - /** - * The transport returned {@link BackupTransport#TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED}. - */ - public static final int LOG_EVENT_ID_TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED = 51; - - - - + public static final int LOG_EVENT_ID_AGENT_LOGGING_RESULTS = 52; /** * This method will be called each time something important happens on BackupManager. diff --git a/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java b/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java index 1e1ca95d69dbb..379ae52018c46 100644 --- a/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java +++ b/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java @@ -23,11 +23,13 @@ import static com.android.server.backup.UserBackupManagerService.BACKUP_MANIFEST import static com.android.server.backup.UserBackupManagerService.BACKUP_METADATA_FILENAME; import static com.android.server.backup.UserBackupManagerService.SHARED_BACKUP_AGENT_PACKAGE; +import android.annotation.Nullable; import android.annotation.UserIdInt; import android.app.ApplicationThreadConstants; import android.app.IBackupAgent; import android.app.backup.BackupTransport; import android.app.backup.FullBackupDataOutput; +import android.app.backup.IBackupManagerMonitor; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; @@ -42,6 +44,7 @@ import com.android.server.backup.OperationStorage.OpType; import com.android.server.backup.UserBackupManagerService; import com.android.server.backup.remote.RemoteCall; import com.android.server.backup.utils.BackupEligibilityRules; +import com.android.server.backup.utils.BackupManagerMonitorUtils; import com.android.server.backup.utils.FullBackupUtils; import java.io.File; @@ -60,12 +63,13 @@ public class FullBackupEngine { private BackupRestoreTask mTimeoutMonitor; private IBackupAgent mAgent; private boolean mIncludeApks; - private PackageInfo mPkg; + private final PackageInfo mPkg; private final long mQuota; private final int mOpToken; private final int mTransportFlags; private final BackupAgentTimeoutParameters mAgentTimeoutParameters; private final BackupEligibilityRules mBackupEligibilityRules; + @Nullable private final IBackupManagerMonitor mMonitor; class FullBackupRunner implements Runnable { private final @UserIdInt int mUserId; @@ -193,7 +197,8 @@ public class FullBackupEngine { long quota, int opToken, int transportFlags, - BackupEligibilityRules backupEligibilityRules) { + BackupEligibilityRules backupEligibilityRules, + IBackupManagerMonitor monitor) { this.backupManagerService = backupManagerService; mOutput = output; mPreflightHook = preflightHook; @@ -208,6 +213,7 @@ public class FullBackupEngine { backupManagerService.getAgentTimeoutParameters(), "Timeout parameters cannot be null"); mBackupEligibilityRules = backupEligibilityRules; + mMonitor = monitor; } public int preflightCheck() throws RemoteException { @@ -260,6 +266,8 @@ public class FullBackupEngine { } result = BackupTransport.TRANSPORT_OK; } + + BackupManagerMonitorUtils.monitorAgentLoggingResults(mMonitor, mPkg, mAgent); } catch (IOException e) { Slog.e(TAG, "Error backing up " + mPkg.packageName + ": " + e.getMessage()); result = BackupTransport.AGENT_ERROR; diff --git a/services/backup/java/com/android/server/backup/fullbackup/PerformAdbBackupTask.java b/services/backup/java/com/android/server/backup/fullbackup/PerformAdbBackupTask.java index ec58e17148b3a..cba1e299ff58c 100644 --- a/services/backup/java/com/android/server/backup/fullbackup/PerformAdbBackupTask.java +++ b/services/backup/java/com/android/server/backup/fullbackup/PerformAdbBackupTask.java @@ -420,7 +420,8 @@ public class PerformAdbBackupTask extends FullBackupTask implements BackupRestor Long.MAX_VALUE, mCurrentOpToken, /*transportFlags=*/ 0, - mBackupEligibilityRules); + mBackupEligibilityRules, + /* monitor= */ null); sendOnBackupPackage(isSharedStorage ? "Shared storage" : pkg.packageName); // Don't need to check preflight result as there is no preflight hook. diff --git a/services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java b/services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java index f0492a8b58b4d..78df304286b4e 100644 --- a/services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java +++ b/services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java @@ -882,7 +882,8 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba mQuota, mCurrentOpToken, mTransportFlags, - mBackupEligibilityRules); + mBackupEligibilityRules, + mMonitor); try { try { if (!mIsCancelled) { diff --git a/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java b/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java index 16aa4ebb06568..fd9c834540e74 100644 --- a/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java +++ b/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java @@ -68,6 +68,7 @@ import com.android.server.backup.transport.BackupTransportClient; import com.android.server.backup.transport.TransportConnection; import com.android.server.backup.transport.TransportNotAvailableException; import com.android.server.backup.utils.BackupEligibilityRules; +import com.android.server.backup.utils.BackupManagerMonitorUtils; import libcore.io.IoUtils; @@ -697,6 +698,8 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable { try { extractAgentData(mCurrentPackage); + BackupManagerMonitorUtils.monitorAgentLoggingResults( + mReporter.getMonitor(), mCurrentPackage, mAgent); int status = sendDataToTransport(mCurrentPackage); cleanUpAgentForTransportStatus(status); } catch (AgentException | TaskException e) { diff --git a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java index b48367db17c25..1d3140061cf21 100644 --- a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java +++ b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java @@ -885,6 +885,10 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask { OpType.RESTORE_WAIT); mAgent.doRestoreFinished(mEphemeralOpToken, backupManagerService.getBackupManagerBinder()); + + // Ask the agent for logs after doRestoreFinished() to allow it to finalize its logs. + BackupManagerMonitorUtils.monitorAgentLoggingResults(mMonitor, mCurrentPackage, mAgent); + // If we get this far, the callback or timeout will schedule the // next restore state, so we're done } catch (Exception e) { diff --git a/services/backup/java/com/android/server/backup/utils/BackupManagerMonitorUtils.java b/services/backup/java/com/android/server/backup/utils/BackupManagerMonitorUtils.java index 6f083760980d7..8eda5b9a32190 100644 --- a/services/backup/java/com/android/server/backup/utils/BackupManagerMonitorUtils.java +++ b/services/backup/java/com/android/server/backup/utils/BackupManagerMonitorUtils.java @@ -16,23 +16,42 @@ package com.android.server.backup.utils; +import static android.app.backup.BackupManagerMonitor.EXTRA_LOG_AGENT_LOGGING_RESULTS; import static android.app.backup.BackupManagerMonitor.EXTRA_LOG_EVENT_PACKAGE_NAME; +import static android.app.backup.BackupManagerMonitor.LOG_EVENT_CATEGORY_AGENT; +import static android.app.backup.BackupManagerMonitor.LOG_EVENT_ID_AGENT_LOGGING_RESULTS; import static com.android.server.backup.BackupManagerService.DEBUG; import static com.android.server.backup.BackupManagerService.TAG; import android.annotation.Nullable; +import android.app.IBackupAgent; import android.app.backup.BackupManagerMonitor; +import android.app.backup.BackupRestoreEventLogger; import android.app.backup.IBackupManagerMonitor; import android.content.pm.PackageInfo; import android.os.Bundle; import android.os.RemoteException; import android.util.Slog; +import com.android.internal.infra.AndroidFuture; + +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + /** * Utility methods to communicate with BackupManagerMonitor. */ public class BackupManagerMonitorUtils { + /** + * Timeout for how long we wait before we give up on getting logs from a {@link IBackupAgent}. + * We expect this to be very fast since the agent immediately returns whatever logs have been + * accumulated. The timeout adds a bit more security and ensures we don't hang the B&R waiting + * for non-essential logs. + */ + private static final int AGENT_LOGGER_RESULTS_TIMEOUT_MILLIS = 500; + /** * Notifies monitor about the event. * @@ -79,6 +98,48 @@ public class BackupManagerMonitorUtils { return null; } + /** + * Extracts logging results from the provided {@code agent} and notifies the {@code monitor} + * about them. + * + *

Note that this method does two separate binder calls (one to the agent and one to the + * monitor). + * + * @param monitor - implementation of {@link IBackupManagerMonitor} to notify. + * @param pkg - package the {@code agent} belongs to. + * @param agent - the {@link IBackupAgent} to retrieve logs from. + * @return {@code null} if the monitor is null. {@code monitor} if we fail to retrieve the logs + * from the {@code agent}. Otherwise, the result of {@link + * #monitorEvent(IBackupManagerMonitor, int, PackageInfo, int, Bundle)}. + */ + public static IBackupManagerMonitor monitorAgentLoggingResults( + @Nullable IBackupManagerMonitor monitor, PackageInfo pkg, IBackupAgent agent) { + if (monitor == null) { + return null; + } + + try { + AndroidFuture> resultsFuture = + new AndroidFuture<>(); + agent.getLoggerResults(resultsFuture); + Bundle loggerResultsBundle = new Bundle(); + loggerResultsBundle.putParcelableList( + EXTRA_LOG_AGENT_LOGGING_RESULTS, + resultsFuture.get(AGENT_LOGGER_RESULTS_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)); + return BackupManagerMonitorUtils.monitorEvent( + monitor, + LOG_EVENT_ID_AGENT_LOGGING_RESULTS, + pkg, + LOG_EVENT_CATEGORY_AGENT, + loggerResultsBundle); + } catch (TimeoutException e) { + Slog.w(TAG, "Timeout while waiting to retrieve logging results from agent", e); + } catch (Exception e) { + Slog.w(TAG, "Failed to retrieve logging results from agent", e); + } + return monitor; + } + /** * Adds given key-value pair in the bundle and returns the bundle. If bundle was null it will * be created. diff --git a/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java b/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java index 962a07ac65534..298cbf3e61b97 100644 --- a/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java +++ b/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java @@ -77,6 +77,8 @@ import android.app.backup.BackupAgent; import android.app.backup.BackupDataInput; import android.app.backup.BackupDataOutput; import android.app.backup.BackupManager; +import android.app.backup.BackupManagerMonitor; +import android.app.backup.BackupRestoreEventLogger; import android.app.backup.BackupTransport; import android.app.backup.IBackupCallback; import android.app.backup.IBackupManager; @@ -89,6 +91,7 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.PackageManagerInternal; import android.net.Uri; +import android.os.Bundle; import android.os.ConditionVariable; import android.os.DeadObjectException; import android.os.Handler; @@ -100,6 +103,7 @@ import android.platform.test.annotations.Presubmit; import android.util.Pair; import com.android.internal.backup.IBackupTransport; +import com.android.internal.infra.AndroidFuture; import com.android.server.EventLogTags; import com.android.server.LocalServices; import com.android.server.backup.BackupRestoreTask; @@ -131,6 +135,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.mockito.InOrder; import org.mockito.Mock; @@ -1447,6 +1452,36 @@ public class KeyValueBackupTaskTest { EventLogTags.BACKUP_PACKAGE, PACKAGE_1.packageName, Files.size(backupData)); } + @Test + public void testRunTask_whenFinishBackupSucceeds_sendsAgentLogsToMonitor() throws Exception { + TransportMock transportMock = setUpInitializedTransport(mTransport); + AgentMock agentMock = setUpAgentWithData(PACKAGE_1); + KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1); + // Mock the agent logging and returning its logs. + List results = new ArrayList<>(); + results.add(new BackupRestoreEventLogger.DataTypeResult("testDataTypeResult")); + doAnswer( + invocation -> { + AndroidFuture> in = + invocation.getArgument(0); + in.complete(results); + return null; + }) + .when(agentMock.agentBinder) + .getLoggerResults(any()); + + runTask(task); + + ArgumentCaptor bundleCaptor = ArgumentCaptor.forClass(Bundle.class); + verify(mMonitor).onEvent(bundleCaptor.capture()); + Bundle eventBundle = bundleCaptor.getValue(); + List sentLoggingResults = + eventBundle.getParcelableArrayList( + BackupManagerMonitor.EXTRA_LOG_AGENT_LOGGING_RESULTS, + BackupRestoreEventLogger.DataTypeResult.class); + assertThat(sentLoggingResults.get(0).getDataType()).isEqualTo("testDataTypeResult"); + } + @Test public void testRunTask_whenFinishBackupSucceeds_notifiesCorrectly() throws Exception { TransportMock transportMock = setUpInitializedTransport(mTransport); diff --git a/services/tests/servicestests/src/com/android/server/backup/utils/BackupManagerMonitorUtilsTest.java b/services/tests/servicestests/src/com/android/server/backup/utils/BackupManagerMonitorUtilsTest.java index d3fd89c3018ad..cadc890f64fdd 100644 --- a/services/tests/servicestests/src/com/android/server/backup/utils/BackupManagerMonitorUtilsTest.java +++ b/services/tests/servicestests/src/com/android/server/backup/utils/BackupManagerMonitorUtilsTest.java @@ -21,13 +21,20 @@ import static android.app.backup.BackupManagerMonitor.EXTRA_LOG_EVENT_ID; import static android.app.backup.BackupManagerMonitor.EXTRA_LOG_EVENT_PACKAGE_LONG_VERSION; import static android.app.backup.BackupManagerMonitor.EXTRA_LOG_EVENT_PACKAGE_NAME; import static android.app.backup.BackupManagerMonitor.EXTRA_LOG_EVENT_PACKAGE_VERSION; +import static android.app.backup.BackupManagerMonitor.LOG_EVENT_CATEGORY_AGENT; +import static android.app.backup.BackupManagerMonitor.LOG_EVENT_ID_AGENT_LOGGING_RESULTS; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; +import android.app.IBackupAgent; +import android.app.backup.BackupManagerMonitor; +import android.app.backup.BackupRestoreEventLogger; import android.app.backup.IBackupManagerMonitor; import android.content.pm.PackageInfo; import android.os.Bundle; @@ -37,6 +44,8 @@ import android.platform.test.annotations.Presubmit; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.internal.infra.AndroidFuture; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -44,6 +53,9 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.ArrayList; +import java.util.List; + @SmallTest @Presubmit @RunWith(AndroidJUnit4.class) @@ -142,6 +154,44 @@ public class BackupManagerMonitorUtilsTest { assertThat(eventBundle.getString("key2")).isEqualTo("value2"); } + @Test + public void monitorAgentLoggingResults_fillsBundleCorrectly() throws Exception { + PackageInfo packageInfo = new PackageInfo(); + packageInfo.packageName = "test.package"; + // Mock an agent that returns a logging result. + IBackupAgent agent = spy(IBackupAgent.class); + List loggingResults = new ArrayList<>(); + loggingResults.add(new BackupRestoreEventLogger.DataTypeResult("testLoggingResult")); + doAnswer( + invocation -> { + AndroidFuture> in = + invocation.getArgument(0); + in.complete(loggingResults); + return null; + }) + .when(agent) + .getLoggerResults(any()); + + IBackupManagerMonitor result = + BackupManagerMonitorUtils.monitorAgentLoggingResults( + mMonitorMock, packageInfo, agent); + + assertThat(result).isEqualTo(mMonitorMock); + ArgumentCaptor bundleCaptor = ArgumentCaptor.forClass(Bundle.class); + verify(mMonitorMock).onEvent(bundleCaptor.capture()); + Bundle eventBundle = bundleCaptor.getValue(); + assertThat(eventBundle.getInt(EXTRA_LOG_EVENT_ID)) + .isEqualTo(LOG_EVENT_ID_AGENT_LOGGING_RESULTS); + assertThat(eventBundle.getInt(EXTRA_LOG_EVENT_CATEGORY)) + .isEqualTo(LOG_EVENT_CATEGORY_AGENT); + assertThat(eventBundle.getString(EXTRA_LOG_EVENT_PACKAGE_NAME)).isEqualTo("test.package"); + List filledLoggingResults = + eventBundle.getParcelableArrayList( + BackupManagerMonitor.EXTRA_LOG_AGENT_LOGGING_RESULTS, + BackupRestoreEventLogger.DataTypeResult.class); + assertThat(filledLoggingResults.get(0).getDataType()).isEqualTo("testLoggingResult"); + } + @Test public void putMonitoringExtraString_bundleExists_fillsBundleCorrectly() throws Exception { Bundle bundle = new Bundle(); @@ -204,5 +254,4 @@ public class BackupManagerMonitorUtilsTest { assertThat(result.size()).isEqualTo(1); assertThat(result.getBoolean("key")).isTrue(); } - } \ No newline at end of file From 2801c741d336624b8f11f1ada8c81d327ed52f64 Mon Sep 17 00:00:00 2001 From: Sarp Misoglu Date: Wed, 9 Nov 2022 16:04:11 +0000 Subject: [PATCH 2/3] Ask transport for a monitor during restore The current API is designed so that the requester of the restore operation passes in a BackupManagerMonitor. This functionality is currently unused, so effectively we'll start asking for a monitor every time. Note that the requester and transport are not necessarily the same entity in the current API definition. This means in the future the requester can start passing in a monitor which would mean we stop asking the transport for one. This is not ideal so while it's infeasible now, in the future we can investigate formally coupling the transport and the requester in the API. Bug: 254815434, 254641961 Test: This class is not testable without a major refactor. atest CtsBackupHostTestCases Also manually by ensuring getBackupManagerMonitor() gets called when running 'adb shell bmgr restore` for KV and full backup packages. Change-Id: I8892ed7e3aa04ca771c9628b44751a9fdef4aad2 --- .../server/backup/restore/PerformUnifiedRestoreTask.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java index 1d3140061cf21..9f89339a9149d 100644 --- a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java +++ b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java @@ -405,6 +405,12 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask { BackupTransportClient transport = mTransportConnection.connectOrThrow("PerformUnifiedRestoreTask.startRestore()"); + // If the requester of the restore has not passed in a monitor, we ask the transport + // for one. + if (mMonitor == null) { + mMonitor = transport.getBackupManagerMonitor(); + } + mStatus = transport.startRestore(mToken, packages); if (mStatus != BackupTransport.TRANSPORT_OK) { Slog.e(TAG, "Transport error " + mStatus + "; no restore possible"); From c6215c0cf4bff479ca8d60aee8eca2c9df57f65b Mon Sep 17 00:00:00 2001 From: Sarp Misoglu Date: Thu, 10 Nov 2022 16:53:28 +0000 Subject: [PATCH 3/3] Unhide BackupTransport#getBackupManagerMonitor() This needs to be a public API so that BackupTransports can implement it. Bug: 241760569 Test: N/A Change-Id: Ic3ac1d3f77bcc1bf5f5babdda7361d949023aa16 --- core/api/system-current.txt | 1 + core/java/android/app/backup/BackupTransport.java | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/core/api/system-current.txt b/core/api/system-current.txt index 2a6383498396a..ad519ce727be3 100644 --- a/core/api/system-current.txt +++ b/core/api/system-current.txt @@ -1564,6 +1564,7 @@ package android.app.backup { method public int finishBackup(); method public void finishRestore(); method public android.app.backup.RestoreSet[] getAvailableRestoreSets(); + method @Nullable public android.app.backup.BackupManagerMonitor getBackupManagerMonitor(); method public long getBackupQuota(String, boolean); method public android.os.IBinder getBinder(); method public long getCurrentRestoreSet(); diff --git a/core/java/android/app/backup/BackupTransport.java b/core/java/android/app/backup/BackupTransport.java index 90e9df47bc4b1..f56c8c3b0bdb8 100644 --- a/core/java/android/app/backup/BackupTransport.java +++ b/core/java/android/app/backup/BackupTransport.java @@ -661,8 +661,6 @@ public class BackupTransport { * *

Backups requested from outside the framework may pass in a monitor with the request, * however backups initiated by the framework will call this method to retrieve one. - * - * @hide */ @Nullable public BackupManagerMonitor getBackupManagerMonitor() {