From a5cc491a9269994c30e4c6efbd281c332951e281 Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Fri, 29 Apr 2022 20:11:31 -0700 Subject: [PATCH] fix(offline): Clean up orphaned segments on abort (#4177) If a storage operation is aborted (via API, not via closing the page), this will now be cleaned up from the database. More work is needed to find and remove orphaned segments in the database. Related to #4166 and follow-up to PR #4176. --- lib/offline/storage.js | 114 ++++++++++++++++++++--------------------- 1 file changed, 56 insertions(+), 58 deletions(-) diff --git a/lib/offline/storage.js b/lib/offline/storage.js index 7bfd42a691..e028b79602 100644 --- a/lib/offline/storage.js +++ b/lib/offline/storage.js @@ -523,42 +523,54 @@ shaka.offline.Storage = class { const usingBgFetch = false; // TODO: Get. - if (this.getManifestIsEncrypted_(manifest) && usingBgFetch && - !this.getManifestIncludesInitData_(manifest)) { - // Background fetch can't make DRM sessions, so if we have to get the - // init data from the init segments, download those first before anything - // else. - await download(toDownload.filter((info) => info.isInitSegment), true); - this.ensureNotDestroyed_(); - toDownload = toDownload.filter((info) => !info.isInitSegment); - - // Copy these and reset them now, before calling await. - const manifestUpdates = pendingManifestUpdates; - const dataSize = pendingDataSize; - pendingManifestUpdates = {}; - pendingDataSize = 0; - - manifestDB = - (await shaka.offline.Storage.assignSegmentsToManifest( - manifestId, manifestUpdates, dataSize, - () => this.ensureNotDestroyed_())) || manifestDB; - this.ensureNotDestroyed_(); - } + try { + if (this.getManifestIsEncrypted_(manifest) && usingBgFetch && + !this.getManifestIncludesInitData_(manifest)) { + // Background fetch can't make DRM sessions, so if we have to get the + // init data from the init segments, download those first before + // anything else. + await download(toDownload.filter((info) => info.isInitSegment), true); + this.ensureNotDestroyed_(); + toDownload = toDownload.filter((info) => !info.isInitSegment); - if (!usingBgFetch) { - await download(toDownload, false); - this.ensureNotDestroyed_(); + // Copy these and reset them now, before calling await. + const manifestUpdates = pendingManifestUpdates; + const dataSize = pendingDataSize; + pendingManifestUpdates = {}; + pendingDataSize = 0; - manifestDB = - (await shaka.offline.Storage.assignSegmentsToManifest( - manifestId, pendingManifestUpdates, pendingDataSize, - () => this.ensureNotDestroyed_())) || manifestDB; - this.ensureNotDestroyed_(); + await shaka.offline.Storage.assignSegmentsToManifest( + storage, manifestId, manifestDB, manifestUpdates, dataSize, + () => this.ensureNotDestroyed_()); + this.ensureNotDestroyed_(); + } - goog.asserts.assert( - !manifestDB.isIncomplete, 'The manifest should be complete by now'); - } else { - // TODO: Send the request to the service worker. Don't await the result. + if (!usingBgFetch) { + await download(toDownload, false); + this.ensureNotDestroyed_(); + + // Copy these and reset them now, before calling await. + const manifestUpdates = pendingManifestUpdates; + const dataSize = pendingDataSize; + pendingManifestUpdates = {}; + pendingDataSize = 0; + + await shaka.offline.Storage.assignSegmentsToManifest( + storage, manifestId, manifestDB, manifestUpdates, dataSize, + () => this.ensureNotDestroyed_()); + this.ensureNotDestroyed_(); + + goog.asserts.assert( + !manifestDB.isIncomplete, 'The manifest should be complete by now'); + } else { + // TODO: Send the request to the service worker. Don't await the result. + } + } catch (error) { + const dataKeys = Object.values(pendingManifestUpdates); + // Remove these pending segments that are not yet linked to the manifest. + await storage.removeSegments(dataKeys, (key) => {}); + + throw error; } } @@ -582,38 +594,27 @@ shaka.offline.Storage = class { } /** - * Load the given manifest, assigns database key to all the segments, then - * stores the updated manifest. + * Updates the given manifest, assigns database keys to segments, then stores + * the updated manifest. * * It is up to the caller to ensure that this method is not called * concurrently on the same manifest. * + * @param {shaka.extern.StorageCell} storage * @param {number} manifestId + * @param {!shaka.extern.ManifestDB} manifestDB * @param {!Object.} manifestUpdates * @param {number} dataSizeUpdate * @param {function()} throwIfAbortedFn A function that should throw if the * download has been aborted. - * @return {!Promise.} + * @return {!Promise} */ static async assignSegmentsToManifest( - manifestId, manifestUpdates, dataSizeUpdate, throwIfAbortedFn) { - /** @type {shaka.offline.StorageMuxer} */ - const muxer = new shaka.offline.StorageMuxer(); - + storage, manifestId, manifestDB, manifestUpdates, dataSizeUpdate, + throwIfAbortedFn) { let manifestUpdated = false; - let activeHandle; - /** @type {!shaka.extern.ManifestDB} */ - let manifestDB; try { - await muxer.init(); - activeHandle = await muxer.getActive(); - - // Load the manifest. - const manifests = await activeHandle.cell.getManifests([manifestId]); - throwIfAbortedFn(); - manifestDB = manifests[0]; - // Assign the stored data to the manifest. let complete = true; for (const stream of manifestDB.streams) { @@ -654,26 +655,23 @@ shaka.offline.Storage = class { manifestDB.isIncomplete = false; } - // Re-store the manifest. - await activeHandle.cell.updateManifest(manifestId, manifestDB); + // Update the manifest. + await storage.updateManifest(manifestId, manifestDB); manifestUpdated = true; throwIfAbortedFn(); } catch (e) { await shaka.offline.Storage.cleanStoredManifest(manifestId); - if (activeHandle && !manifestUpdated) { + if (!manifestUpdated) { const dataKeys = Object.values(manifestUpdates); // The cleanStoredManifest method will not "see" any segments that have // been downloaded but not assigned to the manifest yet. So un-store // them separately. - await activeHandle.cell.removeSegments(dataKeys, (key) => {}); + await storage.removeSegments(dataKeys, (key) => {}); } throw e; - } finally { - await muxer.destroy(); } - return manifestDB; } /**