fix(preload): Fix error handling (shaka-project#6753)

After a previous bugfix to the preload system, we ended up with a
situation where the
overall progress in the preload was tracked by two promises:
`successPromise_`, which is resolved when the preload finishes
successfully.
`destroyPromise_`, which is rejected with an error when the preload
process trips an error condition.
These two promises were confusingly named; it sounds like destroyPromise
is related to the destroy process,
but really it has more to do with errors.
They were also completely redundant, as a single promise can be used to
carry both a resolved and
rejected state.

This PR simply combines the two promises into one.

---------

Co-authored-by: Joey Parrish <[email protected]>

Author: theodab

lib/media/drm_engine.js

@@ -393,7 +393,6 @@ shaka.media.DrmEngine = class {
           this.config_.keySystemsMapping);
     }
 
-
     /** @type {!Map.<string, MediaKeySystemConfiguration>} */
     let configsByKeySystem;
 
@@ -402,6 +401,7 @@ shaka.media.DrmEngine = class {
     await shaka.util.StreamUtils.getDecodingInfosForVariants(variants,
         this.usePersistentLicenses_, this.srcEquals_,
         this.config_.preferredKeySystems);
+    this.destroyer_.ensureNotDestroyed();
 
     const hasDrmInfo = hadDrmInfo || Object.keys(this.config_.servers).length;
     // An unencrypted content is initialized.

lib/media/preload_manager.js

@@ -134,12 +134,6 @@ shaka.media.PreloadManager = class extends shaka.util.FakeEventTarget {
     /** @private {!shaka.util.PublicPromise} */
     this.successPromise_ = new shaka.util.PublicPromise();
 
-    /** @private {!shaka.util.PublicPromise} */
-    this.destroyPromise_ = new shaka.util.PublicPromise();
-
-    // Silence promise failures, in case nobody is catching this.destroyPromise_
-    this.destroyPromise_.catch((error) => {});
-
     /** @private {?shaka.util.FakeEventTarget} */
     this.eventHandoffTarget_ = null;
 
@@ -376,32 +370,30 @@ shaka.media.PreloadManager = class extends shaka.util.FakeEventTarget {
 
   /**
    * Starts the process of loading the asset.
-   * @return {!Promise}
+   * Success or failure will be measured through waitForFinish()
    */
-  async start() {
-    // Force a context switch, to give the player a chance to hook up events
-    // immediately if desired.
-    await Promise.resolve();
-
-    // Perform the preloading process.
-    try {
-      await this.parseManifestInner_();
-      if (this.isDestroyed()) {
-        return;
-      }
-      await this.initializeDrmInner_();
-      if (this.isDestroyed()) {
-        return;
-      }
-      await this.chooseInitialVariantInner_();
-      if (this.isDestroyed()) {
-        return;
+  start() {
+    (async () => {
+      // Force a context switch, to give the player a chance to hook up events
+      // immediately if desired.
+      await Promise.resolve();
+
+      // Perform the preloading process.
+      try {
+        await this.parseManifestInner_();
+        this.throwIfDestroyed_();
+
+        await this.initializeDrmInner_();
+        this.throwIfDestroyed_();
+
+        await this.chooseInitialVariantInner_();
+        this.throwIfDestroyed_();
+
+        this.successPromise_.resolve();
+      } catch (error) {
+        this.successPromise_.reject(error);
       }
-      this.successPromise_.resolve();
-    } catch (error) {
-      this.destroyPromise_.reject(error);
-      throw error;
-    }
+    })();
   }
 
   /**
@@ -423,7 +415,7 @@ shaka.media.PreloadManager = class extends shaka.util.FakeEventTarget {
   onError(error) {
     if (error.severity === shaka.util.Error.Severity.CRITICAL) {
       // Cancel the loading process.
-      this.destroyPromise_.reject(error);
+      this.successPromise_.reject(error);
       this.destroy();
     }
 
@@ -435,6 +427,20 @@ shaka.media.PreloadManager = class extends shaka.util.FakeEventTarget {
     }
   }
 
+  /**
+   * Throw if destroyed, to interrupt processes with a recognizable error.
+   *
+   * @private
+   */
+  throwIfDestroyed_() {
+    if (this.isDestroyed()) {
+      throw new shaka.util.Error(
+          shaka.util.Error.Severity.CRITICAL,
+          shaka.util.Error.Category.PLAYER,
+          shaka.util.Error.Code.OBJECT_DESTROYED);
+    }
+  }
+
   /**
    * Makes a fires an event corresponding to entering a state of the loading
    * process.
@@ -534,6 +540,7 @@ shaka.media.PreloadManager = class extends shaka.util.FakeEventTarget {
       const event = this.makeEvent_(
           shaka.util.FakeEvent.EventName.TracksChanged);
       await Promise.resolve();
+      this.throwIfDestroyed_();
       this.dispatchEvent(event);
     }
 
@@ -542,6 +549,7 @@ shaka.media.PreloadManager = class extends shaka.util.FakeEventTarget {
     await this.drmEngine_.initForPlayback(
         playableVariants,
         this.manifest_.offlineSessionIds);
+    this.throwIfDestroyed_();
 
     // Now that we have drm information, filter the manifest (again) so that
     // we can ensure we only use variants with the selected key system.
@@ -692,10 +700,7 @@ shaka.media.PreloadManager = class extends shaka.util.FakeEventTarget {
    * @export
    */
   waitForFinish() {
-    return Promise.race([
-      this.successPromise_,
-      this.destroyPromise_,
-    ]);
+    return this.successPromise_;
   }
 
   /**

lib/player.js

@@ -1581,7 +1581,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
         if (preloadManager) {
           preloadManager.setEventHandoffTarget(this);
           this.stats_ = preloadManager.getStats();
-          await mutexWrapOperation(() => preloadManager.start(), 'preload');
+          preloadManager.start();
+          // Silence "uncaught error" warnings from this. Unless we are
+          // interrupted, we will check the result of this process and respond
+          // appropriately. If we are interrupted, we can ignore any error
+          // there.
+          preloadManager.waitForFinish().catch(() => {});
         } else {
           this.stats_ = new shaka.util.Stats();
         }
@@ -1843,7 +1848,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
           shaka.util.Error.Category.PLAYER,
           shaka.util.Error.Code.SRC_EQUALS_PRELOAD_NOT_SUPPORTED));
     } else {
-      preloadManager.start().catch((error) => {}); // Catch errors.
+      preloadManager.start();
     }
     return preloadManager;
   }

test/player_load_graph_integration.js

@@ -150,16 +150,16 @@ describe('Player Load Graph', () => {
       'unload',
       'manifest-parser',
       'manifest',
-      'drm-engine',
       'media-source',
+      'drm-engine',
       'load',
 
       // Load 3
       'unload',
       'manifest-parser',
       'manifest',
-      'drm-engine',
       'media-source',
+      'drm-engine',
       'load',
     ]);
   });