From 3f565c33befdbd260958b0a0eb1e2d9468efaf82 Mon Sep 17 00:00:00 2001 From: olly Date: Tue, 9 Apr 2019 20:03:32 +0100 Subject: [PATCH] Simplify offline Requirements - Remove NETWORK_TYPE_NOT_ROAMING and NETWORK_TYPE_METERED because JobScheduler doesn't support them, and they're probably not useful to many people (data when roaming is normally enabled/disabled at the OS level, and restricting to *only* metered networks seems niche) - Convert network requirements proper flags - Stop persisting requirements in DownloadIndex. The direction we're headed to solve the manager start/stop problem is going to involve state in DownloadManager determining whether downloads actually start, and if we're doing that then it's no worse to do it for this as well PiperOrigin-RevId: 242713196 --- .../jobdispatcher/JobDispatcherScheduler.java | 16 +- .../offline/DefaultDownloadIndex.java | 18 +- .../exoplayer2/offline/DownloadIndexUtil.java | 2 +- .../exoplayer2/offline/DownloadManager.java | 10 +- .../exoplayer2/offline/DownloadState.java | 24 +- .../scheduler/PlatformScheduler.java | 33 +-- .../exoplayer2/scheduler/Requirements.java | 205 ++++++------------ .../scheduler/RequirementsWatcher.java | 2 +- .../offline/DefaultDownloadIndexTest.java | 1 - .../offline/DownloadStateBuilder.java | 7 - .../exoplayer2/offline/DownloadStateTest.java | 44 ++-- 11 files changed, 111 insertions(+), 251 deletions(-) diff --git a/extensions/jobdispatcher/src/main/java/com/google/android/exoplayer2/ext/jobdispatcher/JobDispatcherScheduler.java b/extensions/jobdispatcher/src/main/java/com/google/android/exoplayer2/ext/jobdispatcher/JobDispatcherScheduler.java index c676091d32..790f5ca4e5 100644 --- a/extensions/jobdispatcher/src/main/java/com/google/android/exoplayer2/ext/jobdispatcher/JobDispatcherScheduler.java +++ b/extensions/jobdispatcher/src/main/java/com/google/android/exoplayer2/ext/jobdispatcher/JobDispatcherScheduler.java @@ -104,18 +104,10 @@ public final class JobDispatcherScheduler implements Scheduler { .setService(JobDispatcherSchedulerService.class) // the JobService that will be called .setTag(tag); - switch (requirements.getRequiredNetworkType()) { - case Requirements.NETWORK_TYPE_NONE: - // do nothing. - break; - case Requirements.NETWORK_TYPE_ANY: - builder.addConstraint(Constraint.ON_ANY_NETWORK); - break; - case Requirements.NETWORK_TYPE_UNMETERED: - builder.addConstraint(Constraint.ON_UNMETERED_NETWORK); - break; - default: - throw new UnsupportedOperationException(); + if (requirements.isUnmeteredNetworkRequired()) { + builder.addConstraint(Constraint.ON_UNMETERED_NETWORK); + } else if (requirements.isNetworkRequired()) { + builder.addConstraint(Constraint.ON_ANY_NETWORK); } if (requirements.isIdleRequired()) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java index 16e5c37b3d..82d34e983f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java @@ -57,15 +57,20 @@ public final class DefaultDownloadIndex implements DownloadIndex { private static final String COLUMN_DOWNLOADED_BYTES = "downloaded_bytes"; private static final String COLUMN_TOTAL_BYTES = "total_bytes"; private static final String COLUMN_FAILURE_REASON = "failure_reason"; - private static final String COLUMN_NOT_MET_REQUIREMENTS = "not_met_requirements"; private static final String COLUMN_MANUAL_STOP_REASON = "manual_stop_reason"; private static final String COLUMN_START_TIME_MS = "start_time_ms"; private static final String COLUMN_UPDATE_TIME_MS = "update_time_ms"; + /** @deprecated No longer used. */ @SuppressWarnings("DeprecatedIsStillUsed") @Deprecated private static final String COLUMN_STOP_FLAGS = "stop_flags"; + /** @deprecated No longer used. */ + @SuppressWarnings("DeprecatedIsStillUsed") + @Deprecated + private static final String COLUMN_NOT_MET_REQUIREMENTS = "not_met_requirements"; + private static final int COLUMN_INDEX_ID = 0; private static final int COLUMN_INDEX_TYPE = 1; private static final int COLUMN_INDEX_URI = 2; @@ -77,10 +82,9 @@ public final class DefaultDownloadIndex implements DownloadIndex { private static final int COLUMN_INDEX_DOWNLOADED_BYTES = 8; private static final int COLUMN_INDEX_TOTAL_BYTES = 9; private static final int COLUMN_INDEX_FAILURE_REASON = 10; - private static final int COLUMN_INDEX_NOT_MET_REQUIREMENTS = 11; - private static final int COLUMN_INDEX_MANUAL_STOP_REASON = 12; - private static final int COLUMN_INDEX_START_TIME_MS = 13; - private static final int COLUMN_INDEX_UPDATE_TIME_MS = 14; + private static final int COLUMN_INDEX_MANUAL_STOP_REASON = 11; + private static final int COLUMN_INDEX_START_TIME_MS = 12; + private static final int COLUMN_INDEX_UPDATE_TIME_MS = 13; private static final String WHERE_ID_EQUALS = COLUMN_ID + " = ?"; private static final String WHERE_STATE_TERMINAL = @@ -99,7 +103,6 @@ public final class DefaultDownloadIndex implements DownloadIndex { COLUMN_DOWNLOADED_BYTES, COLUMN_TOTAL_BYTES, COLUMN_FAILURE_REASON, - COLUMN_NOT_MET_REQUIREMENTS, COLUMN_MANUAL_STOP_REASON, COLUMN_START_TIME_MS, COLUMN_UPDATE_TIME_MS @@ -204,7 +207,7 @@ public final class DefaultDownloadIndex implements DownloadIndex { values.put(COLUMN_TOTAL_BYTES, downloadState.getTotalBytes()); values.put(COLUMN_FAILURE_REASON, downloadState.failureReason); values.put(COLUMN_STOP_FLAGS, 0); - values.put(COLUMN_NOT_MET_REQUIREMENTS, downloadState.notMetRequirements); + values.put(COLUMN_NOT_MET_REQUIREMENTS, 0); values.put(COLUMN_MANUAL_STOP_REASON, downloadState.manualStopReason); values.put(COLUMN_START_TIME_MS, downloadState.startTimeMs); values.put(COLUMN_UPDATE_TIME_MS, downloadState.updateTimeMs); @@ -356,7 +359,6 @@ public final class DefaultDownloadIndex implements DownloadIndex { action, cursor.getInt(COLUMN_INDEX_STATE), cursor.getInt(COLUMN_INDEX_FAILURE_REASON), - cursor.getInt(COLUMN_INDEX_NOT_MET_REQUIREMENTS), cursor.getInt(COLUMN_INDEX_MANUAL_STOP_REASON), cursor.getLong(COLUMN_INDEX_START_TIME_MS), cursor.getLong(COLUMN_INDEX_UPDATE_TIME_MS), diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndexUtil.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndexUtil.java index 02e95eaf4d..179a2a87f9 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndexUtil.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndexUtil.java @@ -71,7 +71,7 @@ public final class DownloadIndexUtil { throws IOException { DownloadState downloadState = downloadIndex.getDownloadState(action.id); if (downloadState != null) { - downloadState = downloadState.copyWithMergedAction(action); + downloadState = downloadState.copyWithMergedAction(action, /* canStart= */ true); } else { downloadState = new DownloadState(action); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java index 57cea6a54e..05a13f7df0 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java @@ -113,8 +113,7 @@ public final class DownloadManager { /** The default minimum number of times a download must be retried before failing. */ public static final int DEFAULT_MIN_RETRY_COUNT = 5; /** The default requirement is that the device has network connectivity. */ - public static final Requirements DEFAULT_REQUIREMENTS = - new Requirements(Requirements.NETWORK_TYPE_ANY, false, false); + public static final Requirements DEFAULT_REQUIREMENTS = new Requirements(Requirements.NETWORK); // Messages posted to the main handler. private static final int MSG_INITIALIZED = 0; @@ -623,7 +622,8 @@ public final class DownloadManager { downloadState = new DownloadState(action); logd("Download state is created for " + action.id); } else { - downloadState = downloadState.copyWithMergedAction(action); + downloadState = + downloadState.copyWithMergedAction(action, /* canStart= */ notMetRequirements == 0); logd("Download state is loaded for " + action.id); } addDownloadForState(downloadState); @@ -840,7 +840,8 @@ public final class DownloadManager { } public void addAction(DownloadAction newAction) { - downloadState = downloadState.copyWithMergedAction(newAction); + downloadState = + downloadState.copyWithMergedAction(newAction, /* canStart= */ notMetRequirements == 0); initialize(); } @@ -854,7 +855,6 @@ public final class DownloadManager { downloadState.action, state, state != STATE_FAILED ? FAILURE_REASON_NONE : failureReason, - notMetRequirements, manualStopReason, downloadState.startTimeMs, /* updateTimeMs= */ System.currentTimeMillis(), diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadState.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadState.java index 5e7a54b7e7..7a6bcd0753 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadState.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadState.java @@ -17,8 +17,6 @@ package com.google.android.exoplayer2.offline; import androidx.annotation.IntDef; import com.google.android.exoplayer2.C; -import com.google.android.exoplayer2.scheduler.Requirements; -import com.google.android.exoplayer2.scheduler.Requirements.RequirementFlags; import com.google.android.exoplayer2.upstream.cache.CacheUtil.CachingCounters; import com.google.android.exoplayer2.util.Assertions; import java.lang.annotation.Documented; @@ -124,8 +122,6 @@ public final class DownloadState { * #FAILURE_REASON_NONE}. */ @FailureReason public final int failureReason; - /** Not met requirements to download. */ - @Requirements.RequirementFlags public final int notMetRequirements; /** The manual stop reason. */ public final int manualStopReason; @@ -145,7 +141,6 @@ public final class DownloadState { action, /* state= */ STATE_QUEUED, FAILURE_REASON_NONE, - /* notMetRequirements= */ 0, /* manualStopReason= */ 0, /* startTimeMs= */ currentTimeMs, /* updateTimeMs= */ currentTimeMs, @@ -156,20 +151,18 @@ public final class DownloadState { DownloadAction action, @State int state, @FailureReason int failureReason, - @RequirementFlags int notMetRequirements, int manualStopReason, long startTimeMs, long updateTimeMs, CachingCounters counters) { Assertions.checkNotNull(counters); Assertions.checkState((failureReason == FAILURE_REASON_NONE) == (state != STATE_FAILED)); - if (manualStopReason != 0 || notMetRequirements != 0) { + if (manualStopReason != 0) { Assertions.checkState(state != STATE_DOWNLOADING && state != STATE_QUEUED); } this.action = action; this.state = state; this.failureReason = failureReason; - this.notMetRequirements = notMetRequirements; this.manualStopReason = manualStopReason; this.startTimeMs = startTimeMs; this.updateTimeMs = updateTimeMs; @@ -181,14 +174,14 @@ public final class DownloadState { * must have the same id and type. * * @param newAction The {@link DownloadAction} to be merged. + * @param canStart Whether the download is eligible to be started. * @return A new {@link DownloadState}. */ - public DownloadState copyWithMergedAction(DownloadAction newAction) { + public DownloadState copyWithMergedAction(DownloadAction newAction, boolean canStart) { return new DownloadState( action.copyWithMergedAction(newAction), - getNextState(state, manualStopReason != 0 || notMetRequirements != 0), + getNextState(state, canStart && manualStopReason == 0), FAILURE_REASON_NONE, - notMetRequirements, manualStopReason, startTimeMs, /* updateTimeMs= */ System.currentTimeMillis(), @@ -205,7 +198,6 @@ public final class DownloadState { action, state, FAILURE_REASON_NONE, - notMetRequirements, manualStopReason, startTimeMs, /* updateTimeMs= */ System.currentTimeMillis(), @@ -240,13 +232,13 @@ public final class DownloadState { this.counters = counters; } - private static int getNextState(int currentState, boolean isStopped) { + private static int getNextState(int currentState, boolean canStart) { if (currentState == STATE_REMOVING || currentState == STATE_RESTARTING) { return STATE_RESTARTING; - } else if (isStopped) { - return STATE_STOPPED; - } else { + } else if (canStart) { return STATE_QUEUED; + } else { + return STATE_STOPPED; } } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/scheduler/PlatformScheduler.java b/library/core/src/main/java/com/google/android/exoplayer2/scheduler/PlatformScheduler.java index ec0bfe150f..fc8e8b61a5 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/scheduler/PlatformScheduler.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/scheduler/PlatformScheduler.java @@ -93,36 +93,11 @@ public final class PlatformScheduler implements Scheduler { String servicePackage) { JobInfo.Builder builder = new JobInfo.Builder(jobId, jobServiceComponentName); - int networkType; - switch (requirements.getRequiredNetworkType()) { - case Requirements.NETWORK_TYPE_NONE: - networkType = JobInfo.NETWORK_TYPE_NONE; - break; - case Requirements.NETWORK_TYPE_ANY: - networkType = JobInfo.NETWORK_TYPE_ANY; - break; - case Requirements.NETWORK_TYPE_UNMETERED: - networkType = JobInfo.NETWORK_TYPE_UNMETERED; - break; - case Requirements.NETWORK_TYPE_NOT_ROAMING: - if (Util.SDK_INT >= 24) { - networkType = JobInfo.NETWORK_TYPE_NOT_ROAMING; - } else { - throw new UnsupportedOperationException(); - } - break; - case Requirements.NETWORK_TYPE_METERED: - if (Util.SDK_INT >= 26) { - networkType = JobInfo.NETWORK_TYPE_METERED; - } else { - throw new UnsupportedOperationException(); - } - break; - default: - throw new UnsupportedOperationException(); + if (requirements.isUnmeteredNetworkRequired()) { + builder.setRequiredNetworkType(JobInfo.NETWORK_TYPE_UNMETERED); + } else if (requirements.isNetworkRequired()) { + builder.setRequiredNetworkType(JobInfo.NETWORK_TYPE_ANY); } - - builder.setRequiredNetworkType(networkType); builder.setRequiresDeviceIdle(requirements.isIdleRequired()); builder.setRequiresCharging(requirements.isChargingRequired()); builder.setPersisted(true); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/scheduler/Requirements.java b/library/core/src/main/java/com/google/android/exoplayer2/scheduler/Requirements.java index 0aa8a63339..28aa37ee2a 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/scheduler/Requirements.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/scheduler/Requirements.java @@ -25,7 +25,6 @@ import android.net.NetworkInfo; import android.os.BatteryManager; import android.os.PowerManager; import androidx.annotation.IntDef; -import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Log; import com.google.android.exoplayer2.util.Util; import java.lang.annotation.Documented; @@ -38,112 +37,60 @@ import java.lang.annotation.RetentionPolicy; public final class Requirements { /** - * Network types. One of {@link #NETWORK_TYPE_NONE}, {@link #NETWORK_TYPE_ANY}, {@link - * #NETWORK_TYPE_UNMETERED}, {@link #NETWORK_TYPE_NOT_ROAMING} or {@link #NETWORK_TYPE_METERED}. - */ - @Documented - @Retention(RetentionPolicy.SOURCE) - @IntDef({ - NETWORK_TYPE_NONE, - NETWORK_TYPE_ANY, - NETWORK_TYPE_UNMETERED, - NETWORK_TYPE_NOT_ROAMING, - NETWORK_TYPE_METERED, - }) - public @interface NetworkType {} - - /** - * Requirement flags. - * - *

Combination of the following values is possible: - * - *