From 7fcc730e8d5fc00d0945783e6a649997432b9c1c Mon Sep 17 00:00:00 2001
From: JW Wang <wangchun@google.com>
Date: Wed, 12 Feb 2020 11:30:18 +0800
Subject: [PATCH] Always enable apk-in-apex before the apex (2/n)

To keep a rollback object in a consistent state, an apex shouldn't be
enabled (for rollback) until all its embedded apk-in-apex are enabled
successfully.

Bug: 149352598
Test: test RollbackTest StagedRollbackTest
Change-Id: I4d0208118050f948675093f4ed67de8ae148f41b
---
 .../rollback/RollbackManagerServiceImpl.java  | 28 +++++++++++--------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
index 47a1b0e0eca00..9e150fd6a8b3a 100644
--- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
+++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java
@@ -869,14 +869,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
             return false;
         }
 
-        ApplicationInfo appInfo = pkgInfo.applicationInfo;
-        boolean success = rollback.enableForPackage(packageName, newPackage.versionCode,
-                pkgInfo.getLongVersionCode(), isApex, appInfo.sourceDir,
-                appInfo.splitSourceDirs, session.rollbackDataPolicy);
-        if (!success) {
-            return success;
-        }
-
         if (isApex) {
             // Check if this apex contains apks inside it. If true, then they should be added as
             // a RollbackPackageInfo into this rollback
@@ -894,12 +886,24 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub {
                     Slog.e(TAG, apkInApex + " is not installed");
                     return false;
                 }
-                success = rollback.enableForPackageInApex(
-                        apkInApex, apkPkgInfo.getLongVersionCode(), session.rollbackDataPolicy);
-                if (!success) return success;
+                if (!rollback.enableForPackageInApex(
+                        apkInApex, apkPkgInfo.getLongVersionCode(), session.rollbackDataPolicy)) {
+                    return false;
+                }
             }
         }
-        return true;
+
+        /**
+         * The order is important here! Always enable the embedded apk-in-apex (if any) before
+         * enabling the embedding apex. Otherwise the rollback object might be in an inconsistent
+         * state where an embedding apex is successfully enabled while one of its embedded
+         * apk-in-apex failed. Note {@link Rollback#allPackagesEnabled()} won't behave correctly if
+         * a rollback object is inconsistent because it doesn't count apk-in-apex.
+         */
+        ApplicationInfo appInfo = pkgInfo.applicationInfo;
+        return rollback.enableForPackage(packageName, newPackage.versionCode,
+                pkgInfo.getLongVersionCode(), isApex, appInfo.sourceDir,
+                appInfo.splitSourceDirs, session.rollbackDataPolicy);
     }
 
     @Override