From c53d34b4bae01d849735edf32a7fc1022aef162c Mon Sep 17 00:00:00 2001
From: JW Wang <wangchun@google.com>
Date: Mon, 3 Jan 2022 11:03:19 +0800
Subject: [PATCH] Fix handleStreamValidateAndCommit() (5/n)

1. Don't retain finished sessions if isCommitted() is false.
   We don't keep finished sessions if they fail validation to preserve
   the original behavior.
2. Simplify error handling of handleStreamValidateAndCommit()

Bug: 210359798
Test: atest CtsStagedInstallHostTestCases
Change-Id: I713b6b1e88c8978d58c912c8c9fc8b49e4673c82
---
 .../server/pm/PackageInstallerService.java    | 13 +++-
 .../server/pm/PackageInstallerSession.java    | 69 ++++---------------
 2 files changed, 25 insertions(+), 57 deletions(-)

diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java
index b3bb26c098f2f..1f10d77086d3a 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerService.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerService.java
@@ -1661,9 +1661,16 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
                     }
                     synchronized (mSessions) {
                         // Child sessions will be removed along with its parent as a whole
-                        if (!session.hasParentSessionId()
-                                && (!session.isStaged() || session.isDestroyed())) {
-                            removeActiveSession(session);
+                        if (!session.hasParentSessionId()) {
+                            // Retain policy:
+                            // 1. Don't keep non-staged sessions
+                            // 2. Don't keep explicitly abandoned sessions
+                            // 3. Don't keep sessions that fail validation (isCommitted() is false)
+                            boolean shouldRemove = !session.isStaged() || session.isDestroyed()
+                                    || !session.isCommitted();
+                            if (shouldRemove) {
+                                removeActiveSession(session);
+                            }
                         }
 
                         final File appIconFile = buildAppIconFile(session.sessionId);
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index c8133174b6db8..01c7b70cdcfd4 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -1734,61 +1734,22 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
 
     @WorkerThread
     private void handleStreamValidateAndCommit() {
-        PackageManagerException unrecoverableFailure = null;
-        // This will track whether the session and any children were validated and are ready to
-        // progress to the next phase of install
-        boolean allSessionsReady = false;
         try {
-            allSessionsReady = streamValidateAndCommit();
+            // This will track whether the session and any children were validated and are ready to
+            // progress to the next phase of install
+            boolean allSessionsReady = true;
+            for (PackageInstallerSession child : getChildSessions()) {
+                allSessionsReady &= child.streamValidateAndCommit();
+            }
+            if (allSessionsReady && streamValidateAndCommit()) {
+                mHandler.obtainMessage(MSG_INSTALL).sendToTarget();
+            }
         } catch (PackageManagerException e) {
-            unrecoverableFailure = e;
+            destroy();
+            String msg = ExceptionUtils.getCompleteMessage(e);
+            dispatchSessionFinished(e.error, msg, null);
+            maybeFinishChildSessions(e.error, msg);
         }
-
-        if (isMultiPackage()) {
-            final List<PackageInstallerSession> childSessions;
-            synchronized (mLock) {
-                childSessions = getChildSessionsLocked();
-            }
-            int childCount = childSessions.size();
-
-            // This will contain all child sessions that do not encounter an unrecoverable failure
-            ArrayList<PackageInstallerSession> nonFailingSessions = new ArrayList<>(childCount);
-
-            for (int i = childCount - 1; i >= 0; --i) {
-                // commit all children, regardless if any of them fail; we'll throw/return
-                // as appropriate once all children have been processed
-                try {
-                    PackageInstallerSession session = childSessions.get(i);
-                    allSessionsReady &= session.streamValidateAndCommit();
-                    nonFailingSessions.add(session);
-                } catch (PackageManagerException e) {
-                    allSessionsReady = false;
-                    if (unrecoverableFailure == null) {
-                        unrecoverableFailure = e;
-                    }
-                }
-            }
-            // If we encountered any unrecoverable failures, destroy all other sessions including
-            // the parent
-            if (unrecoverableFailure != null) {
-                // {@link #streamValidateAndCommit()} calls
-                // {@link #onSessionValidationFailure(PackageManagerException)}, but we don't
-                // expect it to ever do so for parent sessions. Call that on this parent to clean
-                // it up and notify listeners of the error.
-                onSessionValidationFailure(unrecoverableFailure);
-                // fail other child sessions that did not already fail
-                for (int i = nonFailingSessions.size() - 1; i >= 0; --i) {
-                    PackageInstallerSession session = nonFailingSessions.get(i);
-                    session.onSessionValidationFailure(unrecoverableFailure);
-                }
-            }
-        }
-
-        if (!allSessionsReady) {
-            return;
-        }
-
-        mHandler.obtainMessage(MSG_INSTALL).sendToTarget();
     }
 
     private final class FileSystemConnector extends
@@ -2026,11 +1987,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
             }
             return true;
         } catch (PackageManagerException e) {
-            throw onSessionValidationFailure(e);
+            throw e;
         } catch (Throwable e) {
             // Convert all exceptions into package manager exceptions as only those are handled
             // in the code above.
-            throw onSessionValidationFailure(new PackageManagerException(e));
+            throw new PackageManagerException(e);
         }
     }