fix(ads): Dispose of ad manager on player detach

Change-Id: Ie4cd186885349ecb9081f16e82790295113668c2
cristian-atehortua · Sep 23, 2021 · 7b2a172 · 7b2a172
1 parent 2bdc326
commit 7b2a172
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 0 deletions.
diff --git a/externs/ima.js b/externs/ima.js
@@ -43,6 +43,8 @@ google.ima.AdsLoader = class {
 
   /** @override */
   dispatchEvent() {}
+
+  destroy() {}
 };
 
 
@@ -79,6 +81,8 @@ google.ima.AdsManager = class {
 
   stop() {}
 
+  destroy() {}
+
   /**
    * @param {number} volume
    */

diff --git a/externs/shaka/ads.js b/externs/shaka/ads.js
@@ -64,6 +64,8 @@ shaka.extern.IAdManager = class extends EventTarget {
    */
   setLocale(locale) {}
 
+  release() {}
+
   onAssetUnload() {}
 
   /**

diff --git a/lib/ads/ad_manager.js b/lib/ads/ad_manager.js
@@ -15,6 +15,7 @@ goog.require('shaka.log');
 goog.require('shaka.util.Error');
 goog.require('shaka.util.FakeEvent');
 goog.require('shaka.util.FakeEventTarget');
+goog.require('shaka.util.IReleasable');
 
 
 /**
@@ -358,6 +359,7 @@ goog.require('shaka.util.FakeEventTarget');
 /**
  * A class responsible for ad-related interactions.
  * @implements {shaka.extern.IAdManager}
+ * @implements {shaka.util.IReleasable}
  * @export
  */
 shaka.ads.AdManager = class extends shaka.util.FakeEventTarget {
@@ -402,6 +404,10 @@ shaka.ads.AdManager = class extends shaka.util.FakeEventTarget {
           shaka.util.Error.Code.CS_IMA_SDK_MISSING);
     }
 
+    if (this.csAdManager_) {
+      this.csAdManager_.release();
+    }
+
     this.csAdManager_ = new shaka.ads.ClientSideAdManager(
         adContainer, video, this.locale_,
         (e) => {
@@ -429,6 +435,22 @@ shaka.ads.AdManager = class extends shaka.util.FakeEventTarget {
   }
 
 
+  /**
+   * @override
+   * @export
+   */
+  release() {
+    if (this.csAdManager_) {
+      this.csAdManager_.release();
+      this.csAdManager_ = null;
+    }
+    if (this.ssAdManager_) {
+      this.ssAdManager_.release();
+      this.ssAdManager_ = null;
+    }
+  }
+
+
   /**
   * @override
   * @export
@@ -482,6 +504,10 @@ shaka.ads.AdManager = class extends shaka.util.FakeEventTarget {
           shaka.util.Error.Code.SS_IMA_SDK_MISSING);
     }
 
+    if (this.ssAdManager_) {
+      this.ssAdManager_.release();
+    }
+
     this.ssAdManager_ = new shaka.ads.ServerSideAdManager(
         adContainer, video, this.locale_,
         (e) => {

diff --git a/lib/ads/client_side_ad_manager.js b/lib/ads/client_side_ad_manager.js
@@ -13,10 +13,12 @@ goog.provide('shaka.ads.ClientSideAdManager');
 
 goog.require('goog.asserts');
 goog.require('shaka.ads.ClientSideAd');
+goog.require('shaka.util.IReleasable');
 
 
 /**
  * A class responsible for client-side ad interactions.
+ * @implements {shaka.util.IReleasable}
  */
 shaka.ads.ClientSideAdManager = class {
   /**
@@ -109,6 +111,21 @@ shaka.ads.ClientSideAdManager = class {
     }
   }
 
+  /** @override */
+  release() {
+    this.stop();
+    if (this.resizeObserver_) {
+      this.resizeObserver_.disconnect();
+    }
+    if (this.eventManager_) {
+      this.eventManager_.release();
+    }
+    if (this.imaAdsManager_) {
+      this.imaAdsManager_.destroy();
+    }
+    this.adsLoader_.destroy();
+  }
+
   /**
    * @param {!google.ima.AdErrorEvent} e
    * @private

diff --git a/lib/ads/server_side_ad_manager.js b/lib/ads/server_side_ad_manager.js
@@ -14,10 +14,12 @@ goog.provide('shaka.ads.ServerSideAdManager');
 goog.require('goog.asserts');
 goog.require('shaka.ads.ServerSideAd');
 goog.require('shaka.log');
+goog.require('shaka.util.IReleasable');
 
 
 /**
  * A class responsible for server-side ad interactions.
+ * @implements {shaka.util.IReleasable}
  */
 shaka.ads.ServerSideAdManager = class {
   /**
@@ -218,6 +220,14 @@ shaka.ads.ServerSideAdManager = class {
     this.currentCuePoints_ = [];
   }
 
+  /** @override */
+  release() {
+    this.stop();
+    if (this.eventManager_) {
+      this.eventManager_.release();
+    }
+  }
+
   /**
    * @param {string} type
    * @param {Uint8Array|string} data

diff --git a/lib/player.js b/lib/player.js
@@ -1320,6 +1320,12 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
       has.mediaElement = null;
     }
 
+    if (this.adManager_) {
+      // The ad manager is specific to the video, so dispose of it.
+      this.adManager_.release();
+      this.adManager_ = null;
+    }
+
     // Clear our cached copy of the media element.
     this.video_ = null;
 

diff --git a/test/test/util/fake_ad_manager.js b/test/test/util/fake_ad_manager.js
@@ -25,6 +25,9 @@ shaka.test.FakeAdManager = class extends shaka.util.FakeEventTarget {
     this.stats_ = new shaka.ads.AdsStats();
   }
 
+  /** @override */
+  release() {}
+
   /** @override */
   setLocale(locale) {}