[pm] smooth transition from install progress to incremental loading progress

Previously, regardless of the downloading progress reported by the client,
right before installation completion, we publish the install progress
as 0.9. For incremental installs, because we will publish the
incremnetal loading progress after the installation completion, we
might see the progress going backward, because the incremental loading
progress is likely less than 0.9.

For a smoother transition, we do not overwrite the client progress.
Instead, during committing, publish the incremental loading progress
while making sure the progress doesn't go backwards. After committing,
the launcher will keep showing the incremental loading progress.

+ Making sure the installer's incremental progress listener is
unregistered after session is committed.

See the demo video for an example. Notice that the progress is changed
during "Installing..." (around 00:35).

BUG: 180040329
Test: manual
Change-Id: I6c30b0dac6758fd103ed6a985ca39b6f8557ba8f

3 files changed, 15 insertions, 37 deletions

diff --git a/core/java/android/os/incremental/IncrementalFileStorages.java b/core/java/android/os/incremental/IncrementalFileStorages.java
index a078e0434867..73520e07d118 100644
--- a/core/java/android/os/incremental/IncrementalFileStorages.java
+++ b/core/java/android/os/incremental/IncrementalFileStorages.java
@@ -59,7 +59,6 @@ public final class IncrementalFileStorages {
     /**
      * Set up files and directories used in an installation session. Only used by Incremental.
      * All the files will be created in defaultStorage.
-     * TODO(b/133435829): code clean up
      *
      * @throws IllegalStateException the session is not an Incremental installation session.
      * @throws IOException if fails to setup files or directories.
@@ -73,12 +72,10 @@ public final class IncrementalFileStorages {
             @Nullable IStorageHealthListener healthListener,
             @NonNull List<InstallationFileParcel> addedFiles,
             @NonNull PerUidReadTimeouts[] perUidReadTimeouts,
-            IPackageLoadingProgressCallback progressCallback) throws IOException {
-        // TODO(b/136132412): validity check if session should not be incremental
+            @Nullable IPackageLoadingProgressCallback progressCallback) throws IOException {
         IncrementalManager incrementalManager = (IncrementalManager) context.getSystemService(
                 Context.INCREMENTAL_SERVICE);
         if (incrementalManager == null) {
-            // TODO(b/146080380): add incremental-specific error code
             throw new IOException("Failed to obtain incrementalManager.");
         }
 
@@ -89,7 +86,6 @@ public final class IncrementalFileStorages {
                 try {
                     result.addApkFile(file);
                 } catch (IOException e) {
-                    // TODO(b/146080380): add incremental-specific error code
                     throw new IOException(
                             "Failed to add file to IncFS: " + file.name + ", reason: ", e);
                 }
@@ -203,7 +199,6 @@ public final class IncrementalFileStorages {
 
     /**
      * Resets the states and unbinds storage instances for an installation session.
-     * TODO(b/136132412): make sure unnecessary binds are removed but useful storages are kept
      */
     public void cleanUp() {
         if (mDefaultStorage == null) {
@@ -211,8 +206,8 @@ public final class IncrementalFileStorages {
         }
 
         try {
+            mIncrementalManager.unregisterLoadingProgressCallbacks(mStageDir.getAbsolutePath());
             mDefaultStorage.unBind(mStageDir.getAbsolutePath());
-            mDefaultStorage.unregisterLoadingProgressListener();
         } catch (IOException ignored) {
         }
         mDefaultStorage = null;
diff --git a/core/java/android/os/incremental/IncrementalManager.java b/core/java/android/os/incremental/IncrementalManager.java
index 05899947c3df..cec6a1fb271d 100644
--- a/core/java/android/os/incremental/IncrementalManager.java
+++ b/core/java/android/os/incremental/IncrementalManager.java
@@ -342,7 +342,6 @@ public final class IncrementalManager {
             storage.unregisterLoadingProgressListener();
         }
 
-        // TODO(b/165841827): handle reboot and app update
         public boolean registerCallback(@NonNull IncrementalStorage storage,
                 @NonNull IPackageLoadingProgressCallback callback) {
             final int storageId = storage.getId();
@@ -364,30 +363,6 @@ public final class IncrementalManager {
             return storage.registerLoadingProgressListener(this);
         }
 
-        public boolean unregisterCallback(@NonNull IncrementalStorage storage,
-                @NonNull IPackageLoadingProgressCallback callback) {
-            final int storageId = storage.getId();
-            final RemoteCallbackList<IPackageLoadingProgressCallback> callbacksForStorage;
-            synchronized (mCallbacks) {
-                callbacksForStorage = mCallbacks.get(storageId);
-                if (callbacksForStorage == null) {
-                    // no callback has ever been registered on this storage
-                    return false;
-                }
-                if (!callbacksForStorage.unregister(callback)) {
-                    // the callback was not registered
-                    return false;
-                }
-                if (callbacksForStorage.getRegisteredCallbackCount() > 0) {
-                    // other callbacks are still listening on this storage
-                    return true;
-                }
-                mCallbacks.delete(storageId);
-            }
-            // stop listening for this storage
-            return storage.unregisterLoadingProgressListener();
-        }
-
         @Override
         public void onStorageLoadingProgressChanged(int storageId, float progress) {
             final RemoteCallbackList<IPackageLoadingProgressCallback> callbacksForStorage;
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index f09f33ea95ff..287bcec8ce89 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -1202,8 +1202,13 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
 
     @GuardedBy("mLock")
     private void computeProgressLocked(boolean forcePublish) {
-        mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f)
-                + MathUtils.constrain(mInternalProgress * 0.2f, 0f, 0.2f);
+        if (!mCommitted) {
+            mProgress = MathUtils.constrain(mClientProgress * 0.8f, 0f, 0.8f)
+                    + MathUtils.constrain(mInternalProgress * 0.2f, 0f, 0.2f);
+        } else {
+            // For incremental installs, continue publishing the install progress during committing.
+            mProgress = mIncrementalProgress;
+        }
 
         // Only publish when meaningful change
         if (forcePublish || Math.abs(mProgress - mReportedProgress) >= 0.01) {
@@ -1939,9 +1944,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                     throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR,
                             "Session destroyed");
                 }
-                // Client staging is fully done at this point
-                mClientProgress = 1f;
-                computeProgressLocked(true);
+                if (!isIncrementalInstallation()) {
+                    // For non-incremental installs, client staging is fully done at this point
+                    mClientProgress = 1f;
+                    computeProgressLocked(true);
+                }
 
                 // This ongoing commit should keep session active, even though client
                 // will probably close their end.
@@ -3799,6 +3806,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                             public void onPackageLoadingProgressChanged(float progress) {
                                 synchronized (mLock) {
                                     mIncrementalProgress = progress;
+                                    computeProgressLocked(true);
                                 }
                             }
                         });