From a176d22110a1670f363ba0f745f127d2b6ca2350 Mon Sep 17 00:00:00 2001
From: Christopher Tate <ctate@google.com>
Date: Wed, 16 Jul 2014 12:15:07 -0700
Subject: [PATCH] Always call finishBackup() if performFullBackup() succeeded

Even if we later get an error from sendBackupData() we need to give
the transport its teardown callback.  This simplifies the transport
logic considerably.

Change-Id: Ib8c0e210d4a876ee6b083a4d619dfccc462da4e5
---
 .../android/app/backup/BackupTransport.java   |  4 +++
 .../server/backup/BackupManagerService.java   | 25 +++++++++++++------
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/core/java/android/app/backup/BackupTransport.java b/core/java/android/app/backup/BackupTransport.java
index ba2930b386fae..28108a001566a 100644
--- a/core/java/android/app/backup/BackupTransport.java
+++ b/core/java/android/app/backup/BackupTransport.java
@@ -331,6 +331,10 @@ public class BackupTransport {
      * its datastore, if appropriate, and close the socket that had been provided in
      * {@link #performFullBackup(PackageInfo, ParcelFileDescriptor)}.
      *
+     * <p class="note">If the transport returns TRANSPORT_OK from this method, then the
+     * OS will always provide a matching call to {@link #finishBackup()} even if sending
+     * data via {@link #sendBackupData(int)} failed at some point.
+     *
      * @param targetPackage The package whose data is to follow.
      * @param socket The socket file descriptor through which the data will be provided.
      *    If the transport returns {@link #TRANSPORT_PACKAGE_REJECTED} here, it must still
diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java
index c3a9dbe943317..e8e28139bd698 100644
--- a/services/backup/java/com/android/server/backup/BackupManagerService.java
+++ b/services/backup/java/com/android/server/backup/BackupManagerService.java
@@ -3535,19 +3535,30 @@ public class BackupManagerService extends IBackupManager.Stub {
                                 }
                             } while (nRead > 0 && result == BackupTransport.TRANSPORT_OK);
 
-                            // Done -- how did it turn out?
-                            if (result == BackupTransport.TRANSPORT_OK){
-                                result = transport.finishBackup();
-                            } else {
-                                Slog.w(TAG, "Error backing up " + target.packageName);
+                            // In all cases we need to give the transport its finish callback
+                            int finishResult = transport.finishBackup();
+
+                            // If we were otherwise in a good state, now interpret the final
+                            // result based on what finishBackup() returned.  If we're in a
+                            // failure case already, preserve that result and ignore whatever
+                            // finishBackup() reported.
+                            if (result == BackupTransport.TRANSPORT_OK) {
+                                result = finishResult;
                             }
-                        } else if (result == BackupTransport.TRANSPORT_PACKAGE_REJECTED) {
+
+                            if (result != BackupTransport.TRANSPORT_OK) {
+                                Slog.e(TAG, "Error " + result
+                                        + " backing up " + target.packageName);
+                            }
+                        }
+
+                        if (result == BackupTransport.TRANSPORT_PACKAGE_REJECTED) {
                             if (DEBUG) {
                                 Slog.i(TAG, "Transport rejected backup of " + target.packageName
                                         + ", skipping");
                             }
                             // do nothing, clean up, and continue looping
-                        } else {
+                        } else if (result != BackupTransport.TRANSPORT_OK) {
                             if (DEBUG) {
                                 Slog.i(TAG, "Transport failed; aborting backup");
                                 return;