Fix DownloadService resumption

- DownloadManagerHelper now passes all downloads to the
  DownloadService when the service is attached (and once
  the downloads are known). The service then starts the
  foreground notification updater if necessary. This fixes
  the ref'd issue.
- Don't call getScheduler() if the service is background
  only. This was already documented to be the case on the
  DownloadService constructor.
- If the service is started in the foreground on SDK level
  26 and higher, satisfy the condition to move the service
  to the foreground in onStartCommand rather than in stop().
  It's much more obviously correct, and should produce the
  same end result.

Issue: #6798
PiperOrigin-RevId: 290050024

RELEASENOTES.md

@@ -4,14 +4,18 @@
 
 * Add Java FLAC extractor
   ([#6406](https://github.com/google/ExoPlayer/issues/6406)).
+* Downloads:
+  * Merge downloads in `SegmentDownloader` to improve overall download
+    speed ([#5978](https://github.com/google/ExoPlayer/issues/5978)).
+  * Fix download resumption when the requirements for them to continue are
+    met ([#6733](https://github.com/google/ExoPlayer/issues/6733),
+    [#6798](https://github.com/google/ExoPlayer/issues/6798)).
+  * Fix `DownloadHelper.createMediaSource` to use `customCacheKey` when creating
+    `ProgressiveMediaSource` instances.
 * Update `IcyDecoder` to try ISO-8859-1 decoding if UTF-8 decoding fails.
   Also change `IcyInfo.rawMetadata` from `String` to `byte[]` to allow
   developers to handle data that's neither UTF-8 nor ISO-8859-1
   ([#6753](https://github.com/google/ExoPlayer/issues/6753)).
-* Fix handling of network transitions in `RequirementsWatcher`
-  ([#6733](https://github.com/google/ExoPlayer/issues/6733)). Incorrect handling
-  could previously cause downloads to be paused when they should have been able
-  to proceed.
 * Fix handling of E-AC-3 streams that contain AC-3 syncframes
   ([#6602](https://github.com/google/ExoPlayer/issues/6602)).
 * Fix playback of TrueHD streams in Matroska

extensions/jobdispatcher/src/main/java/com/google/android/exoplayer2/ext/jobdispatcher/JobDispatcherScheduler.java

@@ -76,8 +76,8 @@ public final class JobDispatcherScheduler implements Scheduler {
    *     {@link #schedule(Requirements, String, String)} or {@link #cancel()} are called.
    */
   public JobDispatcherScheduler(Context context, String jobTag) {
-    this.jobDispatcher =
-        new FirebaseJobDispatcher(new GooglePlayDriver(context.getApplicationContext()));
+    context = context.getApplicationContext();
+    this.jobDispatcher = new FirebaseJobDispatcher(new GooglePlayDriver(context));
     this.jobTag = jobTag;
   }
 

library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadService.java

@@ -578,13 +578,13 @@ public abstract class DownloadService extends Service {
           NotificationUtil.IMPORTANCE_LOW);
     }
     Class<? extends DownloadService> clazz = getClass();
-    DownloadManagerHelper downloadManagerHelper = downloadManagerHelpers.get(clazz);
+    @Nullable DownloadManagerHelper downloadManagerHelper = downloadManagerHelpers.get(clazz);
     if (downloadManagerHelper == null) {
+      @Nullable Scheduler scheduler = foregroundNotificationUpdater == null ? null : getScheduler();
       downloadManager = getDownloadManager();
       downloadManager.resumeDownloads();
       downloadManagerHelper =
-          new DownloadManagerHelper(
-              getApplicationContext(), downloadManager, getScheduler(), clazz);
+          new DownloadManagerHelper(getApplicationContext(), downloadManager, scheduler, clazz);
       downloadManagerHelpers.put(clazz, downloadManagerHelper);
     } else {
       downloadManager = downloadManagerHelper.downloadManager;
@@ -600,9 +600,9 @@ public abstract class DownloadService extends Service {
     @Nullable String contentId = null;
     if (intent != null) {
       intentAction = intent.getAction();
+      contentId = intent.getStringExtra(KEY_CONTENT_ID);
       startedInForeground |=
           intent.getBooleanExtra(KEY_FOREGROUND, false) || ACTION_RESTART.equals(intentAction);
-      contentId = intent.getStringExtra(KEY_CONTENT_ID);
     }
     // intentAction is null if the service is restarted or no action is specified.
     if (intentAction == null) {
@@ -660,6 +660,10 @@ public abstract class DownloadService extends Service {
         break;
     }
 
+    if (Util.SDK_INT >= 26 && startedInForeground && foregroundNotificationUpdater != null) {
+      // From API level 26, services started in the foreground are required to show a notification.
+      foregroundNotificationUpdater.showNotificationIfNotAlready();
+    }
     if (downloadManager.isIdle()) {
       stop();
     }
@@ -701,21 +705,21 @@ public abstract class DownloadService extends Service {
    * Returns a {@link Scheduler} to restart the service when requirements allowing downloads to take
    * place are met. If {@code null}, the service will only be restarted if the process is still in
    * memory when the requirements are met.
+   *
+   * <p>This method is not called for services whose {@code foregroundNotificationId} is set to
+   * {@link #FOREGROUND_NOTIFICATION_ID_NONE}. Such services will only be restarted if the process
+   * is still in memory and considered non-idle, meaning that it's either in the foreground or was
+   * backgrounded within the last few minutes.
    */
-  protected abstract @Nullable Scheduler getScheduler();
+  @Nullable
+  protected abstract Scheduler getScheduler();
 
   /**
-   * Returns a notification to be displayed when this service running in the foreground. This method
-   * is called when there is a download state change and periodically while there are active
-   * downloads. The periodic update interval can be set using {@link #DownloadService(int, long)}.
-   *
-   * <p>On API level 26 and above, this method may also be called just before the service stops,
-   * with an empty {@code downloads} array. The returned notification is used to satisfy system
-   * requirements for foreground services.
+   * Returns a notification to be displayed when this service running in the foreground.
    *
    * <p>Download services that do not wish to run in the foreground should be created by setting the
    * {@code foregroundNotificationId} constructor argument to {@link
-   * #FOREGROUND_NOTIFICATION_ID_NONE}. This method will not be called in this case, meaning it can
+   * #FOREGROUND_NOTIFICATION_ID_NONE}. This method is not called for such services, meaning it can
    * be implemented to throw {@link UnsupportedOperationException}.
    *
    * @param downloads The current downloads.
@@ -754,13 +758,32 @@ public abstract class DownloadService extends Service {
     // Do nothing.
   }
 
+  /**
+   * Called after the service is created, once the downloads are known.
+   *
+   * @param downloads The current downloads.
+   */
+  private void notifyDownloads(List<Download> downloads) {
+    if (foregroundNotificationUpdater != null) {
+      for (int i = 0; i < downloads.size(); i++) {
+        if (needsForegroundNotification(downloads.get(i).state)) {
+          foregroundNotificationUpdater.startPeriodicUpdates();
+          break;
+        }
+      }
+    }
+  }
+
+  /**
+   * Called when the state of a download changes.
+   *
+   * @param download The state of the download.
+   */
   @SuppressWarnings("deprecation")
   private void notifyDownloadChanged(Download download) {
     onDownloadChanged(download);
     if (foregroundNotificationUpdater != null) {
-      if (download.state == Download.STATE_DOWNLOADING
-          || download.state == Download.STATE_REMOVING
-          || download.state == Download.STATE_RESTARTING) {
+      if (needsForegroundNotification(download.state)) {
         foregroundNotificationUpdater.startPeriodicUpdates();
       } else {
         foregroundNotificationUpdater.invalidate();
@@ -768,6 +791,11 @@ public abstract class DownloadService extends Service {
     }
   }
 
+  /**
+   * Called when a download is removed.
+   *
+   * @param download The last state of the download before it was removed.
+   */
   @SuppressWarnings("deprecation")
   private void notifyDownloadRemoved(Download download) {
     onDownloadRemoved(download);
@@ -779,10 +807,6 @@ public abstract class DownloadService extends Service {
   private void stop() {
     if (foregroundNotificationUpdater != null) {
       foregroundNotificationUpdater.stopPeriodicUpdates();
-      // Make sure startForeground is called before stopping. Workaround for [Internal: b/69424260].
-      if (startedInForeground && Util.SDK_INT >= 26) {
-        foregroundNotificationUpdater.showNotificationIfNotAlready();
-      }
     }
     if (Util.SDK_INT < 28 && taskRemoved) { // See [Internal: b/74248644].
       stopSelf();
@@ -791,6 +815,12 @@ public abstract class DownloadService extends Service {
     }
   }
 
+  private static boolean needsForegroundNotification(@Download.State int state) {
+    return state == Download.STATE_DOWNLOADING
+        || state == Download.STATE_REMOVING
+        || state == Download.STATE_RESTARTING;
+  }
+
   private static Intent getIntent(
       Context context, Class<? extends DownloadService> clazz, String action, boolean foreground) {
     return getIntent(context, clazz, action).putExtra(KEY_FOREGROUND, foreground);
@@ -884,6 +914,16 @@ public abstract class DownloadService extends Service {
     public void attachService(DownloadService downloadService) {
       Assertions.checkState(this.downloadService == null);
       this.downloadService = downloadService;
+      if (downloadManager.isInitialized()) {
+        // The call to DownloadService.notifyDownloads is posted to avoid it being called directly
+        // from DownloadService.onCreate. This is a good idea because it may in turn call
+        // DownloadService.getForegroundNotification, and concrete subclass implementations may
+        // not anticipate the possibility of this method being called before their onCreate
+        // implementation has finished executing.
+        Util.createHandler()
+            .postAtFrontOfQueue(
+                () -> downloadService.notifyDownloads(downloadManager.getCurrentDownloads()));
+      }
     }
 
     public void detachService(DownloadService downloadService) {
@@ -894,6 +934,15 @@ public abstract class DownloadService extends Service {
       }
     }
 
+    // DownloadManager.Listener implementation.
+
+    @Override
+    public void onInitialized(DownloadManager downloadManager) {
+      if (downloadService != null) {
+        downloadService.notifyDownloads(downloadManager.getCurrentDownloads());
+      }
+    }
+
     @Override
     public void onDownloadChanged(DownloadManager downloadManager, Download download) {
       if (downloadService != null) {
@@ -921,6 +970,10 @@ public abstract class DownloadService extends Service {
         Requirements requirements,
         @Requirements.RequirementFlags int notMetRequirements) {
       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.
       if (downloadService == null && requirementsMet) {
         try {
           Intent intent = getIntent(context, serviceClass, DownloadService.ACTION_INIT);
@@ -935,6 +988,12 @@ public abstract class DownloadService extends Service {
       }
     }
 
+    // Internal methods.
+
+    // 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
+    // necessary to call this method from some additional places.
     @RequiresNonNull("scheduler")
     private void setSchedulerEnabled(boolean enabled, Requirements requirements) {
       if (!enabled) {

library/core/src/main/java/com/google/android/exoplayer2/scheduler/PlatformScheduler.java

@@ -64,6 +64,7 @@ public final class PlatformScheduler implements Scheduler {
    */
   @RequiresPermission(android.Manifest.permission.RECEIVE_BOOT_COMPLETED)
   public PlatformScheduler(Context context, int jobId) {
+    context = context.getApplicationContext();
     this.jobId = jobId;
     jobServiceComponentName = new ComponentName(context, PlatformSchedulerService.class);
     jobScheduler = (JobScheduler) context.getSystemService(Context.JOB_SCHEDULER_SERVICE);