From e87368e1fd17d5d8fa39d1c6fe408dbb133429b2 Mon Sep 17 00:00:00 2001 From: Richard Uhler Date: Thu, 24 Jan 2019 16:34:14 +0000 Subject: [PATCH] Commit rollbacks by ID, not by RollbackInfo. To make it clear that the system only pays attention to the provided rollback ID when committing a rollback. Also, rename executeRollback to commitRollback in IRollbackManager.aidl, now that we are touching the code anyway. Bug: 112431924 Test: atest RollbackTest Change-Id: I315e75c39019536fb2f090a0c84ed4cf7c03ce8c --- api/system-current.txt | 2 +- .../content/rollback/IRollbackManager.aidl | 2 +- .../content/rollback/RollbackManager.java | 7 +++---- .../rollback/RollbackManagerServiceImpl.java | 13 +++++++------ .../RollbackPackageHealthObserver.java | 3 ++- .../android/tests/rollback/RollbackTest.java | 18 +++++++++--------- .../tests/rollback/RollbackTestUtils.java | 5 ++--- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/api/system-current.txt b/api/system-current.txt index 953955af3e953..24793060bfae2 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -1707,7 +1707,7 @@ package android.content.rollback { } public final class RollbackManager { - method @RequiresPermission(android.Manifest.permission.MANAGE_ROLLBACKS) public void commitRollback(@NonNull android.content.rollback.RollbackInfo, @NonNull android.content.IntentSender); + method @RequiresPermission(android.Manifest.permission.MANAGE_ROLLBACKS) public void commitRollback(int, @NonNull android.content.IntentSender); method @RequiresPermission(android.Manifest.permission.MANAGE_ROLLBACKS) public void expireRollbackForPackage(@NonNull String); method @RequiresPermission(android.Manifest.permission.MANAGE_ROLLBACKS) public java.util.List getAvailableRollbacks(); method @RequiresPermission(android.Manifest.permission.MANAGE_ROLLBACKS) @NonNull public java.util.List getRecentlyCommittedRollbacks(); diff --git a/core/java/android/content/rollback/IRollbackManager.aidl b/core/java/android/content/rollback/IRollbackManager.aidl index 63d75a097d24b..226d76abc01c9 100644 --- a/core/java/android/content/rollback/IRollbackManager.aidl +++ b/core/java/android/content/rollback/IRollbackManager.aidl @@ -26,7 +26,7 @@ interface IRollbackManager { ParceledListSlice getAvailableRollbacks(); ParceledListSlice getRecentlyExecutedRollbacks(); - void executeRollback(in RollbackInfo rollback, String callerPackageName, + void commitRollback(int rollbackId, String callerPackageName, in IntentSender statusReceiver); // Exposed for use from the system server only. Callback from the package diff --git a/core/java/android/content/rollback/RollbackManager.java b/core/java/android/content/rollback/RollbackManager.java index 2566ac562e119..f8abcfc8fc764 100644 --- a/core/java/android/content/rollback/RollbackManager.java +++ b/core/java/android/content/rollback/RollbackManager.java @@ -102,16 +102,15 @@ public final class RollbackManager { *

* TODO: Specify the returns status codes. * - * @param rollback to commit + * @param rollbackId ID of the rollback to commit * @param statusReceiver where to deliver the results * @throws SecurityException if the caller does not have the * MANAGE_ROLLBACKS permission. */ @RequiresPermission(android.Manifest.permission.MANAGE_ROLLBACKS) - public void commitRollback(@NonNull RollbackInfo rollback, - @NonNull IntentSender statusReceiver) { + public void commitRollback(int rollbackId, @NonNull IntentSender statusReceiver) { try { - mBinder.executeRollback(rollback, mCallerPackageName, statusReceiver); + mBinder.commitRollback(rollbackId, mCallerPackageName, statusReceiver); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 6487bd7af7ee0..e59228a16fb11 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -227,7 +227,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } @Override - public void executeRollback(RollbackInfo rollback, String callerPackageName, + public void commitRollback(int rollbackId, String callerPackageName, IntentSender statusReceiver) { mContext.enforceCallingOrSelfPermission( android.Manifest.permission.MANAGE_ROLLBACKS, @@ -238,19 +238,19 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { appOps.checkPackage(callingUid, callerPackageName); getHandler().post(() -> - executeRollbackInternal(rollback, callerPackageName, statusReceiver)); + commitRollbackInternal(rollbackId, callerPackageName, statusReceiver)); } /** - * Performs the actual work to execute a rollback. + * Performs the actual work to commit a rollback. * The work is done on the current thread. This may be a long running * operation. */ - private void executeRollbackInternal(RollbackInfo rollback, + private void commitRollbackInternal(int rollbackId, String callerPackageName, IntentSender statusReceiver) { Log.i(TAG, "Initiating rollback"); - RollbackData data = getRollbackForId(rollback.getRollbackId()); + RollbackData data = getRollbackForId(rollbackId); if (data == null) { sendFailure(statusReceiver, "Rollback unavailable"); return; @@ -350,7 +350,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return; } - addRecentlyExecutedRollback(rollback); + addRecentlyExecutedRollback( + new RollbackInfo(data.rollbackId, data.packages)); sendSuccess(statusReceiver); Intent broadcast = new Intent(Intent.ACTION_ROLLBACK_COMMITTED); diff --git a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java index a4ef8dc3049be..fcae61895b36d 100644 --- a/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java +++ b/services/core/java/com/android/server/rollback/RollbackPackageHealthObserver.java @@ -86,7 +86,8 @@ public final class RollbackPackageHealthObserver implements PackageHealthObserve // TODO(zezeozue): Log initiated metrics mHandler.post(() -> - mRollbackManager.commitRollback(rollback, rollbackReceiver.getIntentSender())); + mRollbackManager.commitRollback(rollback.getRollbackId(), + rollbackReceiver.getIntentSender())); // Assume rollback executed successfully return true; } diff --git a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java index 7ab716fac6dfc..0493ef8ab0628 100644 --- a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java +++ b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTest.java @@ -139,7 +139,7 @@ public class RollbackTest { assertNull(broadcastReceiver.poll(0, TimeUnit.SECONDS)); // Roll back the app. - RollbackTestUtils.rollback(rollback); + RollbackTestUtils.rollback(rollback.getRollbackId()); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); // Verify we received a broadcast for the rollback. @@ -211,7 +211,7 @@ public class RollbackTest { assertRollbackInfoEquals(TEST_APP_B, 2, 1, rollbackB); // Rollback of B should not rollback A - RollbackTestUtils.rollback(rollbackB); + RollbackTestUtils.rollback(rollbackB.getRollbackId()); assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_B)); } finally { @@ -268,7 +268,7 @@ public class RollbackTest { assertRollbackInfoForAandB(rollbackB); // Rollback of B should rollback A as well - RollbackTestUtils.rollback(rollbackB); + RollbackTestUtils.rollback(rollbackB.getRollbackId()); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_B)); } finally { @@ -303,7 +303,7 @@ public class RollbackTest { rm.getAvailableRollbacks(), TEST_APP_A); // Roll back the app. - RollbackTestUtils.rollback(rollback); + RollbackTestUtils.rollback(rollback.getRollbackId()); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); // Verify the recent rollback has been recorded. @@ -430,7 +430,7 @@ public class RollbackTest { RollbackManager rm = RollbackTestUtils.getRollbackManager(); RollbackInfo rollback = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TEST_APP_A); - RollbackTestUtils.rollback(rollback); + RollbackTestUtils.rollback(rollback.getRollbackId()); processUserData(TEST_APP_A); } finally { RollbackTestUtils.dropShellPermissionIdentity(); @@ -500,7 +500,7 @@ public class RollbackTest { assertRollbackInfoEquals(TEST_APP_B, 2, 1, rollbackB); // Executing rollback should roll back the correct package. - RollbackTestUtils.rollback(rollbackA); + RollbackTestUtils.rollback(rollbackA.getRollbackId()); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_B)); @@ -509,7 +509,7 @@ public class RollbackTest { RollbackTestUtils.install("RollbackTestAppAv2.apk", true); assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); - RollbackTestUtils.rollback(rollbackB); + RollbackTestUtils.rollback(rollbackB.getRollbackId()); assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_B)); } finally { @@ -544,7 +544,7 @@ public class RollbackTest { try { // TODO: What if the implementation checks arguments for non-null // first? Then this test isn't valid. - rm.commitRollback(null, null); + rm.commitRollback(0, null); fail("expected SecurityException"); } catch (SecurityException e) { // Expected. @@ -598,7 +598,7 @@ public class RollbackTest { // Rollback the app. It should cause both test apps to be rolled // back. - RollbackTestUtils.rollback(rollback); + RollbackTestUtils.rollback(rollback.getRollbackId()); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); assertEquals(1, RollbackTestUtils.getInstalledVersion(TEST_APP_B)); diff --git a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTestUtils.java b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTestUtils.java index f481897c060c3..14786577c8141 100644 --- a/tests/RollbackTest/src/com/android/tests/rollback/RollbackTestUtils.java +++ b/tests/RollbackTest/src/com/android/tests/rollback/RollbackTestUtils.java @@ -21,7 +21,6 @@ import android.content.Intent; import android.content.pm.PackageInfo; import android.content.pm.PackageInstaller; import android.content.pm.PackageManager; -import android.content.rollback.RollbackInfo; import android.content.rollback.RollbackManager; import android.support.test.InstrumentationRegistry; @@ -93,9 +92,9 @@ class RollbackTestUtils { * Commit the given rollback. * @throws AssertionError if the rollback fails. */ - static void rollback(RollbackInfo rollback) throws InterruptedException { + static void rollback(int rollbackId) throws InterruptedException { RollbackManager rm = getRollbackManager(); - rm.commitRollback(rollback, LocalIntentSender.getIntentSender()); + rm.commitRollback(rollbackId, LocalIntentSender.getIntentSender()); assertStatusSuccess(LocalIntentSender.getIntentSenderResult()); }