Commit 9779d71a by olly Committed by Oliver Woodman

Fix stopping downloads

- Changed startDownloads/stopDownloads back to their previous behavior
  of starting and stopping all downloads at the manager level.
- Made setManualStopReason methods for the new case of setting a manual
  stop reason or some or all downloads.
- Added plumbing to specify an initial manual stop reason when adding a
  new download, without also overwriting the manual stop reasons of all
  other preexisting downloads. Using the value is left as a TODO pending
  a bit of further cleanup that'll make it easier.

PiperOrigin-RevId: 242891688
parent 68993f23
...@@ -46,7 +46,7 @@ public final class Download { ...@@ -46,7 +46,7 @@ public final class Download {
// Important: These constants are persisted into DownloadIndex. Do not change them. // Important: These constants are persisted into DownloadIndex. Do not change them.
/** The download is waiting to be started. */ /** The download is waiting to be started. */
public static final int STATE_QUEUED = 0; public static final int STATE_QUEUED = 0;
/** The download is stopped. */ /** The download is manually stopped for the reason specified by {@link #manualStopReason}. */
public static final int STATE_STOPPED = 1; public static final int STATE_STOPPED = 1;
/** The download is currently started. */ /** The download is currently started. */
public static final int STATE_DOWNLOADING = 2; public static final int STATE_DOWNLOADING = 2;
...@@ -71,8 +71,6 @@ public final class Download { ...@@ -71,8 +71,6 @@ public final class Download {
/** The download isn't manually stopped. */ /** The download isn't manually stopped. */
public static final int MANUAL_STOP_REASON_NONE = 0; public static final int MANUAL_STOP_REASON_NONE = 0;
/** The download is manually stopped but a reason isn't specified. */
public static final int MANUAL_STOP_REASON_UNDEFINED = Integer.MAX_VALUE;
/** Returns the state string for the given state value. */ /** Returns the state string for the given state value. */
public static String getStateString(@State int state) { public static String getStateString(@State int state) {
...@@ -122,7 +120,7 @@ public final class Download { ...@@ -122,7 +120,7 @@ public final class Download {
* #FAILURE_REASON_NONE}. * #FAILURE_REASON_NONE}.
*/ */
@FailureReason public final int failureReason; @FailureReason public final int failureReason;
/** The manual stop reason. */ /** The reason the download is manually stopped, or {@link #MANUAL_STOP_REASON_NONE}. */
public final int manualStopReason; public final int manualStopReason;
/*package*/ CachingCounters counters; /*package*/ CachingCounters counters;
...@@ -232,7 +230,7 @@ public final class Download { ...@@ -232,7 +230,7 @@ public final class Download {
this.counters = counters; this.counters = counters;
} }
private static int getNextState(int currentState, boolean canStart) { private static int getNextState(@State int currentState, boolean canStart) {
if (currentState == STATE_REMOVING || currentState == STATE_RESTARTING) { if (currentState == STATE_REMOVING || currentState == STATE_RESTARTING) {
return STATE_RESTARTING; return STATE_RESTARTING;
} else if (canStart) { } else if (canStart) {
......
...@@ -47,14 +47,16 @@ import org.robolectric.shadows.ShadowLog; ...@@ -47,14 +47,16 @@ import org.robolectric.shadows.ShadowLog;
@Config(shadows = {RobolectricUtil.CustomLooper.class, RobolectricUtil.CustomMessageQueue.class}) @Config(shadows = {RobolectricUtil.CustomLooper.class, RobolectricUtil.CustomMessageQueue.class})
public class DownloadManagerTest { public class DownloadManagerTest {
/* Used to check if condition becomes true in this time interval. */ /** Used to check if condition becomes true in this time interval. */
private static final int ASSERT_TRUE_TIMEOUT = 10000; private static final int ASSERT_TRUE_TIMEOUT = 10000;
/* Used to check if condition stays false for this time interval. */ /** Used to check if condition stays false for this time interval. */
private static final int ASSERT_FALSE_TIME = 1000; private static final int ASSERT_FALSE_TIME = 1000;
/* Maximum retry delay in DownloadManager. */ /** Maximum retry delay in DownloadManager. */
private static final int MAX_RETRY_DELAY = 5000; private static final int MAX_RETRY_DELAY = 5000;
/* Maximum number of times a downloader can be restarted before doing a released check. */ /** Maximum number of times a downloader can be restarted before doing a released check. */
private static final int MAX_STARTS_BEFORE_RELEASED = 1; private static final int MAX_STARTS_BEFORE_RELEASED = 1;
/** A manual stop reason. */
private static final int APP_STOP_REASON = 1;
/** The minimum number of times a task must be retried before failing. */ /** The minimum number of times a task must be retried before failing. */
private static final int MIN_RETRY_COUNT = 3; private static final int MIN_RETRY_COUNT = 3;
...@@ -388,17 +390,18 @@ public class DownloadManagerTest { ...@@ -388,17 +390,18 @@ public class DownloadManagerTest {
} }
@Test @Test
public void stopAndResumeSingleDownload() throws Throwable { public void manuallyStopAndResumeSingleDownload() throws Throwable {
DownloadRunner runner = new DownloadRunner(uri1).postDownloadAction(); DownloadRunner runner = new DownloadRunner(uri1).postDownloadAction();
TaskWrapper task = runner.getTask(); TaskWrapper task = runner.getTask();
task.assertDownloading(); task.assertDownloading();
runOnMainThread(() -> downloadManager.stopDownload(task.taskId)); runOnMainThread(() -> downloadManager.setManualStopReason(task.taskId, APP_STOP_REASON));
task.assertStopped(); task.assertStopped();
runOnMainThread(() -> downloadManager.startDownload(task.taskId)); runOnMainThread(
() -> downloadManager.setManualStopReason(task.taskId, Download.MANUAL_STOP_REASON_NONE));
runner.getDownloader(1).assertStarted().unblock(); runner.getDownloader(1).assertStarted().unblock();
...@@ -406,13 +409,13 @@ public class DownloadManagerTest { ...@@ -406,13 +409,13 @@ public class DownloadManagerTest {
} }
@Test @Test
public void stoppedDownloadCanBeCancelled() throws Throwable { public void manuallyStoppedDownloadCanBeCancelled() throws Throwable {
DownloadRunner runner = new DownloadRunner(uri1).postDownloadAction(); DownloadRunner runner = new DownloadRunner(uri1).postDownloadAction();
TaskWrapper task = runner.getTask(); TaskWrapper task = runner.getTask();
task.assertDownloading(); task.assertDownloading();
runOnMainThread(() -> downloadManager.stopDownload(task.taskId)); runOnMainThread(() -> downloadManager.setManualStopReason(task.taskId, APP_STOP_REASON));
task.assertStopped(); task.assertStopped();
...@@ -424,7 +427,7 @@ public class DownloadManagerTest { ...@@ -424,7 +427,7 @@ public class DownloadManagerTest {
} }
@Test @Test
public void stoppedSingleDownload_doesNotAffectOthers() throws Throwable { public void manuallyStoppedSingleDownload_doesNotAffectOthers() throws Throwable {
DownloadRunner runner1 = new DownloadRunner(uri1); DownloadRunner runner1 = new DownloadRunner(uri1);
DownloadRunner runner2 = new DownloadRunner(uri2); DownloadRunner runner2 = new DownloadRunner(uri2);
DownloadRunner runner3 = new DownloadRunner(uri3); DownloadRunner runner3 = new DownloadRunner(uri3);
...@@ -432,7 +435,8 @@ public class DownloadManagerTest { ...@@ -432,7 +435,8 @@ public class DownloadManagerTest {
runner1.postDownloadAction().getTask().assertDownloading(); runner1.postDownloadAction().getTask().assertDownloading();
runner2.postDownloadAction().postRemoveAction().getTask().assertRemoving(); runner2.postDownloadAction().postRemoveAction().getTask().assertRemoving();
runOnMainThread(() -> downloadManager.stopDownload(runner1.getTask().taskId)); runOnMainThread(
() -> downloadManager.setManualStopReason(runner1.getTask().taskId, APP_STOP_REASON));
runner1.getTask().assertStopped(); runner1.getTask().assertStopped();
...@@ -460,9 +464,9 @@ public class DownloadManagerTest { ...@@ -460,9 +464,9 @@ public class DownloadManagerTest {
maxActiveDownloadTasks, maxActiveDownloadTasks,
MIN_RETRY_COUNT, MIN_RETRY_COUNT,
new Requirements(0)); new Requirements(0));
downloadManager.startDownloads();
downloadManagerListener = downloadManagerListener =
new TestDownloadManagerListener(downloadManager, dummyMainThread); new TestDownloadManagerListener(downloadManager, dummyMainThread);
downloadManager.startDownloads();
}); });
downloadManagerListener.waitUntilInitialized(); downloadManagerListener.waitUntilInitialized();
} catch (Throwable throwable) { } catch (Throwable throwable) {
......
...@@ -156,7 +156,7 @@ public class DownloadTest { ...@@ -156,7 +156,7 @@ public class DownloadTest {
DownloadBuilder downloadBuilder = DownloadBuilder downloadBuilder =
new DownloadBuilder(downloadAction) new DownloadBuilder(downloadAction)
.setState(Download.STATE_STOPPED) .setState(Download.STATE_STOPPED)
.setManualStopReason(Download.MANUAL_STOP_REASON_UNDEFINED); .setManualStopReason(/* manualStopReason= */ 1);
Download download = downloadBuilder.build(); Download download = downloadBuilder.build();
Download mergedDownload = download.copyWithMergedAction(downloadAction, /* canStart= */ true); Download mergedDownload = download.copyWithMergedAction(downloadAction, /* canStart= */ true);
...@@ -170,7 +170,7 @@ public class DownloadTest { ...@@ -170,7 +170,7 @@ public class DownloadTest {
DownloadBuilder downloadBuilder = DownloadBuilder downloadBuilder =
new DownloadBuilder(downloadAction) new DownloadBuilder(downloadAction)
.setState(Download.STATE_COMPLETED) .setState(Download.STATE_COMPLETED)
.setManualStopReason(Download.MANUAL_STOP_REASON_UNDEFINED); .setManualStopReason(/* manualStopReason= */ 1);
Download download = downloadBuilder.build(); Download download = downloadBuilder.build();
Download mergedDownload = download.copyWithMergedAction(downloadAction, /* canStart= */ true); Download mergedDownload = download.copyWithMergedAction(downloadAction, /* canStart= */ true);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment