Clean up exceptions on Player.destroy()

This introduces a new method on PublicPromise called destroy(),
which does for Promises what destroy() does for the other classes:
shuts down async processes and cleans up references.

In addition to catching errors and shutting down more cleanly in a
wider array of circumstances, this may also help with GC, since we
no longer leave pending Promises around with handlers attached to
them.

Closes #151

Change-Id: I205c5979418fbc18cd7e35d91d0de2746f337137

lib/media/eme_manager.js

@@ -120,6 +120,9 @@ shaka.media.EmeManager.prototype.destroy = function() {
   this.drmInfo_ = null;
   this.requestGenerated_ = null;
 
+  this.allSessionsPresumedReady_.destroy();
+  this.allSessionsPresumedReady_ = null;
+
   this.eventManager_.destroy();
   this.eventManager_ = null;
 

lib/media/source_buffer_manager.js

@@ -119,8 +119,11 @@ shaka.media.SourceBufferManager.FUDGE_FACTOR_ = 1 / 60;
  * @suppress {checkTypes} to set otherwise non-nullable types to null.
  */
 shaka.media.SourceBufferManager.prototype.destroy = function() {
-  this.abort();
+  this.abort().catch(function() {});
 
+  if (this.operationPromise_) {
+    this.operationPromise_.destroy();
+  }
   this.operationPromise_ = null;
   this.task_ = null;
 

lib/media/stream.js

@@ -202,6 +202,7 @@ shaka.media.Stream.prototype.configure = function(config) {
 shaka.media.Stream.prototype.destroy = function() {
   this.cancelUpdateTimer_();
 
+  this.startedPromise_.destroy();
   this.startedPromise_ = null;
 
   this.streamInfo_ = null;
@@ -250,7 +251,6 @@ shaka.media.Stream.prototype.getSegmentIndex = function() {
  */
 shaka.media.Stream.prototype.started = function(proceed) {
   if (!this.proceeded_) {
-    // The caller should never reject |proceed|.
     proceed.then(
         function() {
           shaka.asserts.assert(this.started_);
@@ -261,7 +261,11 @@ shaka.media.Stream.prototype.started = function(proceed) {
           if (!this.updateTimer_) {
             this.setUpdateTimer_(0);
           }
-        }.bind(this));
+        }.bind(this)).catch(
+        function(error) {
+          // The caller should never reject |proceed| unless it is destroyed.
+          shaka.asserts.assert(error.type == 'destroy');
+        });
   }
   return this.startedPromise_;
 };

lib/media/text_stream.js

@@ -78,6 +78,8 @@ shaka.media.TextStream.prototype.destroy = function() {
     this.video_.removeChild(this.track_);
   }
 
+  this.startedPromise_.destroy();
+  this.startedPromise_ = null;
   this.track_ = null;
   this.segmentIndex_ = null;
   this.streamInfo_ = null;

lib/player/player.js

@@ -301,12 +301,21 @@ shaka.player.Player.prototype.load = function(videoSource) {
   videoSource.setPlaybackStartTime(this.playbackStartTime_);
   this.playbackStartTime_ = null;
 
-  return p.then(shaka.util.TypedBind(this,
+  var loaded = p.then(shaka.util.TypedBind(this,
       function() {
         return videoSource.load();
-      })
-  ).then(shaka.util.TypedBind(this,
+      }));
+  loaded.catch(shaka.util.TypedBind(this,
+      /** @param {*} error */
+      function(error) {
+        // Clean up the local copy, since we haven't set this.videoSource_ yet.
+        videoSource.destroy();
+        // Pass the error along.
+        return Promise.reject(error);
+      }));
+  return loaded.then(shaka.util.TypedBind(this,
       function() {
+        if (!this.video_) return this.rejectDestroyed_();
         this.videoSource_ = videoSource;
         this.emeManager_ = new shaka.media.EmeManager(
             this, this.video_, this.videoSource_);
@@ -314,34 +323,35 @@ shaka.player.Player.prototype.load = function(videoSource) {
       })
   ).then(shaka.util.TypedBind(this,
       function() {
+        if (!this.video_) return this.rejectDestroyed_();
         this.setVideoEventListeners_();
         return this.videoSource_.attach(this, this.video_);
       })
   ).then(shaka.util.TypedBind(this,
       function() {
+        if (!this.video_) return this.rejectDestroyed_();
         this.startWatchdogTimer_();
       })
   ).catch(shaka.util.TypedBind(this,
       /** @param {*} error */
       function(error) {
-        // We own the source now, so we must clean it up.
-        // We may not have set the source on this, so call destroy on the local
-        // var instead.
-        videoSource.destroy();
-
-        // Since we may have set the source on this, set it to null.
-        this.videoSource_ = null;
+        if (this.videoSource_) {
+          this.videoSource_.destroy();
+          this.videoSource_ = null;
+        }
 
         if (this.emeManager_) {
           this.emeManager_.destroy();
           this.emeManager_ = null;
         }
 
-        // Even though we return a rejected promise, we still want to dispatch
-        // an error event to ensure that the application is aware of all errors
-        // from the player.
-        var event = shaka.util.FakeEvent.createErrorEvent(error);
-        this.dispatchEvent(event);
+        if (error.type != 'destroy') {
+          // Even though we return a rejected promise, we still want to
+          // dispatch an error event to ensure that the application is aware of
+          // all errors from the player.
+          var event = shaka.util.FakeEvent.createErrorEvent(error);
+          this.dispatchEvent(event);
+        }
 
         return Promise.reject(error);
       })
@@ -349,6 +359,17 @@ shaka.player.Player.prototype.load = function(videoSource) {
 };
 
 
+/**
+ * @return {!Promise}
+ * @private
+ */
+shaka.player.Player.prototype.rejectDestroyed_ = function() {
+  var error = new Error('Player destroyed');
+  error.type = 'destroy';
+  return Promise.reject(error);
+};
+
+
 /**
  * Sets the video's event listeners.
  *

lib/player/stream_video_source.js

@@ -261,6 +261,12 @@ shaka.player.StreamVideoSource.prototype.configure = function(config) {
  * @suppress {checkTypes} to set otherwise non-nullable types to null.
  */
 shaka.player.StreamVideoSource.prototype.destroy = function() {
+  this.attachPromise_.destroy();
+  this.proceedPromise_.destroy();
+
+  this.attachPromise_ = null;
+  this.proceedPromise_ = null;
+
   this.cancelSeekRangeTimer_();
   this.cancelUpdateTimer_();
 
@@ -269,8 +275,6 @@ shaka.player.StreamVideoSource.prototype.destroy = function() {
   this.eventManager.destroy();
   this.eventManager = null;
 
-  this.proceedPromise_ = null;
-
   shaka.util.MapUtils.values(this.streamsByType_).forEach(
       function(stream) {
         stream.destroy();
@@ -291,7 +295,6 @@ shaka.player.StreamVideoSource.prototype.destroy = function() {
   this.mediaSource = null;
   this.video = null;
   this.stats_ = null;
-  this.attachPromise_ = null;
   this.restrictions_ = null;
 
   this.parent = null;
@@ -1086,7 +1089,10 @@ shaka.player.StreamVideoSource.prototype.onMediaSourceOpen_ = function(event) {
 
   this.createAndStartStreams_().then(shaka.util.TypedBind(this,
       function() {
-        this.attachPromise_.resolve();
+        // We may have been destroyed since.
+        if (this.attachPromise_) {
+          this.attachPromise_.resolve();
+        }
       })
   ).catch(shaka.util.TypedBind(this,
       /** @param {*} error */
@@ -1389,10 +1395,13 @@ shaka.player.StreamVideoSource.prototype.startStreams_ = function(
         // One or more Streams encountered an unrecoverable error during stream
         // startup. There's nothing else to do.
         shaka.asserts.assert(error.type != 'aborted');
-        shaka.log.error('Stream startup failed!');
 
-        var event = shaka.util.FakeEvent.createErrorEvent(error);
-        this.dispatchEvent(event);
+        if (error.type != 'destroy') {
+          shaka.log.error('Stream startup failed!');
+
+          var event = shaka.util.FakeEvent.createErrorEvent(error);
+          this.dispatchEvent(event);
+        }
       })
   );
 

lib/util/ajax_request.js

@@ -185,6 +185,7 @@ shaka.util.AjaxRequest.Parameters = function() {
 shaka.util.AjaxRequest.prototype.destroy_ = function() {
   this.cleanupRequest_();
   this.parameters.body = null;
+  this.promise_.destroy();
   this.promise_ = null;
   this.estimator = null;
 };

lib/util/public_promise.js

@@ -46,6 +46,8 @@ shaka.util.PublicPromise = function() {
 
   promise.resolve = resolvePromise;
   promise.reject = rejectPromise;
+  promise.destroy = shaka.util.PublicPromise.prototype.destroy;
+
   return promise;
 };
 
@@ -57,3 +59,16 @@ shaka.util.PublicPromise.prototype.resolve;
 /** @type {function(*)} */
 shaka.util.PublicPromise.prototype.reject;
 
+
+/**
+ * Rejects the promise and silences errors about uncaught exceptions.
+ */
+shaka.util.PublicPromise.prototype.destroy = function() {
+  // To avoid spurious errors in Chrome's console if nobody is listening:
+  this.catch(function() {});
+
+  var error = new Error('Destroyed!');
+  error.type = 'destroy';
+  this.reject(error);
+};
+

spec/player_integration.js

@@ -1058,8 +1058,10 @@ describe('Player', function() {
         expect(licensePreProcessor.spy).toHaveBeenCalled();
         // done() is called from the LicenseRequest.send() spy above.
       }).catch(function(error) {
-        fail(error);
-        done();
+        if (error.type != 'destroy') {
+          fail(error);
+          done();
+        }
       });
     });
   });
@@ -1534,6 +1536,48 @@ describe('Player', function() {
     });
   });
 
+  describe('destroy', function() {
+    it('does not cause exceptions after load', function(done) {
+      // Satisfy Jasmine, which complains if tests have no expectations.
+      expect(true).toBe(true);
+      // Load the manifest, then wait.
+      player.load(newSource(encryptedManifest)).then(function() {
+        player.destroy();
+        // Wait 1 second for async exceptions.
+        // Jasmine will convert them to test failures.
+        delay(1).then(done);
+      });
+    });
+
+    it('does not cause exceptions immediately', function(done) {
+      // Satisfy Jasmine, which complains if tests have no expectations.
+      expect(true).toBe(true);
+      // Chrome complains if you don't catch all failed Promises.
+      player.load(newSource(encryptedManifest)).catch(function() {});
+      // Now destroy the player without letting it finish loading.
+      player.destroy();
+      // Wait 1 second for async exceptions.
+      // Jasmine will convert them to test failures.
+      delay(1).then(done);
+    });
+
+    for (var ms = 1; ms <= 1000; ms *= 5) {
+      it('does not cause exceptions after ' + ms + ' ms', function(ms, done) {
+        // Satisfy Jasmine, which complains if tests have no expectations.
+        expect(true).toBe(true);
+        // Chrome complains if you don't catch all failed Promises.
+        player.load(newSource(encryptedManifest)).catch(function() {});
+        // Now destroy the player after the specified delay.
+        delay(ms / 1000).then(function() {
+          player.destroy();
+          // Wait 1 additional second for async exceptions.
+          // Jasmine will convert them to test failures.
+          delay(1).then(done);
+        });
+      }.bind(this, ms));
+    }
+  });
+
   // TODO(story 1970528): add tests which exercise PSSH parsing,
   // SegmentTemplate resolution, and SegmentList generation.