Don't create Downloads until DownloadManager is initialized

PiperOrigin-RevId: 225824428
qiqian · Dec 18, 2018 · 3d6707e · 3d6707e
1 parent f4fd3b1
commit 3d6707e
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 14 deletions.
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
@@ -38,7 +38,6 @@
 import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
-import java.util.List;
 import java.util.concurrent.CopyOnWriteArraySet;
 import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
 
@@ -93,6 +92,7 @@ public interface Listener {
   private final HandlerThread fileIOThread;
   private final Handler fileIOHandler;
   private final CopyOnWriteArraySet<Listener> listeners;
+  private final ArrayDeque<DownloadAction> actionQueue;
 
   private boolean initialized;
   private boolean released;
@@ -142,6 +142,7 @@ public DownloadManager(
     fileIOHandler = new Handler(fileIOThread.getLooper());
 
     listeners = new CopyOnWriteArraySet<>();
+    actionQueue = new ArrayDeque<>();
 
     loadActions();
     logd("Created");
@@ -194,8 +195,8 @@ public void stopDownloads() {
    */
   public void handleAction(DownloadAction action) {
     Assertions.checkState(!released);
-    Download download = getDownloadForAction(action);
     if (initialized) {
+      Download download = getOrAddDownloadForAction(action);
       saveActions();
       maybeStartDownloads();
       if (download.state == STATE_QUEUED) {
@@ -204,6 +205,8 @@ public void handleAction(DownloadAction action) {
         // reported to listeners. Do so now.
         notifyListenersDownloadStateChange(download);
       }
+    } else {
+      actionQueue.add(action);
     }
   }
 
@@ -281,7 +284,7 @@ public void release() {
     logd("Released");
   }
 
-  private Download getDownloadForAction(DownloadAction action) {
+  private Download getOrAddDownloadForAction(DownloadAction action) {
     for (int i = 0; i < downloads.size(); i++) {
       Download download = downloads.get(i);
       if (download.action.isSameMedia(action)) {
@@ -380,18 +383,18 @@ private void loadActions() {
                 if (released) {
                   return;
                 }
-                List<Download> pendingDownloads = new ArrayList<>(downloads);
-                downloads.clear();
                 for (DownloadAction action : actions) {
-                  getDownloadForAction(action);
+                  getOrAddDownloadForAction(action);
                 }
                 logd("Downloads are created.");
                 initialized = true;
                 for (Listener listener : listeners) {
                   listener.onInitialized(DownloadManager.this);
                 }
-                if (!pendingDownloads.isEmpty()) {
-                  downloads.addAll(pendingDownloads);
+                if (!actionQueue.isEmpty()) {
+                  while (!actionQueue.isEmpty()) {
+                    getOrAddDownloadForAction(actionQueue.remove());
+                  }
                   saveActions();
                 }
                 maybeStartDownloads();

diff --git a/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadManagerTest.java
@@ -426,9 +426,9 @@ private void setUpDownloadManager(final int maxActiveDownloadTasks) throws Excep
                     actionFile, downloaderFactory, maxActiveDownloadTasks, MIN_RETRY_COUNT);
             downloadManagerListener =
                 new TestDownloadManagerListener(downloadManager, dummyMainThread);
-            downloadManager.addListener(downloadManagerListener);
             downloadManager.startDownloads();
           });
+      downloadManagerListener.waitUntilInitialized();
     } catch (Throwable throwable) {
       throw new Exception(throwable);
     }

diff --git a/.../test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java b/.../test/java/com/google/android/exoplayer2/source/dash/offline/DownloadManagerDashTest.java
@@ -247,7 +247,6 @@ private void createDownloadManager() {
 
           downloadManagerListener =
               new TestDownloadManagerListener(downloadManager, dummyMainThread);
-          downloadManager.addListener(downloadManagerListener);
           downloadManager.startDownloads();
         });
   }

diff --git a/.../test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java b/.../test/java/com/google/android/exoplayer2/source/dash/offline/DownloadServiceDashTest.java
@@ -124,7 +124,6 @@ public void setUp() throws IOException {
                   /* minRetryCount= */ 3);
           downloadManagerListener =
               new TestDownloadManagerListener(dashDownloadManager, dummyMainThread);
-          dashDownloadManager.addListener(downloadManagerListener);
           dashDownloadManager.startDownloads();
 
           dashDownloadService =

diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/DummyMainThread.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/DummyMainThread.java
@@ -21,6 +21,8 @@
 import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.Looper;
+import com.google.android.exoplayer2.util.Util;
+import java.util.concurrent.atomic.AtomicReference;
 
 /** Helper class to simulate main/UI thread in tests. */
 public final class DummyMainThread {
@@ -58,13 +60,21 @@ public void runOnMainThread(int timeoutMs, final Runnable runnable) {
     if (Looper.myLooper() == handler.getLooper()) {
       runnable.run();
     } else {
-      final ConditionVariable finishedCondition = new ConditionVariable();
+      ConditionVariable finishedCondition = new ConditionVariable();
+      AtomicReference<Throwable> thrown = new AtomicReference<>();
       handler.post(
           () -> {
-            runnable.run();
+            try {
+              runnable.run();
+            } catch (Throwable t) {
+              thrown.set(t);
+            }
             finishedCondition.open();
           });
       assertThat(finishedCondition.block(timeoutMs)).isTrue();
+      if (thrown.get() != null) {
+        Util.sneakyThrow(thrown.get());
+      }
     }
   }
 

diff --git a/...ric/src/main/java/com/google/android/exoplayer2/testutil/TestDownloadManagerListener.java b/...ric/src/main/java/com/google/android/exoplayer2/testutil/TestDownloadManagerListener.java
@@ -17,6 +17,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
+import android.os.ConditionVariable;
 import com.google.android.exoplayer2.offline.DownloadManager;
 import com.google.android.exoplayer2.offline.DownloadManager.DownloadState;
 import java.util.HashMap;
@@ -28,10 +29,12 @@
 public final class TestDownloadManagerListener implements DownloadManager.Listener {
 
   private static final int TIMEOUT = 1000;
+  private static final int INITIALIZATION_TIMEOUT = 10000;
 
   private final DownloadManager downloadManager;
   private final DummyMainThread dummyMainThread;
   private final HashMap<String, ArrayBlockingQueue<Integer>> actionStates;
+  private final ConditionVariable initializedCondition;
 
   private CountDownLatch downloadFinishedCondition;
   @DownloadState.FailureReason private int failureReason;
@@ -41,6 +44,8 @@ public TestDownloadManagerListener(
     this.downloadManager = downloadManager;
     this.dummyMainThread = dummyMainThread;
     actionStates = new HashMap<>();
+    initializedCondition = new ConditionVariable();
+    downloadManager.addListener(this);
   }
 
   public Integer pollStateChange(String taskId, long timeoutMs) throws InterruptedException {
@@ -53,7 +58,13 @@ public void clearDownloadError() {
 
   @Override
   public void onInitialized(DownloadManager downloadManager) {
-    // Do nothing.
+    initializedCondition.open();
+  }
+
+  public void waitUntilInitialized() {
+    if (!downloadManager.isInitialized()) {
+      assertThat(initializedCondition.block(INITIALIZATION_TIMEOUT)).isTrue();
+    }
   }
 
   @Override