From c0d627749aeb4cd2dda7a4306e07115f80a96cba Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 10 Jan 2020 14:37:21 +0800 Subject: [PATCH] Fix a todo item (6/n) Add #allPackagesEnabled instead of exposing 2 methods which are always used together. Also remove unused flags. Bug: 147400979 Test: atest RollbackTest RollbackUnitTest Change-Id: I2a6b70e9a089a1a4ebda8ac77b2bae662ca0a373 --- .../com/android/server/rollback/Rollback.java | 64 +++++-------------- .../rollback/RollbackManagerServiceImpl.java | 3 +- .../server/rollback/RollbackUnitTest.java | 30 +++++++++ 3 files changed, 48 insertions(+), 49 deletions(-) diff --git a/services/core/java/com/android/server/rollback/Rollback.java b/services/core/java/com/android/server/rollback/Rollback.java index bf7413b64588a..4d7af9cc0d44c 100644 --- a/services/core/java/com/android/server/rollback/Rollback.java +++ b/services/core/java/com/android/server/rollback/Rollback.java @@ -93,19 +93,6 @@ class Rollback { */ static final int ROLLBACK_STATE_DELETED = 4; - @IntDef(flag = true, prefix = { "MATCH_" }, value = { - MATCH_APK_IN_APEX, - }) - @Retention(RetentionPolicy.SOURCE) - @interface RollbackInfoFlags {} - - /** - * {@link RollbackInfo} flag: include {@code RollbackInfo} packages that are apk-in-apex. - * These packages do not have their own sessions. They are embedded in an apex which has a - * session id. - */ - static final int MATCH_APK_IN_APEX = 1; - /** * The session ID for the staged session if this rollback data represents a staged session, * {@code -1} otherwise. @@ -782,33 +769,6 @@ class Rollback { } } - /** - * Returns the number of {@link PackageRollbackInfo} we are storing in this {@link Rollback} - * instance. By default, this method does not include apk-in-apex package in the count. - * - * @param flags Apk-in-apex packages can be included in the count by passing - * {@link Rollback#MATCH_APK_IN_APEX} - * - * @return Counts number of {@link PackageRollbackInfo} stored in the {@link Rollback} - * according to {@code flags} passed - */ - int getPackageCount(@RollbackInfoFlags int flags) { - synchronized (mLock) { - List packages = info.getPackages(); - if ((flags & MATCH_APK_IN_APEX) != 0) { - return packages.size(); - } - - int packagesWithoutApkInApex = 0; - for (PackageRollbackInfo rollbackInfo : packages) { - if (!rollbackInfo.isApkInApex()) { - packagesWithoutApkInApex++; - } - } - return packagesWithoutApkInApex; - } - } - /** * Adds a rollback token to be associated with this rollback. This may be used to * identify which rollback should be removed in case {@link PackageManager} sends an @@ -841,13 +801,6 @@ class Rollback { return false; } - /** - * Returns the number of package session ids in this rollback. - */ - int getPackageSessionIdCount() { - return mPackageSessionIds.length; - } - /** * Called when a child session finished with success. * Returns true when all child sessions are notified with success. This rollback will be @@ -859,6 +812,23 @@ class Rollback { } } + /** + * Returns true if all packages in this rollback are enabled. We won't enable this rollback + * until all packages are enabled. Note we don't count apk-in-apex here since they are enabled + * automatically when the embedding apex is enabled. + */ + boolean allPackagesEnabled() { + synchronized (mLock) { + int packagesWithoutApkInApex = 0; + for (PackageRollbackInfo rollbackInfo : info.getPackages()) { + if (!rollbackInfo.isApkInApex()) { + packagesWithoutApkInApex++; + } + } + return packagesWithoutApkInApex == mPackageSessionIds.length; + } + } + static String rollbackStateToString(@RollbackState int state) { switch (state) { case Rollback.ROLLBACK_STATE_ENABLING: return "enabling"; diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 3fa114e3b7a33..8bd9533727d65 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -1237,8 +1237,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // equal to the number of sessions we are installing, to ensure we didn't skip enabling // of any sessions. If we successfully enable an apex, then we can assume we enabled // rollback for the embedded apk-in-apex, if any. - // TODO: add a helper instead of exposing 2 methods from Rollback - if (rollback.getPackageCount(0 /*flags*/) != rollback.getPackageSessionIdCount()) { + if (!rollback.allPackagesEnabled()) { Slog.e(TAG, "Failed to enable rollback for all packages in session."); rollback.delete(mAppDataRollbackHelper); return null; diff --git a/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java b/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java index 9719509659ec8..804c1b9e09fde 100644 --- a/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java +++ b/services/tests/servicestests/src/com/android/server/rollback/RollbackUnitTest.java @@ -302,6 +302,22 @@ public class RollbackUnitTest { assertThat(rollback.notifySessionWithSuccess()).isTrue(); } + @Test + public void allPackagesEnabled() { + int[] sessionIds = new int[]{ 7777, 8888 }; + Rollback rollback = new Rollback(123, new File("/test/testing"), -1, USER, INSTALLER, + sessionIds); + // #allPackagesEnabled returns false when 1 out of 2 packages is enabled. + rollback.info.getPackages().add(newPkgInfoFor(PKG_1, 12, 10, false)); + assertThat(rollback.allPackagesEnabled()).isFalse(); + // #allPackagesEnabled returns false for ApkInApex doesn't count. + rollback.info.getPackages().add(newPkgInfoForApkInApex(PKG_3, 157, 156)); + assertThat(rollback.allPackagesEnabled()).isFalse(); + // #allPackagesEnabled returns true when 2 out of 2 packages are enabled. + rollback.info.getPackages().add(newPkgInfoFor(PKG_2, 18, 12, true)); + assertThat(rollback.allPackagesEnabled()).isTrue(); + } + private static PackageRollbackInfo newPkgInfoFor( String packageName, long fromVersion, long toVersion, boolean isApex) { return new PackageRollbackInfo(new VersionedPackage(packageName, fromVersion), @@ -310,6 +326,20 @@ public class RollbackUnitTest { new SparseLongArray()); } + /** + * TODO: merge newPkgInfoFor and newPkgInfoForApkInApex by using enums to specify + * 1. IS_APK + * 2. IS_APEX + * 3. IS_APK_IN_APEX + */ + private static PackageRollbackInfo newPkgInfoForApkInApex( + String packageName, long fromVersion, long toVersion) { + return new PackageRollbackInfo(new VersionedPackage(packageName, fromVersion), + new VersionedPackage(packageName, toVersion), + new IntArray(), new ArrayList<>(), false, true, new IntArray(), + new SparseLongArray()); + } + private static class PackageRollbackInfoForPackage implements ArgumentMatcher { private final String mPkg;