Rework DownloadManager to fix remaining TODOs

- Removed DownloadInternal and its sometimes-out-of-sync
  duplicate state
- Fixed downloads being in STOPPED rather than QUEUED state
  when the manager is paused
- Fixed setMaxParallelDownloads to start/stop downloads if
  necessary when the value changes
- Fixed isWaitingForRequirements

PiperOrigin-RevId: 246164845

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

@@ -67,11 +67,12 @@ public final class ActionFileUpgradeUtil {
     if (actionFile.exists()) {
       boolean success = false;
       try {
+        long nowMs = System.currentTimeMillis();
         for (DownloadRequest request : actionFile.load()) {
           if (downloadIdProvider != null) {
             request = request.copyWithId(downloadIdProvider.getId(request));
           }
-          mergeRequest(request, downloadIndex, addNewDownloadsAsCompleted);
+          mergeRequest(request, downloadIndex, addNewDownloadsAsCompleted, nowMs);
         }
         success = true;
       } finally {
@@ -93,13 +94,13 @@ public final class ActionFileUpgradeUtil {
   /* package */ static void mergeRequest(
       DownloadRequest request,
       DefaultDownloadIndex downloadIndex,
-      boolean addNewDownloadAsCompleted)
+      boolean addNewDownloadAsCompleted,
+      long nowMs)
       throws IOException {
     Download download = downloadIndex.getDownload(request.id);
     if (download != null) {
-      download = DownloadManager.mergeRequest(download, request, download.stopReason);
+      download = DownloadManager.mergeRequest(download, request, download.stopReason, nowMs);
     } else {
-      long nowMs = System.currentTimeMillis();
       download =
           new Download(
               request,

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

@@ -69,7 +69,9 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
   private static final int COLUMN_INDEX_BYTES_DOWNLOADED = 13;
 
   private static final String WHERE_ID_EQUALS = COLUMN_ID + " = ?";
-  private static final String WHERE_STATE_TERMINAL =
+  private static final String WHERE_STATE_IS_DOWNLOADING =
+      COLUMN_STATE + " = " + Download.STATE_DOWNLOADING;
+  private static final String WHERE_STATE_IS_TERMINAL =
       getStateQuery(Download.STATE_COMPLETED, Download.STATE_FAILED);
 
   private static final String[] COLUMNS =
@@ -218,6 +220,19 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
     }
   }
 
+  @Override
+  public void setDownloadingStatesToQueued() throws DatabaseIOException {
+    ensureInitialized();
+    try {
+      ContentValues values = new ContentValues();
+      values.put(COLUMN_STATE, Download.STATE_QUEUED);
+      SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
+      writableDatabase.update(tableName, values, WHERE_STATE_IS_DOWNLOADING, /* whereArgs= */ null);
+    } catch (SQLException e) {
+      throw new DatabaseIOException(e);
+    }
+  }
+
   @Override
   public void setStopReason(int stopReason) throws DatabaseIOException {
     ensureInitialized();
@@ -225,7 +240,7 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
       ContentValues values = new ContentValues();
       values.put(COLUMN_STOP_REASON, stopReason);
       SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
-      writableDatabase.update(tableName, values, WHERE_STATE_TERMINAL, /* whereArgs= */ null);
+      writableDatabase.update(tableName, values, WHERE_STATE_IS_TERMINAL, /* whereArgs= */ null);
     } catch (SQLException e) {
       throw new DatabaseIOException(e);
     }
@@ -239,7 +254,10 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
       values.put(COLUMN_STOP_REASON, stopReason);
       SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
       writableDatabase.update(
-          tableName, values, WHERE_STATE_TERMINAL + " AND " + WHERE_ID_EQUALS, new String[] {id});
+          tableName,
+          values,
+          WHERE_STATE_IS_TERMINAL + " AND " + WHERE_ID_EQUALS,
+          new String[] {id});
     } catch (SQLException e) {
       throw new DatabaseIOException(e);
     }

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

@@ -37,6 +37,13 @@ public interface WritableDownloadIndex extends DownloadIndex {
    */
   void removeDownload(String id) throws IOException;
 
+  /**
+   * Sets all {@link Download#STATE_DOWNLOADING} states to {@link Download#STATE_QUEUED}.
+   *
+   * @throws IOException If an error occurs updating the state.
+   */
+  void setDownloadingStatesToQueued() throws IOException;
+
   /**
    * Sets the stop reason of the downloads in a terminal state ({@link Download#STATE_COMPLETED},
    * {@link Download#STATE_FAILED}).

library/core/src/test/java/com/google/android/exoplayer2/offline/ActionFileUpgradeUtilTest.java

@@ -38,6 +38,8 @@ import org.junit.runner.RunWith;
 @RunWith(AndroidJUnit4.class)
 public class ActionFileUpgradeUtilTest {
 
+  private static final long NOW_MS = 1234;
+
   private File tempFile;
   private ExoDatabaseProvider databaseProvider;
   private DefaultDownloadIndex downloadIndex;
@@ -113,7 +115,7 @@ public class ActionFileUpgradeUtilTest {
             data);
 
     ActionFileUpgradeUtil.mergeRequest(
-        request, downloadIndex, /* addNewDownloadAsCompleted= */ false);
+        request, downloadIndex, /* addNewDownloadAsCompleted= */ false, NOW_MS);
 
     assertDownloadIndexContainsRequest(request, Download.STATE_QUEUED);
   }
