Skip to content

Commit

Permalink
Start service in foreground if allowed
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ojw28 committed Jan 16, 2020
1 parent 775a17c commit cb8391a
Showing 1 changed file with 26 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -580,11 +580,13 @@ public void onCreate() {
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;
Expand Down Expand Up @@ -891,17 +893,20 @@ private static final class DownloadManagerHelper implements DownloadManager.List

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;

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);
Expand Down Expand Up @@ -972,16 +977,9 @@ public void onRequirementsStateChanged(
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);
Expand All @@ -990,6 +988,23 @@ public void onRequirementsStateChanged(

// 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
Expand Down

0 comments on commit cb8391a

Please sign in to comment.