Skip to content

Commit

Permalink
fix: eventBus error after dispose (#218)
Browse files Browse the repository at this point in the history 
* fix: eventBus error after dispose

* improved fix for errors thrown on player dispose with an open key session

* actually this might be better

* fix: add isDisposed to mock eventBus

* skip test

* code review

* code review x2

* fix: update lint standard

* fix: tests and unsafe triggers on event bus

---------

Co-authored-by: Pat O'Neill <pgoneill@gmail.com>
  • Loading branch information
@adrums86 @misteroneill
adrums86 and misteroneill authored May 16, 2024
1 parent e076f7d commit 730bdd9
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 56 deletions.
50 changes: 25 additions & 25 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
"videojs-generate-karma-config": "^8.0.1",
"videojs-generate-rollup-config": "^7.0.0",
"videojs-generator-verify": "^1.2.0",
"videojs-standard": "~9.0.1"
"videojs-standard": "~9.1.0"
},
"generator-videojs-plugin": {
"version": "7.3.2"
Expand Down
48 changes: 39 additions & 9 deletions src/eme.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,24 @@ import EmeError from './consts/errors';

const isFairplayKeySystem = (str) => str.startsWith('com.apple.fps');

/**
* Trigger an event on the event bus component safely.
*
* This is used because there are cases where we can see race conditions
* between asynchronous operations (like closing a key session) and the
* availability of the event bus's DOM element.
*
* @param {Component} eventBus
* @param {...} args
*/
export const safeTriggerOnEventBus = (eventBus, args) => {
if (eventBus.isDisposed()) {
return;
}

eventBus.trigger({...args});
};

/**
* Returns an array of MediaKeySystemConfigurationObjects provided in the keySystem
* options.
Expand Down Expand Up @@ -110,14 +128,14 @@ export const makeNewRequest = (player, requestOptions) => {
try {
const keySession = mediaKeys.createSession();

eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keysessioncreated',
keySession
});

player.on('dispose', () => {
keySession.close().then(() => {
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keysessionclosed',
keySession
});
Expand All @@ -133,7 +151,7 @@ export const makeNewRequest = (player, requestOptions) => {

return new Promise((resolve, reject) => {
keySession.addEventListener('message', (event) => {
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keymessage',
messageEvent: event
});
Expand All @@ -145,7 +163,7 @@ export const makeNewRequest = (player, requestOptions) => {
getLicense(options, event.message, contentId)
.then((license) => {
resolve(keySession.update(license).then(() => {
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keysessionupdated',
keySession
});
Expand All @@ -168,8 +186,13 @@ export const makeNewRequest = (player, requestOptions) => {
keySession.addEventListener(KEY_STATUSES_CHANGE, (event) => {
let expired = false;

// Protect from race conditions causing the player to be disposed.
if (eventBus.isDisposed()) {
return;
}

// Re-emit the keystatuseschange event with the entire keyStatusesMap
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: KEY_STATUSES_CHANGE,
keyStatuses: keySession.keyStatuses
});
Expand All @@ -180,7 +203,7 @@ export const makeNewRequest = (player, requestOptions) => {
// Trigger an event so that outside listeners can take action if appropriate.
// For instance, the `output-restricted` status should result in an
// error being thrown.
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
keyId,
status,
target: keySession,
Expand Down Expand Up @@ -210,7 +233,14 @@ export const makeNewRequest = (player, requestOptions) => {
// session can be created.
videojs.log.debug('Session expired, closing the session.');
keySession.close().then(() => {
eventBus.trigger({

// Because close() is async, this promise could resolve after the
// player has been disposed.
if (eventBus.isDisposed()) {
return;
}

safeTriggerOnEventBus(eventBus, {
type: 'keysessionclosed',
keySession
});
Expand Down Expand Up @@ -408,7 +438,7 @@ const promisifyGetLicense = (keySystem, getLicenseFn, eventBus) => {
return new Promise((resolve, reject) => {
const callback = function(err, license) {
if (eventBus) {
eventBus.trigger('licenserequestattempted');
safeTriggerOnEventBus(eventBus, { type: 'licenserequestattempted' });
}
if (err) {
reject(err);
Expand Down Expand Up @@ -526,7 +556,7 @@ export const standard5July2016 = ({
}).then(() => {
return keySystemAccess.createMediaKeys();
}).then((createdMediaKeys) => {
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keysystemaccesscomplete',
mediaKeys: createdMediaKeys
});
Expand Down
10 changes: 6 additions & 4 deletions src/fairplay.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import window from 'global/window';
import {stringToUint16Array, uint16ArrayToString, getHostnameFromUri, mergeAndRemoveNull} from './utils';
import {httpResponseHandler} from './http-handler.js';
import EmeError from './consts/errors';
import { safeTriggerOnEventBus } from './eme.js';

export const LEGACY_FAIRPLAY_KEY_SYSTEM = 'com.apple.fps.1_0';

Expand Down Expand Up @@ -85,21 +86,22 @@ const addKey = ({video, contentId, initData, cert, options, getLicense, eventBus
return;
}

eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keysessioncreated',
keySession
});

keySession.contentId = contentId;

keySession.addEventListener('webkitkeymessage', (event) => {
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keymessage',
messageEvent: event
});
getLicense(options, contentId, event.message, (err, license) => {
if (eventBus) {
eventBus.trigger('licenserequestattempted');

safeTriggerOnEventBus(eventBus, { type: 'licenserequestattempted' });
}
if (err) {
const metadata = {
Expand All @@ -114,7 +116,7 @@ const addKey = ({video, contentId, initData, cert, options, getLicense, eventBus

keySession.update(new Uint8Array(license));

eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keysessionupdated',
keySession
});
Expand Down
17 changes: 9 additions & 8 deletions src/ms-prefixed.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import window from 'global/window';
import { requestPlayreadyLicense } from './playready';
import { getMediaKeySystemConfigurations } from './utils';
import EmeError from './consts/errors';
import { safeTriggerOnEventBus } from './eme';

export const PLAYREADY_KEY_SYSTEM = 'com.microsoft.playready';

Expand All @@ -25,7 +26,7 @@ export const addKeyToSession = (options, session, event, eventBus, emeError) =>
};

emeError(err, metadata);
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
message: 'Unable to get key: ' + err,
target: session,
type: 'mskeyerror'
Expand All @@ -35,7 +36,7 @@ export const addKeyToSession = (options, session, event, eventBus, emeError) =>

session.update(key);

eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keysessionupdated',
keySession: session
});
Expand All @@ -55,7 +56,7 @@ export const addKeyToSession = (options, session, event, eventBus, emeError) =>

const callback = (err, responseBody) => {
if (eventBus) {
eventBus.trigger('licenserequestattempted');
safeTriggerOnEventBus(eventBus, { type: 'licenserequestattempted' });
}

if (err) {
Expand All @@ -65,7 +66,7 @@ export const addKeyToSession = (options, session, event, eventBus, emeError) =>
};

emeError(err, metadata);
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
message: 'Unable to request key from url: ' + playreadyOptions.url,
target: session,
type: 'mskeyerror'
Expand Down Expand Up @@ -98,7 +99,7 @@ export const createSession = (video, initData, options, eventBus, emeError) => {
throw error;
}

eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keysessioncreated',
keySession: session
});
Expand All @@ -113,7 +114,7 @@ export const createSession = (video, initData, options, eventBus, emeError) => {
// eslint-disable-next-line max-len
// @see [PlayReady License Acquisition]{@link https://msdn.microsoft.com/en-us/library/dn468979.aspx}
session.addEventListener('mskeymessage', (event) => {
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
type: 'keymessage',
messageEvent: event
});
Expand All @@ -127,7 +128,7 @@ export const createSession = (video, initData, options, eventBus, emeError) => {
};

emeError(session.error, metadata);
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
message: 'Unexpected key error from key session with ' +
`code: ${session.error.code} and systemCode: ${session.error.systemCode}`,
target: session,
Expand All @@ -136,7 +137,7 @@ export const createSession = (video, initData, options, eventBus, emeError) => {
});

session.addEventListener('mskeyadded', () => {
eventBus.trigger({
safeTriggerOnEventBus(eventBus, {
target: session,
type: 'mskeyadded'
});
Expand Down
Loading

0 comments on commit 730bdd9

Please sign in to comment.