From bb744e5f9bb267604beea668890559b7b1151a18 Mon Sep 17 00:00:00 2001
From: olly <olly@google.com>
Date: Fri, 1 May 2020 17:26:00 +0100
Subject: [PATCH] DownloadManager.Task: Remove top level thread interrupt

DownloadManager doesn't know enough about the Downloader implementations to
know that interrupting the thread is the right thing to do. In particular,
if a Downloader implementation wants to delegate work to additional worker
threads, then it will probably not want its main thread to be interrupted
on cancelation. Instead, it will want to cancel/interrupt its worker threads,
and keep its main thread blocked until work on those worker threads has
stopped (download() must not return whilst the Downloader is still touching
the cache).

This change moves control over what happens to the individual Downloader
implementations.

Issue: #5978
PiperOrigin-RevId: 309419177
---
 .../android/exoplayer2/offline/DownloadManager.java    |  2 --
 .../exoplayer2/offline/ProgressiveDownloader.java      | 10 ++++++++++
 .../android/exoplayer2/offline/SegmentDownloader.java  | 10 ++++++++++
 3 files changed, 20 insertions(+), 2 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
index 24d7993141..32931d9f32 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
@@ -1284,8 +1284,6 @@ public final class DownloadManager {
       if (!isCanceled) {
         isCanceled = true;
         downloader.cancel();
-        // TODO - This will need propagating deeper as soon as we start using additional threads.
-        interrupt();
       }
     }
 
diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java
index ad3f901653..6ad186b575 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java
@@ -35,6 +35,8 @@ public final class ProgressiveDownloader implements Downloader {
   private final CacheDataSource dataSource;
   private final AtomicBoolean isCanceled;
 
+  @Nullable private volatile Thread downloadThread;
+
   /**
    * @param uri Uri of the data to be downloaded.
    * @param customCacheKey A custom key that uniquely identifies the original stream. Used for cache
@@ -74,6 +76,10 @@ public final class ProgressiveDownloader implements Downloader {
 
   @Override
   public void download(@Nullable ProgressListener progressListener) throws IOException {
+    downloadThread = Thread.currentThread();
+    if (isCanceled.get()) {
+      return;
+    }
     @Nullable PriorityTaskManager priorityTaskManager = dataSource.getUpstreamPriorityTaskManager();
     if (priorityTaskManager != null) {
       priorityTaskManager.add(C.PRIORITY_DOWNLOAD);
@@ -96,6 +102,10 @@ public final class ProgressiveDownloader implements Downloader {
   @Override
   public void cancel() {
     isCanceled.set(true);
+    @Nullable Thread downloadThread = this.downloadThread;
+    if (downloadThread != null) {
+      downloadThread.interrupt();
+    }
   }
 
   @Override
diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/SegmentDownloader.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/SegmentDownloader.java
index b332b85a4d..601945c69d 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/offline/SegmentDownloader.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/SegmentDownloader.java
@@ -76,6 +76,8 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme
   private final Executor executor;
   private final AtomicBoolean isCanceled;
 
+  @Nullable private volatile Thread downloadThread;
+
   /**
    * @param manifestUri The {@link Uri} of the manifest to be downloaded.
    * @param manifestParser A parser for the manifest.
@@ -103,6 +105,10 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme
 
   @Override
   public final void download(@Nullable ProgressListener progressListener) throws IOException {
+    downloadThread = Thread.currentThread();
+    if (isCanceled.get()) {
+      return;
+    }
     @Nullable
     PriorityTaskManager priorityTaskManager =
         cacheDataSourceFactory.getUpstreamPriorityTaskManager();
@@ -183,6 +189,10 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme
   @Override
   public void cancel() {
     isCanceled.set(true);
+    @Nullable Thread downloadThread = this.downloadThread;
+    if (downloadThread != null) {
+      downloadThread.interrupt();
+    }
   }
 
   @Override