@@ -141,9 +143,9 @@ public class ActionFileUpgradeUtilTest {
             /* customCacheKey= */ "key123",
             new byte[] {5, 4, 3, 2, 1});
     ActionFileUpgradeUtil.mergeRequest(
-        request1, downloadIndex, /* addNewDownloadAsCompleted= */ false);
+        request1, downloadIndex, /* addNewDownloadAsCompleted= */ false, NOW_MS);
     ActionFileUpgradeUtil.mergeRequest(
-        request2, downloadIndex, /* addNewDownloadAsCompleted= */ false);
+        request2, downloadIndex, /* addNewDownloadAsCompleted= */ false, NOW_MS);
 
     Download download = downloadIndex.getDownload(request2.id);
     assertThat(download).isNotNull();
@@ -178,16 +180,16 @@ public class ActionFileUpgradeUtilTest {
             /* customCacheKey= */ "key123",
             new byte[] {5, 4, 3, 2, 1});
     ActionFileUpgradeUtil.mergeRequest(
-        request1, downloadIndex, /* addNewDownloadAsCompleted= */ false);
+        request1, downloadIndex, /* addNewDownloadAsCompleted= */ false, NOW_MS);
 
     // Merging existing download, keeps it queued.
     ActionFileUpgradeUtil.mergeRequest(
-        request1, downloadIndex, /* addNewDownloadAsCompleted= */ true);
+        request1, downloadIndex, /* addNewDownloadAsCompleted= */ true, NOW_MS);
     assertThat(downloadIndex.getDownload(request1.id).state).isEqualTo(Download.STATE_QUEUED);
 
     // New download is merged as completed.
     ActionFileUpgradeUtil.mergeRequest(
-        request2, downloadIndex, /* addNewDownloadAsCompleted= */ true);
+        request2, downloadIndex, /* addNewDownloadAsCompleted= */ true, NOW_MS);
     assertThat(downloadIndex.getDownload(request2.id).state).isEqualTo(Download.STATE_COMPLETED);
   }
 

library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java

