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

Commit f5121c96 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge changes from topic "kv-refactor-agent-throws"

* changes:
  [KV] Consider throwing BackupAgent a failure
  [KV] Remove states
parents 44dcfc39 6a422d6a
Loading
Loading
Loading
Loading
+8 −1
Original line number Diff line number Diff line
@@ -125,6 +125,11 @@ public abstract class BackupAgent extends ContextWrapper {
    private static final String TAG = "BackupAgent";
    private static final boolean DEBUG = false;

    /** @hide */
    public static final int RESULT_SUCCESS = 0;
    /** @hide */
    public static final int RESULT_ERROR = -1;

    /** @hide */
    public static final int TYPE_EOF = 0;

@@ -955,8 +960,10 @@ public abstract class BackupAgent extends ContextWrapper {
            BackupDataOutput output = new BackupDataOutput(
                    data.getFileDescriptor(), quotaBytes, transportFlags);

            long result = RESULT_ERROR;
            try {
                BackupAgent.this.onBackup(oldState, output, newState);
                result = RESULT_SUCCESS;
            } catch (IOException ex) {
                Log.d(TAG, "onBackup (" + BackupAgent.this.getClass().getName() + ") threw", ex);
                throw new RuntimeException(ex);
@@ -971,7 +978,7 @@ public abstract class BackupAgent extends ContextWrapper {

                Binder.restoreCallingIdentity(ident);
                try {
                    callbackBinder.operationComplete(0);
                    callbackBinder.operationComplete(result);
                } catch (RemoteException e) {
                    // we'll time out anyway, so we're safe
                }
+10 −10
Original line number Diff line number Diff line
@@ -52,11 +52,9 @@ import java.util.List;
//       verify calls to this object. Add these and more assertions to the test of this class.
@VisibleForTesting
public class KeyValueBackupReporter {
    @VisibleForTesting
    static final String TAG = "KeyValueBackupTask";
    @VisibleForTesting static final String TAG = "KeyValueBackupTask";
    private static final boolean DEBUG = BackupManagerService.DEBUG;
    @VisibleForTesting
    static final boolean MORE_DEBUG = BackupManagerService.MORE_DEBUG || true;
    @VisibleForTesting static final boolean MORE_DEBUG = BackupManagerService.MORE_DEBUG || true;

    static void onNewThread(String threadName) {
        if (DEBUG) {
@@ -132,12 +130,6 @@ public class KeyValueBackupReporter {
        Slog.e(TAG, "Error during PM metadata backup", e);
    }

    void onEmptyQueue() {
        if (MORE_DEBUG) {
            Slog.i(TAG, "Queue now empty");
        }
    }

    void onStartPackageBackup(String packageName) {
        Slog.d(TAG, "Starting key-value backup of " + packageName);
    }
@@ -363,6 +355,14 @@ public class KeyValueBackupReporter {
                                null, BackupManagerMonitor.EXTRA_LOG_CANCEL_ALL, true));
    }

    void onAgentResultError(@Nullable PackageInfo packageInfo) {
        String packageName = getPackageName(packageInfo);
        BackupObserverUtils.sendBackupOnPackageResult(
                mObserver, packageName, BackupManager.ERROR_AGENT_FAILURE);
        EventLog.writeEvent(EventLogTags.BACKUP_AGENT_FAILURE, packageName, "result error");
        Slog.w(TAG, "Agent " + packageName + " error in onBackup()");
    }

    private String getPackageName(@Nullable PackageInfo packageInfo) {
        return (packageInfo != null) ? packageInfo.packageName : "no_package_yet";
    }
+72 −103
Original line number Diff line number Diff line
@@ -251,15 +251,15 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
    @Nullable private final DataChangedJournal mJournal;
    @Nullable private PerformFullTransportBackupTask mFullBackupTask;

    private IBackupAgent mAgentBinder;
    private PackageInfo mCurrentPackage;
    private File mSavedStateFile;
    private File mBackupDataFile;
    private File mNewStateFile;
    private ParcelFileDescriptor mSavedState;
    private ParcelFileDescriptor mBackupData;
    private ParcelFileDescriptor mNewState;
    private int mStatus;
    @Nullable private IBackupAgent mAgentBinder;
    @Nullable private PackageInfo mCurrentPackage;
    @Nullable private File mSavedStateFile;
    @Nullable private File mBackupDataFile;
    @Nullable private File mNewStateFile;
    @Nullable private ParcelFileDescriptor mSavedState;
    @Nullable private ParcelFileDescriptor mBackupData;
    @Nullable private ParcelFileDescriptor mNewState;

    /**
     * This {@link ConditionVariable} is used to signal that the cancel operation has been
@@ -331,44 +331,45 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
    public void run() {
        Process.setThreadPriority(THREAD_PRIORITY);

        BackupState state = startTask();
        while (state == BackupState.RUNNING_QUEUE || state == BackupState.BACKUP_PM) {
            if (mCancelled) {
                state = BackupState.CANCELLED;
            }
            switch (state) {
                case BACKUP_PM:
                    state = backupPm();
                    break;
                case RUNNING_QUEUE:
                    Pair<BackupState, RemoteResult> stateAndResult = extractNextAgentData();
                    state = stateAndResult.first;
                    if (state == null) {
                        state = handleAgentResult(stateAndResult.second);
                    }
                    break;
        boolean processQueue = startTask();
        while (processQueue && !mQueue.isEmpty() && !mCancelled) {
            String packageName = mQueue.remove(0);
            if (PM_PACKAGE.equals(packageName)) {
                processQueue = backupPm();
            } else {
                processQueue = backupPackage(packageName);
            }
        }
        finishTask();
    }

    private BackupState handleAgentResult(RemoteResult result) {
    /** Returns whether to consume next queue package. */
    private boolean handleAgentResult(@Nullable PackageInfo packageInfo, RemoteResult result) {
        if (result == RemoteResult.FAILED_THREAD_INTERRUPTED) {
            // Not an explicit cancel, we need to flag it.
            mCancelled = true;
            handleAgentCancelled();
            return BackupState.CANCELLED;
            mReporter.onAgentCancelled(packageInfo);
            errorCleanup();
            return false;
        }
        if (result == RemoteResult.FAILED_CANCELLED) {
            handleAgentCancelled();
            return BackupState.CANCELLED;
            mReporter.onAgentCancelled(packageInfo);
            errorCleanup();
            return false;
        }
        if (result == RemoteResult.FAILED_TIMED_OUT) {
            handleAgentTimeout();
            return BackupState.RUNNING_QUEUE;
            mReporter.onAgentTimedOut(packageInfo);
            errorCleanup();
            return true;
        }
        Preconditions.checkState(result.isPresent());
        long agentResult = result.get();
        if (agentResult == BackupAgent.RESULT_ERROR) {
            mReporter.onAgentResultError(packageInfo);
            errorCleanup();
            return true;
        }
        Preconditions.checkState(result.succeeded());
        return sendDataToTransport(result.get());
        return sendDataToTransport();
    }

    @Override
@@ -377,11 +378,12 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
    @Override
    public void operationComplete(long unusedResult) {}

    private BackupState startTask() {
    /** Returns whether to consume next queue package. */
    private boolean startTask() {
        synchronized (mBackupManagerService.getCurrentOpLock()) {
            if (mBackupManagerService.isBackupOperationInProgress()) {
                mReporter.onSkipBackup();
                return BackupState.FINAL;
                return false;
            }
        }

@@ -404,14 +406,18 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
        mAgentBinder = null;
        mStatus = BackupTransport.TRANSPORT_OK;

        // Sanity check: if the queue is empty we have no work to do.
        if (mOriginalQueue.isEmpty() && mPendingFullBackups.isEmpty()) {
        if (mQueue.isEmpty() && mPendingFullBackups.isEmpty()) {
            mReporter.onEmptyQueueAtStart();
            return BackupState.FINAL;
            return false;
        }

        // We only backup PM if it was explicitly in the queue or if it's incremental.
        boolean backupPm = mQueue.remove(PM_PACKAGE) || !mNonIncremental;
        if (backupPm) {
            mQueue.add(0, PM_PACKAGE);
        } else {
            mReporter.onSkipPm();
        }

        mReporter.onQueueReady(mQueue);
        File pmState = new File(mStateDirectory, PM_PACKAGE);
@@ -434,18 +440,14 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {

        if (mStatus != BackupTransport.TRANSPORT_OK) {
            mBackupManagerService.resetBackupState(mStateDirectory);
            return BackupState.FINAL;
        }

        if (!backupPm) {
            mReporter.onSkipPm();
            return BackupState.RUNNING_QUEUE;
            return false;
        }

        return BackupState.BACKUP_PM;
        return true;
    }

    private BackupState backupPm() {
    /** Returns whether to consume next queue package. */
    private boolean backupPm() {
        RemoteResult agentResult = null;
        try {
            mCurrentPackage = new PackageInfo();
@@ -466,31 +468,17 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {

        if (mStatus != BackupTransport.TRANSPORT_OK) {
            mBackupManagerService.resetBackupState(mStateDirectory);
            return BackupState.FINAL;
            return false;
        }

        Preconditions.checkNotNull(agentResult);
        return handleAgentResult(agentResult);
        return handleAgentResult(mCurrentPackage, agentResult);
    }

    /**
     * Returns either:
     *
     * <ul>
     *   <li>(next state, {@code null}): In case we failed to call the agent.
     *   <li>({@code null}, agent result): In case we successfully called the agent.
     * </ul>
     */
    private Pair<BackupState, RemoteResult> extractNextAgentData() {
        mStatus = BackupTransport.TRANSPORT_OK;

        if (mQueue.isEmpty()) {
            mReporter.onEmptyQueue();
            return Pair.create(BackupState.FINAL, null);
        }

        String packageName = mQueue.remove(0);
    /** Returns whether to consume next queue package. */
    private boolean backupPackage(String packageName) {
        mReporter.onStartPackageBackup(packageName);
        mStatus = BackupTransport.TRANSPORT_OK;

        // Verify that the requested app is eligible for key-value backup.
        RemoteResult agentResult = null;
@@ -502,19 +490,19 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
                // The manifest has changed. This won't happen again because the app won't be
                // requesting further backups.
                mReporter.onPackageNotEligibleForBackup(packageName);
                return Pair.create(BackupState.RUNNING_QUEUE, null);
                return true;
            }

            if (AppBackupUtils.appGetsFullBackup(mCurrentPackage)) {
                // Initially enqueued for key-value backup, but only supports full-backup now.
                mReporter.onPackageEligibleForFullBackup(packageName);
                return Pair.create(BackupState.RUNNING_QUEUE, null);
                return true;
            }

            if (AppBackupUtils.appIsStopped(applicationInfo)) {
                // Just as it won't receive broadcasts, we won't run it for backup.
                mReporter.onPackageStopped(packageName);
                return Pair.create(BackupState.RUNNING_QUEUE, null);
                return true;
            }

            try {
@@ -550,22 +538,22 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
                mReporter.onAgentError(packageName);
                mBackupManagerService.dataChangedImpl(packageName);
                mStatus = BackupTransport.TRANSPORT_OK;
                return Pair.create(BackupState.RUNNING_QUEUE, null);
                return true;
            }

            if (mStatus == BackupTransport.AGENT_UNKNOWN) {
                mReporter.onAgentUnknown(packageName);
                mStatus = BackupTransport.TRANSPORT_OK;
                return Pair.create(BackupState.RUNNING_QUEUE, null);
                return true;
            }

            // Transport-level failure, re-enqueue everything.
            revertTask();
            return Pair.create(BackupState.FINAL, null);
            return false;
        }

        // Success: caller will figure out the state based on call result
        return Pair.create(null, agentResult);
        Preconditions.checkNotNull(agentResult);
        return handleAgentResult(mCurrentPackage, agentResult);
    }

    private void finishTask() {
@@ -811,7 +799,8 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
        }
    }

    private BackupState sendDataToTransport(long agentResult) {
    /** Returns whether to consume next queue package. */
    private boolean sendDataToTransport() {
        Preconditions.checkState(mBackupData != null);

        String packageName = mCurrentPackage.packageName;
@@ -821,7 +810,7 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
        try {
            if (!validateBackupData(applicationInfo, mBackupDataFile)) {
                errorCleanup();
                return BackupState.RUNNING_QUEUE;
                return true;
            }
            writingWidgetData = true;
            writeWidgetPayloadIfAppropriate(mBackupData.getFileDescriptor(), packageName);
@@ -832,7 +821,7 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
                mReporter.onReadAgentDataError(packageName, e);
            }
            revertTask();
            return BackupState.FINAL;
            return false;
        }

        clearAgentState();
@@ -892,33 +881,31 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
        }
    }

    private BackupState handleTransportStatus(int status, String packageName, long size) {
    /** Returns whether to consume next queue package. */
    private boolean handleTransportStatus(int status, String packageName, long size) {
        if (status == BackupTransport.TRANSPORT_OK) {
            mReporter.onPackageBackupComplete(packageName, size);
            return BackupState.RUNNING_QUEUE;
            return true;
        }
        if (status == BackupTransport.TRANSPORT_PACKAGE_REJECTED) {
            mReporter.onPackageBackupRejected(packageName);
            return BackupState.RUNNING_QUEUE;
            return true;
        }
        if (status == BackupTransport.TRANSPORT_NON_INCREMENTAL_BACKUP_REQUIRED) {
            mReporter.onPackageBackupNonIncrementalRequired(mCurrentPackage);
            // Immediately retry the current package.
            if (PM_PACKAGE.equals(packageName)) {
                return BackupState.BACKUP_PM;
            }
            mQueue.add(0, packageName);
            return BackupState.RUNNING_QUEUE;
            return true;
        }
        if (status == BackupTransport.TRANSPORT_QUOTA_EXCEEDED) {
            mReporter.onPackageBackupQuotaExceeded(packageName);
            agentDoQuotaExceeded(mAgentBinder, packageName, size);
            return BackupState.RUNNING_QUEUE;
            return true;
        }
        // Any other error here indicates a transport-level failure.
        mReporter.onPackageBackupTransportFailure(packageName);
        revertTask();
        return BackupState.FINAL;
        return false;
    }

    private void agentDoQuotaExceeded(
@@ -1017,16 +1004,6 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
        mCancelAcknowledged.block();
    }

    private void handleAgentTimeout() {
        mReporter.onAgentTimedOut(mCurrentPackage);
        errorCleanup();
    }

    private void handleAgentCancelled() {
        mReporter.onAgentCancelled(mCurrentPackage);
        errorCleanup();
    }

    private void revertTask() {
        mReporter.onRevertTask();
        long delay;
@@ -1087,12 +1064,4 @@ public class KeyValueBackupTask implements BackupRestoreTask, Runnable {
        mPendingCall = null;
        return result;
    }

    private enum BackupState {
        INITIAL,
        BACKUP_PM,
        RUNNING_QUEUE,
        CANCELLED,
        FINAL
    }
}
+2 −2
Original line number Diff line number Diff line
@@ -23,7 +23,7 @@ import java.util.concurrent.CompletableFuture;

/**
 * An implementation of {@link IBackupCallback} that completes the {@link CompletableFuture}
 * provided in the constructor with a successful {@link RemoteResult}.
 * provided in the constructor with a present {@link RemoteResult}.
 */
public class FutureBackupCallback extends IBackupCallback.Stub {
    private final CompletableFuture<RemoteResult> mFuture;
@@ -34,6 +34,6 @@ public class FutureBackupCallback extends IBackupCallback.Stub {

    @Override
    public void operationComplete(long result) throws RemoteException {
        mFuture.complete(RemoteResult.successful(result));
        mFuture.complete(RemoteResult.of(result));
    }
}
+1 −1
Original line number Diff line number Diff line
@@ -83,7 +83,7 @@ public class RemoteCall {
     *
     * <ul>
     *   <li>The callback passed to {@link RemoteCallable} is called with the result. We return a
     *       successful {@link RemoteResult} with the result.
     *       present {@link RemoteResult} with the result.
     *   <li>Time-out happens. We return {@link RemoteResult#FAILED_TIMED_OUT}.
     *   <li>Someone calls {@link #cancel()} on this object. We return {@link
     *       RemoteResult#FAILED_CANCELLED}.
Loading