From 07946db845e62a94eb5adc7f722fcc5fe9e14d21 Mon Sep 17 00:00:00 2001
From: Pat O'Neill <pgoneill@gmail.com>
Date: Wed, 20 Mar 2019 15:07:11 -0400
Subject: [PATCH] fix: Clean up player when a preroll fails and prevent
 uncaught play promise exceptions. (#470)

---
 src/states/Preroll.js            | 29 ++++++++++++++++++++++++-----
 test/unit/states/test.Preroll.js | 17 ++++++++++++++---
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/states/Preroll.js b/src/states/Preroll.js
index ff2f3556..855bf5a0 100644
--- a/src/states/Preroll.js
+++ b/src/states/Preroll.js
@@ -154,7 +154,7 @@ class Preroll extends AdState {
     const player = this.player;
 
     if (this.adsReady && !player.ads.inAdBreak() && !this.isContentResuming()) {
-      player.clearTimeout(this._timeout);
+      this.clearTimeout(player);
       player.ads.adType = 'preroll';
       this.waitingForAdBreak = false;
       adBreak.start(player);
@@ -227,16 +227,21 @@ class Preroll extends AdState {
   }
 
   resumeAfterNoPreroll(player) {
+
     // Resume to content and unblock play as there is no preroll ad
     this.contentResuming = true;
     player.ads._shouldBlockPlay = false;
+    this.cleanupPartial(player);
 
     // Play the content if we had requested play or we paused on 'contentupdate'
     // and we haven't played yet. This happens if there was no preroll or if it
     // errored, timed out, etc. Otherwise snapshot restore would play.
-    if (player.paused() &&
-        (player.ads._playRequested || player.ads._pausedOnContentupdate)) {
-      player.play();
+    if (player.paused() && (player.ads._playRequested || player.ads._pausedOnContentupdate)) {
+      const playPromise = player.play();
+
+      if (playPromise && playPromise.then) {
+        playPromise.then(null, (e) => {});
+      }
     }
   }
 
@@ -247,12 +252,26 @@ class Preroll extends AdState {
     if (!player.ads._hasThereBeenALoadStartDuringPlayerLife) {
       videojs.log.warn('Leaving Preroll state before loadstart event can cause issues.');
     }
+    this.cleanupPartial(player);
+  }
 
+  /*
+   * Performs cleanup tasks without depending on a state transition. This is
+   * used mainly in cases where a preroll failed.
+   */
+  cleanupPartial(player) {
     player.removeClass('vjs-ad-loading');
     player.removeClass('vjs-ad-content-resuming');
-    player.clearTimeout(this._timeout);
+    this.clearTimeout(player);
   }
 
+  /*
+   * Clear the preroll timeout and nulls out the pointer.
+   */
+  clearTimeout(player) {
+    player.clearTimeout(this._timeout);
+    this._timeout = null;
+  }
 }
 
 States.registerState('Preroll', Preroll);
diff --git a/test/unit/states/test.Preroll.js b/test/unit/states/test.Preroll.js
index 0d1931f6..ffd47d8e 100644
--- a/test/unit/states/test.Preroll.js
+++ b/test/unit/states/test.Preroll.js
@@ -11,6 +11,7 @@ QUnit.module('Preroll', {
   beforeEach() {
     this.events = [];
     this.playTriggered = false;
+    this.classes = [];
 
     this.player = {
       ads: {
@@ -22,8 +23,9 @@ QUnit.module('Preroll', {
       },
       setTimeout: () => {},
       clearTimeout: () => {},
-      addClass: () => {},
-      removeClass: () => {},
+      addClass: (name) => this.classes.push(name),
+      removeClass: (name) => this.classes.splice(this.classes.indexOf(name), 1),
+      hasClass: (name) => this.classes.indexOf(name) !== -1,
       one: () => {},
       trigger: (event) => {
         this.events.push(event);
@@ -120,6 +122,9 @@ QUnit.test('can handle adscanceled', function(assert) {
   this.preroll.init(this.player, false, false);
   this.preroll.onAdsCanceled(this.player);
   assert.equal(this.preroll.isContentResuming(), true);
+  assert.notOk(this.player.hasClass('vjs-ad-loading'));
+  assert.notOk(this.player.hasClass('vjs-ad-content-resuming'));
+  assert.notOk(this.preroll._timeout);
   this.preroll.onPlaying(this.player);
   assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback');
 });
@@ -128,6 +133,9 @@ QUnit.test('can handle adserror', function(assert) {
   this.preroll.init(this.player, false, false);
   this.preroll.onAdsError(this.player);
   assert.equal(this.preroll.isContentResuming(), true);
+  assert.notOk(this.player.hasClass('vjs-ad-loading'));
+  assert.notOk(this.player.hasClass('vjs-ad-content-resuming'));
+  assert.notOk(this.preroll._timeout);
   this.preroll.onPlaying(this.player);
   assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback');
 });
@@ -140,10 +148,13 @@ QUnit.test('can skip linear ad mode', function(assert) {
   assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback');
 });
 
-QUnit.test('plays content after ad timeout', function(assert) {
+QUnit.test('can handle adtimeout', function(assert) {
   this.preroll.init(this.player, false, false);
   this.preroll.onAdTimeout(this.player);
   assert.equal(this.preroll.isContentResuming(), true);
+  assert.notOk(this.player.hasClass('vjs-ad-loading'));
+  assert.notOk(this.player.hasClass('vjs-ad-content-resuming'));
+  assert.notOk(this.preroll._timeout);
   this.preroll.onPlaying(this.player);
   assert.equal(this.newState, 'ContentPlayback', 'transitioned to ContentPlayback');
 });