From 73e65ba47e613d453e4b69587f5035d21985190c Mon Sep 17 00:00:00 2001
From: Felipe Leme <felipeal@google.com>
Date: Wed, 13 Mar 2019 10:57:09 -0700
Subject: Keep ContentCapturePerUserService alive while the package is being
 updated.

Test: manual verification (cannot be tested using CTS because it would kill the test process)
Test: atest CtsContentCaptureServiceTestCases

Bug: 126266412
Fixes: 128466656

Change-Id: I73e89f41b58615070c38103fa2f1fa04ac015dca
---
 .../ContentCaptureManagerService.java              |  2 +-
 .../server/infra/AbstractMasterSystemService.java  | 87 +++++++++++++++++++---
 2 files changed, 78 insertions(+), 11 deletions(-)

diff --git a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
index c75b4c6f113c..9995d8e4d893 100644
--- a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
+++ b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
@@ -115,7 +115,7 @@ public final class ContentCaptureManagerService extends
     public ContentCaptureManagerService(@NonNull Context context) {
         super(context, new FrameworkResourcesServiceNameResolver(context,
                 com.android.internal.R.string.config_defaultContentCaptureService),
-                UserManager.DISALLOW_CONTENT_CAPTURE);
+                UserManager.DISALLOW_CONTENT_CAPTURE, /* refreshServiceOnPackageUpdate=*/ false);
         DeviceConfig.addOnPropertyChangedListener(DeviceConfig.NAMESPACE_CONTENT_CAPTURE,
                 ActivityThread.currentApplication().getMainExecutor(),
                 (namespace, key, value) -> onDeviceConfigChange(key, value));
diff --git a/services/core/java/com/android/server/infra/AbstractMasterSystemService.java b/services/core/java/com/android/server/infra/AbstractMasterSystemService.java
index d2ccfcc732a9..4cafd4ba40c1 100644
--- a/services/core/java/com/android/server/infra/AbstractMasterSystemService.java
+++ b/services/core/java/com/android/server/infra/AbstractMasterSystemService.java
@@ -125,9 +125,27 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
     @GuardedBy("mLock")
     private final SparseArray<S> mServicesCache = new SparseArray<>();
 
