Skip to content

Commit

Permalink
Fix a no-op uninstall being treated as a failure
Browse files Browse the repository at this point in the history
Treat a stageUninstall() doing nothing as a success, not a
failure. This prevents the system retrying the uninstall
later.

Unit tests run with:

make -j30 FrameworksServicesTests
adb install -r -g \
  "out/target/product/marlin/data/app/FrameworksServicesTests/FrameworksServicesTests.apk"
adb shell am instrument -e package com.android.server.timezone -w \
  com.android.frameworks.servicestests \
  "com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner"

Test: See above
Test: Manual testing, adb dumpsys timezone
Test: PTS: run pts -m PtsTimeZoneTestCases
Bug: 65657176
Merged-in: Ifd205da90e848552711ac3f4207cd22ad2c2747b
Change-Id: Ifd205da90e848552711ac3f4207cd22ad2c2747b
(cherry picked from commit 8e27c92)
  • Loading branch information
nfuller committed Sep 15, 2017
1 parent 293e2f9 commit 249b2e3
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -343,24 +343,28 @@ private class UninstallRunnable implements Runnable {
@Override
public void run() {
EventLogTags.writeTimezoneUninstallStarted(toStringOrNull(mCheckToken));
boolean success = false;
boolean packageTrackerStatus = false;
try {
success = mInstaller.stageUninstall();
// Right now we just have success (0) / failure (1). All clients should be checking
// against SUCCESS. More granular failures may be added in future.
int resultCode = success ? Callback.SUCCESS
: Callback.ERROR_UNKNOWN_FAILURE;
int uninstallResult = mInstaller.stageUninstall();
packageTrackerStatus = (uninstallResult == TimeZoneDistroInstaller.UNINSTALL_SUCCESS
|| uninstallResult == TimeZoneDistroInstaller.UNINSTALL_NOTHING_INSTALLED);

// Right now we just have Callback.SUCCESS / Callback.ERROR_UNKNOWN_FAILURE for
// uninstall. All clients should be checking against SUCCESS. More granular failures
// may be added in future.
int callbackResultCode =
packageTrackerStatus ? Callback.SUCCESS : Callback.ERROR_UNKNOWN_FAILURE;
EventLogTags.writeTimezoneUninstallComplete(
toStringOrNull(mCheckToken), resultCode);
sendFinishedStatus(mCallback, resultCode);
toStringOrNull(mCheckToken), callbackResultCode);
sendFinishedStatus(mCallback, callbackResultCode);
} catch (Exception e) {
EventLogTags.writeTimezoneUninstallComplete(
toStringOrNull(mCheckToken), Callback.ERROR_UNKNOWN_FAILURE);
Slog.w(TAG, "Failed to uninstall distro.", e);
sendFinishedStatus(mCallback, Callback.ERROR_UNKNOWN_FAILURE);
} finally {
// Notify the package tracker that the operation is now complete.
mPackageTracker.recordCheckResult(mCheckToken, success);
mPackageTracker.recordCheckResult(mCheckToken, packageTrackerStatus);

mOperationInProgress.set(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,39 @@ public void requestUninstall_asyncSuccess() throws Exception {
verifyNoPackageTrackerCallsMade();

// Set up the installer.
configureStageUninstallExpectation(true /* success */);
configureStageUninstallExpectation(TimeZoneDistroInstaller.UNINSTALL_SUCCESS);

// Simulate the async execution.
mFakeExecutor.simulateAsyncExecutionOfLastCommand();

// Verify the expected calls were made to other components.
verifyStageUninstallCalled();
verifyPackageTrackerCalled(token, true /* success */);

// Check the callback was called.
callback.assertResultReceived(Callback.SUCCESS);
}

@Test
public void requestUninstall_asyncNothingInstalled() throws Exception {
configureCallerHasPermission();

CheckToken token = createArbitraryToken();
byte[] tokenBytes = token.toByteArray();

TestCallback callback = new TestCallback();

// Request the uninstall.
assertEquals(RulesManager.SUCCESS,
mRulesManagerService.requestUninstall(tokenBytes, callback));

// Assert nothing has happened yet.
callback.assertNoResultReceived();
verifyNoInstallerCallsMade();
verifyNoPackageTrackerCallsMade();

// Set up the installer.
configureStageUninstallExpectation(TimeZoneDistroInstaller.UNINSTALL_NOTHING_INSTALLED);

// Simulate the async execution.
mFakeExecutor.simulateAsyncExecutionOfLastCommand();
Expand Down Expand Up @@ -613,7 +645,7 @@ public void requestUninstall_nullTokenBytes() throws Exception {
callback.assertNoResultReceived();

// Set up the installer.
configureStageUninstallExpectation(true /* success */);
configureStageUninstallExpectation(TimeZoneDistroInstaller.UNINSTALL_SUCCESS);

// Simulate the async execution.
mFakeExecutor.simulateAsyncExecutionOfLastCommand();
Expand Down Expand Up @@ -644,7 +676,7 @@ public void requestUninstall_asyncUninstallFail() throws Exception {
callback.assertNoResultReceived();

// Set up the installer.
configureStageUninstallExpectation(false /* success */);
configureStageUninstallExpectation(TimeZoneDistroInstaller.UNINSTALL_FAIL);

// Simulate the async execution.
mFakeExecutor.simulateAsyncExecutionOfLastCommand();
Expand Down Expand Up @@ -849,8 +881,8 @@ private void configureStageInstallExpectation(int resultCode)
.thenReturn(resultCode);
}

private void configureStageUninstallExpectation(boolean success) throws Exception {
doReturn(success).when(mMockTimeZoneDistroInstaller).stageUninstall();
private void configureStageUninstallExpectation(int resultCode) throws Exception {
doReturn(resultCode).when(mMockTimeZoneDistroInstaller).stageUninstall();
}

private void verifyStageInstallCalled() throws Exception {
Expand Down

0 comments on commit 249b2e3

Please sign in to comment.