From a67a1a98413fd1a2ffc15e67777182f5ad364bf8 Mon Sep 17 00:00:00 2001 From: Songchun Fan Date: Fri, 19 Feb 2021 01:10:12 +0000 Subject: [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 --- .../os/incremental/IncrementalFileStorages.java | 9 ++------ .../android/os/incremental/IncrementalManager.java | 25 ---------------------- .../android/server/pm/PackageInstallerSession.java | 18 +++++++++++----- 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 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 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 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); } } }); -- cgit v1.2.3-59-g8ed1b