Merge "Unsubscribe listeners when stopping UserBackupManagerService and remove unused code"

5 files changed, 29 insertions, 43 deletions

diff --git a/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java b/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java
index ae850ac8b0d5..0e99b34e9dd1 100644
--- a/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java
+++ b/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java
@@ -33,8 +33,6 @@ import com.android.internal.annotations.VisibleForTesting;
  * are represented as a comma-delimited key value list.
  */
 public class BackupAgentTimeoutParameters extends KeyValueSettingObserver {
-    private static final String TAG = "BackupAgentTimeout";
-
     @VisibleForTesting
     public static final String SETTING = Settings.Global.BACKUP_AGENT_TIMEOUT_PARAMETERS;
 
diff --git a/services/backup/java/com/android/server/backup/BackupManagerService.java b/services/backup/java/com/android/server/backup/BackupManagerService.java
index 302e3ffa5200..8df25b5a6465 100644
--- a/services/backup/java/com/android/server/backup/BackupManagerService.java
+++ b/services/backup/java/com/android/server/backup/BackupManagerService.java
@@ -39,7 +39,6 @@ import android.content.IntentFilter;
 import android.content.pm.PackageManager;
 import android.os.Binder;
 import android.os.FileUtils;
-import android.os.HandlerThread;
 import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
 import android.os.Trace;
@@ -87,7 +86,6 @@ public class BackupManagerService {
 
     private final Context mContext;
     private final Trampoline mTrampoline;
-    private final HandlerThread mBackupThread;
 
     // Keeps track of all unlocked users registered with this service. Indexed by user id.
     private final SparseArray<UserBackupManagerService> mServiceUsers = new SparseArray<>();
@@ -107,11 +105,9 @@ public class BackupManagerService {
     };
 
     /** Instantiate a new instance of {@link BackupManagerService}. */
-    public BackupManagerService(
-            Context context, Trampoline trampoline, HandlerThread backupThread) {
+    public BackupManagerService(Context context, Trampoline trampoline) {
         mContext = checkNotNull(context);
         mTrampoline = checkNotNull(trampoline);
-        mBackupThread = checkNotNull(backupThread);
 
         // Set up our transport options.
         SystemConfig systemConfig = SystemConfig.getInstance();
diff --git a/services/backup/java/com/android/server/backup/Trampoline.java b/services/backup/java/com/android/server/backup/Trampoline.java
index a9b292b37903..53bbac4d41fe 100644
--- a/services/backup/java/com/android/server/backup/Trampoline.java
+++ b/services/backup/java/com/android/server/backup/Trampoline.java
@@ -110,15 +110,15 @@ public class Trampoline extends IBackupManager.Stub {
     private final Object mStateLock = new Object();
 
     private volatile BackupManagerService mService;
-    private HandlerThread mHandlerThread;
-    private Handler mHandler;
+    private final Handler mHandler;
 
     public Trampoline(Context context) {
         mContext = context;
         mGlobalDisable = isBackupDisabled();
-        mHandlerThread = new HandlerThread(BACKUP_THREAD, Process.THREAD_PRIORITY_BACKGROUND);
-        mHandlerThread.start();
-        mHandler = new Handler(mHandlerThread.getLooper());
+        HandlerThread handlerThread =
+                new HandlerThread(BACKUP_THREAD, Process.THREAD_PRIORITY_BACKGROUND);
+        handlerThread.start();
+        mHandler = new Handler(handlerThread.getLooper());
         mUserManager = UserManager.get(context);
     }
 
@@ -232,7 +232,7 @@ public class Trampoline extends IBackupManager.Stub {
     }
 
     protected BackupManagerService createBackupManagerService() {
-        return new BackupManagerService(mContext, this, mHandlerThread);
+        return new BackupManagerService(mContext, this);
     }
 
     protected void postToHandler(Runnable runnable) {
diff --git a/services/backup/java/com/android/server/backup/UserBackupManagerService.java b/services/backup/java/com/android/server/backup/UserBackupManagerService.java
index c0af99c30542..ed4e59611c22 100644
--- a/services/backup/java/com/android/server/backup/UserBackupManagerService.java
+++ b/services/backup/java/com/android/server/backup/UserBackupManagerService.java
@@ -367,6 +367,9 @@ public class UserBackupManagerService {
     private long mCurrentToken = 0;
     @Nullable private File mAncestralSerialNumberFile;
 
+    private final ContentObserver mSetupObserver;
+    private final BroadcastReceiver mRunBackupReceiver;
+    private final BroadcastReceiver mRunInitReceiver;
 
     /**
      * Creates an instance of {@link UserBackupManagerService} and initializes state for it. This
@@ -493,11 +496,11 @@ public class UserBackupManagerService {
         mAutoRestore = Settings.Secure.getIntForUser(resolver,
                 Settings.Secure.BACKUP_AUTO_RESTORE, 1, userId) != 0;
 
-        ContentObserver setupObserver = new SetupObserver(this, mBackupHandler);
+        mSetupObserver = new SetupObserver(this, mBackupHandler);
         resolver.registerContentObserver(
                 Settings.Secure.getUriFor(Settings.Secure.USER_SETUP_COMPLETE),
                 /* notifyForDescendents */ false,
-                setupObserver,
+                mSetupObserver,
                 mUserId);
 
         mBaseStateDir = checkNotNull(baseStateDir, "baseStateDir cannot be null");
@@ -516,21 +519,21 @@ public class UserBackupManagerService {
         mBackupPasswordManager = new BackupPasswordManager(mContext, mBaseStateDir, mRng);
 
         // Receivers for scheduled backups and transport initialization operations.
-        BroadcastReceiver runBackupReceiver = new RunBackupReceiver(this);
+        mRunBackupReceiver = new RunBackupReceiver(this);
         IntentFilter filter = new IntentFilter();
         filter.addAction(RUN_BACKUP_ACTION);
         context.registerReceiverAsUser(
-                runBackupReceiver,
+                mRunBackupReceiver,
                 UserHandle.of(userId),
                 filter,
                 android.Manifest.permission.BACKUP,
                 /* scheduler */ null);
 
-        BroadcastReceiver runInitReceiver = new RunInitializeReceiver(this);
+        mRunInitReceiver = new RunInitializeReceiver(this);
         filter = new IntentFilter();
         filter.addAction(RUN_INITIALIZE_ACTION);
         context.registerReceiverAsUser(
-                runInitReceiver,
+                mRunInitReceiver,
                 UserHandle.of(userId),
                 filter,
                 android.Manifest.permission.BACKUP,
@@ -599,6 +602,12 @@ public class UserBackupManagerService {
 
     /** Cleans up state when the user of this service is stopped. */
     void tearDownService() {
+        mAgentTimeoutParameters.stop();
+        mConstants.stop();
+        mContext.getContentResolver().unregisterContentObserver(mSetupObserver);
+        mContext.unregisterReceiver(mRunBackupReceiver);
+        mContext.unregisterReceiver(mRunInitReceiver);
+        mContext.unregisterReceiver(mPackageTrackingReceiver);
         mUserBackupThread.quit();
     }
 
@@ -747,7 +756,7 @@ public class UserBackupManagerService {
 
     @VisibleForTesting
     BroadcastReceiver getPackageTrackingReceiver() {
-        return mBroadcastReceiver;
+        return mPackageTrackingReceiver;
     }
 
     @Nullable
@@ -788,10 +797,6 @@ public class UserBackupManagerService {
         mPendingInits.clear();
     }
 
-    public PerformFullTransportBackupTask getRunningFullBackupTask() {
-        return mRunningFullBackupTask;
-    }
-
     public void setRunningFullBackupTask(
             PerformFullTransportBackupTask runningFullBackupTask) {
         mRunningFullBackupTask = runningFullBackupTask;
@@ -874,7 +879,7 @@ public class UserBackupManagerService {
         filter.addAction(Intent.ACTION_PACKAGE_CHANGED);
         filter.addDataScheme("package");
         mContext.registerReceiverAsUser(
-                mBroadcastReceiver,
+                mPackageTrackingReceiver,
                 UserHandle.of(mUserId),
                 filter,
                 /* broadcastPermission */ null,
@@ -885,7 +890,7 @@ public class UserBackupManagerService {
         sdFilter.addAction(Intent.ACTION_EXTERNAL_APPLICATIONS_AVAILABLE);
         sdFilter.addAction(Intent.ACTION_EXTERNAL_APPLICATIONS_UNAVAILABLE);
         mContext.registerReceiverAsUser(
-                mBroadcastReceiver,
+                mPackageTrackingReceiver,
                 UserHandle.of(mUserId),
                 sdFilter,
                 /* broadcastPermission */ null,
@@ -1158,7 +1163,7 @@ public class UserBackupManagerService {
      * A {@link BroadcastReceiver} tracking changes to packages and sd cards in order to update our
      * internal bookkeeping.
      */
-    private BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {
+    private BroadcastReceiver mPackageTrackingReceiver = new BroadcastReceiver() {
         public void onReceive(Context context, Intent intent) {
             if (MORE_DEBUG) {
                 Slog.d(TAG, "Received broadcast " + intent);
diff --git a/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceTest.java b/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceTest.java
index 0cb21d014054..be93d6ba5352 100644
--- a/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceTest.java
+++ b/services/robotests/backup/src/com/android/server/backup/BackupManagerServiceTest.java
@@ -21,7 +21,6 @@ import static android.Manifest.permission.DUMP;
 import static android.Manifest.permission.INTERACT_ACROSS_USERS_FULL;
 import static android.Manifest.permission.PACKAGE_USAGE_STATS;
 
-import static com.android.server.backup.testing.BackupManagerServiceTestUtils.startBackupThread;
 import static com.android.server.backup.testing.TransportData.backupTransport;
 
 import static com.google.common.truth.Truth.assertThat;
@@ -133,8 +132,7 @@ public class BackupManagerServiceTest {
                 () ->
                         new BackupManagerService(
                                 /* context */ null,
-                                new Trampoline(mContext),
-                                startBackupThread(null)));
+                                new Trampoline(mContext)));
     }
 
     /** Test that the constructor handles {@code null} parameters. */
@@ -144,17 +142,7 @@ public class BackupManagerServiceTest {
                 NullPointerException.class,
                 () ->
                         new BackupManagerService(
-                                mContext, /* trampoline */ null, startBackupThread(null)));
-    }
-
-    /** Test that the constructor handles {@code null} parameters. */
-    @Test
-    public void testConstructor_withNullBackupThread_throws() throws Exception {
-        expectThrows(
-                NullPointerException.class,
-                () ->
-                        new BackupManagerService(
-                                mContext, new Trampoline(mContext), /* backupThread */ null));
+                                mContext, /* trampoline */ null));
     }
 
     /** Test that the service registers users. */
@@ -1581,8 +1569,7 @@ public class BackupManagerServiceTest {
 
     private BackupManagerService createService() {
         mShadowContext.grantPermissions(BACKUP);
-        return new BackupManagerService(
-                mContext, new Trampoline(mContext), startBackupThread(null));
+        return new BackupManagerService(mContext, new Trampoline(mContext));
     }
 
     private BackupManagerService createServiceAndRegisterUser(