Skip to content

Commit

Permalink
fix(ads): Dispose of ad manager on player detach
Browse files Browse the repository at this point in the history
Change-Id: Ie4cd186885349ecb9081f16e82790295113668c2
  • Loading branch information
theodab committed Sep 23, 2021
1 parent 2bdc326 commit 7b2a172
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 0 deletions.
4 changes: 4 additions & 0 deletions externs/ima.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ google.ima.AdsLoader = class {

/** @override */
dispatchEvent() {}

destroy() {}
};


Expand Down Expand Up @@ -79,6 +81,8 @@ google.ima.AdsManager = class {

stop() {}

destroy() {}

/**
* @param {number} volume
*/
Expand Down
2 changes: 2 additions & 0 deletions externs/shaka/ads.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ shaka.extern.IAdManager = class extends EventTarget {
*/
setLocale(locale) {}

release() {}

onAssetUnload() {}

/**
Expand Down
26 changes: 26 additions & 0 deletions lib/ads/ad_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');


/**
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) => {
Expand Down
17 changes: 17 additions & 0 deletions lib/ads/client_side_ad_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions lib/ads/server_side_ad_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions lib/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 3 additions & 0 deletions test/test/util/fake_ad_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ shaka.test.FakeAdManager = class extends shaka.util.FakeEventTarget {
this.stats_ = new shaka.ads.AdsStats();
}

/** @override */
release() {}

/** @override */
setLocale(locale) {}

Expand Down

0 comments on commit 7b2a172

Please sign in to comment.