Merge "Wait for sessions of dependencies to complete" into main

7 files changed, 186 insertions, 21 deletions

diff --git a/services/core/java/com/android/server/pm/InstallDependencyHelper.java b/services/core/java/com/android/server/pm/InstallDependencyHelper.java
index 527d68049537..65140bf971f8 100644
--- a/services/core/java/com/android/server/pm/InstallDependencyHelper.java
+++ b/services/core/java/com/android/server/pm/InstallDependencyHelper.java
@@ -23,6 +23,7 @@ import android.annotation.NonNull;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
+import android.content.pm.PackageInstaller.SessionInfo;
 import android.content.pm.ResolveInfo;
 import android.content.pm.SharedLibraryInfo;
 import android.content.pm.dependencyinstaller.DependencyInstallerCallback;
@@ -33,12 +34,14 @@ import android.os.Handler;
 import android.os.OutcomeReceiver;
 import android.os.Process;
 import android.os.RemoteException;
+import android.util.ArraySet;
 import android.util.Slog;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.infra.AndroidFuture;
 import com.android.internal.infra.ServiceConnector;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
@@ -54,16 +57,21 @@ public class InstallDependencyHelper {
     private static final long UNBIND_TIMEOUT_MILLIS = TimeUnit.HOURS.toMillis(6);
     private static final long REQUEST_TIMEOUT_MILLIS = TimeUnit.MINUTES.toMillis(1);
 
-    private final SharedLibrariesImpl mSharedLibraries;
     private final Context mContext;
+    private final SharedLibrariesImpl mSharedLibraries;
+    private final PackageInstallerService mPackageInstallerService;
     private final Object mRemoteServiceLock = new Object();
+    @GuardedBy("mTrackers")
+    private final List<DependencyInstallTracker> mTrackers = new ArrayList<>();
 
     @GuardedBy("mRemoteServiceLock")
     private ServiceConnector<IDependencyInstallerService> mRemoteService = null;
 
-    InstallDependencyHelper(Context context, SharedLibrariesImpl sharedLibraries) {
+    InstallDependencyHelper(Context context, SharedLibrariesImpl sharedLibraries,
+            PackageInstallerService packageInstallerService) {
         mContext = context;
         mSharedLibraries = sharedLibraries;
+        mPackageInstallerService = packageInstallerService;
     }
 
     void resolveLibraryDependenciesIfNeeded(PackageLite pkg, Computer snapshot, int userId,
@@ -98,19 +106,8 @@ public class InstallDependencyHelper {
             return;
         }
 
-        IDependencyInstallerCallback serviceCallback = new IDependencyInstallerCallback.Stub() {
-            @Override
-            public void onAllDependenciesResolved(int[] sessionIds) throws RemoteException {
-                // TODO(b/372862145): Implement waiting for sessions to finish installation
-                callback.onResult(null);
-            }
-
-            @Override
-            public void onFailureToResolveAllDependencies() throws RemoteException {
-                onError(callback, "Failed to resolve all dependencies automatically");
-            }
-        };
-
+        IDependencyInstallerCallback serviceCallback =
+                new DependencyInstallerCallbackCallOnce(handler, callback);
         boolean scheduleSuccess;
         synchronized (mRemoteServiceLock) {
             scheduleSuccess = mRemoteService.run(service -> {
@@ -123,10 +120,28 @@ public class InstallDependencyHelper {
         }
     }
 
-    private void onError(CallOnceProxy callback, String msg) {
+    void notifySessionComplete(int sessionId, boolean success) {
+        if (DEBUG) {
+            Slog.i(TAG, "Session complete for " + sessionId + " result: " + success);
+        }
+        synchronized (mTrackers) {
+            List<DependencyInstallTracker> completedTrackers = new ArrayList<>();
+            for (DependencyInstallTracker tracker: mTrackers) {
+                if (!tracker.onSessionComplete(sessionId, success)) {
+                    completedTrackers.add(tracker);
+                }
+            }
+            mTrackers.removeAll(completedTrackers);
+        }
+    }
+
+    private static void onError(CallOnceProxy callback, String msg) {
         PackageManagerException pe = new PackageManagerException(
                 INSTALL_FAILED_MISSING_SHARED_LIBRARY, msg);
         callback.onError(pe);
+        if (DEBUG) {
+            Slog.i(TAG, "Orig session error: " + msg);
+        }
     }
 
     private boolean bindToDependencyInstallerIfNeeded(int userId, Handler handler,
@@ -253,4 +268,135 @@ public class InstallDependencyHelper {
             }
         }
     }
+
+    /**
+     * Ensure we call one of the outcomes only once, on the right handler.
+     *
+     * Repeated calls will be no-op.
+     */
+    private class DependencyInstallerCallbackCallOnce extends IDependencyInstallerCallback.Stub {
+
+        private final Handler mHandler;
+        private final CallOnceProxy mCallback;
+
+        @GuardedBy("this")
+        private boolean mCalled = false;
+
+        DependencyInstallerCallbackCallOnce(Handler handler, CallOnceProxy callback) {
+            mHandler = handler;
+            mCallback = callback;
+        }
+
+        // TODO(b/372862145): Consider turning the binder call to two-way so that we can
+        //  throw IllegalArgumentException
+        @Override
+        public void onAllDependenciesResolved(int[] sessionIds) throws RemoteException {
+            synchronized (this) {
+                if (mCalled) {
+                    return;
+                }
+                mCalled = true;
+            }
+
+            ArraySet<Integer> set = new ArraySet<>();
+            for (int i = 0; i < sessionIds.length; i++) {
+                if (DEBUG) {
+                    Slog.i(TAG, "onAllDependenciesResolved called with " + sessionIds[i]);
+                }
+                set.add(sessionIds[i]);
+            }
+
+            DependencyInstallTracker tracker = new DependencyInstallTracker(mCallback, set);
+            synchronized (mTrackers) {
+                mTrackers.add(tracker);
+            }
+
+            // In case any of the session ids have already been installed, check if they
+            // are valid.
+            mHandler.post(() -> {
+                if (DEBUG) {
+                    Slog.i(TAG, "onAllDependenciesResolved cleaning up invalid sessions");
+                }
+
+                for (int i = 0; i < sessionIds.length; i++) {
+                    int sessionId = sessionIds[i];
+                    SessionInfo sessionInfo = mPackageInstallerService.getSessionInfo(sessionId);
+
+                    // Continue waiting if session exists and hasn't passed or failed yet.
+                    if (sessionInfo != null && !sessionInfo.isSessionApplied
+                            && !sessionInfo.isSessionFailed) {
+                        continue;
+                    }
+
+                    if (DEBUG) {
+                        Slog.i(TAG, "onAllDependenciesResolved cleaning up finished"
+                                + " session: " + sessionId);
+                    }
+
+                    // If session info is null, we assume it to be success.
+                    // TODO(b/372862145): Check historical sessions to be more precise.
+                    boolean success = sessionInfo == null || sessionInfo.isSessionApplied;
+
+                    notifySessionComplete(sessionId, /*success=*/success);
+                }
+            });
+        }
+
+        @Override
+        public void onFailureToResolveAllDependencies() throws RemoteException {
+            synchronized (this) {
+                if (mCalled) {
+                    return;
+                }
+                onError(mCallback, "Failed to resolve all dependencies automatically");
+                mCalled = true;
+            }
+        }
+    }
+
+    /**
+     * Tracks a list of session ids against a particular callback.
+     *
+     * If all the sessions completes successfully, it invokes the positive flow. If any of the
+     * sessions fails, it invokes the failure flow immediately.
+     */
+    // TODO(b/372862145): Determine and add support for rebooting while dependency is being resolved
+    private static class DependencyInstallTracker {
+        private final CallOnceProxy mCallback;
+        private final ArraySet<Integer> mPendingSessionIds;
+
+        DependencyInstallTracker(CallOnceProxy callback, ArraySet<Integer> pendingSessionIds) {
+            mCallback = callback;
+            mPendingSessionIds = pendingSessionIds;
+        }
+
+        /**
+         * Process a session complete event.
+         *
+         * Returns true if we still need to continue tracking.
+         */
+        public boolean onSessionComplete(int sessionId, boolean success) {
+            synchronized (this) {
+                if (!mPendingSessionIds.contains(sessionId)) {
+                    // This had no impact on tracker, so continue tracking
+                    return true;
+                }
+
+                if (!success) {
+                    // If one of the dependency fails, the orig session would fail too.
+                    onError(mCallback, "Failed to install all dependencies");
+                    // TODO(b/372862145): Abandon the rest of the pending sessions.
+                    return false; // No point in tracking anymore
+                }
+
+                mPendingSessionIds.remove(sessionId);
+                if (mPendingSessionIds.isEmpty()) {
+                    mCallback.onResult(null);
+                    return false; // Nothing to track anymore
+                }
+                return true; // Keep on tracking
+            }
+        }
+
+    }
 }
diff --git a/services/core/java/com/android/server/pm/InstallPackageHelper.java b/services/core/java/com/android/server/pm/InstallPackageHelper.java
index 8168c5493304..e5e274450655 100644
--- a/services/core/java/com/android/server/pm/InstallPackageHelper.java
+++ b/services/core/java/com/android/server/pm/InstallPackageHelper.java
@@ -229,6 +229,7 @@ final class InstallPackageHelper {
     private final SharedLibrariesImpl mSharedLibraries;
     private final PackageManagerServiceInjector mInjector;
     private final UpdateOwnershipHelper mUpdateOwnershipHelper;
+    private final InstallDependencyHelper mInstallDependencyHelper;
 
     private final Object mInternalLock = new Object();
     @GuardedBy("mInternalLock")
@@ -239,7 +240,8 @@ final class InstallPackageHelper {
                          AppDataHelper appDataHelper,
                          RemovePackageHelper removePackageHelper,
                          DeletePackageHelper deletePackageHelper,
-                         BroadcastHelper broadcastHelper) {
+                         BroadcastHelper broadcastHelper,
+                         InstallDependencyHelper installDependencyHelper) {
         mPm = pm;
         mInjector = pm.mInjector;
         mAppDataHelper = appDataHelper;
@@ -253,6 +255,7 @@ final class InstallPackageHelper {
         mPackageAbiHelper = pm.mInjector.getAbiHelper();
         mSharedLibraries = pm.mInjector.getSharedLibrariesImpl();
         mUpdateOwnershipHelper = pm.mInjector.getUpdateOwnershipHelper();
+        mInstallDependencyHelper = installDependencyHelper;
     }
 
     /**
@@ -1364,6 +1367,10 @@ final class InstallPackageHelper {
                 }
             }
         }
+
+        for (InstallRequest request : requests) {
+            mInstallDependencyHelper.notifySessionComplete(request.getSessionId(), success);
+        }
     }
 
     @GuardedBy("mPm.mInstallLock")
diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java
index a4152a724d64..47b785040d44 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerService.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerService.java
@@ -327,7 +327,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
                 context, mInstallThread.getLooper(), new AppStateHelper(context));
         mPackageArchiver = new PackageArchiver(mContext, mPm);
         mInstallDependencyHelper = new InstallDependencyHelper(mContext,
-                mPm.mInjector.getSharedLibrariesImpl());
+                mPm.mInjector.getSharedLibrariesImpl(), this);
 
         LocalServices.getService(SystemServiceManager.class).startService(
                 new Lifecycle(context, this));
@@ -337,6 +337,10 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements
         return mStagingManager;
     }
 
+    InstallDependencyHelper getInstallDependencyHelper() {
+        return mInstallDependencyHelper;
+    }
+
     boolean okToSendBroadcasts()  {
         return mOkToSendBroadcasts;
     }
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 807445ef062d..715633410575 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -2118,7 +2118,8 @@ public class PackageManagerService implements PackageSender, TestUtilityService
         mDeletePackageHelper = new DeletePackageHelper(this, mRemovePackageHelper,
                 mBroadcastHelper);
         mInstallPackageHelper = new InstallPackageHelper(this, mAppDataHelper, mRemovePackageHelper,
-                mDeletePackageHelper, mBroadcastHelper);
+                mDeletePackageHelper, mBroadcastHelper,
+                injector.getPackageInstallerService().getInstallDependencyHelper());
 
         mInstantAppRegistry = new InstantAppRegistry(mContext, mPermissionManager,
                 mInjector.getUserManagerInternal(), mDeletePackageHelper);
diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/ApexManagerTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/ApexManagerTest.java
index 769f071e3ddc..405024cc0e34 100644
--- a/services/tests/mockingservicestests/src/com/android/server/pm/ApexManagerTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/pm/ApexManagerTest.java
@@ -134,7 +134,7 @@ public class ApexManagerTest {
         mMockSystem.system().validateFinalState();
         mInstallPackageHelper = new InstallPackageHelper(mPmService, mock(AppDataHelper.class),
                 mock(RemovePackageHelper.class), mock(DeletePackageHelper.class),
-                mock(BroadcastHelper.class));
+                mock(BroadcastHelper.class), mock(InstallDependencyHelper.class));
     }
 
     @NonNull
diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/InstallDependencyHelperTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/InstallDependencyHelperTest.java
index 20ac0781e2ed..0304a74f7654 100644
--- a/services/tests/mockingservicestests/src/com/android/server/pm/InstallDependencyHelperTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/pm/InstallDependencyHelperTest.java
@@ -79,12 +79,14 @@ public class InstallDependencyHelperTest {
     @Mock private SharedLibrariesImpl mSharedLibraries;
     @Mock private Context mContext;
     @Mock private Computer mComputer;
+    @Mock private PackageInstallerService mPackageInstallerService;
     private InstallDependencyHelper mInstallDependencyHelper;
 
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
-        mInstallDependencyHelper = new InstallDependencyHelper(mContext, mSharedLibraries);
+        mInstallDependencyHelper = new InstallDependencyHelper(mContext, mSharedLibraries,
+                mPackageInstallerService);
     }
 
     @Test
diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/MockSystem.kt b/services/tests/mockingservicestests/src/com/android/server/pm/MockSystem.kt
index 0a6edf1b9831..b53dbc834351 100644
--- a/services/tests/mockingservicestests/src/com/android/server/pm/MockSystem.kt
+++ b/services/tests/mockingservicestests/src/com/android/server/pm/MockSystem.kt
@@ -218,6 +218,8 @@ class MockSystem(withSession: (StaticMockitoSessionBuilder) -> Unit = {}) {
         val handler = TestHandler(null)
         val defaultAppProvider: DefaultAppProvider = mock()
         val backgroundHandler = TestHandler(null)
+        val packageInstallerService: PackageInstallerService = mock()
+        val installDependencyHelper: InstallDependencyHelper = mock()
         val updateOwnershipHelper: UpdateOwnershipHelper = mock()
     }
 
@@ -306,6 +308,7 @@ class MockSystem(withSession: (StaticMockitoSessionBuilder) -> Unit = {}) {
         whenever(mocks.injector.handler) { mocks.handler }
         whenever(mocks.injector.defaultAppProvider) { mocks.defaultAppProvider }
         whenever(mocks.injector.backgroundHandler) { mocks.backgroundHandler }
+        whenever(mocks.injector.packageInstallerService) { mocks.packageInstallerService }
         whenever(mocks.injector.updateOwnershipHelper) { mocks.updateOwnershipHelper }
         whenever(mocks.injector.getSystemService(AppOpsManager::class.java)) { mocks.appOpsManager }
         wheneverStatic { SystemConfig.getInstance() }.thenReturn(mocks.systemConfig)
@@ -332,6 +335,8 @@ class MockSystem(withSession: (StaticMockitoSessionBuilder) -> Unit = {}) {
             DEVICE_PROVISIONING_PACKAGE_NAME
         }
         whenever(mocks.apexManager.activeApexInfos).thenReturn(DEFAULT_ACTIVE_APEX_INFO_LIST)
+        whenever(mocks.packageInstallerService.installDependencyHelper).thenReturn(
+            mocks.installDependencyHelper)
         whenever(mocks.settings.packagesLocked).thenReturn(mSettingsMap)
         whenever(mocks.settings.internalVersion).thenReturn(DEFAULT_VERSION_INFO)
         whenever(mocks.settings.keySetManagerService).thenReturn(mocks.keySetManagerService)