Merge "Fix issue #37306208: CTS: ServiceTest#testForegroundService..." into oc-dev

services/core/java/com/android/server/am/ActiveServices.java

@@ -756,7 +756,7 @@ public final class ActiveServices {
                 }
             }
             if (r.fgRequired) {
-                if (DEBUG_BACKGROUND_CHECK) {
+                if (DEBUG_SERVICE || DEBUG_BACKGROUND_CHECK) {
                     Slog.i(TAG, "Service called startForeground() as required: " + r);
                 }
                 r.fgRequired = false;
@@ -1334,16 +1334,19 @@ public final class ActiveServices {
         final ComponentName comp = service.getComponent();
         if (comp != null) {
             r = smap.mServicesByName.get(comp);
+            if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE, "Retrieved by component: " + r);
         }
         if (r == null && !isBindExternal) {
             Intent.FilterComparison filter = new Intent.FilterComparison(service);
             r = smap.mServicesByIntent.get(filter);
+            if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE, "Retrieved by intent: " + r);
         }
         if (r != null && (r.serviceInfo.flags & ServiceInfo.FLAG_EXTERNAL_SERVICE) != 0
                 && !callingPackage.equals(r.packageName)) {
             // If an external service is running within its own package, other packages
             // should not bind to that instance.
             r = null;
+            if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Whoops, can't use existing external service");
         }
         if (r == null) {
             try {
@@ -1403,12 +1406,14 @@ public final class ActiveServices {
                     sInfo.applicationInfo = mAm.getAppInfoForUser(sInfo.applicationInfo, userId);
                 }
                 r = smap.mServicesByName.get(name);
+                if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE,
+                        "Retrieved via pm by intent: " + r);
                 if (r == null && createIfNeeded) {
-                    Intent.FilterComparison filter
+                    final Intent.FilterComparison filter
                             = new Intent.FilterComparison(service.cloneFilter());
-                    ServiceRestarter res = new ServiceRestarter();
+                    final ServiceRestarter res = new ServiceRestarter();
-                    BatteryStatsImpl.Uid.Pkg.Serv ss = null;
+                    final BatteryStatsImpl.Uid.Pkg.Serv ss;
-                    BatteryStatsImpl stats = mAm.mBatteryStatsService.getActiveStatistics();
+                    final BatteryStatsImpl stats = mAm.mBatteryStatsService.getActiveStatistics();
                     synchronized (stats) {
                         ss = stats.getServiceStatsLocked(
                                 sInfo.applicationInfo.uid, sInfo.packageName,
@@ -1421,12 +1426,14 @@ public final class ActiveServices {
 
                     // Make sure this component isn't in the pending list.
                     for (int i=mPendingServices.size()-1; i>=0; i--) {
-                        ServiceRecord pr = mPendingServices.get(i);
+                        final ServiceRecord pr = mPendingServices.get(i);
                         if (pr.serviceInfo.applicationInfo.uid == sInfo.applicationInfo.uid
                                 && pr.name.equals(name)) {
+                            if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Remove pending: " + pr);
                             mPendingServices.remove(i);
                         }
                     }
+                    if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Retrieve created new service: " + r);
                 }
             } catch (RemoteException ex) {
                 // pm is in same process, this will never happen.
@@ -2114,7 +2121,28 @@ public final class ActiveServices {
             }
         }
 
-        if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Bringing down " + r + " " + r.intent);
+        // Check to see if the service had been started as foreground, but being
+        // brought down before actually showing a notification.  That is not allowed.
+        if (r.fgRequired) {
+            Slog.w(TAG_SERVICE, "Bringing down service while still waiting for start foreground: "
+                    + r);
+            r.fgRequired = false;
+            r.fgWaiting = false;
+            mAm.mHandler.removeMessages(
+                    ActivityManagerService.SERVICE_FOREGROUND_TIMEOUT_MSG, r);
+            if (r.app != null) {
+                Message msg = mAm.mHandler.obtainMessage(
+                        ActivityManagerService.SERVICE_FOREGROUND_CRASH_MSG);
+                msg.obj = r.app;
+                mAm.mHandler.sendMessage(msg);
+            }
+        }
+
+        if (DEBUG_SERVICE) {
+            RuntimeException here = new RuntimeException();
+            here.fillInStackTrace();
+            Slog.v(TAG_SERVICE, "Bringing down " + r + " " + r.intent, here);
+        }
         r.destroyTime = SystemClock.uptimeMillis();
         if (LOG_SERVICE_START_STOP) {
             EventLogTags.writeAmDestroyService(
@@ -2122,7 +2150,14 @@ public final class ActiveServices {
         }
 
         final ServiceMap smap = getServiceMapLocked(r.userId);
-        smap.mServicesByName.remove(r.name);
+        ServiceRecord found = smap.mServicesByName.remove(r.name);
+        if (found != r) {
+            // This is not actually the service we think is running...  this should not happen,
+            // but if it does, fail hard.
+            smap.mServicesByName.put(r.name, found);
+            throw new IllegalStateException("Bringing down " + r + " but actually running "
+                    + found);
+        }
         smap.mServicesByIntent.remove(r.intent);
         r.totalRestartCount = 0;
         unscheduleServiceRestartLocked(r, 0, true);
@@ -2962,7 +2997,7 @@ public final class ActiveServices {
     void serviceForegroundTimeout(ServiceRecord r) {
         ProcessRecord app;
         synchronized (mAm) {
-            if (!r.fgRequired) {
+            if (!r.fgRequired || r.destroying) {
                 return;
             }
 
@@ -2980,6 +3015,11 @@ public final class ActiveServices {
         }
     }
 
+    void serviceForegroundCrash(ProcessRecord app) {
+        mAm.crashApplication(app.uid, app.pid, app.info.packageName, app.userId,
+                "Context.startForegroundService() did not then call Service.startForeground()");
+    }
+
     void scheduleServiceTimeoutLocked(ProcessRecord proc) {
         if (proc.executingServices.size() == 0 || proc.thread == null) {
             return;

services/core/java/com/android/server/am/ActivityManagerService.java

@@ -1703,6 +1703,7 @@ public class ActivityManagerService extends IActivityManager.Stub
     static final int SERVICE_FOREGROUND_TIMEOUT_MSG = 66;
     static final int DISPATCH_PENDING_INTENT_CANCEL_MSG = 67;
     static final int PUSH_TEMP_WHITELIST_UI_MSG = 68;
+    static final int SERVICE_FOREGROUND_CRASH_MSG = 69;
     static final int START_USER_SWITCH_FG_MSG = 712;
 
     static final int FIRST_ACTIVITY_STACK_MSG = 100;
@@ -1974,6 +1975,9 @@ public class ActivityManagerService extends IActivityManager.Stub
             case SERVICE_FOREGROUND_TIMEOUT_MSG: {
                 mServices.serviceForegroundTimeout((ServiceRecord)msg.obj);
             } break;
+            case SERVICE_FOREGROUND_CRASH_MSG: {
+                mServices.serviceForegroundCrash((ProcessRecord)msg.obj);
+            } break;
             case DISPATCH_PENDING_INTENT_CANCEL_MSG: {
                 RemoteCallbackList<IResultReceiver> callbacks
                         = (RemoteCallbackList<IResultReceiver>)msg.obj;