@@ -61,6 +61,8 @@ public class DownloadManagerTest {
   private static final int APP_STOP_REASON = 1;
   /** The minimum number of times a task must be retried before failing. */
   private static final int MIN_RETRY_COUNT = 3;
+  /** Dummy value for the current time. */
+  private static final long NOW_MS = 1234;
 
   private Uri uri1;
   private Uri uri2;
@@ -132,6 +134,7 @@ public class DownloadManagerTest {
     task.assertCompleted();
     runner.assertCreatedDownloaderCount(1);
     downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError();
+    assertThat(downloadManager.getCurrentDownloads()).isEmpty();
   }
 
   @Test
@@ -143,6 +146,7 @@ public class DownloadManagerTest {
     task.assertRemoved();
     runner.assertCreatedDownloaderCount(2);
     downloadManagerListener.blockUntilTasksCompleteAndThrowAnyDownloadError();
+    assertThat(downloadManager.getCurrentDownloads()).isEmpty();
   }
 
   @Test
@@ -158,6 +162,7 @@ public class DownloadManagerTest {
     downloader.assertReleased().assertStartCount(MIN_RETRY_COUNT + 1);
     runner.getTask().assertFailed();
     downloadManagerListener.blockUntilTasksComplete();
+    assertThat(downloadManager.getCurrentDownloads()).isEmpty();
   }
 
   @Test
@@ -174,6 +179,7 @@ public class DownloadManagerTest {
     downloader.assertReleased().assertStartCount(MIN_RETRY_COUNT + 1);
     runner.getTask().assertCompleted();
     downloadManagerListener.blockUntilTasksComplete();
+    assertThat(downloadManager.getCurrentDownloads()).isEmpty();
   }
 
   @Test
@@ -341,7 +347,7 @@ public class DownloadManagerTest {
   }
 
   @Test
-  public void getTasks_returnTasks() {
+  public void getCurrentDownloads_returnsCurrentDownloads() {
     TaskWrapper task1 = new DownloadRunner(uri1).postDownloadRequest().getTask();
     TaskWrapper task2 = new DownloadRunner(uri2).postDownloadRequest().getTask();
     TaskWrapper task3 =
@@ -370,13 +376,11 @@ public class DownloadManagerTest {
 
     runOnMainThread(() -> downloadManager.pauseDownloads());
 
-    // TODO: This should be assertQueued. Fix implementation and update test.
+    runner1.getTask().assertQueued();
-    runner1.getTask().assertStopped();
 
     // remove requests aren't stopped.
     runner2.getDownloader(1).unblock().assertReleased();
-    // TODO: This should be assertQueued. Fix implementation and update test.
+    runner2.getTask().assertQueued();
-    runner2.getTask().assertStopped();
     // Although remove2 is finished, download2 doesn't start.
     runner2.getDownloader(2).assertDoesNotStart();
 
@@ -397,7 +401,7 @@ public class DownloadManagerTest {
   }
 
   @Test
-  public void manuallyStopAndResumeSingleDownload() throws Throwable {
+  public void setAndClearSingleDownloadStopReason() throws Throwable {
     DownloadRunner runner = new DownloadRunner(uri1).postDownloadRequest();
     TaskWrapper task = runner.getTask();
 
@@ -415,7 +419,7 @@ public class DownloadManagerTest {
   }
 
   @Test
-  public void manuallyStoppedDownloadCanBeCancelled() throws Throwable {
+  public void setSingleDownloadStopReasonThenRemove_removesDownload() throws Throwable {
     DownloadRunner runner = new DownloadRunner(uri1).postDownloadRequest();
     TaskWrapper task = runner.getTask();
 
@@ -433,7 +437,7 @@ public class DownloadManagerTest {
   }
 
   @Test
-  public void manuallyStoppedSingleDownload_doesNotAffectOthers() throws Throwable {
+  public void setSingleDownloadStopReason_doesNotAffectOtherDownloads() throws Throwable {
     DownloadRunner runner1 = new DownloadRunner(uri1);
     DownloadRunner runner2 = new DownloadRunner(uri2);
     DownloadRunner runner3 = new DownloadRunner(uri3);
@@ -455,21 +459,22 @@ public class DownloadManagerTest {
   }
 
   @Test
-  public void mergeRequest_removingDownload_becomesRestarting() {
+  public void mergeRequest_removing_becomesRestarting() {
     DownloadRequest downloadRequest = createDownloadRequest();
     DownloadBuilder downloadBuilder =
         new DownloadBuilder(downloadRequest).setState(Download.STATE_REMOVING);
     Download download = downloadBuilder.build();
 
     Download mergedDownload =
-        DownloadManager.mergeRequest(download, downloadRequest, download.stopReason);
+        DownloadManager.mergeRequest(download, downloadRequest, download.stopReason, NOW_MS);
 
-    Download expectedDownload = downloadBuilder.setState(Download.STATE_RESTARTING).build();
+    Download expectedDownload =
-    assertEqualIgnoringTimeFields(mergedDownload, expectedDownload);
+        downloadBuilder.setStartTimeMs(NOW_MS).setState(Download.STATE_RESTARTING).build();
+    assertEqualIgnoringUpdateTime(mergedDownload, expectedDownload);
   }
 
   @Test
-  public void mergeRequest_failedDownload_becomesQueued() {
+  public void mergeRequest_failed_becomesQueued() {
     DownloadRequest downloadRequest = createDownloadRequest();
     DownloadBuilder downloadBuilder =
         new DownloadBuilder(downloadRequest)
@@ -478,18 +483,19 @@ public class DownloadManagerTest {
     Download download = downloadBuilder.build();
 
     Download mergedDownload =
-        DownloadManager.mergeRequest(download, downloadRequest, download.stopReason);
+        DownloadManager.mergeRequest(download, downloadRequest, download.stopReason, NOW_MS);
 
     Download expectedDownload =
         downloadBuilder
+            .setStartTimeMs(NOW_MS)
             .setState(Download.STATE_QUEUED)
             .setFailureReason(Download.FAILURE_REASON_NONE)
             .build();
-    assertEqualIgnoringTimeFields(mergedDownload, expectedDownload);
+    assertEqualIgnoringUpdateTime(mergedDownload, expectedDownload);
   }
 
   @Test
-  public void mergeRequest_stoppedDownload_staysStopped() {
+  public void mergeRequest_stopped_staysStopped() {
     DownloadRequest downloadRequest = createDownloadRequest();
     DownloadBuilder downloadBuilder =
         new DownloadBuilder(downloadRequest)
@@ -498,13 +504,13 @@ public class DownloadManagerTest {
     Download download = downloadBuilder.build();
 
     Download mergedDownload =
-        DownloadManager.mergeRequest(download, downloadRequest, download.stopReason);
+        DownloadManager.mergeRequest(download, downloadRequest, download.stopReason, NOW_MS);
 
-    assertEqualIgnoringTimeFields(mergedDownload, download);
+    assertEqualIgnoringUpdateTime(mergedDownload, download);
   }
 
   @Test
-  public void mergeRequest_stopReasonSetButNotStopped_becomesStopped() {
+  public void mergeRequest_completedWithStopReason_becomesStopped() {
     DownloadRequest downloadRequest = createDownloadRequest();
     DownloadBuilder downloadBuilder =
         new DownloadBuilder(downloadRequest)
@@ -513,10 +519,11 @@ public class DownloadManagerTest {
     Download download = downloadBuilder.build();
 
     Download mergedDownload =
-        DownloadManager.mergeRequest(download, downloadRequest, download.stopReason);
+        DownloadManager.mergeRequest(download, downloadRequest, download.stopReason, NOW_MS);
 
-    Download expectedDownload = downloadBuilder.setState(Download.STATE_STOPPED).build();
+    Download expectedDownload =
-    assertEqualIgnoringTimeFields(mergedDownload, expectedDownload);
+        downloadBuilder.setStartTimeMs(NOW_MS).setState(Download.STATE_STOPPED).build();
+    assertEqualIgnoringUpdateTime(mergedDownload, expectedDownload);
   }
 
   private void setUpDownloadManager(final int maxParallelDownloads) throws Exception {
@@ -554,9 +561,10 @@ public class DownloadManagerTest {
     dummyMainThread.runTestOnMainThread(r);
   }
 
-  private static void assertEqualIgnoringTimeFields(Download download, Download that) {
+  private static void assertEqualIgnoringUpdateTime(Download download, Download that) {
     assertThat(download.request).isEqualTo(that.request);
     assertThat(download.state).isEqualTo(that.state);
+    assertThat(download.startTimeMs).isEqualTo(that.startTimeMs);
     assertThat(download.contentLength).isEqualTo(that.contentLength);
     assertThat(download.failureReason).isEqualTo(that.failureReason);
     assertThat(download.stopReason).isEqualTo(that.stopReason);