[KV] Only discard state if non-null
To avoid NPEs. Added tests. This design is not optimal but left any
re-design to be done when we move applyStateTransition() to more upper
levels, which is going to happen when we extract package-backup.
Bug: 117269444
Test: atest FrameworksServicesRoboTests
Test: 1. Set BackupApp to time-out on agent onCreate()
2. adb shell bmgr backupnow --non-incremental
com.google.android.apps.backupapp
3. Verify it doesn't crash
Change-Id: I7a3fd3d3d5a4b5931206564c197edd86b6321933
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 6904b3f..3a5232a 100644
--- a/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java
+++ b/services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java
@@ -708,8 +708,6 @@
} else {
throw TaskException.create();
}
- } finally {
- mBlankStateFile.delete();
}
checkAgentResult(packageInfo, agentResult);
}
@@ -1037,8 +1035,13 @@
private void cleanUpAgent(@StateTransaction int stateTransaction) {
applyStateTransaction(stateTransaction);
- mBackupDataFile.delete();
+ if (mBackupDataFile != null) {
+ mBackupDataFile.delete();
+ }
mBlankStateFile.delete();
+ mSavedStateFile = null;
+ mBackupDataFile = null;
+ mNewStateFile = null;
tryCloseFileDescriptor(mSavedState, "old state");
tryCloseFileDescriptor(mBackupData, "backup data");
tryCloseFileDescriptor(mNewState, "new state");
@@ -1059,7 +1062,9 @@
mNewStateFile.renameTo(mSavedStateFile);
break;
case StateTransaction.DISCARD_NEW:
- mNewStateFile.delete();
+ if (mNewStateFile != null) {
+ mNewStateFile.delete();
+ }
break;
case StateTransaction.DISCARD_ALL:
mSavedStateFile.delete();
diff --git a/services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java b/services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java
index fb57d68..7a847f3 100644
--- a/services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java
+++ b/services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java
@@ -726,6 +726,39 @@
}
@Test
+ public void testRunTask_whenSecondAgentUnavailable_commitsFirstAgentState() throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ AgentMock agentMock = setUpAgent(PACKAGE_1);
+ setUpAgent(PACKAGE_2.unavailable());
+ agentOnBackupDo(
+ agentMock,
+ (oldState, dataOutput, newState) -> {
+ writeData(dataOutput, "key", "data".getBytes());
+ writeState(newState, "newState".getBytes());
+ });
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, PACKAGE_1, PACKAGE_2);
+
+ runTask(task);
+
+ assertThat(Files.readAllBytes(getStateFile(mTransport, PACKAGE_1))).isEqualTo(
+ "newState".getBytes());
+ }
+
+ @Test
+ public void testRunTask_whenNonIncrementalAndAgentUnavailable() throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ setUpAgent(PACKAGE_1.unavailable());
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, true, PACKAGE_1);
+
+ runTask(task);
+
+ verify(mBackupManagerService).setWorkSource(null);
+ verify(mObserver).onResult(PACKAGE_1.packageName, ERROR_AGENT_FAILURE);
+ verify(mObserver).backupFinished(BackupManager.SUCCESS);
+ assertBackupPendingFor(PACKAGE_1);
+ }
+
+ @Test
public void testRunTask_whenBindToAgentThrowsSecurityException() throws Exception {
TransportMock transportMock = setUpInitializedTransport(mTransport);
setUpAgent(PACKAGE_1);
@@ -743,6 +776,23 @@
}
@Test
+ public void testRunTask_whenNonIncrementalAndBindToAgentThrowsSecurityException() throws Exception {
+ TransportMock transportMock = setUpInitializedTransport(mTransport);
+ setUpAgent(PACKAGE_1);
+ doThrow(SecurityException.class)
+ .when(mBackupManagerService)
+ .bindToAgentSynchronous(argThat(applicationInfo(PACKAGE_1)), anyInt());
+ KeyValueBackupTask task = createKeyValueBackupTask(transportMock, true, PACKAGE_1);
+
+ runTask(task);
+
+ verify(mBackupManagerService).setWorkSource(null);
+ verify(mObserver).onResult(PACKAGE_1.packageName, ERROR_AGENT_FAILURE);
+ verify(mObserver).backupFinished(BackupManager.SUCCESS);
+ assertBackupPendingFor(PACKAGE_1);
+ }
+
+ @Test
public void testRunTask_whenTransportGetBackupQuotaThrows_notifiesCorrectly() throws Exception {
TransportMock transportMock = setUpInitializedTransport(mTransport);
when(transportMock.transport.getBackupQuota(PACKAGE_1.packageName, false))