DefaultDownloadIndex: Clear failure reason when removing download

The Download constructor considers it invalid to have a failure
reason if the download isn't in the failed state. Unfortunately,
calling DefaultDownloadIndex.removeAllDownloads when there's a
failed download will change the state without clearing the reason.
If the downloads are then read back from the DefaultDownloadIndex
we end up violating the Download constructor assertion.

This change clears the failed reason for any existing rows in the
invalid state, and also fixes the root cause that allows invalid
rows to enter the table in the first place.

Issue: #6785
PiperOrigin-RevId: 286576242
This commit is contained in:
olly 2019-12-20 15:41:01 +00:00 committed by Oliver Woodman
parent 406acfc38f
commit 87865a5cc4
4 changed files with 38 additions and 4 deletions

View File

@ -16,6 +16,10 @@
`SsaStyle$SsaAlignment`
([#6771](https://github.com/google/ExoPlayer/issues/6771)).
* Fix `CacheDataSource` to correctly propagate `DataSpec.httpRequestHeaders`.
* Fix issue with `DefaultDownloadIndex` that could result in an
`IllegalStateException` being thrown from
`DefaultDownloadIndex.getDownloadForCurrentRow`
([#6785](https://github.com/google/ExoPlayer/issues/6785)).
* Add missing @Nullable to `MediaCodecAudioRenderer.getMediaClock` and
`SimpleDecoderAudioRenderer.getMediaClock`
([#6792](https://github.com/google/ExoPlayer/issues/6792)).

View File

@ -26,6 +26,8 @@ import androidx.annotation.VisibleForTesting;
import com.google.android.exoplayer2.database.DatabaseIOException;
import com.google.android.exoplayer2.database.DatabaseProvider;
import com.google.android.exoplayer2.database.VersionTable;
import com.google.android.exoplayer2.offline.Download.FailureReason;
import com.google.android.exoplayer2.offline.Download.State;
import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Util;
import java.util.ArrayList;
@ -239,6 +241,9 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
try {
ContentValues values = new ContentValues();
values.put(COLUMN_STATE, Download.STATE_REMOVING);
// Only downloads in STATE_FAILED are allowed a failure reason, so we need to clear it here in
// case we're moving downloads from STATE_FAILED to STATE_REMOVING.
values.put(COLUMN_FAILURE_REASON, Download.FAILURE_REASON_NONE);
SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
writableDatabase.update(tableName, values, /* whereClause= */ null, /* whereArgs= */ null);
} catch (SQLException e) {
@ -351,14 +356,22 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
DownloadProgress downloadProgress = new DownloadProgress();
downloadProgress.bytesDownloaded = cursor.getLong(COLUMN_INDEX_BYTES_DOWNLOADED);
downloadProgress.percentDownloaded = cursor.getFloat(COLUMN_INDEX_PERCENT_DOWNLOADED);
@State int state = cursor.getInt(COLUMN_INDEX_STATE);
// It's possible the database contains failure reasons for non-failed downloads, which is
// invalid. Clear them here. See https://github.com/google/ExoPlayer/issues/6785.
@FailureReason
int failureReason =
state == Download.STATE_FAILED
? cursor.getInt(COLUMN_INDEX_FAILURE_REASON)
: Download.FAILURE_REASON_NONE;
return new Download(
request,
/* state= */ cursor.getInt(COLUMN_INDEX_STATE),
state,
/* startTimeMs= */ cursor.getLong(COLUMN_INDEX_START_TIME_MS),
/* updateTimeMs= */ cursor.getLong(COLUMN_INDEX_UPDATE_TIME_MS),
/* contentLength= */ cursor.getLong(COLUMN_INDEX_CONTENT_LENGTH),
/* stopReason= */ cursor.getInt(COLUMN_INDEX_STOP_REASON),
/* failureReason= */ cursor.getInt(COLUMN_INDEX_FAILURE_REASON),
failureReason,
downloadProgress);
}

View File

@ -130,9 +130,9 @@ public final class Download {
@FailureReason int failureReason,
DownloadProgress progress) {
Assertions.checkNotNull(progress);
Assertions.checkState((failureReason == FAILURE_REASON_NONE) == (state != STATE_FAILED));
Assertions.checkArgument((failureReason == FAILURE_REASON_NONE) == (state != STATE_FAILED));
if (stopReason != 0) {
Assertions.checkState(state != STATE_DOWNLOADING && state != STATE_QUEUED);
Assertions.checkArgument(state != STATE_DOWNLOADING && state != STATE_QUEUED);
}
this.request = request;
this.state = state;

View File

@ -248,6 +248,23 @@ public class DefaultDownloadIndexTest {
assertEqual(readDownload, download);
}
@Test
public void setStatesToRemoving_setsStateAndClearsFailureReason() throws Exception {
String id = "id";
DownloadBuilder downloadBuilder =
new DownloadBuilder(id)
.setState(Download.STATE_FAILED)
.setFailureReason(Download.FAILURE_REASON_UNKNOWN);
Download download = downloadBuilder.build();
downloadIndex.putDownload(download);
downloadIndex.setStatesToRemoving();
download = downloadIndex.getDownload(id);
assertThat(download.state).isEqualTo(Download.STATE_REMOVING);
assertThat(download.failureReason).isEqualTo(Download.FAILURE_REASON_NONE);
}
@Test
public void setSingleDownloadStopReason_setReasonToNone() throws Exception {
String id = "id";