Merge "[KV] Exceptions for error-handling"

services/backup/java/com/android/server/backup/keyvalue/AgentException.java

@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.server.backup.keyvalue;
+
+/**
+ * This represents something wrong with a specific package. For example:
+ * <ul>
+ *     <li>Package unknown.
+ *     <li>Package is not eligible for backup anymore.
+ *     <li>Backup agent timed out.
+ *     <li>Backup agent wrote protected keys.
+ *     <li>...
+ * </ul>
+ *
+ * @see KeyValueBackupTask
+ * @see TaskException
+ */
+class AgentException extends BackupException {
+    static AgentException transitory() {
+        return new AgentException(/* transitory */ true);
+    }
+
+    static AgentException transitory(Exception cause) {
+        return new AgentException(/* transitory */ true, cause);
+    }
+
+    static AgentException permanent() {
+        return new AgentException(/* transitory */ false);
+    }
+
+    static AgentException permanent(Exception cause) {
+        return new AgentException(/* transitory */ false, cause);
+    }
+
+    private final boolean mTransitory;
+
+    private AgentException(boolean transitory) {
+        mTransitory = transitory;
+    }
+
+    private AgentException(boolean transitory, Exception cause) {
+        super(cause);
+        mTransitory = transitory;
+    }
+
+    boolean isTransitory() {
+        return mTransitory;
+    }
+}

services/backup/java/com/android/server/backup/keyvalue/BackupException.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.server.backup.keyvalue;
+
+import android.util.AndroidException;
+
+/**
+ * Key-value backup task exception.
+ *
+ * @see AgentException
+ * @see TaskException
+ */
+class BackupException extends AndroidException {
+    BackupException() {}
+
+    BackupException(Exception cause) {
+        super(cause);
+    }
+}

services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupReporter.java

@@ -153,16 +153,18 @@ public class KeyValueBackupReporter {
                 mObserver, packageName, BackupManager.ERROR_BACKUP_NOT_ALLOWED);
     }
 
-    void onBindAgentError(SecurityException e) {
-        Slog.d(TAG, "Error in bind/backup", e);
-    }
-
     void onAgentUnknown(String packageName) {
         Slog.d(TAG, "Package does not exist, skipping");
         BackupObserverUtils.sendBackupOnPackageResult(
                 mObserver, packageName, BackupManager.ERROR_PACKAGE_NOT_FOUND);
     }
 
+    void onBindAgentError(String packageName, SecurityException e) {
+        Slog.d(TAG, "Error in bind/backup", e);
+        BackupObserverUtils.sendBackupOnPackageResult(
+                mObserver, packageName, BackupManager.ERROR_AGENT_FAILURE);
+    }
+
     void onAgentError(String packageName) {
         if (MORE_DEBUG) {
             Slog.i(TAG, "Agent failure for " + packageName + ", re-staging");
@@ -190,6 +192,8 @@ public class KeyValueBackupReporter {
     void onCallAgentDoBackupError(String packageName, boolean callingAgent, Exception e) {
         if (callingAgent) {
             Slog.e(TAG, "Error invoking agent on " + packageName + ": " + e);
+            BackupObserverUtils.sendBackupOnPackageResult(
+                    mObserver, packageName, BackupManager.ERROR_AGENT_FAILURE);
         } else {
             Slog.e(TAG, "Error before invoking agent on " + packageName + ": " + e);
         }
@@ -220,12 +224,8 @@ public class KeyValueBackupReporter {
         }
     }
 
-    void onReadAgentDataError(String packageName, IOException e) {
+    void onAgentDataError(String packageName, IOException e) {
-        Slog.w(TAG, "Unable read backup data for " + packageName + ": " + e);
+        Slog.w(TAG, "Unable to read/write agent data for " + packageName + ": " + e);
-    }
-
-    void onWriteWidgetDataError(String packageName, IOException e) {
-        Slog.w(TAG, "Unable to save widget data for " + packageName + ": " + e);
     }
 
     void onDigestError(NoSuchAlgorithmException e) {
@@ -243,16 +243,12 @@ public class KeyValueBackupReporter {
         }
     }
 
-    void onSendDataToTransport(String packageName) {
+    void onTransportPerformBackup(String packageName) {
         if (MORE_DEBUG) {
             Slog.v(TAG, "Sending non-empty data to transport for " + packageName);
         }
     }
 
-    void onNonIncrementalAndNonIncrementalRequired() {
-        Slog.e(TAG, "Transport requested non-incremental but already the case");
-    }
-
     void onEmptyData(PackageInfo packageInfo) {
         if (MORE_DEBUG) {
             Slog.i(TAG, "No backup data written, not calling transport");
@@ -302,13 +298,20 @@ public class KeyValueBackupReporter {
                 /* extras */ null);
     }
 
+    void onPackageBackupNonIncrementalAndNonIncrementalRequired(String packageName) {
+        Slog.e(TAG, "Transport requested non-incremental but already the case");
+        BackupObserverUtils.sendBackupOnPackageResult(
+                mObserver, packageName, BackupManager.ERROR_TRANSPORT_ABORTED);
+        EventLog.writeEvent(EventLogTags.BACKUP_TRANSPORT_FAILURE, packageName);
+    }
+
     void onPackageBackupTransportFailure(String packageName) {
         BackupObserverUtils.sendBackupOnPackageResult(
                 mObserver, packageName, BackupManager.ERROR_TRANSPORT_ABORTED);
         EventLog.writeEvent(EventLogTags.BACKUP_TRANSPORT_FAILURE, packageName);
     }
 
-    void onPackageBackupError(String packageName, Exception e) {
+    void onPackageBackupTransportError(String packageName, Exception e) {
         Slog.e(TAG, "Transport error backing up " + packageName, e);
         BackupObserverUtils.sendBackupOnPackageResult(
                 mObserver, packageName, BackupManager.ERROR_TRANSPORT_ABORTED);

services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java

@@ -16,6 +16,7 @@
 
 package com.android.server.backup.keyvalue;
 
+import static android.app.ApplicationThreadConstants.BACKUP_MODE_INCREMENTAL;
 import static android.os.ParcelFileDescriptor.MODE_CREATE;
 import static android.os.ParcelFileDescriptor.MODE_READ_ONLY;
 import static android.os.ParcelFileDescriptor.MODE_READ_WRITE;
@@ -25,8 +26,8 @@ import static com.android.server.backup.BackupManagerService.KEY_WIDGET_STATE;
 import static com.android.server.backup.BackupManagerService.OP_PENDING;
 import static com.android.server.backup.BackupManagerService.OP_TYPE_BACKUP;
 
+import android.annotation.IntDef;
 import android.annotation.Nullable;
-import android.app.ApplicationThreadConstants;
 import android.app.IBackupAgent;
 import android.app.backup.BackupAgent;
 import android.app.backup.BackupDataInput;
@@ -47,7 +48,6 @@ import android.os.RemoteException;
 import android.os.SELinux;
 import android.os.UserHandle;
 import android.os.WorkSource;
-import android.util.Pair;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
@@ -77,6 +77,8 @@ import java.io.FileDescriptor;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
@@ -173,10 +175,8 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
     private static final AtomicInteger THREAD_COUNT = new AtomicInteger();
     private static final String BLANK_STATE_FILE_NAME = "blank_state";
     private static final String PM_PACKAGE = BackupManagerService.PACKAGE_MANAGER_SENTINEL;
-    @VisibleForTesting
+    @VisibleForTesting public static final String STAGING_FILE_SUFFIX = ".data";
-    public static final String STAGING_FILE_SUFFIX = ".data";
+    @VisibleForTesting public static final String NEW_STATE_FILE_SUFFIX = ".new";
-    @VisibleForTesting
-    public static final String NEW_STATE_FILE_SUFFIX = ".new";
 
     /**
      * Creates a new {@link KeyValueBackupTask} for key-value backup operation, spins up a new
@@ -244,13 +244,13 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
     private final int mCurrentOpToken;
     private final File mStateDirectory;
     private final File mDataDirectory;
+    private final File mBlankStateFile;
     private final List<String> mOriginalQueue;
     private final List<String> mQueue;
     private final List<String> mPendingFullBackups;
     private final Object mQueueLock;
     @Nullable private final DataChangedJournal mJournal;
 
-    private int mStatus;
     @Nullable private PerformFullTransportBackupTask mFullBackupTask;
     @Nullable private IBackupAgent mAgent;
     @Nullable private PackageInfo mCurrentPackage;
@@ -316,6 +316,7 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         mDataDirectory = mBackupManagerService.getDataDir();
         mCurrentOpToken = backupManagerService.generateRandomIntegerToken();
         mQueueLock = mBackupManagerService.getQueueLock();
+        mBlankStateFile = new File(mStateDirectory, BLANK_STATE_FILE_NAME);
     }
 
     private void registerTask() {
@@ -331,45 +332,43 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
     public void run() {
         Process.setThreadPriority(THREAD_PRIORITY);
 
-        boolean processQueue = startTask();
+        int status = BackupTransport.TRANSPORT_OK;
-        while (processQueue && !mQueue.isEmpty() && !mCancelled) {
+        try {
-            String packageName = mQueue.remove(0);
+            startTask();
-            if (PM_PACKAGE.equals(packageName)) {
+            while (!mQueue.isEmpty() && !mCancelled) {
-                processQueue = backupPm();
+                String packageName = mQueue.remove(0);
-            } else {
+                try {
-                processQueue = backupPackage(packageName);
+                    if (PM_PACKAGE.equals(packageName)) {
+                        backupPm();
+                    } else {
+                        backupPackage(packageName);
+                    }
+                } catch (AgentException e) {
+                    if (e.isTransitory()) {
+                        // We try again this package in the next backup pass.
+                        mBackupManagerService.dataChangedImpl(packageName);
+                    }
+                }
             }
+        } catch (TaskException e) {
+            if (e.isStateCompromised()) {
+                mBackupManagerService.resetBackupState(mStateDirectory);
+            }
+            revertTask();
+            status = e.getStatus();
         }
-        finishTask();
+        finishTask(status);
     }
 
-    /** Returns whether to consume next queue package. */
+    /** Returns transport status. */
-    private boolean handleAgentResult(@Nullable PackageInfo packageInfo, RemoteResult result) {
+    private int sendDataToTransport(@Nullable PackageInfo packageInfo)
-        if (result == RemoteResult.FAILED_THREAD_INTERRUPTED) {
+            throws AgentException, TaskException {
-            // Not an explicit cancel, we need to flag it.
+        try {
-            mCancelled = true;
+            return sendDataToTransport();
-            mReporter.onAgentCancelled(packageInfo);
+        } catch (IOException e) {
-            cleanUpAgentForAgentError();
+            mReporter.onAgentDataError(packageInfo.packageName, e);
-            return false;
+            throw TaskException.causedBy(e);
         }
-        if (result == RemoteResult.FAILED_CANCELLED) {
-            mReporter.onAgentCancelled(packageInfo);
-            cleanUpAgentForAgentError();
-            return false;
-        }
-        if (result == RemoteResult.FAILED_TIMED_OUT) {
-            mReporter.onAgentTimedOut(packageInfo);
-            cleanUpAgentForAgentError();
-            return true;
-        }
-        Preconditions.checkState(result.isPresent());
-        long agentResult = result.get();
-        if (agentResult == BackupAgent.RESULT_ERROR) {
-            mReporter.onAgentResultError(packageInfo);
-            cleanUpAgentForAgentError();
-            return true;
-        }
-        return sendDataToTransport();
     }
 
     @Override
@@ -378,11 +377,10 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
     @Override
     public void operationComplete(long unusedResult) {}
 
-    /** Returns whether to consume next queue package. */
+    private void startTask() throws TaskException {
-    private boolean startTask() {
         if (mBackupManagerService.isBackupOperationInProgress()) {
             mReporter.onSkipBackup();
-            return false;
+            throw TaskException.create();
         }
 
         // Unfortunately full backup task constructor registers the task with BMS, so we have to
@@ -390,11 +388,9 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         mFullBackupTask = createFullBackupTask(mPendingFullBackups);
         registerTask();
 
-        mStatus = BackupTransport.TRANSPORT_OK;
-
         if (mQueue.isEmpty() && mPendingFullBackups.isEmpty()) {
             mReporter.onEmptyQueueAtStart();
-            return false;
+            return;
         }
         // We only backup PM if it was explicitly in the queue or if it's incremental.
         boolean backupPm = mQueue.remove(PM_PACKAGE) || !mNonIncremental;
@@ -415,20 +411,18 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
             if (pmState.length() <= 0) {
                 mReporter.onInitializeTransport(transportName);
                 mBackupManagerService.resetBackupState(mStateDirectory);
-                mStatus = transport.initializeDevice();
+                int status = transport.initializeDevice();
-                mReporter.onTransportInitialized(mStatus);
+                mReporter.onTransportInitialized(status);
+                if (status != BackupTransport.TRANSPORT_OK) {
+                    throw TaskException.stateCompromised();
+                }
             }
+        } catch (TaskException e) {
+            throw e;
         } catch (Exception e) {
             mReporter.onInitializeTransportError(e);
-            mStatus = BackupTransport.TRANSPORT_ERROR;
+            throw TaskException.stateCompromised();
         }
-
-        if (mStatus != BackupTransport.TRANSPORT_OK) {
-            mBackupManagerService.resetBackupState(mStateDirectory);
-            return false;
-        }
-
-        return true;
     }
 
     private PerformFullTransportBackupTask createFullBackupTask(List<String> packages) {
@@ -446,120 +440,82 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
                 mUserInitiated);
     }
 
-    /** Returns whether to consume next queue package. */
+    private void backupPm() throws TaskException {
-    private boolean backupPm() {
+        mReporter.onStartPackageBackup(PM_PACKAGE);
-        RemoteResult agentResult = null;
+        mCurrentPackage = new PackageInfo();
+        mCurrentPackage.packageName = PM_PACKAGE;
+
         try {
-            mCurrentPackage = new PackageInfo();
+            extractPmAgentData(mCurrentPackage);
-            mCurrentPackage.packageName = PM_PACKAGE;
+            int status = sendDataToTransport(mCurrentPackage);
-
+            cleanUpAgentForTransportStatus(status);
-            // Since PM is running in the system process we can set up its agent directly.
+        } catch (AgentException | TaskException e) {
-            BackupAgent pmAgent = mBackupManagerService.makeMetadataAgent();
-            mAgent = IBackupAgent.Stub.asInterface(pmAgent.onBind());
-
-            Pair<Integer, RemoteResult> statusAndResult = extractAgentData(PM_PACKAGE, mAgent);
-            mStatus = statusAndResult.first;
-            agentResult = statusAndResult.second;
-        } catch (Exception e) {
             mReporter.onExtractPmAgentDataError(e);
-            mStatus = BackupTransport.TRANSPORT_ERROR;
+            cleanUpAgentForError(e);
+            // PM agent failure is task failure.
+            throw TaskException.stateCompromised(e);
         }
-
-        if (mStatus != BackupTransport.TRANSPORT_OK) {
-            // In this case either extractAgentData() already made the agent clean-up or we haven't
-            // prepared the state for calling the agent, in either case we don't need to clean-up.
-            mBackupManagerService.resetBackupState(mStateDirectory);
-            return false;
-        }
-
-        Preconditions.checkNotNull(agentResult);
-        return handleAgentResult(mCurrentPackage, agentResult);
     }
 
-    /** Returns whether to consume next queue package. */
+    private void backupPackage(String packageName) throws AgentException, TaskException {
-    private boolean backupPackage(String packageName) {
         mReporter.onStartPackageBackup(packageName);
-        mStatus = BackupTransport.TRANSPORT_OK;
+        mCurrentPackage = getPackageForBackup(packageName);
 
-        // Verify that the requested app is eligible for key-value backup.
-        RemoteResult agentResult = null;
         try {
-            mCurrentPackage = mPackageManager.getPackageInfo(
+            extractAgentData(mCurrentPackage);
-                    packageName, PackageManager.GET_SIGNING_CERTIFICATES);
+            int status = sendDataToTransport(mCurrentPackage);
-            ApplicationInfo applicationInfo = mCurrentPackage.applicationInfo;
+            cleanUpAgentForTransportStatus(status);
-            if (!AppBackupUtils.appIsEligibleForBackup(applicationInfo, mPackageManager)) {
+        } catch (AgentException | TaskException e) {
-                // The manifest has changed. This won't happen again because the app won't be
+            cleanUpAgentForError(e);
-                // requesting further backups.
+            throw e;
-                mReporter.onPackageNotEligibleForBackup(packageName);
-                return true;
-            }
-
-            if (AppBackupUtils.appGetsFullBackup(mCurrentPackage)) {
-                // Initially enqueued for key-value backup, but only supports full-backup now.
-                mReporter.onPackageEligibleForFullBackup(packageName);
-                return true;
-            }
-
-            if (AppBackupUtils.appIsStopped(applicationInfo)) {
-                // Just as it won't receive broadcasts, we won't run it for backup.
-                mReporter.onPackageStopped(packageName);
-                return true;
-            }
-
-            try {
-                mBackupManagerService.setWorkSource(new WorkSource(applicationInfo.uid));
-                IBackupAgent agent =
-                        mBackupManagerService.bindToAgentSynchronous(
-                                applicationInfo,
-                                ApplicationThreadConstants.BACKUP_MODE_INCREMENTAL);
-                if (agent != null) {
-                    mAgent = agent;
-                    Pair<Integer, RemoteResult> statusAndResult =
-                            extractAgentData(packageName, agent);
-                    mStatus = statusAndResult.first;
-                    agentResult = statusAndResult.second;
-                } else {
-                    // Timeout waiting for the agent to bind.
-                    mStatus = BackupTransport.AGENT_ERROR;
-                }
-            } catch (SecurityException e) {
-                mReporter.onBindAgentError(e);
-                mStatus = BackupTransport.AGENT_ERROR;
-            }
-        } catch (PackageManager.NameNotFoundException e) {
-            mStatus = BackupTransport.AGENT_UNKNOWN;
-        } finally {
-            mBackupManagerService.setWorkSource(null);
         }
-
-        if (mStatus != BackupTransport.TRANSPORT_OK) {
-            // In this case either extractAgentData() already made the agent clean-up or we haven't
-            // prepared the state for calling the agent, in either case we don't need to clean-up.
-            Preconditions.checkState(mAgent == null);
-
-            if (mStatus == BackupTransport.AGENT_ERROR) {
-                mReporter.onAgentError(packageName);
-                mBackupManagerService.dataChangedImpl(packageName);
-                mStatus = BackupTransport.TRANSPORT_OK;
-                return true;
-            }
-
-            if (mStatus == BackupTransport.AGENT_UNKNOWN) {
-                mReporter.onAgentUnknown(packageName);
-                mStatus = BackupTransport.TRANSPORT_OK;
-                return true;
-            }
-
-            // Transport-level failure, re-enqueue everything.
-            revertTask();
-            return false;
-        }
-
-        Preconditions.checkNotNull(agentResult);
-        return handleAgentResult(mCurrentPackage, agentResult);
     }
 
-    private void finishTask() {
+    private PackageInfo getPackageForBackup(String packageName) throws AgentException {
+        final PackageInfo packageInfo;
+        try {
+            packageInfo =
+                    mPackageManager.getPackageInfo(
+                            packageName, PackageManager.GET_SIGNING_CERTIFICATES);
+        } catch (PackageManager.NameNotFoundException e) {
+            mReporter.onAgentUnknown(packageName);
+            throw AgentException.permanent(e);
+        }
+        ApplicationInfo applicationInfo = packageInfo.applicationInfo;
+        if (!AppBackupUtils.appIsEligibleForBackup(applicationInfo, mPackageManager)) {
+            mReporter.onPackageNotEligibleForBackup(packageName);
+            throw AgentException.permanent();
+        }
+        if (AppBackupUtils.appGetsFullBackup(packageInfo)) {
+            mReporter.onPackageEligibleForFullBackup(packageName);
+            throw AgentException.permanent();
+        }
+        if (AppBackupUtils.appIsStopped(applicationInfo)) {
+            mReporter.onPackageStopped(packageName);
+            throw AgentException.permanent();
+        }
+        return packageInfo;
+    }
+
+    private IBackupAgent bindAgent(PackageInfo packageInfo) throws AgentException {
+        String packageName = packageInfo.packageName;
+        final IBackupAgent agent;
+        try {
+            agent =
+                    mBackupManagerService.bindToAgentSynchronous(
+                            packageInfo.applicationInfo, BACKUP_MODE_INCREMENTAL);
+            if (agent == null) {
+                mReporter.onAgentError(packageName);
+                throw AgentException.transitory();
+            }
+        } catch (SecurityException e) {
+            mReporter.onBindAgentError(packageName, e);
+            throw AgentException.transitory(e);
+        }
+        return agent;
+    }
+
+    private void finishTask(int status) {
         // Mark packages that we couldn't backup as pending backup.
         for (String packageName : mQueue) {
             mBackupManagerService.dataChangedImpl(packageName);
@@ -576,7 +532,7 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         // If we succeeded and this is the first time we've done a backup, we can record the current
         // backup dataset token.
         long currentToken = mBackupManagerService.getCurrentToken();
-        if ((mStatus == BackupTransport.TRANSPORT_OK) && (currentToken == 0)) {
+        if ((status == BackupTransport.TRANSPORT_OK) && (currentToken == 0)) {
             try {
                 IBackupTransport transport = mTransportClient.connectOrThrow(callerLogString);
                 mBackupManagerService.setCurrentToken(transport.getCurrentRestoreSet());
@@ -589,9 +545,14 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
 
         synchronized (mQueueLock) {
             mBackupManagerService.setBackupRunning(false);
-            if (mStatus == BackupTransport.TRANSPORT_NOT_INITIALIZED) {
+            if (status == BackupTransport.TRANSPORT_NOT_INITIALIZED) {
                 mReporter.onTransportNotInitialized();
-                triggerTransportInitializationLocked();
+                try {
+                    triggerTransportInitializationLocked();
+                } catch (Exception e) {
+                    mReporter.onPendingInitializeTransportError(e);
+                    status = BackupTransport.TRANSPORT_ERROR;
+                }
             }
         }
 
@@ -605,7 +566,7 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         }
 
         if (!mCancelled
-                && mStatus == BackupTransport.TRANSPORT_OK
+                && status == BackupTransport.TRANSPORT_OK
                 && mFullBackupTask != null
                 && !mPendingFullBackups.isEmpty()) {
             mReporter.onStartFullBackup(mPendingFullBackups);
@@ -621,7 +582,7 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
             mFullBackupTask.unregisterTask();
         }
         mTaskFinishedListener.onFinished(callerLogString);
-        mReporter.onBackupFinished(getBackupFinishedStatus(mCancelled, mStatus));
+        mReporter.onBackupFinished(getBackupFinishedStatus(mCancelled, status));
         mBackupManagerService.getWakelock().release();
     }
 
@@ -642,17 +603,12 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
     }
 
     @GuardedBy("mQueueLock")
-    private void triggerTransportInitializationLocked() {
+    private void triggerTransportInitializationLocked() throws Exception {
-        try {
+        IBackupTransport transport =
-            IBackupTransport transport =
+                mTransportClient.connectOrThrow("KVBT.triggerTransportInitializationLocked");
-                    mTransportClient.connectOrThrow("KVBT.triggerTransportInitializationLocked");
+        mBackupManagerService.getPendingInits().add(transport.name());
-            mBackupManagerService.getPendingInits().add(transport.name());
+        deletePmStateFile();
-            deletePmStateFile();
+        mBackupManagerService.backupNow();
-            mBackupManagerService.backupNow();
-        } catch (Exception e) {
-            mReporter.onPendingInitializeTransportError(e);
-            mStatus = BackupTransport.TRANSPORT_ERROR;
-        }
     }
 
     /** Removes PM state, triggering initialization in the next key-value task. */
@@ -660,35 +616,69 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         new File(mStateDirectory, PM_PACKAGE).delete();
     }
 
+    /** Same as {@link #extractAgentData(PackageInfo)}, but only for PM package. */
+    private void extractPmAgentData(PackageInfo packageInfo) throws AgentException, TaskException {
+        Preconditions.checkArgument(packageInfo.packageName.equals(PM_PACKAGE));
+        BackupAgent pmAgent = mBackupManagerService.makeMetadataAgent();
+        mAgent = IBackupAgent.Stub.asInterface(pmAgent.onBind());
+        extractAgentData(packageInfo, mAgent);
+    }
+
     /**
-     * Returns a {@link Pair}. The first of the pair contains the status. In case the status is
+     * Binds to the agent and extracts its backup data. If this method returns, the data in {@code
-     * {@link BackupTransport#TRANSPORT_OK}, the second of the pair contains the agent result,
+     * mBackupData} is ready to be sent to the transport, otherwise it will throw.
-     * otherwise {@code null}.
+     *
+     * <p>This method leaves agent resources (agent binder, files and file-descriptors) opened that
+     * need to be cleaned up after terminating, either successfully or exceptionally. This clean-up
+     * can be done with methods {@link #cleanUpAgentForTransportStatus(int)} and {@link
+     * #cleanUpAgentForError(BackupException)}, depending on whether data was successfully sent to
+     * the transport or not. It's the caller responsibility to do the clean-up or delegate it.
      */
-    private Pair<Integer, RemoteResult> extractAgentData(String packageName, IBackupAgent agent) {
+    private void extractAgentData(PackageInfo packageInfo) throws AgentException, TaskException {
+        mBackupManagerService.setWorkSource(new WorkSource(packageInfo.applicationInfo.uid));
+        try {
+            mAgent = bindAgent(packageInfo);
+            extractAgentData(packageInfo, mAgent);
+        } finally {
+            mBackupManagerService.setWorkSource(null);
+        }
+    }
+
+    /**
+     * Calls agent {@link IBackupAgent#doBackup(ParcelFileDescriptor, ParcelFileDescriptor,
+     * ParcelFileDescriptor, long, IBackupCallback, int)} and waits for the result. If this method
+     * returns, the data in {@code mBackupData} is ready to be sent to the transport, otherwise it
+     * will throw.
+     *
+     * <p>This method creates files and file-descriptors for the agent that need to be deleted and
+     * closed after terminating, either successfully or exceptionally. This clean-up can be done
+     * with methods {@link #cleanUpAgentForTransportStatus(int)} and {@link
+     * #cleanUpAgentForError(BackupException)}, depending on whether data was successfully sent to
+     * the transport or not. It's the caller responsibility to do the clean-up or delegate it.
+     */
+    private void extractAgentData(PackageInfo packageInfo, IBackupAgent agent)
+            throws AgentException, TaskException {
+        String packageName = packageInfo.packageName;
         mReporter.onExtractAgentData(packageName);
 
-        File blankStateFile = new File(mStateDirectory, BLANK_STATE_FILE_NAME);
         mSavedStateFile = new File(mStateDirectory, packageName);
         mBackupDataFile = new File(mDataDirectory, packageName + STAGING_FILE_SUFFIX);
         mNewStateFile = new File(mStateDirectory, packageName + NEW_STATE_FILE_SUFFIX);
         mReporter.onAgentFilesReady(mBackupDataFile);
 
-        mSavedState = null;
-        mBackupData = null;
-        mNewState = null;
-
         boolean callingAgent = false;
         final RemoteResult agentResult;
         try {
-            File savedStateFileForAgent = (mNonIncremental) ? blankStateFile : mSavedStateFile;
+            File savedStateFileForAgent = (mNonIncremental) ? mBlankStateFile : mSavedStateFile;
             // MODE_CREATE to make an empty file if necessary
-            mSavedState = ParcelFileDescriptor.open(
+            mSavedState =
-                    savedStateFileForAgent, MODE_READ_ONLY | MODE_CREATE);
+                    ParcelFileDescriptor.open(savedStateFileForAgent, MODE_READ_ONLY | MODE_CREATE);
-            mBackupData = ParcelFileDescriptor.open(
+            mBackupData =
-                    mBackupDataFile, MODE_READ_WRITE | MODE_CREATE | MODE_TRUNCATE);
+                    ParcelFileDescriptor.open(
-            mNewState = ParcelFileDescriptor.open(
+                            mBackupDataFile, MODE_READ_WRITE | MODE_CREATE | MODE_TRUNCATE);
-                    mNewStateFile, MODE_READ_WRITE | MODE_CREATE | MODE_TRUNCATE);
+            mNewState =
+                    ParcelFileDescriptor.open(
+                            mNewStateFile, MODE_READ_WRITE | MODE_CREATE | MODE_TRUNCATE);
 
             if (!SELinux.restorecon(mBackupDataFile)) {
                 mReporter.onRestoreconFailed(mBackupDataFile);
@@ -713,15 +703,40 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
                             "doBackup()");
         } catch (Exception e) {
             mReporter.onCallAgentDoBackupError(packageName, callingAgent, e);
-            cleanUpAgentForAgentError();
+            if (callingAgent) {
-            // TODO: Remove the check on callingAgent when RemoteCall supports local agent calls.
+                throw AgentException.transitory(e);
-            int status =
+            } else {
-                    callingAgent ? BackupTransport.AGENT_ERROR : BackupTransport.TRANSPORT_ERROR;
+                throw TaskException.create();
-            return Pair.create(status, null);
+            }
+        } finally {
+            mBlankStateFile.delete();
         }
-        blankStateFile.delete();
+        checkAgentResult(packageInfo, agentResult);
+    }
 
-        return Pair.create(BackupTransport.TRANSPORT_OK, agentResult);
+    private void checkAgentResult(PackageInfo packageInfo, RemoteResult result)
+            throws AgentException, TaskException {
+        if (result == RemoteResult.FAILED_THREAD_INTERRUPTED) {
+            // Not an explicit cancel, we need to flag it.
+            mCancelled = true;
+            mReporter.onAgentCancelled(packageInfo);
+            throw TaskException.create();
+        }
+        if (result == RemoteResult.FAILED_CANCELLED) {
+            mReporter.onAgentCancelled(packageInfo);
+            throw TaskException.create();
+        }
+        if (result == RemoteResult.FAILED_TIMED_OUT) {
+            mReporter.onAgentTimedOut(packageInfo);
+            throw AgentException.transitory();
+        }
+        Preconditions.checkState(result.isPresent());
+        long resultCode = result.get();
+        if (resultCode == BackupAgent.RESULT_ERROR) {
+            mReporter.onAgentResultError(packageInfo);
+            throw AgentException.transitory();
+        }
+        Preconditions.checkState(resultCode == BackupAgent.RESULT_SUCCESS);
     }
 
     private void agentFail(IBackupAgent agent, String message) {
@@ -801,94 +816,79 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         }
     }
 
-    /** Returns whether to consume next queue package. */
+    /** Returns transport status. */
-    private boolean sendDataToTransport() {
+    private int sendDataToTransport() throws AgentException, TaskException, IOException {
         Preconditions.checkState(mBackupData != null);
+        checkBackupData(mCurrentPackage.applicationInfo, mBackupDataFile);
 
         String packageName = mCurrentPackage.packageName;
-        ApplicationInfo applicationInfo = mCurrentPackage.applicationInfo;
+        writeWidgetPayloadIfAppropriate(mBackupData.getFileDescriptor(), packageName);
-
-        boolean writingWidgetData = false;
-        try {
-            if (!validateBackupData(applicationInfo, mBackupDataFile)) {
-                cleanUpAgentForAgentError();
-                return true;
-            }
-            writingWidgetData = true;
-            writeWidgetPayloadIfAppropriate(mBackupData.getFileDescriptor(), packageName);
-        } catch (IOException e) {
-            if (writingWidgetData) {
-                mReporter.onWriteWidgetDataError(packageName, e);
-            } else {
-                mReporter.onReadAgentDataError(packageName, e);
-            }
-            cleanUpAgentForAgentError();
-            revertTask();
-            return false;
-        }
 
         boolean nonIncremental = mSavedStateFile.length() == 0;
-        long size = mBackupDataFile.length();
+        int status = transportPerformBackup(mCurrentPackage, mBackupDataFile, nonIncremental);
-        if (size > 0) {
+        handleTransportStatus(status, packageName, mBackupDataFile.length());
-            try (ParcelFileDescriptor backupData =
+        return status;
-                         ParcelFileDescriptor.open(mBackupDataFile, MODE_READ_ONLY)) {
-                IBackupTransport transport =
-                        mTransportClient.connectOrThrow("KVBT.sendDataToTransport()");
-                mReporter.onSendDataToTransport(packageName);
-                int flags = getPerformBackupFlags(mUserInitiated, nonIncremental);
-
-                mStatus = transport.performBackup(mCurrentPackage, backupData, flags);
-                if (mStatus == BackupTransport.TRANSPORT_OK) {
-                    mStatus = transport.finishBackup();
-                }
-            } catch (Exception e) {
-                mReporter.onPackageBackupError(packageName, e);
-                mStatus = BackupTransport.TRANSPORT_ERROR;
-            }
-        } else {
-            mReporter.onEmptyData(mCurrentPackage);
-            mStatus = BackupTransport.TRANSPORT_OK;
-        }
-
-        if (nonIncremental
-                && mStatus == BackupTransport.TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED) {
-            mReporter.onNonIncrementalAndNonIncrementalRequired();
-            mStatus = BackupTransport.TRANSPORT_ERROR;
-        }
-
-
-        boolean processQueue = handleTransportStatus(mStatus, packageName, size);
-        // We might report quota exceeded to the agent in handleTransportStatus() above, so we
-        // only clean-up after it.
-        cleanUpAgentForTransportStatus(mStatus);
-        return processQueue;
     }
 
-    /** Returns whether to consume next queue package. */
+    private int transportPerformBackup(
-    private boolean handleTransportStatus(int status, String packageName, long size) {
+            PackageInfo packageInfo, File backupDataFile, boolean nonIncremental)
+            throws TaskException {
+        String packageName = packageInfo.packageName;
+        long size = backupDataFile.length();
+        if (size <= 0) {
+            mReporter.onEmptyData(packageInfo);
+            return BackupTransport.TRANSPORT_OK;
+        }
+
+        int status;
+        try (ParcelFileDescriptor backupData =
+                ParcelFileDescriptor.open(backupDataFile, MODE_READ_ONLY)) {
+            IBackupTransport transport =
+                    mTransportClient.connectOrThrow("KVBT.transportPerformBackup()");
+            mReporter.onTransportPerformBackup(packageName);
+            int flags = getPerformBackupFlags(mUserInitiated, nonIncremental);
+
+            status = transport.performBackup(packageInfo, backupData, flags);
+            if (status == BackupTransport.TRANSPORT_OK) {
+                status = transport.finishBackup();
+            }
+        } catch (Exception e) {
+            mReporter.onPackageBackupTransportError(packageName, e);
+            throw TaskException.causedBy(e);
+        }
+
+        if (nonIncremental && status == BackupTransport.TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED) {
+            mReporter.onPackageBackupNonIncrementalAndNonIncrementalRequired(packageName);
+            throw TaskException.create();
+        }
+
+        return status;
+    }
+
+    private void handleTransportStatus(int status, String packageName, long size)
+            throws TaskException, AgentException {
         if (status == BackupTransport.TRANSPORT_OK) {
             mReporter.onPackageBackupComplete(packageName, size);
-            return true;
+            return;
-        }
-        if (status == BackupTransport.TRANSPORT_PACKAGE_REJECTED) {
-            mReporter.onPackageBackupRejected(packageName);
-            return true;
         }
         if (status == BackupTransport.TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED) {
             mReporter.onPackageBackupNonIncrementalRequired(mCurrentPackage);
             // Immediately retry the current package.
             mQueue.add(0, packageName);
-            return true;
+            return;
+        }
+        if (status == BackupTransport.TRANSPORT_PACKAGE_REJECTED) {
+            mReporter.onPackageBackupRejected(packageName);
+            throw AgentException.permanent();
         }
         if (status == BackupTransport.TRANSPORT_QUOTA_EXCEEDED) {
             mReporter.onPackageBackupQuotaExceeded(packageName);
             agentDoQuotaExceeded(mAgent, packageName, size);
-            return true;
+            throw AgentException.permanent();
         }
         // Any other error here indicates a transport-level failure.
         mReporter.onPackageBackupTransportFailure(packageName);
-        revertTask();
+        throw TaskException.forStatus(status);
-        return false;
     }
 
     private void agentDoQuotaExceeded(@Nullable IBackupAgent agent, String packageName, long size) {
@@ -908,19 +908,17 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
     }
 
     /**
-     * For system apps and pseudo-apps always return {@code true}. For regular apps returns whether
+     * For system apps and pseudo-apps never throws. For regular apps throws {@link AgentException}
-     * {@code backupDataFile} doesn't have any protected keys.
+     * if {@code backupDataFile} has any protected keys, also crashing the app.
-     *
-     * <p>If the app has attempted to write any protected keys we also crash them.
      */
-    private boolean validateBackupData(
+    private void checkBackupData(@Nullable ApplicationInfo applicationInfo, File backupDataFile)
-            @Nullable ApplicationInfo applicationInfo, File backupDataFile) throws IOException {
+            throws IOException, AgentException {
         if (applicationInfo == null || (applicationInfo.flags & ApplicationInfo.FLAG_SYSTEM) != 0) {
             // System apps and pseudo-apps can write what they want.
-            return true;
+            return;
         }
         try (ParcelFileDescriptor backupData =
-                     ParcelFileDescriptor.open(backupDataFile, MODE_READ_ONLY)) {
+                ParcelFileDescriptor.open(backupDataFile, MODE_READ_ONLY)) {
             BackupDataInput backupDataInput = new BackupDataInput(backupData.getFileDescriptor());
             while (backupDataInput.readNextHeader()) {
                 String key = backupDataInput.getKey();
@@ -928,12 +926,11 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
                     mReporter.onAgentIllegalKey(mCurrentPackage, key);
                     // Crash them if they wrote any protected keys.
                     agentFail(mAgent, "Illegal backup key: " + key);
-                    return false;
+                    throw AgentException.permanent();
                 }
                 backupDataInput.skipEntityData();
             }
         }
-        return true;
     }
 
     private int getPerformBackupFlags(boolean userInitiated, boolean nonIncremental) {
@@ -1009,44 +1006,39 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         }
     }
 
-    /** Cleans-up after having called the agent. */
+    /**
+     * Cleans up agent resources opened by {@link #extractAgentData(PackageInfo)} for exceptional
+     * case.
+     *
+     * <p>Note: Declaring exception parameter so that the caller only calls this when an exception
+     * is thrown.
+     */
+    private void cleanUpAgentForError(BackupException exception) {
+        cleanUpAgent(StateTransaction.DISCARD_NEW);
+    }
+
+    /**
+     * Cleans up agent resources opened by {@link #extractAgentData(PackageInfo)} according to
+     * transport status returned in {@link #sendDataToTransport(PackageInfo)}.
+     */
     private void cleanUpAgentForTransportStatus(int status) {
-        updateFiles(status);
-        cleanUpAgent();
-    }
-
-    /** Cleans-up if we failed to call the agent. */
-    private void cleanUpAgentForAgentError() {
-        mBackupDataFile.delete();
-        mNewStateFile.delete();
-        cleanUpAgent();
-    }
-
-    private void updateFiles(int status) {
         switch (status) {
             case BackupTransport.TRANSPORT_OK:
-                mBackupDataFile.delete();
+                cleanUpAgent(StateTransaction.COMMIT_NEW);
-                mNewStateFile.renameTo(mSavedStateFile);
                 break;
             case BackupTransport.TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED:
-                mSavedStateFile.delete();
+                cleanUpAgent(StateTransaction.DISCARD_ALL);
-                mBackupDataFile.delete();
-                mNewStateFile.delete();
                 break;
             default:
-                // Includes:
+                // All other transport statuses are properly converted to agent or task exceptions.
-                // * BackupTransport.TRANSPORT_PACKAGE_REJECTED
+                throw new AssertionError();
-                // * BackupTransport.TRANSPORT_QUOTA_EXCEEDED
-                // * BackupTransport.TRANSPORT_ERROR
-                mBackupDataFile.delete();
-                mNewStateFile.delete();
-                break;
         }
     }
 
-    /** Cleans-up file-descriptors and unbinds agent. */
+    private void cleanUpAgent(@StateTransaction int stateTransaction) {
-    private void cleanUpAgent() {
+        applyStateTransaction(stateTransaction);
-        mAgent = null;
+        mBackupDataFile.delete();
+        mBlankStateFile.delete();
         tryCloseFileDescriptor(mSavedState, "old state");
         tryCloseFileDescriptor(mBackupData, "backup data");
         tryCloseFileDescriptor(mNewState, "new state");
@@ -1058,6 +1050,24 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         if (mCurrentPackage.applicationInfo != null) {
             mBackupManagerService.unbindAgent(mCurrentPackage.applicationInfo);
         }
+        mAgent = null;
+    }
+
+    private void applyStateTransaction(@StateTransaction int stateTransaction) {
+        switch (stateTransaction) {
+            case StateTransaction.COMMIT_NEW:
+                mNewStateFile.renameTo(mSavedStateFile);
+                break;
+            case StateTransaction.DISCARD_NEW:
+                mNewStateFile.delete();
+                break;
+            case StateTransaction.DISCARD_ALL:
+                mSavedStateFile.delete();
+                mNewStateFile.delete();
+                break;
+            default:
+                throw new IllegalArgumentException("Unknown state transaction " + stateTransaction);
+        }
     }
 
     private void tryCloseFileDescriptor(@Nullable Closeable closeable, String logName) {
@@ -1079,4 +1089,16 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
         mPendingCall = null;
         return result;
     }
+
+    @IntDef({
+        StateTransaction.COMMIT_NEW,
+        StateTransaction.DISCARD_NEW,
+        StateTransaction.DISCARD_ALL,
+    })
+    @Retention(RetentionPolicy.SOURCE)
+    private @interface StateTransaction {
+        int COMMIT_NEW = 0;
+        int DISCARD_NEW = 1;
+        int DISCARD_ALL = 2;
+    }
 }

services/backup/java/com/android/server/backup/keyvalue/TaskException.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.server.backup.keyvalue;
+
+import android.app.backup.BackupTransport;
+
+import com.android.internal.util.Preconditions;
+
+/**
+ * The key-value backup task has failed, no more packages will be processed and we shouldn't attempt
+ * any more backups now. These can be caused by transport failures (as opposed to agent failures).
+ *
+ * @see KeyValueBackupTask
+ * @see AgentException
+ */
+class TaskException extends BackupException {
+    private static final int DEFAULT_STATUS = BackupTransport.TRANSPORT_ERROR;
+
+    static TaskException stateCompromised() {
+        return new TaskException(/* stateCompromised */ true, DEFAULT_STATUS);
+    }
+
+    static TaskException stateCompromised(Exception cause) {
+        if (cause instanceof TaskException) {
+            TaskException exception = (TaskException) cause;
+            return new TaskException(cause, /* stateCompromised */ true, exception.getStatus());
+        }
+        return new TaskException(cause, /* stateCompromised */ true, DEFAULT_STATUS);
+    }
+
+    static TaskException forStatus(int status) {
+        Preconditions.checkArgument(
+                status != BackupTransport.TRANSPORT_OK, "Exception based on TRANSPORT_OK");
+        return new TaskException(/* stateCompromised */ false, status);
+    }
+
+    static TaskException causedBy(Exception cause) {
+        if (cause instanceof TaskException) {
+            return (TaskException) cause;
+        }
+        return new TaskException(cause, /* stateCompromised */ false, DEFAULT_STATUS);
+    }
+
+    static TaskException create() {
+        return new TaskException(/* stateCompromised */ false, DEFAULT_STATUS);
+    }
+
+    private final boolean mStateCompromised;
+    private final int mStatus;
+
+    private TaskException(Exception cause, boolean stateCompromised, int status) {
+        super(cause);
+        mStateCompromised = stateCompromised;
+        mStatus = status;
+    }
+
+    private TaskException(boolean stateCompromised, int status) {
+        mStateCompromised = stateCompromised;
+        mStatus = status;
+    }
+
+    boolean isStateCompromised() {
+        return mStateCompromised;
+    }
+
+    int getStatus() {
+        return mStatus;
+    }
+}

services/robotests/src/com/android/server/backup/keyvalue/AgentExceptionTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.server.backup.keyvalue;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import android.platform.test.annotations.Presubmit;
+
+import com.android.server.testing.FrameworkRobolectricTestRunner;
+import com.android.server.testing.SystemLoaderPackages;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+
+import java.io.IOException;
+
+@RunWith(FrameworkRobolectricTestRunner.class)
+@Config(manifest = Config.NONE, sdk = 26)
+@SystemLoaderPackages({"com.android.server.backup"})
+@Presubmit
+public class AgentExceptionTest {
+    @Test
+    public void testTransitory_isTransitory() throws Exception {
+        AgentException exception = AgentException.transitory();
+
+        assertThat(exception.isTransitory()).isTrue();
+    }
+
+    @Test
+    public void testTransitory_withCause() throws Exception {
+        Exception cause = new IOException();
+
+        AgentException exception = AgentException.transitory(cause);
+
+        assertThat(exception.isTransitory()).isTrue();
+        assertThat(exception.getCause()).isEqualTo(cause);
+    }
+
+    @Test
+    public void testPermanent_isNotTransitory() throws Exception {
+        AgentException exception = AgentException.permanent();
+
+        assertThat(exception.isTransitory()).isFalse();
+    }
+
+    @Test
+    public void testPermanent_withCause() throws Exception {
+        Exception cause = new IOException();
+
+        AgentException exception = AgentException.permanent(cause);
+
+        assertThat(exception.isTransitory()).isFalse();
+        assertThat(exception.getCause()).isEqualTo(cause);
+    }
+}

services/robotests/src/com/android/server/backup/keyvalue/BackupExceptionTest.java

@@ -0,0 +1,45 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.server.backup.keyvalue;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import android.platform.test.annotations.Presubmit;
+
+import com.android.server.testing.FrameworkRobolectricTestRunner;
+import com.android.server.testing.SystemLoaderPackages;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+
+import java.io.IOException;
+
+@RunWith(FrameworkRobolectricTestRunner.class)
+@Config(manifest = Config.NONE, sdk = 26)
+@SystemLoaderPackages({"com.android.server.backup"})
+@Presubmit
+public class BackupExceptionTest {
+    @Test
+    public void testConstructor_passesCause() {
+        Exception cause = new IOException();
+
+        Exception exception = new BackupException(cause);
+
+        assertThat(exception.getCause()).isEqualTo(cause);
+    }
+}

services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java

@@ -155,9 +155,7 @@ import java.util.List;
 import java.util.concurrent.TimeoutException;
 import java.util.stream.Stream;
 
-// TODO: When returning to RUNNING_QUEUE vs FINAL, RUNNING_QUEUE sets status = OK. Why? Verify?
+// TODO: Test agents timing out
-// TODO: Check queue in general, behavior w/ multiple packages
-// TODO: Test PM invocation
 @RunWith(FrameworkRobolectricTestRunner.class)
 @Config(
         manifest = Config.NONE,
@@ -369,6 +367,47 @@ public class KeyValueBackupTaskTest {
         assertThat(mWakeLock.isHeld()).isFalse();
     }
 
+    @Test
+    public void testRunTask_whenOnePackage_cleansUpPmFiles() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        assertCleansUpFiles(mTransport, PM_PACKAGE);
+    }
+
+    @Test
+    public void testRunTask_whenTransportReturnsTransportErrorForPm_cleansUpPmFiles()
+            throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        when(transportMock.transport.performBackup(
+                        argThat(packageInfo(PM_PACKAGE)), any(), anyInt()))
+                .thenReturn(BackupTransport.TRANSPORT_PACKAGE_REJECTED);
+        setUpAgent(PACKAGE_1);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        assertCleansUpFiles(mTransport, PM_PACKAGE);
+    }
+
+    @Test
+    public void testRunTask_whenTransportReturnsTransportErrorForPm_resetsBackupState()
+            throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        when(transportMock.transport.performBackup(
+                        argThat(packageInfo(PM_PACKAGE)), any(), anyInt()))
+                .thenReturn(BackupTransport.TRANSPORT_PACKAGE_REJECTED);
+        setUpAgent(PACKAGE_1);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        verify(mBackupManagerService).resetBackupState(getStateDirectory(mTransport).toFile());
+    }
+
     @Test
     public void testRunTask_whenOnePackage_updatesBookkeeping() throws Exception {
         // Transport has to be initialized to not reset current token
@@ -418,7 +457,7 @@ public class KeyValueBackupTaskTest {
     public void testRunTask_whenNonPmPackageAndNonIncremental_doesNotBackUpPm() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
         setUpAgentWithData(PACKAGE_1);
-        PackageManagerBackupAgent pmAgent = spy(createPmAgent());
+        BackupAgent pmAgent = spy(createPmAgent());
         when(mBackupManagerService.makeMetadataAgent()).thenReturn(forward(pmAgent));
         KeyValueBackupTask task = createKeyValueBackupTask(transportMock, true, PACKAGE_1);
 
@@ -431,7 +470,7 @@ public class KeyValueBackupTaskTest {
     public void testRunTask_whenNonPmPackageAndPmAndNonIncremental_backsUpPm() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
         setUpAgentWithData(PACKAGE_1);
-        PackageManagerBackupAgent pmAgent = spy(createPmAgent());
+        BackupAgent pmAgent = spy(createPmAgent());
         when(mBackupManagerService.makeMetadataAgent()).thenReturn(forward(pmAgent));
         KeyValueBackupTask task =
                 createKeyValueBackupTask(transportMock, true, PACKAGE_1, PM_PACKAGE);
@@ -445,7 +484,7 @@ public class KeyValueBackupTaskTest {
     public void testRunTask_whenNonPmPackageAndIncremental_backsUpPm() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
         setUpAgentWithData(PACKAGE_1);
-        PackageManagerBackupAgent pmAgent = spy(createPmAgent());
+        BackupAgent pmAgent = spy(createPmAgent());
         when(mBackupManagerService.makeMetadataAgent()).thenReturn(forward(pmAgent));
         KeyValueBackupTask task = createKeyValueBackupTask(transportMock, false, PACKAGE_1);
 
@@ -528,6 +567,35 @@ public class KeyValueBackupTaskTest {
         assertBackupPendingFor(PACKAGE_1);
     }
 
+    @Test
+    public void testRunTask_whenPackageUnknown() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        // Not calling setUpAgent() for PACKAGE_1
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        verify(transportMock.transport, never())
+                .performBackup(argThat(packageInfo(PACKAGE_1)), any(), anyInt());
+        verify(mObserver).onResult(PACKAGE_1.packageName, ERROR_PACKAGE_NOT_FOUND);
+        verify(mObserver).backupFinished(SUCCESS);
+        assertBackupNotPendingFor(PACKAGE_1);
+    }
+
+    @Test
+    public void testRunTask_whenFirstPackageUnknown_callsTransportForSecondPackage()
+            throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        // Not calling setUpAgent() for PACKAGE_1
+        setUpAgentWithData(PACKAGE_2);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1, PACKAGE_2);
+
+        runTask(task);
+
+        verify(transportMock.transport)
+                .performBackup(argThat(packageInfo(PACKAGE_2)), any(), anyInt());
+    }
+
     @Test
     public void testRunTask_whenPackageNotEligibleForBackup() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
@@ -544,6 +612,19 @@ public class KeyValueBackupTaskTest {
         assertBackupNotPendingFor(PACKAGE_1);
     }
 
+    @Test
+    public void testRunTask_whenFirstPackageNotEligibleForBackup_callsTransportForSecondPackage()
+            throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgentsWithData(PACKAGE_1.backupNotAllowed(), PACKAGE_2);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1, PACKAGE_2);
+
+        runTask(task);
+
+        verify(transportMock.transport)
+                .performBackup(argThat(packageInfo(PACKAGE_2)), any(), anyInt());
+    }
+
     @Test
     public void testRunTask_whenPackageDoesFullBackup() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
@@ -560,6 +641,20 @@ public class KeyValueBackupTaskTest {
         assertBackupNotPendingFor(PACKAGE_1);
     }
 
+    @Test
+    public void testRunTask_whenFirstPackageDoesFullBackup_callsTransportForSecondPackage()
+            throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        PackageData packageData = fullBackupPackage(1);
+        setUpAgentsWithData(packageData, PACKAGE_2);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, packageData, PACKAGE_2);
+
+        runTask(task);
+
+        verify(transportMock.transport)
+                .performBackup(argThat(packageInfo(PACKAGE_2)), any(), anyInt());
+    }
+
     @Test
     public void testRunTask_whenPackageIsStopped() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
@@ -575,18 +670,16 @@ public class KeyValueBackupTaskTest {
     }
 
     @Test
-    public void testRunTask_whenPackageUnknown() throws Exception {
+    public void testRunTask_whenFirstPackageIsStopped_callsTransportForSecondPackage()
+            throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
-        // Not calling setUpAgent()
+        setUpAgentsWithData(PACKAGE_1.stopped(), PACKAGE_2);
-        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1, PACKAGE_2);
 
         runTask(task);
 
-        verify(transportMock.transport, never())
+        verify(transportMock.transport)
-                .performBackup(argThat(packageInfo(PACKAGE_1)), any(), anyInt());
+                .performBackup(argThat(packageInfo(PACKAGE_2)), any(), anyInt());
-        verify(mObserver).onResult(PACKAGE_1.packageName, ERROR_PACKAGE_NOT_FOUND);
-        verify(mObserver).backupFinished(SUCCESS);
-        assertBackupNotPendingFor(PACKAGE_1);
     }
 
     @Test
@@ -629,6 +722,7 @@ public class KeyValueBackupTaskTest {
         verify(mBackupManagerService).setWorkSource(null);
         verify(mObserver).onResult(PACKAGE_1.packageName, ERROR_AGENT_FAILURE);
         verify(mObserver).backupFinished(BackupManager.SUCCESS);
+        assertBackupPendingFor(PACKAGE_1);
     }
 
     @Test
@@ -645,6 +739,7 @@ public class KeyValueBackupTaskTest {
         verify(mBackupManagerService).setWorkSource(null);
         verify(mObserver).onResult(PACKAGE_1.packageName, ERROR_AGENT_FAILURE);
         verify(mObserver).backupFinished(BackupManager.SUCCESS);
+        assertBackupPendingFor(PACKAGE_1);
     }
 
     @Test
@@ -798,7 +893,7 @@ public class KeyValueBackupTaskTest {
 
         runTask(task);
 
-        assertBackupNotPendingFor(PACKAGE_1);
+        assertBackupPendingFor(PACKAGE_1);
     }
 
     @Test
@@ -1139,6 +1234,38 @@ public class KeyValueBackupTaskTest {
         }
     }
 
+    @Test
+    public void testRunTask_whenPmAgentWritesData_callsTransportPerformBackupWithAgentData()
+            throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        Path backupDataPath = createTemporaryFile();
+        when(transportMock.transport.performBackup(
+                        argThat(packageInfo(PM_PACKAGE)), any(), anyInt()))
+                .then(copyBackupDataTo(backupDataPath));
+        BackupAgent pmAgent = spy(createPmAgent());
+        when(mBackupManagerService.makeMetadataAgent()).thenReturn(forward(pmAgent));
+        agentOnBackupDo(
+                pmAgent,
+                (oldState, dataOutput, newState) -> {
+                    writeData(dataOutput, "key1", "data1".getBytes());
+                    writeData(dataOutput, "key2", "data2".getBytes());
+                    writeState(newState, "newState".getBytes());
+                });
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        verify(transportMock.transport)
+                .performBackup(argThat(packageInfo(PM_PACKAGE)), any(), anyInt());
+        try (FileInputStream inputStream = new FileInputStream(backupDataPath.toFile())) {
+            BackupDataInput backupData = new BackupDataInput(inputStream.getFD());
+            assertDataHasKeyValue(backupData, "key1", "data1".getBytes());
+            assertDataHasKeyValue(backupData, "key2", "data2".getBytes());
+            assertThat(backupData.readNextHeader()).isFalse();
+        }
+    }
+
     @Test
     public void testRunTask_whenPerformBackupSucceeds_callsTransportFinishBackup()
             throws Exception {
@@ -1175,6 +1302,50 @@ public class KeyValueBackupTaskTest {
         assertCleansUpFilesAndAgent(mTransport, PACKAGE_1);
     }
 
+    @Test
+    public void testRunTask_whenFinishBackupSucceedsForPm_cleansUp() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        when(transportMock.transport.finishBackup()).thenReturn(BackupTransport.TRANSPORT_OK);
+        BackupAgent pmAgent = spy(createPmAgent());
+        when(mBackupManagerService.makeMetadataAgent()).thenReturn(forward(pmAgent));
+        agentOnBackupDo(
+                pmAgent,
+                (oldState, dataOutput, newState) -> {
+                    writeData(dataOutput, "key", "data".getBytes());
+                    writeState(newState, "newState".getBytes());
+                });
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        assertThat(Files.readAllBytes(getStateFile(mTransport, PM_PACKAGE)))
+                .isEqualTo("newState".getBytes());
+        assertCleansUpFiles(mTransport, PM_PACKAGE);
+        // We don't unbind PM
+        verify(mBackupManagerService, never()).unbindAgent(argThat(applicationInfo(PM_PACKAGE)));
+    }
+
+    @Test
+    public void testRunTask_whenFinishBackupSucceedsForPm_doesNotUnbindPm() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        when(transportMock.transport.finishBackup()).thenReturn(BackupTransport.TRANSPORT_OK);
+        BackupAgent pmAgent = spy(createPmAgent());
+        when(mBackupManagerService.makeMetadataAgent()).thenReturn(forward(pmAgent));
+        agentOnBackupDo(
+                pmAgent,
+                (oldState, dataOutput, newState) -> {
+                    writeData(dataOutput, "key", "data".getBytes());
+                    writeState(newState, "newState".getBytes());
+                });
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        verify(mBackupManagerService, never()).unbindAgent(argThat(applicationInfo(PM_PACKAGE)));
+    }
+
     @Test
     public void testRunTask_whenFinishBackupSucceeds_logsBackupPackageEvent() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
@@ -1354,6 +1525,7 @@ public class KeyValueBackupTaskTest {
     public void testRunTask_whenTransportReturnsQuotaExceeded_updatesBookkeeping()
             throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgentWithData(PACKAGE_1);
         when(transportMock.transport.performBackup(
                         argThat(packageInfo(PACKAGE_1)), any(), anyInt()))
                 .thenReturn(BackupTransport.TRANSPORT_QUOTA_EXCEEDED);
@@ -1701,9 +1873,9 @@ public class KeyValueBackupTaskTest {
     }
 
     @Test
-    public void testRunTask_whenPmAgentFails() throws Exception {
+    public void testRunTask_whenPmAgentFails_reportsCorrectly() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
-        PackageManagerBackupAgent pmAgent = createThrowingPmAgent(new RuntimeException());
+        BackupAgent pmAgent = createThrowingPmAgent(new RuntimeException());
         when(mBackupManagerService.makeMetadataAgent()).thenReturn(pmAgent);
         KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
 
@@ -1717,6 +1889,75 @@ public class KeyValueBackupTaskTest {
                 new RuntimeException().toString());
     }
 
+    @Test
+    public void testRunTask_whenPmAgentFails_revertsTask() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        BackupAgent pmAgent = createThrowingPmAgent(new RuntimeException());
+        when(mBackupManagerService.makeMetadataAgent()).thenReturn(pmAgent);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        assertTaskReverted(transportMock, PACKAGE_1);
+    }
+
+    @Test
+    public void testRunTask_whenPmAgentFails_cleansUpFiles() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        BackupAgent pmAgent = createThrowingPmAgent(new RuntimeException());
+        when(mBackupManagerService.makeMetadataAgent()).thenReturn(pmAgent);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        assertCleansUpFiles(mTransport, PM_PACKAGE);
+    }
+
+    @Test
+    public void testRunTask_whenPmAgentFails_resetsBackupState() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        BackupAgent pmAgent = createThrowingPmAgent(new RuntimeException());
+        when(mBackupManagerService.makeMetadataAgent()).thenReturn(pmAgent);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+
+        runTask(task);
+
+        verify(mBackupManagerService).resetBackupState(getStateDirectory(mTransport).toFile());
+    }
+
+    @Test
+    public void testRunTask_whenMarkCancelDuringPmOnBackup_resetsBackupState() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        BackupAgent pmAgent = spy(createPmAgent());
+        when(mBackupManagerService.makeMetadataAgent()).thenReturn(forward(pmAgent));
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+        agentOnBackupDo(
+                pmAgent, (oldState, dataOutput, newState) -> runInWorkerThread(task::markCancel));
+
+        runTask(task);
+
+        verify(mBackupManagerService).resetBackupState(getStateDirectory(mTransport).toFile());
+    }
+
+    @Test
+    public void testRunTask_whenMarkCancelDuringPmOnBackup_cleansUpFiles() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        setUpAgent(PACKAGE_1);
+        BackupAgent pmAgent = spy(createPmAgent());
+        when(mBackupManagerService.makeMetadataAgent()).thenReturn(forward(pmAgent));
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+        agentOnBackupDo(
+                pmAgent, (oldState, dataOutput, newState) -> runInWorkerThread(task::markCancel));
+
+        runTask(task);
+
+        assertCleansUpFiles(mTransport, PM_PACKAGE);
+    }
+
     @Test
     public void testRunTask_whenBackupRunning_doesNotThrow() throws Exception {
         TransportMock transportMock = setUpInitializedTransport(mTransport);
@@ -1736,7 +1977,7 @@ public class KeyValueBackupTaskTest {
 
         runTask(task);
 
-        verify(mReporter).onReadAgentDataError(eq(PACKAGE_1.packageName), any());
+        verify(mReporter).onAgentDataError(eq(PACKAGE_1.packageName), any());
     }
 
     @Test
@@ -1778,6 +2019,24 @@ public class KeyValueBackupTaskTest {
         assertTaskReverted(transportMock, PACKAGE_1, PACKAGE_2);
     }
 
+    @Test
+    public void testRunTask_whenMarkCancelDuringAgentOnBackup_cleansUpFiles() throws Exception {
+        TransportMock transportMock = setUpInitializedTransport(mTransport);
+        AgentMock agentMock = setUpAgent(PACKAGE_1);
+        KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1);
+        agentOnBackupDo(
+                agentMock,
+                (oldState, dataOutput, newState) -> {
+                    writeData(dataOutput, "key", "data".getBytes());
+                    writeState(newState, "newState".getBytes());
+                    runInWorkerThread(task::markCancel);
+                });
+
+        runTask(task);
+
+        assertCleansUpFiles(mTransport, PACKAGE_1);
+    }
+
     @Test
     public void
             testRunTask_whenMarkCancelDuringFirstAgentOnBackup_doesNotCallTransportAfterWaitCancel()
@@ -2293,20 +2552,28 @@ public class KeyValueBackupTaskTest {
      */
     private static void agentOnBackupDo(AgentMock agentMock, BackupAgentOnBackup function)
             throws Exception {
-        doAnswer(
+        agentOnBackupDo(
-                        (BackupAgentOnBackup)
+                agentMock.agent,
-                                (oldState, dataOutput, newState) -> {
+                (oldState, dataOutput, newState) -> {
-                                    ByteArrayOutputStream outputStream =
+                    ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
-                                            new ByteArrayOutputStream();
+                    transferStreamedData(
-                                    transferStreamedData(
+                            new FileInputStream(oldState.getFileDescriptor()), outputStream);
-                                            new FileInputStream(oldState.getFileDescriptor()),
+                    agentMock.oldState = outputStream.toByteArray();
-                                            outputStream);
+                    agentMock.oldStateHistory.add(agentMock.oldState);
-                                    agentMock.oldState = outputStream.toByteArray();
+                    function.onBackup(oldState, dataOutput, newState);
-                                    agentMock.oldStateHistory.add(agentMock.oldState);
+                });
-                                    function.onBackup(oldState, dataOutput, newState);
+    }
-                                })
+
-                .when(agentMock.agent)
+    /**
-                .onBackup(any(), any(), any());
+     * Implements {@code function} for {@link BackupAgent#onBackup(ParcelFileDescriptor,
+     * BackupDataOutput, ParcelFileDescriptor)} of {@code agentMock}.
+     *
+     * @see #agentOnBackupDo(AgentMock, BackupAgentOnBackup)
+     * @see #remoteAgentOnBackupThrows(AgentMock, BackupAgentOnBackup)
+     */
+    private static void agentOnBackupDo(BackupAgent backupAgent, BackupAgentOnBackup function)
+            throws IOException {
+        doAnswer(function).when(backupAgent).onBackup(any(), any(), any());
     }
 
     /**
@@ -2400,6 +2667,10 @@ public class KeyValueBackupTaskTest {
             // constructor
             assertJournalDoesNotContain(mBackupManagerService.getJournal(), packageName);
             assertThat(mBackupManagerService.getPendingBackups()).doesNotContainKey(packageName);
+            // Also verifying BMS is never called since for some cases the package wouldn't be
+            // pending for other reasons (for example it's not eligible for backup). Regardless of
+            // these reasons, we shouldn't mark them as pending backup (call dataChangedImpl()).
+            verify(mBackupManagerService, never()).dataChangedImpl(packageName);
         }
     }
 

services/robotests/src/com/android/server/backup/keyvalue/TaskExceptionTest.java

@@ -0,0 +1,152 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.server.backup.keyvalue;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.testng.Assert.expectThrows;
+
+import android.app.backup.BackupTransport;
+import android.platform.test.annotations.Presubmit;
+
+import com.android.server.testing.FrameworkRobolectricTestRunner;
+import com.android.server.testing.SystemLoaderPackages;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.annotation.Config;
+
+import java.io.IOException;
+
+@RunWith(FrameworkRobolectricTestRunner.class)
+@Config(manifest = Config.NONE, sdk = 26)
+@SystemLoaderPackages({"com.android.server.backup"})
+@Presubmit
+public class TaskExceptionTest {
+    @Test
+    public void testStateCompromised() {
+        TaskException exception = TaskException.stateCompromised();
+
+        assertThat(exception.isStateCompromised()).isTrue();
+        assertThat(exception.getStatus()).isEqualTo(BackupTransport.TRANSPORT_ERROR);
+    }
+
+    @Test
+    public void testStateCompromised_whenCauseInstanceOfTaskException() {
+        Exception cause = TaskException.forStatus(BackupTransport.TRANSPORT_NOT_INITIALIZED);
+
+        TaskException exception = TaskException.stateCompromised(cause);
+
+        assertThat(exception.isStateCompromised()).isTrue();
+        assertThat(exception.getStatus()).isEqualTo(BackupTransport.TRANSPORT_NOT_INITIALIZED);
+        assertThat(exception.getCause()).isEqualTo(cause);
+    }
+
+    @Test
+    public void testStateCompromised_whenCauseNotInstanceOfTaskException() {
+        Exception cause = new IOException();
+
+        TaskException exception = TaskException.stateCompromised(cause);
+
+        assertThat(exception.isStateCompromised()).isTrue();
+        assertThat(exception.getStatus()).isEqualTo(BackupTransport.TRANSPORT_ERROR);
+        assertThat(exception.getCause()).isEqualTo(cause);
+    }
+
+    @Test
+    public void testForStatus_whenTransportOk_throws() {
+        expectThrows(
+                IllegalArgumentException.class,
+                () -> TaskException.forStatus(BackupTransport.TRANSPORT_OK));
+    }
+
+    @Test
+    public void testForStatus_whenTransportNotInitialized() {
+        TaskException exception =
+                TaskException.forStatus(BackupTransport.TRANSPORT_NOT_INITIALIZED);
+
+        assertThat(exception.isStateCompromised()).isFalse();
+        assertThat(exception.getStatus()).isEqualTo(BackupTransport.TRANSPORT_NOT_INITIALIZED);
+    }
+
+    @Test
+    public void testCausedBy_whenCauseInstanceOfTaskException_returnsCause() {
+        Exception cause = TaskException.forStatus(BackupTransport.TRANSPORT_NOT_INITIALIZED);
+
+        TaskException exception = TaskException.causedBy(cause);
+
+        assertThat(exception).isEqualTo(cause);
+    }
+
+    @Test
+    public void testCausedBy_whenCauseNotInstanceOfTaskException() {
+        Exception cause = new IOException();
+
+        TaskException exception = TaskException.causedBy(cause);
+
+        assertThat(exception).isNotEqualTo(cause);
+        assertThat(exception.isStateCompromised()).isFalse();
+        assertThat(exception.getStatus()).isEqualTo(BackupTransport.TRANSPORT_ERROR);
+        assertThat(exception.getCause()).isEqualTo(cause);
+    }
+
+    @Test
+    public void testCreate() {
+        TaskException exception = TaskException.create();
+
+        assertThat(exception.isStateCompromised()).isFalse();
+        assertThat(exception.getStatus()).isEqualTo(BackupTransport.TRANSPORT_ERROR);
+    }
+
+    @Test
+    public void testIsStateCompromised_whenStateCompromised_returnsTrue() {
+        TaskException taskException = TaskException.stateCompromised();
+
+        boolean stateCompromised = taskException.isStateCompromised();
+
+        assertThat(stateCompromised).isTrue();
+    }
+
+    @Test
+    public void testIsStateCompromised_whenCreatedWithCreate_returnsFalse() {
+        TaskException taskException = TaskException.create();
+
+        boolean stateCompromised = taskException.isStateCompromised();
+
+        assertThat(stateCompromised).isFalse();
+    }
+
+    @Test
+    public void testGetStatus_whenStatusIsTransportPackageRejected() {
+        TaskException taskException =
+                TaskException.forStatus(BackupTransport.TRANSPORT_PACKAGE_REJECTED);
+
+        int status = taskException.getStatus();
+
+        assertThat(status).isEqualTo(BackupTransport.TRANSPORT_PACKAGE_REJECTED);
+    }
+
+    @Test
+    public void testGetStatus_whenStatusIsTransportNotInitialized() {
+        TaskException taskException =
+                TaskException.forStatus(BackupTransport.TRANSPORT_NOT_INITIALIZED);
+
+        int status = taskException.getStatus();
+
+        assertThat(status).isEqualTo(BackupTransport.TRANSPORT_NOT_INITIALIZED);
+    }
+}