From 0ac22d340130e6ea5b4b32bb4595ca46f079559a Mon Sep 17 00:00:00 2001
From: olly <olly@google.com>
Date: Thu, 16 Jan 2020 16:09:28 +0000
Subject: [PATCH] Start service in foreground if allowed

This fixes an issue where a DownloadService implementation
that allows foreground but doesn't provide a scheduler would
not be restarted in the case that it was still in memory but
classed as idle by the platform.

It also speeds up service restart in the case that a
scheduler is provided.

Issue: #6798
PiperOrigin-RevId: 290068960
---
 .../exoplayer2/offline/DownloadService.java   | 37 +++++++++++++------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadService.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadService.java
index 7d1ec48b64..d8a0608825 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadService.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadService.java
@@ -580,11 +580,13 @@ public abstract class DownloadService extends Service {
     Class<? extends DownloadService> clazz = getClass();
     @Nullable DownloadManagerHelper downloadManagerHelper = downloadManagerHelpers.get(clazz);
     if (downloadManagerHelper == null) {
-      @Nullable Scheduler scheduler = foregroundNotificationUpdater == null ? null : getScheduler();
+      boolean foregroundAllowed = foregroundNotificationUpdater != null;
+      @Nullable Scheduler scheduler = foregroundAllowed ? getScheduler() : null;
       downloadManager = getDownloadManager();
       downloadManager.resumeDownloads();
       downloadManagerHelper =
-          new DownloadManagerHelper(getApplicationContext(), downloadManager, scheduler, clazz);
+          new DownloadManagerHelper(
+              getApplicationContext(), downloadManager, foregroundAllowed, scheduler, clazz);
       downloadManagerHelpers.put(clazz, downloadManagerHelper);
     } else {
       downloadManager = downloadManagerHelper.downloadManager;
@@ -891,6 +893,7 @@ public abstract class DownloadService extends Service {
 
     private final Context context;
     private final DownloadManager downloadManager;
+    private final boolean foregroundAllowed;
     @Nullable private final Scheduler scheduler;
     private final Class<? extends DownloadService> serviceClass;
     @Nullable private DownloadService downloadService;
@@ -898,10 +901,12 @@ public abstract class DownloadService extends Service {
     private DownloadManagerHelper(
         Context context,
         DownloadManager downloadManager,
+        boolean foregroundAllowed,
         @Nullable Scheduler scheduler,
         Class<? extends DownloadService> serviceClass) {
       this.context = context;
       this.downloadManager = downloadManager;
+      this.foregroundAllowed = foregroundAllowed;
       this.scheduler = scheduler;
       this.serviceClass = serviceClass;
       downloadManager.addListener(this);
@@ -972,16 +977,9 @@ public abstract class DownloadService extends Service {
       boolean requirementsMet = notMetRequirements == 0;
       // TODO: Fix this logic to only start the service if the DownloadManager is actually going to
       // make progress (in addition to the requirements being met, it also needs to be not paused
-      // and have some current downloads). Start in service in the foreground if the service has a
-      // ForegroundNotificationUpdater.
+      // and have some current downloads).
       if (downloadService == null && requirementsMet) {
-        try {
-          Intent intent = getIntent(context, serviceClass, DownloadService.ACTION_INIT);
-          context.startService(intent);
-        } catch (IllegalStateException e) {
-          /* startService fails if the app is in the background then don't stop the scheduler. */
-          return;
-        }
+        restartService();
       }
       if (scheduler != null) {
         setSchedulerEnabled(/* enabled= */ !requirementsMet, requirements);
@@ -990,6 +988,23 @@ public abstract class DownloadService extends Service {
 
     // Internal methods.
 
+    private void restartService() {
+      if (foregroundAllowed) {
+        Intent intent = getIntent(context, serviceClass, DownloadService.ACTION_RESTART);
+        Util.startForegroundService(context, intent);
+      } else {
+        // The service is background only. Use ACTION_INIT rather than ACTION_RESTART because
+        // ACTION_RESTART is handled as though KEY_FOREGROUND is set to true.
+        try {
+          Intent intent = getIntent(context, serviceClass, DownloadService.ACTION_INIT);
+          context.startService(intent);
+        } catch (IllegalArgumentException e) {
+          // The process is classed as idle by the platform. Starting a background service is not
+          // allowed in this state.
+        }
+      }
+    }
+
     // TODO: Fix callers to this method so that the scheduler is only enabled if the DownloadManager
     // would actually make progress were the requirements met (or if it's not initialized yet, in
     // which case we should be cautious until we know better). To implement this properly it'll be