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 f1c90e67c1..d4ea9fc3ff 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 @@ -310,9 +310,10 @@ public final class DownloadManager { /** Returns whether there are no active downloads. */ public boolean isIdle() { Assertions.checkState(!released); - if (!initialized) { + if (!initialized || !activeDownloads.isEmpty()) { return false; } + // Still need to check all downloads as there might be remove tasks going on. for (int i = 0; i < downloads.size(); i++) { if (!downloads.get(i).isIdle()) { return false; @@ -365,6 +366,14 @@ public final class DownloadManager { } } + private void maybeRestartDownload(Download download) { + if (activeDownloads.contains(download)) { + download.start(); + } else { + maybeStartDownload(download); + } + } + private void maybeNotifyListenersIdle() { if (!isIdle()) { return; @@ -540,7 +549,7 @@ public final class DownloadManager { this.startTimeMs = System.currentTimeMillis(); actionQueue = new ArrayDeque<>(); actionQueue.add(action); - initialize(/* restart= */ false); + initialize(); } public boolean addAction(DownloadAction newAction) { @@ -562,12 +571,12 @@ public final class DownloadManager { setState(STATE_REMOVING); } } else if (!action.equals(updatedAction)) { + Assertions.checkState( + state == STATE_DOWNLOADING || state == STATE_QUEUED || state == STATE_STOPPED); if (state == STATE_DOWNLOADING) { stopDownloadThread(); - } else { - Assertions.checkState(state == STATE_QUEUED || state == STATE_STOPPED); - initialize(/* restart= */ false); } + initialize(); } return true; } @@ -633,13 +642,14 @@ public final class DownloadManager { public void updateStopFlags(int flags, int values) { stopFlags = (values & flags) | (stopFlags & ~flags); if (stopFlags != 0) { - if (state == STATE_DOWNLOADING) { - stopDownloadThread(); - } else if (state == STATE_QUEUED) { + if (state == STATE_DOWNLOADING || state == STATE_QUEUED) { + if (state == STATE_DOWNLOADING) { + stopDownloadThread(); + } setState(STATE_STOPPED); } } else if (state == STATE_STOPPED) { - startOrQueue(/* restart= */ false); + startOrQueue(); } } @@ -650,7 +660,7 @@ public final class DownloadManager { notMetRequirements != 0 ? STOP_FLAG_REQUIREMENTS_NOT_MET : 0); } - private void initialize(boolean restart) { + private void initialize() { DownloadAction action = actionQueue.peek(); if (action.isRemoveAction) { if (!downloadManager.released) { @@ -660,18 +670,14 @@ public final class DownloadManager { } else if (stopFlags != 0) { setState(STATE_STOPPED); } else { - startOrQueue(restart); + startOrQueue(); } } - private void startOrQueue(boolean restart) { + private void startOrQueue() { // Set to queued state but don't notify listeners until we make sure we can't start now. state = STATE_QUEUED; - if (restart) { - start(); - } else { - downloadManager.maybeStartDownload(this); - } + downloadManager.maybeRestartDownload(this); if (state == STATE_QUEUED) { downloadManager.onDownloadStateChange(this); } @@ -683,6 +689,9 @@ public final class DownloadManager { } private void startDownloadThread(DownloadAction action) { + if (downloadThread != null) { + return; + } downloader = downloaderFactory.createDownloader(action); downloadThread = new DownloadThread( @@ -690,29 +699,37 @@ public final class DownloadManager { } private void stopDownloadThread() { - Assertions.checkNotNull(downloadThread).cancel(); + if (!downloadThread.isRemoveThread) { + Assertions.checkNotNull(downloadThread).cancel(); + } } - private void onDownloadThreadStopped(@Nullable Throwable finalError) { + private void onDownloadThreadStopped(@Nullable Throwable error) { failureReason = FAILURE_REASON_NONE; - if (!downloadThread.isCanceled) { - if (finalError != null && state != STATE_REMOVING && state != STATE_RESTARTING) { - failureReason = FAILURE_REASON_UNKNOWN; - setState(STATE_FAILED); - return; + boolean isCanceled = downloadThread.isCanceled; + downloadThread = null; + if (isCanceled) { + if (!isIdle()) { + startDownloadThread(actionQueue.peek()); } - if (actionQueue.size() == 1) { - if (state == STATE_REMOVING) { - setState(STATE_REMOVED); - } else { - Assertions.checkState(state == STATE_DOWNLOADING); - setState(STATE_COMPLETED); - } - return; - } - actionQueue.remove(); + return; } - initialize(/* restart= */ state == STATE_DOWNLOADING); + if (error != null && state == STATE_DOWNLOADING) { + failureReason = FAILURE_REASON_UNKNOWN; + setState(STATE_FAILED); + return; + } + if (actionQueue.size() == 1) { + if (state == STATE_REMOVING) { + setState(STATE_REMOVED); + } else { + Assertions.checkState(state == STATE_DOWNLOADING); + setState(STATE_COMPLETED); + } + return; + } + actionQueue.remove(); + initialize(); } } @@ -720,7 +737,7 @@ public final class DownloadManager { private final Download download; private final Downloader downloader; - private final boolean remove; + private final boolean isRemoveThread; private final int minRetryCount; private final Handler callbackHandler; private final Thread thread; @@ -729,12 +746,12 @@ public final class DownloadManager { private DownloadThread( Download download, Downloader downloader, - boolean remove, + boolean isRemoveThread, int minRetryCount, Handler callbackHandler) { this.download = download; this.downloader = downloader; - this.remove = remove; + this.isRemoveThread = isRemoveThread; this.minRetryCount = minRetryCount; this.callbackHandler = callbackHandler; thread = new Thread(this); @@ -754,7 +771,7 @@ public final class DownloadManager { logd("Download is started", download); Throwable error = null; try { - if (remove) { + if (isRemoveThread) { downloader.remove(); } else { int errorCount = 0; 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 dd55d6ea31..f5b3287b32 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 @@ -182,13 +182,11 @@ public final class DownloadState { byte[] customMetadata) { Assertions.checkState( failureReason == FAILURE_REASON_NONE ? state != STATE_FAILED : state == STATE_FAILED); + Assertions.checkState(stopFlags == 0 || (state != STATE_DOWNLOADING && state != STATE_QUEUED)); Assertions.checkState( (stopFlags & STOP_FLAG_REQUIREMENTS_NOT_MET) == 0 ? notMetRequirements == 0 : notMetRequirements != 0); - // TODO enable this when we start changing state immediately - // Assertions.checkState(stopFlags == 0 || (state != STATE_DOWNLOADING && state != - // STATE_QUEUED)); this.id = id; this.type = type; this.uri = uri; diff --git a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java index 28040ec538..8d36ea68a4 100644 --- a/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java +++ b/library/dash/src/test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java @@ -53,6 +53,7 @@ import org.mockito.MockitoAnnotations; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowLog; /** Tests {@link DownloadManager}. */ @RunWith(RobolectricTestRunner.class) @@ -73,6 +74,7 @@ public class DownloadManagerDashTest { @Before public void setUp() throws Exception { + ShadowLog.stream = System.out; dummyMainThread = new DummyMainThread(); Context context = RuntimeEnvironment.application; tempFolder = Util.createTempDirectory(context, "ExoPlayerTest");