Skip to content

Commit

Permalink
Merge pull request #100 from xwp/fix/cancel-downloads-before-removeall
Browse files Browse the repository at this point in the history
Cancel any ongoing downloads before removeAll
  • Loading branch information
derekherman authored Mar 17, 2021
2 parents 935c674 + 9ee98b5 commit d08e337
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 7 deletions.
12 changes: 11 additions & 1 deletion src/js/components/VideoDownloader.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ export default class extends HTMLElement {
};
this.storageManager.ondone = () => {
this.progress = 100;
this.downloading = false;
this.state = 'done';
};

Expand Down Expand Up @@ -427,6 +428,16 @@ export default class extends HTMLElement {
}
}

/**
* Cancels any ongoing operation.
*/
cancel() {
if (this.downloadManager) this.downloadManager.cancel();
if (this.storageManager) this.storageManager.cancel();

this.downloading = false;
}

/**
* @returns {VideoMeta} Video meta value.
*/
Expand Down Expand Up @@ -475,7 +486,6 @@ export default class extends HTMLElement {
async removeFromIDB() {
const db = await getIDBConnection();

this.state = 'removing';
await db.removeVideo(this.getId(), this.internal.files);
this.init(this.internal.apiData, this.internal.cacheName);
}
Expand Down
10 changes: 9 additions & 1 deletion src/js/components/VideoDownloader/DownloadManager.module.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export default class {
constructor(videoDownloader) {
this.files = videoDownloader.internal.files || [];
this.paused = false;
this.cancelled = false;

this.internal = {
videoDownloader,
Expand Down Expand Up @@ -138,12 +139,19 @@ export default class {
this.paused = true;
}

/**
* Cancels the download.
*/
cancel() {
this.cancelled = true;
}

/**
* Starts downloading files.
*/
async run() {
this.paused = false;
while (!this.done && !this.paused && this.currentFileMeta) {
while (!this.done && !this.paused && !this.cancelled && this.currentFileMeta) {
/* eslint-disable no-await-in-loop */
// Await in loop is OK. We don't want to download files in parallel.
await this.downloadFile();
Expand Down
9 changes: 9 additions & 0 deletions src/js/components/VideoDownloader/StorageManager.module.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ export default class {
};
}

/**
* Cancels all storage operations.
*/
cancel() {
this.cancelled = true;
}

/**
* Receives the updated file meta object and a file chunk and updates the
* database with new information.
Expand All @@ -35,6 +42,8 @@ export default class {
* @returns {Promise} Promise that resolves when the write operations are complete.
*/
async storeChunk(fileMeta, fileChunk, isDone) {
if (this.cancelled) return true;

const db = await getIDBConnection();
const videoMeta = {
done: isDone,
Expand Down
19 changes: 14 additions & 5 deletions src/js/modules/VideoDownloaderRegistry.module.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import VideoDownloader from '../components/VideoDownloader';
*/
export default class VideoDownloaderRegistry {
constructor() {
this.instances = {};
this.instances = new Map();
}

/**
Expand All @@ -20,9 +20,9 @@ export default class VideoDownloaderRegistry {
* @returns {VideoDownloader} Instantiated VideoDownloader.
*/
create(videoId) {
this.instances[videoId] = new VideoDownloader();
this.instances.set(videoId, new VideoDownloader());

return this.instances[videoId];
return this.instances.get(videoId);
}

/**
Expand All @@ -33,14 +33,23 @@ export default class VideoDownloaderRegistry {
* @returns {VideoDownloader|null} `VideoDownloader` instance.
*/
get(videoId) {
return this.instances[videoId] || null;
return this.instances.get(videoId) || null;
}

/**
* Returns an iterator over all stored instances.
*
* @returns {Iterator<string, VideoDownloader>} Iterator over all VideoDownloader instance.
*/
getAll() {
return this.instances.entries();
}

/**
* Detaches the `VideoDownload` instances and leaves them to be
* garbage collected.
*/
destroyAll() {
this.instances = {};
this.instances.clear();
}
}
8 changes: 8 additions & 0 deletions src/js/pages/Downloads.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ export default async (routerContext) => {
deleteAllBtn.addEventListener('click', async (e) => {
const btn = e.target;

/**
* @type {Iterator<string, VideoDownloader>}
*/
const allDownloaders = Array.from(videoDownloaderRegistry.getAll());
allDownloaders.forEach(
([, downloader]) => downloader.cancel(),
);

grid.classList.add('clearing');
btn.classList.add('clearing');

Expand Down
1 change: 1 addition & 0 deletions src/js/typedefs.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
* @typedef {object} VideoDownloaderRegistry
* @property {Function} create Creates a new `VideoDownload` instance.
* @property {Function} get Returns a previously created `VideoDownload` instance or null.
* @property {Function} getAll Returns an iterator over all `VideoDownload` instances.
* @property {Function} destroyAll Detaches all `VideoDownload` instances, clears the registry.
* @property {object} instances Holds `VideoDownload` instances keyed by video IDs.
*/
Expand Down

0 comments on commit d08e337

Please sign in to comment.