+    /**
+     * Whether the per-user service should be removed from the cache when its apk is updated.
+     */
+    private final boolean mRefreshServiceOnPackageUpdate;
+
+    /**
+     * Name of the service's package that was active but then was removed because its package
+     * update.
+     *
+     * <p>It's a temporary state set / used by the {@link PackageMonitor} implementation, but
+     * defined here so it can be dumped.
+     */
+    @GuardedBy("mLock")
+    private String mLastActivePackageName;
+
     /**
      * Default constructor.
      *
+     * <p>When using this constructor, the {@link AbstractPerUserSystemService} is removed from
+     * the cache (and re-added) when the service package is updated.
+     *
      * @param context system context.
      * @param serviceNameResolver resolver for
      * {@link com.android.internal.infra.AbstractRemoteService} instances, or
@@ -139,8 +157,32 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
     protected AbstractMasterSystemService(@NonNull Context context,
             @Nullable ServiceNameResolver serviceNameResolver,
             @Nullable String disallowProperty) {
+        this(context, serviceNameResolver, disallowProperty,
+                /* refreshServiceOnPackageUpdate=*/ true);
+    }
+
+    /**
+     * Full constructor.
+     *
+     * @param context system context.
+     * @param serviceNameResolver resolver for
+     * {@link com.android.internal.infra.AbstractRemoteService} instances, or
+     * {@code null} when the service doesn't bind to remote services.
+     * @param disallowProperty when not {@code null}, defines a {@link UserManager} restriction that
+     *        disables the service. <b>NOTE: </b> you'll also need to add it to
+     *        {@code UserRestrictionsUtils.USER_RESTRICTIONS}.
+     * @param refreshServiceOnPackageUpdate when {@code true}, the
+     *        {@link AbstractPerUserSystemService} is removed from the cache (and re-added) when the
+     *        service package is updated; when {@code false}, the service is untouched during the
+     *        update.
+     */
+    protected AbstractMasterSystemService(@NonNull Context context,
+            @Nullable ServiceNameResolver serviceNameResolver,
+            @Nullable String disallowProperty, boolean refreshServiceOnPackageUpdate) {
         super(context);
 
+        mRefreshServiceOnPackageUpdate = refreshServiceOnPackageUpdate;
+
         mServiceNameResolver = serviceNameResolver;
         if (mServiceNameResolver != null) {
             mServiceNameResolver
@@ -553,6 +595,8 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
             final int size = mServicesCache.size();
             pw.print(prefix); pw.print("Debug: "); pw.print(realDebug);
             pw.print(" Verbose: "); pw.println(realVerbose);
+            pw.print(" Refresh on package update: "); pw.println(mRefreshServiceOnPackageUpdate);
+            pw.print(" Last active service on update: "); pw.println(mLastActivePackageName);
             if (mServiceNameResolver != null) {
                 pw.print(prefix); pw.print("Name resolver: ");
                 mServiceNameResolver.dumpShort(pw); pw.println();
@@ -590,21 +634,42 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
     }
 
     private void startTrackingPackageChanges() {
-        PackageMonitor monitor = new PackageMonitor() {
+        final PackageMonitor monitor = new PackageMonitor() {
+
             @Override
-            public void onSomePackagesChanged() {
+            public void onPackageUpdateStarted(String packageName, int uid) {
                 synchronized (mLock) {
-                    updateCachedServiceLocked(getChangingUserId());
+                    final String activePackageName = getActiveServicePackageNameLocked();
+                    if (packageName.equals(activePackageName)) {
+                        final int userId = getChangingUserId();
+                        if (mRefreshServiceOnPackageUpdate) {
+                            if (debug) {
+                                Slog.d(mTag, "Removing service for user " + userId
+                                        + " because package " + activePackageName
+                                        + " is being updated");
+                            }
+                            mLastActivePackageName = activePackageName;
+                            removeCachedServiceLocked(userId);
+                        } else {
+                            if (debug) {
+                                Slog.d(mTag, "Holding service for user " + userId
+                                        + " while package " + activePackageName
+                                        + " is being updated");
+                            }
+                        }
+                    }
                 }
             }
 
             @Override
             public void onPackageUpdateFinished(String packageName, int uid) {
                 synchronized (mLock) {
-                    final String activePackageName = getActiveServicePackageName();
-                    if (packageName.equals(activePackageName)) {
-                        removeCachedServiceLocked(getChangingUserId());
-                    } else {
+                    String activePackageName = getActiveServicePackageNameLocked();
+                    if (activePackageName == null) {
+                        activePackageName = mLastActivePackageName;
+                        mLastActivePackageName = null;
+                    }
+                    if (!packageName.equals(activePackageName)) {
                         handlePackageUpdateLocked(packageName);
                     }
                 }
@@ -630,7 +695,7 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
             public boolean onHandleForceStop(Intent intent, String[] packages,
                     int uid, boolean doit) {
                 synchronized (mLock) {
-                    final String activePackageName = getActiveServicePackageName();
+                    final String activePackageName = getActiveServicePackageNameLocked();
                     for (String pkg : packages) {
                         if (pkg.equals(activePackageName)) {
                             if (!doit) {
@@ -646,7 +711,9 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
             }
 
             private void handleActiveServiceRemoved(@UserIdInt int userId) {
-                removeCachedServiceLocked(userId);
+                synchronized (mLock) {
+                    removeCachedServiceLocked(userId);
+                }
                 final String serviceSettingsProperty = getServiceSettingsProperty();
                 if (serviceSettingsProperty != null) {
                     Settings.Secure.putStringForUser(getContext().getContentResolver(),
@@ -654,7 +721,7 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
                 }
             }
 
-            private String getActiveServicePackageName() {
+            private String getActiveServicePackageNameLocked() {
                 final int userId = getChangingUserId();
                 final S service = peekServiceForUserLocked(userId);
                 if (service == null) {
-- 
cgit v1.2.3-59-g8ed1b