Commit a0c21461 by olly Committed by Oliver Woodman

Ensure getAllDownloadStates order is stable

HashMap.values does not guarantee a stable order, but our tests
assert that the order is stable. It's probably a nice property
that the order is the same as that in which downloads are added,
so this change makes the order stable, with that order.

A future change will likely improve the way this works, by
changing the internal thread "downloads" variable to have type
ArrayList<Download>, at which point the internal thread can just
pass a read-only copy of its list to the main thread whenever a
state changes.

PiperOrigin-RevId: 242460725
parent 8b766ba4
...@@ -162,7 +162,7 @@ public final class DownloadManager { ...@@ -162,7 +162,7 @@ public final class DownloadManager {
// Collections that are accessed on the main thread. // Collections that are accessed on the main thread.
private final CopyOnWriteArraySet<Listener> listeners; private final CopyOnWriteArraySet<Listener> listeners;
private final HashMap<String, DownloadState> downloadStates; private final ArrayList<DownloadState> downloadStates;
// Collections that are accessed on the internal thread. // Collections that are accessed on the internal thread.
private final ArrayList<Download> downloads; private final ArrayList<Download> downloads;
...@@ -249,7 +249,7 @@ public final class DownloadManager { ...@@ -249,7 +249,7 @@ public final class DownloadManager {
manualStopReason = MANUAL_STOP_REASON_UNDEFINED; manualStopReason = MANUAL_STOP_REASON_UNDEFINED;
downloads = new ArrayList<>(); downloads = new ArrayList<>();
downloadStates = new HashMap<>(); downloadStates = new ArrayList<>();
activeDownloads = new HashMap<>(); activeDownloads = new HashMap<>();
listeners = new CopyOnWriteArraySet<>(); listeners = new CopyOnWriteArraySet<>();
releaseLock = new Object(); releaseLock = new Object();
...@@ -292,7 +292,7 @@ public final class DownloadManager { ...@@ -292,7 +292,7 @@ public final class DownloadManager {
/** Returns the states of all current downloads. */ /** Returns the states of all current downloads. */
public DownloadState[] getAllDownloadStates() { public DownloadState[] getAllDownloadStates() {
return downloadStates.values().toArray(new DownloadState[0]); return downloadStates.toArray(new DownloadState[0]);
} }
/** Returns the requirements needed to be met to start downloads. */ /** Returns the requirements needed to be met to start downloads. */
...@@ -497,20 +497,22 @@ public final class DownloadManager { ...@@ -497,20 +497,22 @@ public final class DownloadManager {
// allows updating idle at the same point as the downloads that can be queried changes. // allows updating idle at the same point as the downloads that can be queried changes.
private void onInitialized(List<DownloadState> downloadStates) { private void onInitialized(List<DownloadState> downloadStates) {
initialized = true; initialized = true;
for (int i = 0; i < downloadStates.size(); i++) { this.downloadStates.addAll(downloadStates);
DownloadState downloadState = downloadStates.get(i);
this.downloadStates.put(downloadState.id, downloadState);
}
for (Listener listener : listeners) { for (Listener listener : listeners) {
listener.onInitialized(DownloadManager.this); listener.onInitialized(DownloadManager.this);
} }
} }
private void onDownloadStateChanged(DownloadState downloadState) { private void onDownloadStateChanged(DownloadState downloadState) {
int downloadStateIndex = getDownloadStateIndex(downloadState.id);
if (downloadState.state == STATE_COMPLETED || downloadState.state == STATE_FAILED) { if (downloadState.state == STATE_COMPLETED || downloadState.state == STATE_FAILED) {
downloadStates.remove(downloadState.id); if (downloadStateIndex != C.INDEX_UNSET) {
downloadStates.remove(downloadStateIndex);
}
} else if (downloadStateIndex != C.INDEX_UNSET) {
downloadStates.set(downloadStateIndex, downloadState);
} else { } else {
downloadStates.put(downloadState.id, downloadState); downloadStates.add(downloadState);
} }
for (Listener listener : listeners) { for (Listener listener : listeners) {
listener.onDownloadStateChanged(this, downloadState); listener.onDownloadStateChanged(this, downloadState);
...@@ -518,7 +520,7 @@ public final class DownloadManager { ...@@ -518,7 +520,7 @@ public final class DownloadManager {
} }
private void onDownloadRemoved(DownloadState downloadState) { private void onDownloadRemoved(DownloadState downloadState) {
downloadStates.remove(downloadState.id); downloadStates.remove(getDownloadStateIndex(downloadState.id));
for (Listener listener : listeners) { for (Listener listener : listeners) {
listener.onDownloadRemoved(this, downloadState); listener.onDownloadRemoved(this, downloadState);
} }
...@@ -534,6 +536,15 @@ public final class DownloadManager { ...@@ -534,6 +536,15 @@ public final class DownloadManager {
} }
} }
private int getDownloadStateIndex(String id) {
for (int i = 0; i < downloadStates.size(); i++) {
if (downloadStates.get(i).id.equals(id)) {
return i;
}
}
return C.INDEX_UNSET;
}
// Internal thread message handling. // Internal thread message handling.
private boolean handleInternalMessage(Message message) { private boolean handleInternalMessage(Message message) {
......
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