diff options
11 files changed, 201 insertions, 141 deletions
diff --git a/services/backup/java/com/android/server/backup/BackupAgentConnectionManager.java b/services/backup/java/com/android/server/backup/BackupAgentConnectionManager.java index 02f186557ca4..6ced096e8778 100644 --- a/services/backup/java/com/android/server/backup/BackupAgentConnectionManager.java +++ b/services/backup/java/com/android/server/backup/BackupAgentConnectionManager.java @@ -101,11 +101,16 @@ public class BackupAgentConnectionManager { private static final class BackupAgentConnection { public final ApplicationInfo appInfo; + public final int backupMode; + public final boolean inRestrictedMode; public IBackupAgent backupAgent; public boolean connecting = true; // Assume we are trying to connect on creation. - private BackupAgentConnection(ApplicationInfo appInfo) { + private BackupAgentConnection(ApplicationInfo appInfo, int backupMode, + boolean inRestrictedMode) { this.appInfo = appInfo; + this.backupMode = backupMode; + this.inRestrictedMode = inRestrictedMode; } } @@ -113,26 +118,31 @@ public class BackupAgentConnectionManager { * Fires off a backup agent, blocking until it attaches (i.e. ActivityManager calls * {@link #agentConnected(String, IBinder)}) or until this operation times out. * - * @param mode a {@code BACKUP_MODE} from {@link android.app.ApplicationThreadConstants}. + * @param backupMode a {@code BACKUP_MODE} from {@link android.app.ApplicationThreadConstants}. */ @Nullable - public IBackupAgent bindToAgentSynchronous(ApplicationInfo app, int mode, + public IBackupAgent bindToAgentSynchronous(ApplicationInfo app, int backupMode, @BackupAnnotations.BackupDestination int backupDestination) { + if (app == null) { + Slog.w(TAG, mUserIdMsg + "bindToAgentSynchronous for null app"); + return null; + } + synchronized (mAgentConnectLock) { - boolean useRestrictedMode = shouldUseRestrictedBackupModeForPackage(mode, + boolean useRestrictedMode = shouldUseRestrictedBackupModeForPackage(backupMode, app.packageName); if (mCurrentConnection != null) { Slog.e(TAG, mUserIdMsg + "binding to new agent before unbinding from old one: " + mCurrentConnection.appInfo.packageName); } - mCurrentConnection = new BackupAgentConnection(app); + mCurrentConnection = new BackupAgentConnection(app, backupMode, useRestrictedMode); // bindBackupAgent() is an async API. It will kick off the app's process and call // agentConnected() when it receives the agent from the app. boolean startedBindSuccessfully = false; try { - startedBindSuccessfully = mActivityManager.bindBackupAgent(app.packageName, mode, - mUserId, backupDestination, useRestrictedMode); + startedBindSuccessfully = mActivityManager.bindBackupAgent(app.packageName, + backupMode, mUserId, backupDestination, useRestrictedMode); } catch (RemoteException e) { // can't happen - ActivityManager is local } @@ -173,28 +183,66 @@ public class BackupAgentConnectionManager { /** * Tell the ActivityManager that we are done with the {@link IBackupAgent} of this {@code app}. * It will tell the app to destroy the agent. + * + * <p>If {@code allowKill} is set, this will kill the app's process if the app is in restricted + * mode or if it was started for restore and specified {@code android:killAfterRestore} in its + * manifest. + * + * @see #shouldUseRestrictedBackupModeForPackage(int, String) */ - public void unbindAgent(ApplicationInfo app) { - synchronized (mAgentConnectLock) { - if (mCurrentConnection == null) { - Slog.w(TAG, mUserIdMsg + "unbindAgent but no current connection"); - } else if (!mCurrentConnection.appInfo.packageName.equals(app.packageName)) { - Slog.w(TAG, mUserIdMsg + "unbindAgent for unexpected package: " + app.packageName - + " expected: " + mCurrentConnection.appInfo.packageName); - } else { - mCurrentConnection = null; - } + public void unbindAgent(ApplicationInfo app, boolean allowKill) { + if (app == null) { + Slog.w(TAG, mUserIdMsg + "unbindAgent for null app"); + return; + } + synchronized (mAgentConnectLock) { // Even if we weren't expecting to be bound to this agent, we should still call // ActivityManager just in case. It will ignore the call if it also wasn't expecting it. try { mActivityManager.unbindBackupAgent(app); + + // Evaluate this before potentially setting mCurrentConnection = null. + boolean willKill = allowKill && shouldKillAppOnUnbind(app); + + if (mCurrentConnection == null) { + Slog.w(TAG, mUserIdMsg + "unbindAgent but no current connection"); + } else if (!mCurrentConnection.appInfo.packageName.equals(app.packageName)) { + Slog.w(TAG, + mUserIdMsg + "unbindAgent for unexpected package: " + app.packageName + + " expected: " + mCurrentConnection.appInfo.packageName); + } else { + mCurrentConnection = null; + } + + if (willKill) { + Slog.i(TAG, mUserIdMsg + "Killing agent host process"); + mActivityManager.killApplicationProcess(app.processName, app.uid); + } } catch (RemoteException e) { // Can't happen - activity manager is local } } } + @GuardedBy("mAgentConnectLock") + private boolean shouldKillAppOnUnbind(ApplicationInfo app) { + // We don't ask system UID processes to be killed. + if (UserHandle.isCore(app.uid)) { + return false; + } + + // If the app is in restricted mode or if we're not sure if it is because our internal + // state is messed up, we need to avoid it being stuck in it. + if (mCurrentConnection == null || mCurrentConnection.inRestrictedMode) { + return true; + } + + // App was doing restore and asked to be killed afterwards. + return isBackupModeRestore(mCurrentConnection.backupMode) + && (app.flags & ApplicationInfo.FLAG_KILL_AFTER_RESTORE) != 0; + } + /** * Callback: a requested backup agent has been instantiated. This should only be called from * the {@link ActivityManager} when it's telling us that an agent is ready after a call to @@ -307,16 +355,16 @@ public class BackupAgentConnectionManager { */ private boolean shouldUseRestrictedBackupModeForPackage( @BackupAnnotations.OperationType int mode, String packageName) { - if (!Flags.enableRestrictedModeChanges()) { - return true; - } - // Key/Value apps are never put in restricted mode. if (mode == ApplicationThreadConstants.BACKUP_MODE_INCREMENTAL || mode == ApplicationThreadConstants.BACKUP_MODE_RESTORE) { return false; } + if (!Flags.enableRestrictedModeChanges()) { + return true; + } + try { PackageManager.Property property = mPackageManager.getPropertyAsUser( PackageManager.PROPERTY_USE_RESTRICTED_BACKUP_MODE, @@ -352,6 +400,11 @@ public class BackupAgentConnectionManager { return true; } + private static boolean isBackupModeRestore(int backupMode) { + return backupMode == ApplicationThreadConstants.BACKUP_MODE_RESTORE + || backupMode == ApplicationThreadConstants.BACKUP_MODE_RESTORE_FULL; + } + @VisibleForTesting Thread getThreadForCancellation(Runnable operation) { return new Thread(operation, /* operationName */ "agent-disconnected"); diff --git a/services/backup/java/com/android/server/backup/KeyValueAdbBackupEngine.java b/services/backup/java/com/android/server/backup/KeyValueAdbBackupEngine.java index 136bacdd6399..b343ec8e709b 100644 --- a/services/backup/java/com/android/server/backup/KeyValueAdbBackupEngine.java +++ b/services/backup/java/com/android/server/backup/KeyValueAdbBackupEngine.java @@ -300,7 +300,8 @@ public class KeyValueAdbBackupEngine { } private void cleanup() { - mBackupManagerService.tearDownAgentAndKill(mCurrentPackage.applicationInfo); + mBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + mCurrentPackage.applicationInfo, /* allowKill= */ true); mBlankStateName.delete(); mNewStateName.delete(); mBackupDataName.delete(); diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java index e085f6e63067..a90b693c5a1d 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java @@ -1998,39 +1998,6 @@ public class UserBackupManagerService { return mOperationStorage.isBackupOperationInProgress(); } - /** Unbind the backup agent and kill the app if it's a non-system app. */ - public void tearDownAgentAndKill(ApplicationInfo app) { - if (app == null) { - // Null means the system package, so just quietly move on. :) - return; - } - - try { - // unbind and tidy up even on timeout or failure, just in case - mActivityManager.unbindBackupAgent(app); - - // The agent was running with a stub Application object, so shut it down. - // !!! We hardcode the confirmation UI's package name here rather than use a - // manifest flag! TODO something less direct. - if (!UserHandle.isCore(app.uid) - && !app.packageName.equals("com.android.backupconfirm")) { - if (MORE_DEBUG) { - Slog.d(TAG, addUserIdToLogMessage(mUserId, "Killing agent host process")); - } - mActivityManager.killApplicationProcess(app.processName, app.uid); - } else { - if (MORE_DEBUG) { - Slog.d( - TAG, - addUserIdToLogMessage( - mUserId, "Not killing after operation: " + app.processName)); - } - } - } catch (RemoteException e) { - Slog.d(TAG, addUserIdToLogMessage(mUserId, "Lost app trying to shut down")); - } - } - // ----- Full-data backup scheduling ----- /** diff --git a/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java b/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java index b98cb1086680..cf617a523bec 100644 --- a/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java +++ b/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java @@ -323,7 +323,8 @@ public class FullBackupEngine { private void tearDown() { if (mPkg != null) { - backupManagerService.tearDownAgentAndKill(mPkg.applicationInfo); + backupManagerService.getBackupAgentConnectionManager().unbindAgent( + mPkg.applicationInfo, /* allowKill= */ true); } } } diff --git a/services/backup/java/com/android/server/backup/fullbackup/PerformAdbBackupTask.java b/services/backup/java/com/android/server/backup/fullbackup/PerformAdbBackupTask.java index dc6709141b25..0ba0d710af38 100644 --- a/services/backup/java/com/android/server/backup/fullbackup/PerformAdbBackupTask.java +++ b/services/backup/java/com/android/server/backup/fullbackup/PerformAdbBackupTask.java @@ -503,7 +503,8 @@ public class PerformAdbBackupTask extends FullBackupTask implements BackupRestor Slog.w(TAG, "adb backup cancel of " + target); } if (target != null) { - mUserBackupManagerService.tearDownAgentAndKill(mCurrentTarget.applicationInfo); + mUserBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + target.applicationInfo, /* allowKill= */ true); } mOperationStorage.removeOperation(mCurrentOpToken); } diff --git a/services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java b/services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java index 65730c9591a8..799494831f19 100644 --- a/services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java +++ b/services/backup/java/com/android/server/backup/fullbackup/PerformFullTransportBackupTask.java @@ -596,8 +596,8 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba // from the preflight pass. If we got as far as preflight, we now need // to tear down the target process. if (mBackupRunner != null) { - mUserBackupManagerService.tearDownAgentAndKill( - currentPackage.applicationInfo); + mUserBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + currentPackage.applicationInfo, /* allowKill= */ true); } // ... and continue looping. } else if (backupPackageStatus == BackupTransport.TRANSPORT_QUOTA_EXCEEDED) { @@ -609,7 +609,8 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba EventLog.writeEvent(EventLogTags.FULL_BACKUP_QUOTA_EXCEEDED, packageName); } - mUserBackupManagerService.tearDownAgentAndKill(currentPackage.applicationInfo); + mUserBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + currentPackage.applicationInfo, /* allowKill= */ true); // Do nothing, clean up, and continue looping. } else if (backupPackageStatus == BackupTransport.AGENT_ERROR) { BackupObserverUtils @@ -617,7 +618,8 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba BackupManager.ERROR_AGENT_FAILURE); Slog.w(TAG, "Application failure for package: " + packageName); EventLog.writeEvent(EventLogTags.BACKUP_AGENT_FAILURE, packageName); - mUserBackupManagerService.tearDownAgentAndKill(currentPackage.applicationInfo); + mUserBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + currentPackage.applicationInfo, /* allowKill= */ true); // Do nothing, clean up, and continue looping. } else if (backupPackageStatus == BackupManager.ERROR_BACKUP_CANCELLED) { BackupObserverUtils @@ -626,7 +628,8 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba Slog.w(TAG, "Backup cancelled. package=" + packageName + ", cancelAll=" + mCancelAll); EventLog.writeEvent(EventLogTags.FULL_BACKUP_CANCELLED, packageName); - mUserBackupManagerService.tearDownAgentAndKill(currentPackage.applicationInfo); + mUserBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + currentPackage.applicationInfo, /* allowKill= */ true); // Do nothing, clean up, and continue looping. } else if (backupPackageStatus != BackupTransport.TRANSPORT_OK) { BackupObserverUtils @@ -636,7 +639,8 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba EventLog.writeEvent(EventLogTags.FULL_BACKUP_TRANSPORT_FAILURE); // Abort entire backup pass. backupRunStatus = BackupManager.ERROR_TRANSPORT_ABORTED; - mUserBackupManagerService.tearDownAgentAndKill(currentPackage.applicationInfo); + mUserBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + currentPackage.applicationInfo, /* allowKill= */ true); return; } else { // Success! @@ -1005,7 +1009,8 @@ public class PerformFullTransportBackupTask extends FullBackupTask implements Ba mIsCancelled = true; // Cancel tasks spun off by this task. mUserBackupManagerService.handleCancel(mEphemeralToken, cancelAll); - mUserBackupManagerService.tearDownAgentAndKill(mTarget.applicationInfo); + mUserBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + mTarget.applicationInfo, /* allowKill= */ true); // Free up everyone waiting on this task and its children. mPreflightLatch.countDown(); mBackupLatch.countDown(); diff --git a/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java b/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java index 82232a653858..689c43a1cc96 100644 --- a/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java +++ b/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java @@ -1303,7 +1303,7 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable { // For PM metadata (for which applicationInfo is null) there is no agent-bound state. if (mCurrentPackage.applicationInfo != null) { mBackupManagerService.getBackupAgentConnectionManager().unbindAgent( - mCurrentPackage.applicationInfo); + mCurrentPackage.applicationInfo, /* allowKill= */ false); } mAgent = null; } diff --git a/services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java b/services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java index b59e860f81fe..237ffa3bdb21 100644 --- a/services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java +++ b/services/backup/java/com/android/server/backup/restore/FullRestoreEngine.java @@ -727,7 +727,8 @@ public class FullRestoreEngine extends RestoreEngine { latch.await(); } - mBackupManagerService.tearDownAgentAndKill(app); + mBackupManagerService.getBackupAgentConnectionManager().unbindAgent( + app, /* allowKill= */ true); } catch (RemoteException e) { Slog.d(TAG, "Lost app trying to shut down"); } diff --git a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java index e5c7e5cce757..dad84c86deef 100644 --- a/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java +++ b/services/backup/java/com/android/server/backup/restore/PerformUnifiedRestoreTask.java @@ -51,7 +51,6 @@ import android.os.ParcelFileDescriptor; import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; -import android.os.UserHandle; import android.provider.Settings; import android.util.ArraySet; import android.util.EventLog; @@ -1487,49 +1486,10 @@ public class PerformUnifiedRestoreTask implements BackupRestoreTask { // If this wasn't the PM pseudopackage, tear down the agent side if (mCurrentPackage.applicationInfo != null) { - // unbind and tidy up even on timeout or failure - try { - backupManagerService - .getActivityManager() - .unbindBackupAgent(mCurrentPackage.applicationInfo); - - // The agent was probably running with a stub Application object, - // which isn't a valid run mode for the main app logic. Shut - // down the app so that next time it's launched, it gets the - // usual full initialization. Note that this is only done for - // full-system restores: when a single app has requested a restore, - // it is explicitly not killed following that operation. - // - // We execute this kill when these conditions hold: - // 1. it's not a system-uid process, - // 2. the app did not request its own restore (mTargetPackage == null), and - // either - // 3a. the app is a full-data target (TYPE_FULL_STREAM) or - // b. the app does not state android:killAfterRestore="false" in its manifest - final int appFlags = mCurrentPackage.applicationInfo.flags; - final boolean killAfterRestore = - !UserHandle.isCore(mCurrentPackage.applicationInfo.uid) - && ((mRestoreDescription.getDataType() - == RestoreDescription.TYPE_FULL_STREAM) - || ((appFlags & ApplicationInfo.FLAG_KILL_AFTER_RESTORE) - != 0)); - - if (mTargetPackage == null && killAfterRestore) { - if (DEBUG) { - Slog.d( - TAG, - "Restore complete, killing host process of " - + mCurrentPackage.applicationInfo.processName); - } - backupManagerService - .getActivityManager() - .killApplicationProcess( - mCurrentPackage.applicationInfo.processName, - mCurrentPackage.applicationInfo.uid); - } - } catch (RemoteException e) { - // can't happen; we run in the same process as the activity manager - } + // If mTargetPackage is not null it means the app requested its own restore, in which + // case we won't allow the app to be killed. + backupManagerService.getBackupAgentConnectionManager().unbindAgent( + mCurrentPackage.applicationInfo, /* allowKill= */ mTargetPackage == null); } // The caller is responsible for reestablishing the state machine; our diff --git a/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java b/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java index aeb1ba93f049..de16b7ee8126 100644 --- a/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java +++ b/services/robotests/backup/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java @@ -865,7 +865,8 @@ public class KeyValueBackupTaskTest { runTask(task); verify(mBackupManagerService).setWorkSource(null); - verify(mBackupAgentConnectionManager).unbindAgent(argThat(applicationInfo(PACKAGE_1))); + verify(mBackupAgentConnectionManager).unbindAgent(argThat(applicationInfo(PACKAGE_1)), + eq(false)); } @Test @@ -1101,7 +1102,8 @@ public class KeyValueBackupTaskTest { runTask(task); verify(agentMock.agentBinder).fail(any()); - verify(mBackupAgentConnectionManager).unbindAgent(argThat(applicationInfo(PACKAGE_1))); + verify(mBackupAgentConnectionManager).unbindAgent(argThat(applicationInfo(PACKAGE_1)), + eq(false)); } @Test @@ -1423,7 +1425,7 @@ public class KeyValueBackupTaskTest { assertCleansUpFiles(mTransport, PM_PACKAGE); // We don't unbind PM verify(mBackupAgentConnectionManager, never()).unbindAgent( - argThat(applicationInfo(PM_PACKAGE))); + argThat(applicationInfo(PM_PACKAGE)), eq(false)); } @Test @@ -1445,7 +1447,7 @@ public class KeyValueBackupTaskTest { runTask(task); verify(mBackupAgentConnectionManager, never()).unbindAgent( - argThat(applicationInfo(PM_PACKAGE))); + argThat(applicationInfo(PM_PACKAGE)), eq(false)); } @Test @@ -1651,7 +1653,7 @@ public class KeyValueBackupTaskTest { InOrder inOrder = inOrder(agentMock.agent, mBackupAgentConnectionManager); inOrder.verify(agentMock.agent).onQuotaExceeded(anyLong(), eq(1234L)); inOrder.verify(mBackupAgentConnectionManager).unbindAgent( - argThat(applicationInfo(PACKAGE_1))); + argThat(applicationInfo(PACKAGE_1)), eq(false)); } @Test @@ -2983,7 +2985,8 @@ public class KeyValueBackupTaskTest { private void assertCleansUpFilesAndAgent(TransportData transport, PackageData packageData) { assertCleansUpFiles(transport, packageData); - verify(mBackupAgentConnectionManager).unbindAgent(argThat(applicationInfo(packageData))); + verify(mBackupAgentConnectionManager).unbindAgent(argThat(applicationInfo(packageData)), + eq(false)); } private void assertCleansUpFiles(TransportData transport, PackageData packageData) { diff --git a/services/tests/mockingservicestests/src/com/android/server/backup/BackupAgentConnectionManagerTest.java b/services/tests/mockingservicestests/src/com/android/server/backup/BackupAgentConnectionManagerTest.java index 2461e9e79acf..8aaa72339c5b 100644 --- a/services/tests/mockingservicestests/src/com/android/server/backup/BackupAgentConnectionManagerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/backup/BackupAgentConnectionManagerTest.java @@ -119,6 +119,8 @@ public class BackupAgentConnectionManagerTest { mTestApplicationInfo = new ApplicationInfo(); mTestApplicationInfo.packageName = TEST_PACKAGE; + mTestApplicationInfo.processName = TEST_PACKAGE; + mTestApplicationInfo.uid = Process.FIRST_APPLICATION_UID + 1; mBackupAgentResult = null; mTestThread = null; @@ -134,8 +136,8 @@ public class BackupAgentConnectionManagerTest { @Test public void bindToAgentSynchronous_amReturnsFailure_returnsNullAndClearsPendingBackups() throws Exception { - when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), - anyInt(), anyBoolean())).thenReturn(false); + when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), anyInt(), + anyBoolean())).thenReturn(false); IBackupAgent result = mConnectionManager.bindToAgentSynchronous(mTestApplicationInfo, ApplicationThreadConstants.BACKUP_MODE_FULL, BackupDestination.CLOUD); @@ -147,8 +149,8 @@ public class BackupAgentConnectionManagerTest { @Test public void bindToAgentSynchronous_agentDisconnectedCalled_returnsNullAndClearsPendingBackups() throws Exception { - when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), - anyInt(), anyBoolean())).thenReturn(true); + when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), anyInt(), + anyBoolean())).thenReturn(true); // This is so that IBackupAgent.Stub.asInterface() works. when(mBackupAgentStub.queryLocalInterface(any())).thenReturn(mBackupAgentStub); when(mConnectionManager.getCallingUid()).thenReturn(Process.SYSTEM_UID); @@ -172,24 +174,7 @@ public class BackupAgentConnectionManagerTest { @Test public void bindToAgentSynchronous_agentConnectedCalled_returnsBackupAgent() throws Exception { - when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), - anyInt(), anyBoolean())).thenReturn(true); - // This is so that IBackupAgent.Stub.asInterface() works. - when(mBackupAgentStub.queryLocalInterface(any())).thenReturn(mBackupAgentStub); - when(mConnectionManager.getCallingUid()).thenReturn(Process.SYSTEM_UID); - - // This is going to block until it receives the callback so we need to run it on a - // separate thread. - Thread testThread = new Thread(() -> setBackupAgentResult( - mConnectionManager.bindToAgentSynchronous(mTestApplicationInfo, - ApplicationThreadConstants.BACKUP_MODE_FULL, BackupDestination.CLOUD)), - "backup-agent-connection-manager-test"); - testThread.start(); - // Give the testThread a head start, otherwise agentConnected() might run before - // bindToAgentSynchronous() is called. - Thread.sleep(500); - mConnectionManager.agentConnected(TEST_PACKAGE, mBackupAgentStub); - testThread.join(); + bindAndConnectToTestAppAgent(ApplicationThreadConstants.BACKUP_MODE_FULL); assertThat(mBackupAgentResult).isEqualTo(mBackupAgentStub); verify(mActivityManagerInternal, never()).clearPendingBackup(anyInt()); @@ -198,8 +183,8 @@ public class BackupAgentConnectionManagerTest { @Test public void bindToAgentSynchronous_unexpectedAgentConnected_doesNotReturnWrongAgent() throws Exception { - when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), - anyInt(), anyBoolean())).thenReturn(true); + when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), anyInt(), + anyBoolean())).thenReturn(true); // This is so that IBackupAgent.Stub.asInterface() works. when(mBackupAgentStub.queryLocalInterface(any())).thenReturn(mBackupAgentStub); when(mConnectionManager.getCallingUid()).thenReturn(Process.SYSTEM_UID); @@ -390,11 +375,75 @@ public class BackupAgentConnectionManagerTest { @Test public void unbindAgent_callsAmUnbindBackupAgent() throws Exception { - mConnectionManager.unbindAgent(mTestApplicationInfo); + mConnectionManager.unbindAgent(mTestApplicationInfo, /* allowKill= */ false); verify(mActivityManager).unbindBackupAgent(eq(mTestApplicationInfo)); } + @Test + public void unbindAgent_doNotAllowKill_doesNotKillApp() throws Exception { + mConnectionManager.unbindAgent(mTestApplicationInfo, /* allowKill= */ false); + + verify(mActivityManager, never()).killApplicationProcess(any(), anyInt()); + } + + @Test + public void unbindAgent_allowKill_isCoreApp_doesNotKillApp() throws Exception { + mTestApplicationInfo.uid = 1000; + + mConnectionManager.unbindAgent(mTestApplicationInfo, /* allowKill= */ true); + + verify(mActivityManager, never()).killApplicationProcess(any(), anyInt()); + } + + @Test + public void unbindAgent_allowKill_notCurrentConnection_killsApp() throws Exception { + mConnectionManager.unbindAgent(mTestApplicationInfo, /* allowKill= */ true); + + verify(mActivityManager).killApplicationProcess(eq(TEST_PACKAGE), anyInt()); + } + + @Test + public void unbindAgent_allowKill_inRestrictedMode_killsApp() throws Exception { + bindAndConnectToTestAppAgent(ApplicationThreadConstants.BACKUP_MODE_FULL); + + mConnectionManager.unbindAgent(mTestApplicationInfo, /* allowKill= */ true); + + verify(mActivityManager).killApplicationProcess(eq(TEST_PACKAGE), anyInt()); + } + + @Test + public void unbindAgent_allowKill_notInRestrictedMode_doesNotKillApp() throws Exception { + bindAndConnectToTestAppAgent(ApplicationThreadConstants.BACKUP_MODE_INCREMENTAL); + + mConnectionManager.unbindAgent(mTestApplicationInfo, /* allowKill= */ true); + + verify(mActivityManager, never()).killApplicationProcess(any(), anyInt()); + } + + @Test + public void unbindAgent_allowKill_isRestore_noKillAfterRestore_doesNotKillApp() + throws Exception { + bindAndConnectToTestAppAgent(ApplicationThreadConstants.BACKUP_MODE_RESTORE); + mTestApplicationInfo.flags = 0; + verify(mActivityManager).bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), anyInt(), + /* useRestrictedMode= */ eq(false)); + + mConnectionManager.unbindAgent(mTestApplicationInfo, /* allowKill= */ true); + + verify(mActivityManager, never()).killApplicationProcess(any(), anyInt()); + } + + @Test + public void unbindAgent_allowKill_isRestore_killAfterRestore_killsApp() throws Exception { + bindAndConnectToTestAppAgent(ApplicationThreadConstants.BACKUP_MODE_RESTORE); + mTestApplicationInfo.flags |= ApplicationInfo.FLAG_KILL_AFTER_RESTORE; + + mConnectionManager.unbindAgent(mTestApplicationInfo, /* allowKill= */ true); + + verify(mActivityManager).killApplicationProcess(eq(TEST_PACKAGE), anyInt()); + } + // Needed because variables can't be assigned directly inside lambdas in Java. private void setBackupAgentResult(IBackupAgent result) { mBackupAgentResult = result; @@ -404,4 +453,23 @@ public class BackupAgentConnectionManagerTest { private void setTestThread(Thread thread) { mTestThread = thread; } + + private void bindAndConnectToTestAppAgent(int backupMode) throws Exception { + when(mActivityManager.bindBackupAgent(eq(TEST_PACKAGE), anyInt(), anyInt(), anyInt(), + anyBoolean())).thenReturn(true); + // This is going to block until it receives the callback so we need to run it on a + // separate thread. + Thread testThread = new Thread(() -> setBackupAgentResult( + mConnectionManager.bindToAgentSynchronous(mTestApplicationInfo, backupMode, + BackupDestination.CLOUD)), "backup-agent-connection-manager-test"); + testThread.start(); + // Give the testThread a head start, otherwise agentConnected() might run before + // bindToAgentSynchronous() is called. + Thread.sleep(500); + when(mConnectionManager.getCallingUid()).thenReturn(Process.SYSTEM_UID); + // This is so that IBackupAgent.Stub.asInterface() works. + when(mBackupAgentStub.queryLocalInterface(any())).thenReturn(mBackupAgentStub); + mConnectionManager.agentConnected(TEST_PACKAGE, mBackupAgentStub); + testThread.join(); + } } |