Merge "Make service start whitelisting for background activity starts…

… persist after service start." into qt-dev
AOSP-10-CAF · May 10, 2019 · 7b0194c · 7b0194c
2 parents 07b0f13 + b8137aa
commit 7b0194c
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 25 deletions.
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
@@ -634,28 +634,13 @@ ComponentName startServiceLocked(IApplicationThread caller, Intent service, Stri
         }
 
         if (allowBackgroundActivityStarts) {
-            r.setHasStartedWhitelistingBgActivityStarts(true);
-            scheduleCleanUpHasStartedWhitelistingBgActivityStartsLocked(r);
+            r.whitelistBgActivityStartsOnServiceStart();
         }
 
         ComponentName cmp = startServiceInnerLocked(smap, service, r, callerFg, addToStarting);
         return cmp;
     }
 
-    private void scheduleCleanUpHasStartedWhitelistingBgActivityStartsLocked(ServiceRecord r) {
-        // if there's a request pending from the past, drop it before scheduling a new one
-        if (r.startedWhitelistingBgActivityStartsCleanUp == null) {
-            r.startedWhitelistingBgActivityStartsCleanUp = () -> {
-                synchronized(mAm) {
-                    r.setHasStartedWhitelistingBgActivityStarts(false);
-                }
-            };
-        }
-        mAm.mHandler.removeCallbacks(r.startedWhitelistingBgActivityStartsCleanUp);
-        mAm.mHandler.postDelayed(r.startedWhitelistingBgActivityStartsCleanUp,
-                mAm.mConstants.SERVICE_BG_ACTIVITY_START_TIMEOUT);
-    }
-
     private boolean requestStartTargetPermissionsReviewIfNeededLocked(ServiceRecord r,
             String callingPackage, int callingUid, Intent service, boolean callerFg,
             final int userId) {

diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java
@@ -135,7 +135,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN
     // allowBackgroundActivityStarts=true to startServiceLocked()?
     private boolean mHasStartedWhitelistingBgActivityStarts;
     // used to clean up the state of hasStartedWhitelistingBgActivityStarts after a timeout
-    Runnable startedWhitelistingBgActivityStartsCleanUp;
+    private Runnable mStartedWhitelistingBgActivityStartsCleanUp;
+    private ProcessRecord mAppForStartedWhitelistingBgActivityStarts;
 
     String stringName;      // caching of toString
 
@@ -541,14 +542,35 @@ public void makeRestarting(int memFactor, long now) {
 
     public void setProcess(ProcessRecord _proc) {
         if (_proc != null) {
+            // We're starting a new process for this service, but a previous one is whitelisted.
+            // Remove that whitelisting now (unless the new process is the same as the previous one,
+            // which is a common case).
+            if (mAppForStartedWhitelistingBgActivityStarts != null) {
+                if (mAppForStartedWhitelistingBgActivityStarts != _proc) {
+                    mAppForStartedWhitelistingBgActivityStarts
+                            .removeAllowBackgroundActivityStartsToken(this);
+                    ams.mHandler.removeCallbacks(mStartedWhitelistingBgActivityStartsCleanUp);
+                }
+                mAppForStartedWhitelistingBgActivityStarts = null;
+            }
+            if (mHasStartedWhitelistingBgActivityStarts) {
+                // Make sure the cleanup callback knows about the new process.
+                mAppForStartedWhitelistingBgActivityStarts = _proc;
+            }
             if (mHasStartedWhitelistingBgActivityStarts
                     || mHasBindingWhitelistingBgActivityStarts) {
                 _proc.addAllowBackgroundActivityStartsToken(this);
             } else {
                 _proc.removeAllowBackgroundActivityStartsToken(this);
             }
-        } else if (app != null) {
-            app.removeAllowBackgroundActivityStartsToken(this);
+        }
+        if (app != null && app != _proc) {
+            // If the old app is whitelisted because of a service start, leave it whitelisted until
+            // the cleanup callback runs. Otherwise we can remove it from the whitelist immediately
+            // (it can't be bound now).
+            if (!mHasStartedWhitelistingBgActivityStarts) {
+                app.removeAllowBackgroundActivityStartsToken(this);
+            }
             app.updateBoundClientUids();
         }
         app = _proc;
@@ -616,10 +638,7 @@ void updateHasBindingWhitelistingBgActivityStarts() {
                 break;
             }
         }
-        if (mHasBindingWhitelistingBgActivityStarts != hasWhitelistingBinding) {
-            mHasBindingWhitelistingBgActivityStarts = hasWhitelistingBinding;
-            updateParentProcessBgActivityStartsWhitelistingToken();
-        }
+        setHasBindingWhitelistingBgActivityStarts(hasWhitelistingBinding);
     }
 
     void setHasBindingWhitelistingBgActivityStarts(boolean newValue) {
@@ -629,7 +648,42 @@ void setHasBindingWhitelistingBgActivityStarts(boolean newValue) {
         }
     }
 
-    void setHasStartedWhitelistingBgActivityStarts(boolean newValue) {
+    /**
+     * Called when the service is started with allowBackgroundActivityStarts set. We whitelist
+     * it for background activity starts, setting up a callback to remove the whitelisting after a
+     * timeout. Note that the whitelisting persists for the process even if the service is
+     * subsequently stopped.
+     */
+    void whitelistBgActivityStartsOnServiceStart() {
+        setHasStartedWhitelistingBgActivityStarts(true);
+
+        // This callback is stateless, so we create it once when we first need it.
+        if (mStartedWhitelistingBgActivityStartsCleanUp == null) {
+            mStartedWhitelistingBgActivityStartsCleanUp = () -> {
+                synchronized (ams) {
+                    if (app == mAppForStartedWhitelistingBgActivityStarts) {
+                        // The process we whitelisted is still running the service. We remove
+                        // the started whitelisting, but it may still be whitelisted via bound
+                        // connections.
+                        setHasStartedWhitelistingBgActivityStarts(false);
+                    } else  if (mAppForStartedWhitelistingBgActivityStarts != null) {
+                        // The process we whitelisted is not running the service. It therefore
+                        // can't be bound so we can unconditionally remove the whitelist.
+                        mAppForStartedWhitelistingBgActivityStarts
+                                .removeAllowBackgroundActivityStartsToken(ServiceRecord.this);
+                    }
+                    mAppForStartedWhitelistingBgActivityStarts = null;
+                }
+            };
+        }
+
+        // if there's a request pending from the past, drop it before scheduling a new one
+        ams.mHandler.removeCallbacks(mStartedWhitelistingBgActivityStartsCleanUp);
+        ams.mHandler.postDelayed(mStartedWhitelistingBgActivityStartsCleanUp,
+                ams.mConstants.SERVICE_BG_ACTIVITY_START_TIMEOUT);
+    }
+
+    private void setHasStartedWhitelistingBgActivityStarts(boolean newValue) {
         if (mHasStartedWhitelistingBgActivityStarts != newValue) {
             mHasStartedWhitelistingBgActivityStarts = newValue;
             updateParentProcessBgActivityStartsWhitelistingToken();
@@ -650,7 +704,7 @@ private void updateParentProcessBgActivityStartsWhitelistingToken() {
             return;
         }
         if (mHasStartedWhitelistingBgActivityStarts || mHasBindingWhitelistingBgActivityStarts) {
-            // if the token is already there it's safe to "re-add it" - we're deadling with
+            // if the token is already there it's safe to "re-add it" - we're dealing with
             // a set of Binder objects
             app.addAllowBackgroundActivityStartsToken(this);
         } else {