Merge changes from topic "kv-refactor-agent-throws"
* changes:
[KV] Consider throwing BackupAgent a failure
[KV] Remove states
diff --git a/core/java/android/app/backup/BackupAgent.java b/core/java/android/app/backup/BackupAgent.java
index 097dd9c..62332d6 100644
--- a/core/java/android/app/backup/BackupAgent.java
+++ b/core/java/android/app/backup/BackupAgent.java
@@ -126,6 +126,11 @@
private static final boolean DEBUG = false;
/** @hide */
+ public static final int RESULT_SUCCESS = 0;
+ /** @hide */
+ public static final int RESULT_ERROR = -1;
+
+ /** @hide */
public static final int TYPE_EOF = 0;
/**
@@ -955,8 +960,10 @@
BackupDataOutput output = new BackupDataOutput(
data.getFileDescriptor(), quotaBytes, transportFlags);
+ long result = RESULT_ERROR;
try {
BackupAgent.this.onBackup(oldState, output, newState);
+ result = RESULT_SUCCESS;
} catch (IOException ex) {
Log.d(TAG, "onBackup (" + BackupAgent.this.getClass().getName() + ") threw", ex);
throw new RuntimeException(ex);
@@ -971,7 +978,7 @@
Binder.restoreCallingIdentity(ident);
try {
- callbackBinder.operationComplete(0);
+ callbackBinder.operationComplete(result);
} catch (RemoteException e) {
// we'll time out anyway, so we're safe
}
diff --git a/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupReporter.java b/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupReporter.java
index 7f5ddc28..2f32775 100644
--- a/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupReporter.java
+++ b/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupReporter.java
@@ -52,11 +52,9 @@
// verify calls to this object. Add these and more assertions to the test of this class.
@VisibleForTesting
public class KeyValueBackupReporter {
- @VisibleForTesting
- static final String TAG = "KeyValueBackupTask";
+ @VisibleForTesting static final String TAG = "KeyValueBackupTask";
private static final boolean DEBUG = BackupManagerService.DEBUG;
- @VisibleForTesting
- static final boolean MORE_DEBUG = BackupManagerService.MORE_DEBUG || true;
+ @VisibleForTesting static final boolean MORE_DEBUG = BackupManagerService.MORE_DEBUG || true;
static void onNewThread(String threadName) {
if (DEBUG) {
@@ -132,12 +130,6 @@
Slog.e(TAG, "Error during PM metadata backup", e);
}
- void onEmptyQueue() {
- if (MORE_DEBUG) {
- Slog.i(TAG, "Queue now empty");
- }
- }
-
void onStartPackageBackup(String packageName) {
Slog.d(TAG, "Starting key-value backup of " + packageName);
}
@@ -363,6 +355,14 @@
null, BackupManagerMonitor.EXTRA_LOG_CANCEL_ALL, true));
}
+ void onAgentResultError(@Nullable PackageInfo packageInfo) {
+ String packageName = getPackageName(packageInfo);
+ BackupObserverUtils.sendBackupOnPackageResult(
+ mObserver, packageName, BackupManager.ERROR_AGENT_FAILURE);
+ EventLog.writeEvent(EventLogTags.BACKUP_AGENT_FAILURE, packageName, "result error");
+ Slog.w(TAG, "Agent " + packageName + " error in onBackup()");
+ }
+
private String getPackageName(@Nullable PackageInfo packageInfo) {
return (packageInfo != null) ? packageInfo.packageName : "no_package_yet";
}
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 7d035cb..fbe0d6f 100644
--- a/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java
+++ b/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java
@@ -251,15 +251,15 @@
@Nullable private final DataChangedJournal mJournal;
@Nullable private PerformFullTransportBackupTask mFullBackupTask;
- private IBackupAgent mAgentBinder;
- private PackageInfo mCurrentPackage;
- private File mSavedStateFile;
- private File mBackupDataFile;
- private File mNewStateFile;
- private ParcelFileDescriptor mSavedState;
- private ParcelFileDescriptor mBackupData;
- private ParcelFileDescriptor mNewState;
private int mStatus;
+ @Nullable private IBackupAgent mAgentBinder;
+ @Nullable private PackageInfo mCurrentPackage;
+ @Nullable private File mSavedStateFile;
+ @Nullable private File mBackupDataFile;
+ @Nullable private File mNewStateFile;
+ @Nullable private ParcelFileDescriptor mSavedState;
+ @Nullable private ParcelFileDescriptor mBackupData;
+ @Nullable private ParcelFileDescriptor mNewState;
/**
* This {@link ConditionVariable} is used to signal that the cancel operation has been
@@ -331,44 +331,45 @@
public void run() {
Process.setThreadPriority(THREAD_PRIORITY);
- BackupState state = startTask();
- while (state == BackupState.RUNNING_QUEUE || state == BackupState.BACKUP_PM) {
- if (mCancelled) {
- state = BackupState.CANCELLED;
- }
- switch (state) {
- case BACKUP_PM:
- state = backupPm();
- break;
- case RUNNING_QUEUE:
- Pair<BackupState, RemoteResult> stateAndResult = extractNextAgentData();
- state = stateAndResult.first;
- if (state == null) {
- state = handleAgentResult(stateAndResult.second);
- }
- break;
+ boolean processQueue = startTask();
+ while (processQueue && !mQueue.isEmpty() && !mCancelled) {
+ String packageName = mQueue.remove(0);
+ if (PM_PACKAGE.equals(packageName)) {
+ processQueue = backupPm();
+ } else {
+ processQueue = backupPackage(packageName);
}
}
finishTask();
}
- private BackupState handleAgentResult(RemoteResult result) {
+ /** Returns whether to consume next queue package. */
+ private boolean handleAgentResult(@Nullable PackageInfo packageInfo, RemoteResult result) {
if (result == RemoteResult.FAILED_THREAD_INTERRUPTED) {
// Not an explicit cancel, we need to flag it.
mCancelled = true;
- handleAgentCancelled();
- return BackupState.CANCELLED;
+ mReporter.onAgentCancelled(packageInfo);
+ errorCleanup();
+ return false;
}
if (result == RemoteResult.FAILED_CANCELLED) {
- handleAgentCancelled();
- return BackupState.CANCELLED;
+ mReporter.onAgentCancelled(packageInfo);
+ errorCleanup();
+ return false;
}
if (result == RemoteResult.FAILED_TIMED_OUT) {
- handleAgentTimeout();
- return BackupState.RUNNING_QUEUE;
+ mReporter.onAgentTimedOut(packageInfo);
+ errorCleanup();
+ return true;
}
- Preconditions.checkState(result.succeeded());
- return sendDataToTransport(result.get());
+ Preconditions.checkState(result.isPresent());
+ long agentResult = result.get();
+ if (agentResult == BackupAgent.RESULT_ERROR) {
+ mReporter.onAgentResultError(packageInfo);
+ errorCleanup();
+ return true;
+ }
+ return sendDataToTransport();
}
@Override
@@ -377,11 +378,12 @@
@Override
public void operationComplete(long unusedResult) {}
- private BackupState startTask() {
+ /** Returns whether to consume next queue package. */
+ private boolean startTask() {
synchronized (mBackupManagerService.getCurrentOpLock()) {
if (mBackupManagerService.isBackupOperationInProgress()) {
mReporter.onSkipBackup();
- return BackupState.FINAL;
+ return false;
}
}
@@ -404,14 +406,18 @@
mAgentBinder = null;
mStatus = BackupTransport.TRANSPORT_OK;
- // Sanity check: if the queue is empty we have no work to do.
- if (mOriginalQueue.isEmpty() && mPendingFullBackups.isEmpty()) {
+ if (mQueue.isEmpty() && mPendingFullBackups.isEmpty()) {
mReporter.onEmptyQueueAtStart();
- return BackupState.FINAL;
+ return false;
}
// We only backup PM if it was explicitly in the queue or if it's incremental.
boolean backupPm = mQueue.remove(PM_PACKAGE) || !mNonIncremental;
+ if (backupPm) {
+ mQueue.add(0, PM_PACKAGE);
+ } else {
+ mReporter.onSkipPm();
+ }
mReporter.onQueueReady(mQueue);
File pmState = new File(mStateDirectory, PM_PACKAGE);
@@ -434,18 +440,14 @@
if (mStatus != BackupTransport.TRANSPORT_OK) {
mBackupManagerService.resetBackupState(mStateDirectory);
- return BackupState.FINAL;
+ return false;
}
- if (!backupPm) {
- mReporter.onSkipPm();
- return BackupState.RUNNING_QUEUE;
- }
-
- return BackupState.BACKUP_PM;
+ return true;
}
- private BackupState backupPm() {
+ /** Returns whether to consume next queue package. */
+ private boolean backupPm() {
RemoteResult agentResult = null;
try {
mCurrentPackage = new PackageInfo();
@@ -466,31 +468,17 @@
if (mStatus != BackupTransport.TRANSPORT_OK) {
mBackupManagerService.resetBackupState(mStateDirectory);
- return BackupState.FINAL;
+ return false;
}
Preconditions.checkNotNull(agentResult);
- return handleAgentResult(agentResult);
+ return handleAgentResult(mCurrentPackage, agentResult);
}
- /**
- * Returns either:
- *
- * <ul>
- * <li>(next state, {@code null}): In case we failed to call the agent.
- * <li>({@code null}, agent result): In case we successfully called the agent.
- * </ul>
- */
- private Pair<BackupState, RemoteResult> extractNextAgentData() {
- mStatus = BackupTransport.TRANSPORT_OK;
-
- if (mQueue.isEmpty()) {
- mReporter.onEmptyQueue();
- return Pair.create(BackupState.FINAL, null);
- }
-
- String packageName = mQueue.remove(0);
+ /** Returns whether to consume next queue package. */
+ private boolean backupPackage(String packageName) {
mReporter.onStartPackageBackup(packageName);
+ mStatus = BackupTransport.TRANSPORT_OK;
// Verify that the requested app is eligible for key-value backup.
RemoteResult agentResult = null;
@@ -502,19 +490,19 @@
// The manifest has changed. This won't happen again because the app won't be
// requesting further backups.
mReporter.onPackageNotEligibleForBackup(packageName);
- return Pair.create(BackupState.RUNNING_QUEUE, null);
+ return true;
}
if (AppBackupUtils.appGetsFullBackup(mCurrentPackage)) {
// Initially enqueued for key-value backup, but only supports full-backup now.
mReporter.onPackageEligibleForFullBackup(packageName);
- return Pair.create(BackupState.RUNNING_QUEUE, null);
+ return true;
}
if (AppBackupUtils.appIsStopped(applicationInfo)) {
// Just as it won't receive broadcasts, we won't run it for backup.
mReporter.onPackageStopped(packageName);
- return Pair.create(BackupState.RUNNING_QUEUE, null);
+ return true;
}
try {
@@ -550,22 +538,22 @@
mReporter.onAgentError(packageName);
mBackupManagerService.dataChangedImpl(packageName);
mStatus = BackupTransport.TRANSPORT_OK;
- return Pair.create(BackupState.RUNNING_QUEUE, null);
+ return true;
}
if (mStatus == BackupTransport.AGENT_UNKNOWN) {
mReporter.onAgentUnknown(packageName);
mStatus = BackupTransport.TRANSPORT_OK;
- return Pair.create(BackupState.RUNNING_QUEUE, null);
+ return true;
}
// Transport-level failure, re-enqueue everything.
revertTask();
- return Pair.create(BackupState.FINAL, null);
+ return false;
}
- // Success: caller will figure out the state based on call result
- return Pair.create(null, agentResult);
+ Preconditions.checkNotNull(agentResult);
+ return handleAgentResult(mCurrentPackage, agentResult);
}
private void finishTask() {
@@ -811,7 +799,8 @@
}
}
- private BackupState sendDataToTransport(long agentResult) {
+ /** Returns whether to consume next queue package. */
+ private boolean sendDataToTransport() {
Preconditions.checkState(mBackupData != null);
String packageName = mCurrentPackage.packageName;
@@ -821,7 +810,7 @@
try {
if (!validateBackupData(applicationInfo, mBackupDataFile)) {
errorCleanup();
- return BackupState.RUNNING_QUEUE;
+ return true;
}
writingWidgetData = true;
writeWidgetPayloadIfAppropriate(mBackupData.getFileDescriptor(), packageName);
@@ -832,7 +821,7 @@
mReporter.onReadAgentDataError(packageName, e);
}
revertTask();
- return BackupState.FINAL;
+ return false;
}
clearAgentState();
@@ -892,33 +881,31 @@
}
}
- private BackupState handleTransportStatus(int status, String packageName, long size) {
+ /** Returns whether to consume next queue package. */
+ private boolean handleTransportStatus(int status, String packageName, long size) {
if (status == BackupTransport.TRANSPORT_OK) {
mReporter.onPackageBackupComplete(packageName, size);
- return BackupState.RUNNING_QUEUE;
+ return true;
}
if (status == BackupTransport.TRANSPORT_PACKAGE_REJECTED) {
mReporter.onPackageBackupRejected(packageName);
- return BackupState.RUNNING_QUEUE;
+ return true;
}
if (status == BackupTransport.TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED) {
mReporter.onPackageBackupNonIncrementalRequired(mCurrentPackage);
// Immediately retry the current package.
- if (PM_PACKAGE.equals(packageName)) {
- return BackupState.BACKUP_PM;
- }
mQueue.add(0, packageName);
- return BackupState.RUNNING_QUEUE;
+ return true;
}
if (status == BackupTransport.TRANSPORT_QUOTA_EXCEEDED) {
mReporter.onPackageBackupQuotaExceeded(packageName);
agentDoQuotaExceeded(mAgentBinder, packageName, size);
- return BackupState.RUNNING_QUEUE;
+ return true;
}
// Any other error here indicates a transport-level failure.
mReporter.onPackageBackupTransportFailure(packageName);
revertTask();
- return BackupState.FINAL;
+ return false;
}
private void agentDoQuotaExceeded(
@@ -1017,16 +1004,6 @@
mCancelAcknowledged.block();
}
- private void handleAgentTimeout() {
- mReporter.onAgentTimedOut(mCurrentPackage);
- errorCleanup();
- }
-
- private void handleAgentCancelled() {
- mReporter.onAgentCancelled(mCurrentPackage);
- errorCleanup();
- }
-
private void revertTask() {
mReporter.onRevertTask();
long delay;
@@ -1087,12 +1064,4 @@
mPendingCall = null;
return result;
}
-
- private enum BackupState {
- INITIAL,
- BACKUP_PM,
- RUNNING_QUEUE,
- CANCELLED,
- FINAL
- }
}
diff --git a/services/backup/java/com/android/server/backup/remote/FutureBackupCallback.java b/services/backup/java/com/android/server/backup/remote/FutureBackupCallback.java
index 1445cc3..1ea4249 100644
--- a/services/backup/java/com/android/server/backup/remote/FutureBackupCallback.java
+++ b/services/backup/java/com/android/server/backup/remote/FutureBackupCallback.java
@@ -23,7 +23,7 @@
/**
* An implementation of {@link IBackupCallback} that completes the {@link CompletableFuture}
- * provided in the constructor with a successful {@link RemoteResult}.
+ * provided in the constructor with a present {@link RemoteResult}.
*/
public class FutureBackupCallback extends IBackupCallback.Stub {
private final CompletableFuture<RemoteResult> mFuture;
@@ -34,6 +34,6 @@
@Override
public void operationComplete(long result) throws RemoteException {
- mFuture.complete(RemoteResult.successful(result));
+ mFuture.complete(RemoteResult.of(result));
}
}
diff --git a/services/backup/java/com/android/server/backup/remote/RemoteCall.java b/services/backup/java/com/android/server/backup/remote/RemoteCall.java
index ac84811..b3e802e 100644
--- a/services/backup/java/com/android/server/backup/remote/RemoteCall.java
+++ b/services/backup/java/com/android/server/backup/remote/RemoteCall.java
@@ -83,7 +83,7 @@
*
* <ul>
* <li>The callback passed to {@link RemoteCallable} is called with the result. We return a
- * successful {@link RemoteResult} with the result.
+ * present {@link RemoteResult} with the result.
* <li>Time-out happens. We return {@link RemoteResult#FAILED_TIMED_OUT}.
* <li>Someone calls {@link #cancel()} on this object. We return {@link
* RemoteResult#FAILED_CANCELLED}.
diff --git a/services/backup/java/com/android/server/backup/remote/RemoteResult.java b/services/backup/java/com/android/server/backup/remote/RemoteResult.java
index 7f4f469..63c79db 100644
--- a/services/backup/java/com/android/server/backup/remote/RemoteResult.java
+++ b/services/backup/java/com/android/server/backup/remote/RemoteResult.java
@@ -29,7 +29,7 @@
* #FAILED_CANCELLED}, {@link #FAILED_THREAD_INTERRUPTED} or a successful result, in which case
* {@link #get()} returns its value.
*
- * <p>Use {@link #succeeded()} to check for successful result, or direct identity comparison to
+ * <p>Use {@link #isPresent()} to check for successful result, or direct identity comparison to
* check for specific failures, like {@code result == RemoteResult.FAILED_CANCELLED}.
*/
public class RemoteResult {
@@ -38,7 +38,7 @@
public static final RemoteResult FAILED_THREAD_INTERRUPTED =
new RemoteResult(Type.FAILED_THREAD_INTERRUPTED, 0);
- public static RemoteResult successful(long value) {
+ public static RemoteResult of(long value) {
return new RemoteResult(Type.SUCCESS, value);
}
@@ -50,7 +50,7 @@
mValue = value;
}
- public boolean succeeded() {
+ public boolean isPresent() {
return mType == Type.SUCCESS;
}
@@ -60,7 +60,7 @@
* @throws IllegalStateException in case this is not a successful result.
*/
public long get() {
- Preconditions.checkState(succeeded(), "Can't obtain value of failed result");
+ Preconditions.checkState(isPresent(), "Can't obtain value of failed result");
return mValue;
}
@@ -79,8 +79,9 @@
return "FAILED_CANCELLED";
case Type.FAILED_THREAD_INTERRUPTED:
return "FAILED_THREAD_INTERRUPTED";
+ default:
+ throw new AssertionError("Unknown type");
}
- throw new AssertionError("Unknown type");
}
@Override
diff --git a/services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java b/services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java
index 63b0ea8..9fae43a 100644
--- a/services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java
+++ b/services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java
@@ -62,6 +62,7 @@
import static org.mockito.Mockito.when;
import static org.robolectric.Shadows.shadowOf;
import static org.robolectric.shadow.api.Shadow.extract;
+import static org.testng.Assert.fail;
import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
import static java.util.Collections.emptyList;
@@ -106,6 +107,7 @@
import com.android.server.backup.TransportManager;
import com.android.server.backup.internal.BackupHandler;
import com.android.server.backup.internal.OnTaskFinishedListener;
+import com.android.server.backup.remote.RemoteCall;
import com.android.server.backup.testing.PackageData;
import com.android.server.backup.testing.TestUtils.ThrowingRunnable;
import com.android.server.backup.testing.TransportData;
@@ -704,14 +706,13 @@
}
/**
- * For local agents the exception is thrown in our stack, so it hits the catch clause around
- * invocation earlier than the {@link KeyValueBackupTask#operationComplete(long)} code-path,
- * invalidating the latter. Note that this happens because {@link
- * BackupManagerService#opComplete(int, long)} schedules the actual execution to the backup
- * handler.
+ * For local agents the exception is thrown in our stack, before {@link RemoteCall} has a chance
+ * to complete cleanly.
*/
+ // TODO: When RemoteCall spins up a new thread the assertions on this method should be the same
+ // as the methods below (non-local call).
@Test
- public void testRunTask_whenLocalAgentOnBackupThrows() throws Exception {
+ public void testRunTask_whenLocalAgentOnBackupThrows_setsNullWorkSource() throws Exception {
TransportMock transportMock = setUpInitializedTransport(mTransport);
AgentMock agentMock = setUpAgent(PACKAGE_1);
agentOnBackupDo(
@@ -724,16 +725,119 @@
runTask(task);
verify(mBackupManagerService).setWorkSource(null);
+ }
+
+ @Test
+ public void testRunTask_whenLocalAgentOnBackupThrows_reportsCorrectly() throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ AgentMock agentMock = setUpAgent(PACKAGE_1);
+ agentOnBackupDo(
+ agentMock,
+ (oldState, dataOutput, newState) -> {
+ throw new RuntimeException();
+ });
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+ runTask(task);
+
verify(mObserver).onResult(PACKAGE_1.packageName, ERROR_AGENT_FAILURE);
verify(mObserver).backupFinished(SUCCESS);
+ verify(mReporter)
+ .onCallAgentDoBackupError(
+ eq(PACKAGE_1.packageName), eq(true), any(RuntimeException.class));
assertEventLogged(
EventLogTags.BACKUP_AGENT_FAILURE,
PACKAGE_1.packageName,
new RuntimeException().toString());
+ }
+
+ @Test
+ public void testRunTask_whenLocalAgentOnBackupThrows_doesNotUpdateBookkeping()
+ throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ AgentMock agentMock = setUpAgent(PACKAGE_1);
+ agentOnBackupDo(
+ agentMock,
+ (oldState, dataOutput, newState) -> {
+ throw new RuntimeException();
+ });
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+ runTask(task);
+
assertBackupPendingFor(PACKAGE_1);
}
@Test
+ public void testRunTask_whenAgentOnBackupThrows_reportsCorrectly() throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ AgentMock agentMock = setUpAgent(PACKAGE_1);
+ remoteAgentOnBackupThrows(
+ agentMock,
+ (oldState, dataOutput, newState) -> {
+ throw new RuntimeException();
+ });
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+ runTask(task);
+
+ verify(mReporter).onAgentResultError(argThat(packageInfo(PACKAGE_1)));
+ }
+
+ @Test
+ public void testRunTask_whenAgentOnBackupThrows_updatesBookkeeping() throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ AgentMock agentMock = setUpAgent(PACKAGE_1);
+ remoteAgentOnBackupThrows(
+ agentMock,
+ (oldState, dataOutput, newState) -> {
+ throw new RuntimeException();
+ });
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+ runTask(task);
+
+ assertBackupNotPendingFor(PACKAGE_1);
+ }
+
+ @Test
+ public void testRunTask_whenAgentOnBackupThrows_doesNotCallTransport() throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ AgentMock agentMock = setUpAgent(PACKAGE_1);
+ remoteAgentOnBackupThrows(
+ agentMock,
+ (oldState, dataOutput, newState) -> {
+ throw new RuntimeException();
+ });
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+ runTask(task);
+
+ verify(transportMock.transport, never())
+ .performBackup(argThat(packageInfo(PACKAGE_1)), any(), anyInt());
+ }
+
+ @Test
+ public void testRunTask_whenAgentOnBackupThrows_updatesAndCleansUpFiles() throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ AgentMock agentMock = setUpAgent(PACKAGE_1);
+ remoteAgentOnBackupThrows(
+ agentMock,
+ (oldState, dataOutput, newState) -> {
+ throw new RuntimeException();
+ });
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+ Files.write(getStateFile(mTransport, PACKAGE_1), "oldState".getBytes());
+
+ runTask(task);
+
+ assertThat(Files.readAllBytes(getStateFile(mTransport, PACKAGE_1)))
+ .isEqualTo("oldState".getBytes());
+ assertThat(Files.exists(getTemporaryStateFile(mTransport, PACKAGE_1))).isFalse();
+ assertThat(Files.exists(getStagingFile(PACKAGE_1))).isFalse();
+ }
+
+ @Test
public void testRunTask_whenTransportProvidesFlags_passesThemToTheAgent() throws Exception {
TransportMock transportMock = setUpInitializedTransport(mTransport);
int flags = BackupAgent.FLAG_CLIENT_SIDE_ENCRYPTION_ENABLED;
@@ -2130,6 +2234,10 @@
* Implements {@code function} for {@link BackupAgent#onBackup(ParcelFileDescriptor,
* BackupDataOutput, ParcelFileDescriptor)} of {@code agentMock} and populates {@link
* AgentMock#oldState}.
+ *
+ * <p>Note that for throwing agents this will simulate a local agent (the exception will be
+ * thrown in our stack), use {@link #remoteAgentOnBackupThrows(AgentMock, BackupAgentOnBackup)}
+ * if you want to simulate a remote agent.
*/
private static void agentOnBackupDo(AgentMock agentMock, BackupAgentOnBackup function)
throws Exception {
@@ -2150,6 +2258,33 @@
}
/**
+ * Use this method to simulate a remote agent throwing. We catch the exception thrown, thus
+ * simulating a one-way call. It also populates {@link AgentMock#oldState}.
+ *
+ * @param agentMock The Agent mock.
+ * @param function A function that throws, otherwise the test will fail.
+ */
+ // TODO: Remove when RemoteCall spins up a dedicated thread for calls
+ private static void remoteAgentOnBackupThrows(AgentMock agentMock, BackupAgentOnBackup function)
+ throws Exception {
+ agentOnBackupDo(agentMock, function);
+ doAnswer(
+ invocation -> {
+ try {
+ invocation.callRealMethod();
+ fail("Agent method expected to throw");
+ } catch (RuntimeException e) {
+ // This silences the exception just like a one-way call would, the
+ // normal completion via IBackupCallback binder still happens, check
+ // finally() block of IBackupAgent.doBackup().
+ }
+ return null;
+ })
+ .when(agentMock.agentBinder)
+ .doBackup(any(), any(), any(), anyLong(), any(), anyInt());
+ }
+
+ /**
* Returns an {@link Answer} that can be used for mocking {@link
* IBackupTransport#performBackup(PackageInfo, ParcelFileDescriptor, int)} that copies the
* backup data received to {@code backupDataPath} and returns {@code result}.
diff --git a/services/robotests/src/com/android/server/backup/remote/FutureBackupCallbackTest.java b/services/robotests/src/com/android/server/backup/remote/FutureBackupCallbackTest.java
index aec207d..f3621e2 100644
--- a/services/robotests/src/com/android/server/backup/remote/FutureBackupCallbackTest.java
+++ b/services/robotests/src/com/android/server/backup/remote/FutureBackupCallbackTest.java
@@ -41,6 +41,6 @@
callback.operationComplete(7);
- assertThat(future.get()).isEqualTo(RemoteResult.successful(7));
+ assertThat(future.get()).isEqualTo(RemoteResult.of(7));
}
}
diff --git a/services/robotests/src/com/android/server/backup/remote/RemoteCallTest.java b/services/robotests/src/com/android/server/backup/remote/RemoteCallTest.java
index 55db616..6434c4f 100644
--- a/services/robotests/src/com/android/server/backup/remote/RemoteCallTest.java
+++ b/services/robotests/src/com/android/server/backup/remote/RemoteCallTest.java
@@ -161,7 +161,7 @@
}
@Test
- public void testCall_whenCallbackIsCalledBeforeTimeOut_returnsSuccess() throws Exception {
+ public void testCall_whenCallbackIsCalledBeforeTimeOut_returnsResult() throws Exception {
ConditionVariable scheduled = new ConditionVariable(false);
RemoteCall remoteCall =
new RemoteCall(
@@ -176,11 +176,11 @@
scheduled.block();
runToEndOfTasks(Looper.getMainLooper());
- assertThat(result.get()).isEqualTo(RemoteResult.successful(3));
+ assertThat(result.get()).isEqualTo(RemoteResult.of(3));
}
@Test
- public void testCall_whenCallbackIsCalledBeforeCancel_returnsSuccess() throws Exception {
+ public void testCall_whenCallbackIsCalledBeforeCancel_returnsResult() throws Exception {
CompletableFuture<IBackupCallback> callbackFuture = new CompletableFuture<>();
RemoteCall remoteCall = new RemoteCall(callbackFuture::complete, 1000);
@@ -191,7 +191,7 @@
IBackupCallback callback = callbackFuture.get();
callback.operationComplete(3);
remoteCall.cancel();
- assertThat(result.get()).isEqualTo(RemoteResult.successful(3));
+ assertThat(result.get()).isEqualTo(RemoteResult.of(3));
}
@Test
diff --git a/services/robotests/src/com/android/server/backup/remote/RemoteResultTest.java b/services/robotests/src/com/android/server/backup/remote/RemoteResultTest.java
index f1c4f27..7f6fd57 100644
--- a/services/robotests/src/com/android/server/backup/remote/RemoteResultTest.java
+++ b/services/robotests/src/com/android/server/backup/remote/RemoteResultTest.java
@@ -35,28 +35,38 @@
@Presubmit
public class RemoteResultTest {
@Test
- public void testSucceeded_whenSuccessfulResult_returnsTrue() {
- RemoteResult result = RemoteResult.successful(3);
+ public void testIsPresent_whenNonFailedResult_returnsTrue() {
+ RemoteResult result = RemoteResult.of(3);
- boolean succeeded = result.succeeded();
+ boolean isPresent = result.isPresent();
- assertThat(succeeded).isTrue();
+ assertThat(isPresent).isTrue();
}
@Test
- public void testSucceeded_whenFailedResults_returnsFalse() {
- boolean timeOutSucceeded = RemoteResult.FAILED_TIMED_OUT.succeeded();
- boolean cancelledSucceeded = RemoteResult.FAILED_CANCELLED.succeeded();
- boolean threadInterruptedSucceeded = RemoteResult.FAILED_THREAD_INTERRUPTED.succeeded();
+ public void testIsPresent_whenTimeOutResult_returnsFalse() {
+ boolean timeOutIsPresent = RemoteResult.FAILED_TIMED_OUT.isPresent();
- assertThat(timeOutSucceeded).isFalse();
- assertThat(cancelledSucceeded).isFalse();
- assertThat(threadInterruptedSucceeded).isFalse();
+ assertThat(timeOutIsPresent).isFalse();
+ }
+
+ @Test
+ public void testIsPresent_whenCancelledResult_returnsFalse() {
+ boolean cancelledIsPresent = RemoteResult.FAILED_CANCELLED.isPresent();
+
+ assertThat(cancelledIsPresent).isFalse();
+ }
+
+ @Test
+ public void testIsPresent_whenThreadInterruptedResult_returnsFalse() {
+ boolean threadInterruptedIsPresent = RemoteResult.FAILED_THREAD_INTERRUPTED.isPresent();
+
+ assertThat(threadInterruptedIsPresent).isFalse();
}
@Test
public void testGet_whenSuccessfulResult_returnsValue() {
- RemoteResult result = RemoteResult.successful(7);
+ RemoteResult result = RemoteResult.of(7);
long value = result.get();
@@ -72,7 +82,7 @@
@Test
public void testToString() {
- assertThat(RemoteResult.successful(3).toString()).isEqualTo("RemoteResult{3}");
+ assertThat(RemoteResult.of(3).toString()).isEqualTo("RemoteResult{3}");
assertThat(RemoteResult.FAILED_TIMED_OUT.toString())
.isEqualTo("RemoteResult{FAILED_TIMED_OUT}");
assertThat(RemoteResult.FAILED_CANCELLED.toString())
@@ -83,14 +93,14 @@
@Test
public void testEquals() {
- assertThat(RemoteResult.successful(3).equals(RemoteResult.successful(3))).isTrue();
- assertThat(RemoteResult.successful(3).equals(RemoteResult.successful(7))).isFalse();
- assertThat(RemoteResult.successful(-1).equals(RemoteResult.successful(1))).isFalse();
- assertThat(RemoteResult.successful(Long.MAX_VALUE).equals(RemoteResult.successful(-1)))
+ assertThat(RemoteResult.of(3).equals(RemoteResult.of(3))).isTrue();
+ assertThat(RemoteResult.of(3).equals(RemoteResult.of(7))).isFalse();
+ assertThat(RemoteResult.of(-1).equals(RemoteResult.of(1))).isFalse();
+ assertThat(RemoteResult.of(Long.MAX_VALUE).equals(RemoteResult.of(-1)))
.isFalse();
- assertThat(RemoteResult.successful(3).equals(RemoteResult.FAILED_TIMED_OUT)).isFalse();
- assertThat(RemoteResult.successful(3).equals("3")).isFalse();
- assertThat(RemoteResult.successful(3).equals(null)).isFalse();
+ assertThat(RemoteResult.of(3).equals(RemoteResult.FAILED_TIMED_OUT)).isFalse();
+ assertThat(RemoteResult.of(3).equals("3")).isFalse();
+ assertThat(RemoteResult.of(3).equals(null)).isFalse();
assertThat(RemoteResult.FAILED_TIMED_OUT.equals(RemoteResult.FAILED_TIMED_OUT)).isTrue();
assertThat(RemoteResult.FAILED_TIMED_OUT.equals(RemoteResult.FAILED_CANCELLED)).isFalse();
}
@@ -98,9 +108,9 @@
/** @see Object#hashCode() */
@Test
public void testHashCode() {
- RemoteResult result3 = RemoteResult.successful(3);
+ RemoteResult result3 = RemoteResult.of(3);
assertThat(result3.hashCode()).isEqualTo(result3.hashCode());
- assertThat(result3.hashCode()).isEqualTo(RemoteResult.successful(3).hashCode());
+ assertThat(result3.hashCode()).isEqualTo(RemoteResult.of(3).hashCode());
assertThat(RemoteResult.FAILED_TIMED_OUT.hashCode())
.isEqualTo(RemoteResult.FAILED_TIMED_OUT.hashCode());
assertThat(RemoteResult.FAILED_CANCELLED.hashCode())