From 801f2ec567fbeb9d2a76b2c062ef3cc39c0e4d2a Mon Sep 17 00:00:00 2001 From: Chandan Nath Date: Fri, 4 Jan 2019 01:27:01 +0000 Subject: [PATCH 1/2] [Multi-user] Make backup dirs user-specific 1. For system user, functionality remains almost (see 2) exactly the same. 2. Change the full backup dir which is used only to write temporary manifest and meta dirs by the system process when doing full backup. This is so that we dont have to worry unnecessarily about yet another dir. Bug: 120424138 Test: 1) atest RunBackupFrameworksServicesRoboTests 2) atest $(find \ frameworks/base/services/tests/servicestests/src/com/android/server/backup \ -name '*Test.java') 3) atest CtsBackupTestCases 4) atest CtsBackupHostTestCases 5) atest GtsBackupTestCases 6) atest GtsBackupHostTestCases 7) 'adb shell bmgr' enabled/backupnow flow Change-Id: I9a33547c9595a86b62869ee731d4c75a029922e8 --- .../server/backup/UserBackupManagerFiles.java | 26 ++++++++++++------- .../backup/UserBackupManagerService.java | 10 +------ .../backup/fullbackup/FullBackupEngine.java | 23 ++++++++++------ 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerFiles.java b/services/backup/java/com/android/server/backup/UserBackupManagerFiles.java index a0feaf9c8d15d..aabd41a611a10 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerFiles.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerFiles.java @@ -17,29 +17,35 @@ package com.android.server.backup; import android.os.Environment; +import android.os.UserHandle; import java.io.File; /** Directories used for user specific backup/restore persistent state and book-keeping. */ -public final class UserBackupManagerFiles { +final class UserBackupManagerFiles { // Name of the directories the service stores bookkeeping data under. private static final String BACKUP_PERSISTENT_DIR = "backup"; private static final String BACKUP_STAGING_DIR = "backup_stage"; + private static File getBaseDir(int userId) { + return Environment.getDataSystemCeDirectory(userId); + } + static File getBaseStateDir(int userId) { - // TODO (b/120424138) this should be per user + if (userId != UserHandle.USER_SYSTEM) { + return new File(getBaseDir(userId), BACKUP_PERSISTENT_DIR); + } + // TODO (b/120424138) remove if clause above and use same logic for system user. + // simultaneously, copy below dir to new system user dir return new File(Environment.getDataDirectory(), BACKUP_PERSISTENT_DIR); } static File getDataDir(int userId) { - // TODO (b/120424138) this should be per user - // This dir on /cache is managed directly in init.rc + if (userId != UserHandle.USER_SYSTEM) { + return new File(getBaseDir(userId), BACKUP_STAGING_DIR); + } + // TODO (b/120424138) remove if clause above and use same logic for system user. Since this + // is a staging dir, we dont need to copy below dir to new system user dir return new File(Environment.getDownloadCacheDirectory(), BACKUP_STAGING_DIR); } - - /** Directory used by full backup engine to store state. */ - public static File getFullBackupEngineFilesDir(int userId) { - // TODO (b/120424138) this should be per user - return new File("/data/system"); - } } diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java index 198a258a33b69..ce90704aa6c6b 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java @@ -551,7 +551,7 @@ public class UserBackupManagerService { mUserBackupThread.quit(); } - public int getUserId() { + public @UserIdInt int getUserId() { return mUserId; } @@ -567,10 +567,6 @@ public class UserBackupManagerService { return mContext; } - public void setContext(Context context) { - mContext = context; - } - public PackageManager getPackageManager() { return mPackageManager; } @@ -583,10 +579,6 @@ public class UserBackupManagerService { return mPackageManagerBinder; } - public void setPackageManagerBinder(IPackageManager packageManagerBinder) { - mPackageManagerBinder = packageManagerBinder; - } - public IActivityManager getActivityManager() { return mActivityManager; } 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 45ca2af54b525..03d4e97cb9985 100644 --- a/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java +++ b/services/backup/java/com/android/server/backup/fullbackup/FullBackupEngine.java @@ -34,14 +34,12 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.os.ParcelFileDescriptor; import android.os.RemoteException; -import android.os.UserHandle; import android.util.Slog; import com.android.internal.util.Preconditions; import com.android.server.AppWidgetBackupBridge; import com.android.server.backup.BackupAgentTimeoutParameters; import com.android.server.backup.BackupRestoreTask; -import com.android.server.backup.UserBackupManagerFiles; import com.android.server.backup.UserBackupManagerService; import com.android.server.backup.remote.RemoteCall; import com.android.server.backup.utils.FullBackupUtils; @@ -78,21 +76,21 @@ public class FullBackupEngine { private final File mFilesDir; FullBackupRunner( + UserBackupManagerService userBackupManagerService, PackageInfo packageInfo, IBackupAgent agent, ParcelFileDescriptor pipe, int token, boolean includeApks) throws IOException { - // TODO: http://b/22388012 - mUserId = UserHandle.USER_SYSTEM; + mUserId = userBackupManagerService.getUserId(); mPackageManager = backupManagerService.getPackageManager(); mPackage = packageInfo; mAgent = agent; mPipe = ParcelFileDescriptor.dup(pipe.getFileDescriptor()); mToken = token; mIncludeApks = includeApks; - mFilesDir = UserBackupManagerFiles.getFullBackupEngineFilesDir(mUserId); + mFilesDir = userBackupManagerService.getDataDir(); } @Override @@ -155,9 +153,12 @@ public class FullBackupEngine { backupManagerService.getBackupManagerBinder(), mTransportFlags); } catch (IOException e) { - Slog.e(TAG, "Error running full backup for " + mPackage.packageName); + Slog.e(TAG, "Error running full backup for " + mPackage.packageName, e); } catch (RemoteException e) { - Slog.e(TAG, "Remote agent vanished during full backup of " + mPackage.packageName); + Slog.e( + TAG, + "Remote agent vanished during full backup of " + mPackage.packageName, + e); } finally { try { mPipe.close(); @@ -233,7 +234,13 @@ public class FullBackupEngine { pipes = ParcelFileDescriptor.createPipe(); FullBackupRunner runner = - new FullBackupRunner(mPkg, mAgent, pipes[1], mOpToken, mIncludeApks); + new FullBackupRunner( + backupManagerService, + mPkg, + mAgent, + pipes[1], + mOpToken, + mIncludeApks); pipes[1].close(); // the runner has dup'd it pipes[1] = null; Thread t = new Thread(runner, "app-data-runner"); From b3bf0c63e22c539c44173beb2376358a21772c83 Mon Sep 17 00:00:00 2001 From: Chandan Nath Date: Fri, 4 Jan 2019 22:28:49 +0000 Subject: [PATCH 2/2] backup: no-op code cleanup 1. remove unused methods,imports,members 2. make members private/final as much as possible Bug: 122371936 Test: 1) atest RunBackupFrameworksServicesRoboTests 2) atest $(find \ frameworks/base/services/tests/servicestests/src/com/android/server/backup \ -name '*Test.java') 3) atest CtsBackupTestCases 4) atest CtsBackupHostTestCases Change-Id: Ibd37ae124be87487b0b30bcb92b012c1713f6555 --- .../android/server/backup/FullBackupJob.java | 5 +- .../backup/KeyValueAdbRestoreEngine.java | 11 +-- .../backup/UserBackupManagerService.java | 91 ++++--------------- 3 files changed, 23 insertions(+), 84 deletions(-) diff --git a/services/backup/java/com/android/server/backup/FullBackupJob.java b/services/backup/java/com/android/server/backup/FullBackupJob.java index 82638b4ecee4d..5708b1c368225 100644 --- a/services/backup/java/com/android/server/backup/FullBackupJob.java +++ b/services/backup/java/com/android/server/backup/FullBackupJob.java @@ -24,15 +24,12 @@ import android.content.ComponentName; import android.content.Context; public class FullBackupJob extends JobService { - private static final String TAG = "FullBackupJob"; - private static final boolean DEBUG = true; - private static ComponentName sIdleService = new ComponentName("android", FullBackupJob.class.getName()); private static final int JOB_ID = 0x5038; - JobParameters mParams; + private JobParameters mParams; public static void schedule(Context ctx, long minDelay, BackupManagerConstants constants) { JobScheduler js = (JobScheduler) ctx.getSystemService(Context.JOB_SCHEDULER_SERVICE); diff --git a/services/backup/java/com/android/server/backup/KeyValueAdbRestoreEngine.java b/services/backup/java/com/android/server/backup/KeyValueAdbRestoreEngine.java index bed520e9f068a..3184bd87601a1 100644 --- a/services/backup/java/com/android/server/backup/KeyValueAdbRestoreEngine.java +++ b/services/backup/java/com/android/server/backup/KeyValueAdbRestoreEngine.java @@ -13,8 +13,6 @@ import android.os.ParcelFileDescriptor; import android.os.RemoteException; import android.util.Slog; -import com.android.server.backup.restore.PerformAdbRestoreTask; - import libcore.io.IoUtils; import java.io.File; @@ -42,11 +40,10 @@ public class KeyValueAdbRestoreEngine implements Runnable { private final UserBackupManagerService mBackupManagerService; private final File mDataDir; - FileMetadata mInfo; - PerformAdbRestoreTask mRestoreTask; - ParcelFileDescriptor mInFD; - IBackupAgent mAgent; - int mToken; + private final FileMetadata mInfo; + private final ParcelFileDescriptor mInFD; + private final IBackupAgent mAgent; + private final int mToken; public KeyValueAdbRestoreEngine(UserBackupManagerService backupManagerService, File dataDir, FileMetadata info, ParcelFileDescriptor inFD, IBackupAgent agent, diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java index ce90704aa6c6b..696942ab2c493 100644 --- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java @@ -208,8 +208,8 @@ public class UserBackupManagerService { public static final String RUN_BACKUP_ACTION = "android.app.backup.intent.RUN"; public static final String RUN_INITIALIZE_ACTION = "android.app.backup.intent.INIT"; - public static final String BACKUP_FINISHED_ACTION = "android.intent.action.BACKUP_FINISHED"; - public static final String BACKUP_FINISHED_PACKAGE_EXTRA = "packageName"; + private static final String BACKUP_FINISHED_ACTION = "android.intent.action.BACKUP_FINISHED"; + private static final String BACKUP_FINISHED_PACKAGE_EXTRA = "packageName"; // Bookkeeping of in-flight operations. The operation token is the index of the entry in the // pending operations list. @@ -247,25 +247,25 @@ public class UserBackupManagerService { private final TransportManager mTransportManager; private final HandlerThread mUserBackupThread; - private Context mContext; - private PackageManager mPackageManager; - private IPackageManager mPackageManagerBinder; - private IActivityManager mActivityManager; + private final Context mContext; + private final PackageManager mPackageManager; + private final IPackageManager mPackageManagerBinder; + private final IActivityManager mActivityManager; private PowerManager mPowerManager; - private AlarmManager mAlarmManager; - private IStorageManager mStorageManager; - private BackupManagerConstants mConstants; - private PowerManager.WakeLock mWakelock; - private BackupHandler mBackupHandler; + private final AlarmManager mAlarmManager; + private final IStorageManager mStorageManager; + private final BackupManagerConstants mConstants; + private final PowerManager.WakeLock mWakelock; + private final BackupHandler mBackupHandler; - private IBackupManager mBackupManagerBinder; + private final IBackupManager mBackupManagerBinder; private boolean mEnabled; // access to this is synchronized on 'this' private boolean mSetupComplete; private boolean mAutoRestore; - private PendingIntent mRunBackupIntent; - private PendingIntent mRunInitIntent; + private final PendingIntent mRunBackupIntent; + private final PendingIntent mRunInitIntent; private final ArraySet mPendingInits = new ArraySet<>(); // transport names @@ -273,7 +273,7 @@ public class UserBackupManagerService { private final SparseArray> mBackupParticipants = new SparseArray<>(); // Backups that we haven't started yet. Keys are package names. - private HashMap mPendingBackups = new HashMap<>(); + private final HashMap mPendingBackups = new HashMap<>(); // locking around the pending-backup management private final Object mQueueLock = new Object(); @@ -340,7 +340,7 @@ public class UserBackupManagerService { private final SparseArray mCurrentOperations = new SparseArray<>(); private final Object mCurrentOpLock = new Object(); private final Random mTokenGenerator = new Random(); - final AtomicInteger mNextToken = new AtomicInteger(); + private final AtomicInteger mNextToken = new AtomicInteger(); // Where we keep our journal files and other bookkeeping. private final File mBaseStateDir; @@ -348,7 +348,7 @@ public class UserBackupManagerService { private final File mJournalDir; @Nullable private DataChangedJournal mJournal; - private File mFullBackupScheduleFile; + private final File mFullBackupScheduleFile; // Keep a log of all the apps we've ever backed up. private ProcessedPackagesJournal mProcessedPackagesJournal; @@ -571,10 +571,6 @@ public class UserBackupManagerService { return mPackageManager; } - public void setPackageManager(PackageManager packageManager) { - mPackageManager = packageManager; - } - public IPackageManager getPackageManagerBinder() { return mPackageManagerBinder; } @@ -583,27 +579,15 @@ public class UserBackupManagerService { return mActivityManager; } - public void setActivityManager(IActivityManager activityManager) { - mActivityManager = activityManager; - } - public AlarmManager getAlarmManager() { return mAlarmManager; } - public void setAlarmManager(AlarmManager alarmManager) { - mAlarmManager = alarmManager; - } - @VisibleForTesting void setPowerManager(PowerManager powerManager) { mPowerManager = powerManager; } - public void setBackupManagerBinder(IBackupManager backupManagerBinder) { - mBackupManagerBinder = backupManagerBinder; - } - public TransportManager getTransportManager() { return mTransportManager; } @@ -638,35 +622,18 @@ public class UserBackupManagerService { mWakelock.setWorkSource(workSource); } - public void setWakelock(PowerManager.WakeLock wakelock) { - mWakelock = wakelock; - } - public Handler getBackupHandler() { return mBackupHandler; } - public void setBackupHandler(BackupHandler backupHandler) { - mBackupHandler = backupHandler; - } - public PendingIntent getRunInitIntent() { return mRunInitIntent; } - public void setRunInitIntent(PendingIntent runInitIntent) { - mRunInitIntent = runInitIntent; - } - public HashMap getPendingBackups() { return mPendingBackups; } - public void setPendingBackups( - HashMap pendingBackups) { - mPendingBackups = pendingBackups; - } - public Object getQueueLock() { return mQueueLock; } @@ -679,10 +646,6 @@ public class UserBackupManagerService { mBackupRunning = backupRunning; } - public long getLastBackupPass() { - return mLastBackupPass; - } - public void setLastBackupPass(long lastBackupPass) { mLastBackupPass = lastBackupPass; } @@ -691,10 +654,6 @@ public class UserBackupManagerService { return mClearDataLock; } - public boolean isClearingData() { - return mClearingData; - } - public void setClearingData(boolean clearingData) { mClearingData = clearingData; } @@ -715,11 +674,6 @@ public class UserBackupManagerService { return mActiveRestoreSession; } - public void setActiveRestoreSession( - ActiveRestoreSession activeRestoreSession) { - mActiveRestoreSession = activeRestoreSession; - } - public SparseArray getCurrentOperations() { return mCurrentOperations; } @@ -753,18 +707,10 @@ public class UserBackupManagerService { return mRng; } - public Set getAncestralPackages() { - return mAncestralPackages; - } - public void setAncestralPackages(Set ancestralPackages) { mAncestralPackages = ancestralPackages; } - public long getAncestralToken() { - return mAncestralToken; - } - public void setAncestralToken(long ancestralToken) { mAncestralToken = ancestralToken; } @@ -3129,8 +3075,7 @@ public class UserBackupManagerService { synchronized (mAgentConnectLock) { if (Binder.getCallingUid() == Process.SYSTEM_UID) { Slog.d(TAG, "agentConnected pkg=" + packageName + " agent=" + agentBinder); - IBackupAgent agent = IBackupAgent.Stub.asInterface(agentBinder); - mConnectedAgent = agent; + mConnectedAgent = IBackupAgent.Stub.asInterface(agentBinder); mConnecting = false; } else { Slog.w(TAG, "Non-system process uid=" + Binder.getCallingUid()