From 8108251a87831ee7c0afd6d62e0e7a3083dedd98 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 31 Dec 2019 15:52:12 +0800 Subject: [PATCH 1/3] Implement VersionedPackage#equals (1/n) So we don't need to call VersionedPackage#getPackageName and VersionedPackage#getVersionCode to compare 2 VersionedPackages. Note you must also override #hashCode whenever you override #equals. Bug: 147028082 Test: atest StagedRollbackTest Change-Id: Ib1fac7e0521e2ccde0c2bb1f3fd28c0a7cd70234 --- core/java/android/content/pm/VersionedPackage.java | 14 ++++++++++++++ .../rollback/RollbackPackageHealthObserver.java | 6 +----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/core/java/android/content/pm/VersionedPackage.java b/core/java/android/content/pm/VersionedPackage.java index 3e22eb21d3693..21df7ecef0156 100644 --- a/core/java/android/content/pm/VersionedPackage.java +++ b/core/java/android/content/pm/VersionedPackage.java @@ -95,6 +95,20 @@ public final class VersionedPackage implements Parcelable { return "VersionedPackage[" + mPackageName + "/" + mVersionCode + "]"; } + @Override + public boolean equals(Object o) { + return o instanceof VersionedPackage + && ((VersionedPackage) o).mPackageName.equals(mPackageName) + && ((VersionedPackage) o).mVersionCode == mVersionCode; + } + + @Override + public int hashCode() { + // Roll our own hash function without using Objects#hash which incurs the overhead + // of autoboxing. + return 31 * mPackageName.hashCode() + Long.hashCode(mVersionCode); + } + @Override public int describeContents() { return 0; diff --git a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java index bb095841bbaa5..59c1b4e2f0fb1 100644 --- a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java +++ b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java @@ -212,11 +212,7 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve RollbackManager rollbackManager = mContext.getSystemService(RollbackManager.class); for (RollbackInfo rollback : rollbackManager.getAvailableRollbacks()) { for (PackageRollbackInfo packageRollback : rollback.getPackages()) { - boolean hasFailedPackage = packageRollback.getPackageName().equals( - failedPackage.getPackageName()) - && packageRollback.getVersionRolledBackFrom().getVersionCode() - == failedPackage.getVersionCode(); - if (hasFailedPackage) { + if (packageRollback.getVersionRolledBackFrom().equals(failedPackage)) { return rollback; } } From b059e2f6d0d7f3b68e304a8ecb79aebbfd5112c3 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 31 Dec 2019 15:58:31 +0800 Subject: [PATCH 2/3] Use PackageRollbackInfo#getVersionRolledBackFrom to simplify code (2/n) PackageRollbackInfo#getVersionRolledBackFrom returns a VersionedPackage which contains a version code. No need to query the version code from the package manager. Bug: 147028082 Test: atest StagedRollbackTest Change-Id: I769e726183ee99bb9e8c7576bb8b6c21a75fe2ff --- .../RollbackPackageHealthObserver.java | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java index 59c1b4e2f0fb1..9dab6a1c05107 100644 --- a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java +++ b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java @@ -357,15 +357,6 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve } } - private VersionedPackage getVersionedPackage(String packageName) { - try { - return new VersionedPackage(packageName, mContext.getPackageManager().getPackageInfo( - packageName, 0 /* flags */).getLongVersionCode()); - } catch (PackageManager.NameNotFoundException e) { - return null; - } - } - /** * Rolls back the session that owns {@code failedPackage} * @@ -428,14 +419,8 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve List rollbacks = rollbackManager.getAvailableRollbacks(); for (RollbackInfo rollback : rollbacks) { - String samplePackageName = rollback.getPackages().get(0).getPackageName(); - VersionedPackage sampleVersionedPackage = getVersionedPackage(samplePackageName); - if (sampleVersionedPackage == null) { - Slog.e(TAG, "Failed to rollback " + samplePackageName); - continue; - } - rollbackPackage(rollback, sampleVersionedPackage, - PackageWatchdog.FAILURE_REASON_NATIVE_CRASH); + VersionedPackage sample = rollback.getPackages().get(0).getVersionRolledBackFrom(); + rollbackPackage(rollback, sample, PackageWatchdog.FAILURE_REASON_NATIVE_CRASH); } } From 41c017e09cb8514f633d94f5f3be7cd66761aa19 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 31 Dec 2019 16:04:39 +0800 Subject: [PATCH 3/3] Simplify a logging message (3/n) VersionedPackage overrides toString() which can be used in string concatenation. Bug: 147028082 Test: build doesn't fail Change-Id: Id7ba8465c8e5df6e7c409e5b0c14e99eb31bbcef --- .../server/rollback/RollbackPackageHealthObserver.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java index 9dab6a1c05107..162a695463185 100644 --- a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java +++ b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java @@ -120,9 +120,7 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve RollbackInfo rollback = getAvailableRollback(failedPackage); if (rollback == null) { - Slog.w(TAG, "Expected rollback but no valid rollback found for package: [ " - + failedPackage.getPackageName() + "] with versionCode: [" - + failedPackage.getVersionCode() + "]"); + Slog.w(TAG, "Expected rollback but no valid rollback found for " + failedPackage); return false; } rollbackPackage(rollback, failedPackage, rollbackReason);