Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5dcff706 authored by Bernardo Rufino's avatar Bernardo Rufino
Browse files

[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
parent 38304b59

services/backup/java/com/android/server/backup/keyvalue/KeyValueBackupTask.java

+9 −4
Original line number Diff line number Diff line
@@ -708,8 +708,6 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable { 
            } else {

                throw TaskException.create();

            }

        } finally {

            mBlankStateFile.delete();

        }

        checkAgentResult(packageInfo, agentResult);

    }
@@ -1037,8 +1035,13 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable { 


    private void cleanUpAgent(@StateTransaction int stateTransaction) {

        applyStateTransaction(stateTransaction);

        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 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable { 
                mNewStateFile.renameTo(mSavedStateFile);

                break;

            case StateTransaction.DISCARD_NEW:

                if (mNewStateFile != null) {

                    mNewStateFile.delete();

                }

                break;

            case StateTransaction.DISCARD_ALL:

                mSavedStateFile.delete();

services/robotests/src/com/android/server/backup/keyvalue/KeyValueBackupTaskTest.java

+50 −0
Original line number Diff line number Diff line
@@ -725,6 +725,39 @@ public class KeyValueBackupTaskTest { 
        assertBackupPendingFor(PACKAGE_1);

    }



    @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);
@@ -742,6 +775,23 @@ public class KeyValueBackupTaskTest { 
        assertBackupPendingFor(PACKAGE_1);

    }



    @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);