From b18925badcb1a2f93fb981c25321dc7726144664 Mon Sep 17 00:00:00 2001
From: JW Wang <wangchun@google.com>
Date: Tue, 11 Feb 2020 16:20:06 +0800
Subject: [PATCH] Tweak the lifecycle of rollbacks (2/n)

1. #createNewRollbackLocked now adds newly created rollbacks to
   mRollbacks
2. #completeEnableRollback no longer adds the rollback object to
   mRollbacks.
3. #completeEnableRollback delete and remove rollbacks from
   mRollbacks should any error happen.

Bug: 149258640
Test: atest RollbackTest StagedRollbackTest
Change-Id: Ie6668cf6bd00ce9b83096383776bc75198c4f194
---
 .../rollback/RollbackManagerServiceImpl.java  | 45 +++++++++----------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
index 950aae8e42aba..f9981d0d9c968 100644
--- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
+++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
@@ -802,7 +802,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
             newRollback = getRollbackForSessionLocked(packageSession.getSessionId());
             if (newRollback == null) {
                 newRollback = createNewRollbackLocked(parentSession);
-                mRollbacks.add(newRollback);
             }
         }
         newRollback.addToken(token);
@@ -1157,16 +1156,10 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
                 Rollback rollback;
                 synchronized (mLock) {
                     rollback = getRollbackForSessionLocked(sessionId);
-                    if (rollback == null || rollback.isStaged() || !rollback.isEnabling()
-                            || !rollback.notifySessionWithSuccess()) {
-                        return;
-                    }
-                    // All child sessions finished with success. We can enable this rollback now.
-                    // TODO: refactor #completeEnableRollback so we won't remove 'rollback' from
-                    // mRollbacks here and add it back in #completeEnableRollback later.
-                    mRollbacks.remove(rollback);
                 }
-                if (completeEnableRollback(rollback)) {
+                if (rollback != null && !rollback.isStaged() && rollback.isEnabling()
+                        && rollback.notifySessionWithSuccess()
+                        && completeEnableRollback(rollback)) {
                     makeRollbackAvailable(rollback);
                 }
             } else {
@@ -1184,7 +1177,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
     }
 
     /**
-     * Add a rollback to the list of rollbacks. It does not make the rollback available yet.
+     * Persist a rollback as enable-completed. It does not make the rollback available yet.
+     * This rollback will be deleted and removed from {@link #mRollbacks} should any error happens.
      *
      * @return {code true} if {code rollback} is successfully enable-completed,
      * or {code false} otherwise.
@@ -1201,24 +1195,22 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
         // rollback for the embedded apk-in-apex, if any.
         if (!rollback.allPackagesEnabled()) {
             Slog.e(TAG, "Failed to enable rollback for all packages in session.");
+            mRollbacks.remove(rollback);
             rollback.delete(mAppDataRollbackHelper);
             return false;
         }
 
+        // Note: There is a small window of time between when
+        // the session has been committed by the package
+        // manager and when we make the rollback available
+        // here. Presumably the window is small enough that
+        // nobody will want to roll back the newly installed
+        // package before we make the rollback available.
+        // TODO: We'll lose the rollback if the
+        // device reboots between when the session is
+        // committed and this point. Revisit this after
+        // adding support for rollback of staged installs.
         rollback.saveRollback();
-        synchronized (mLock) {
-            // Note: There is a small window of time between when
-            // the session has been committed by the package
-            // manager and when we make the rollback available
-            // here. Presumably the window is small enough that
-            // nobody will want to roll back the newly installed
-            // package before we make the rollback available.
-            // TODO: We'll lose the rollback if the
-            // device reboots between when the session is
-            // committed and this point. Revisit this after
-            // adding support for rollback of staged installs.
-            mRollbacks.add(rollback);
-        }
 
         return true;
     }
@@ -1300,6 +1292,10 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
         }
     }
 
+    /**
+     * Creates and returns a Rollback according to the given SessionInfo
+     * and adds it to {@link #mRollbacks}.
+     */
     @WorkerThread
     @GuardedBy("mLock")
     private Rollback createNewRollbackLocked(PackageInstaller.SessionInfo parentSession) {
@@ -1334,6 +1330,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
                     installerPackageName, packageSessionIds);
         }
 
+        mRollbacks.add(rollback);
         return rollback;
     }