From 698148f3256badc6d958a097eddc3e5c1744ab70 Mon Sep 17 00:00:00 2001 From: Kevin Date: Thu, 1 Aug 2024 16:31:21 -0700 Subject: [PATCH 1/7] feat: listen for navigator permissions changes --- lib/twilio/audiohelper.ts | 10 ++++++++ tests/audiohelper.js | 52 ++++++++++++++++++++++++++++++++++++++- tests/index.ts | 5 ++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/lib/twilio/audiohelper.ts b/lib/twilio/audiohelper.ts index 79f73a183..221e56d12 100644 --- a/lib/twilio/audiohelper.ts +++ b/lib/twilio/audiohelper.ts @@ -275,6 +275,16 @@ class AudioHelper extends EventEmitter { if (isEnumerationSupported) { this._initializeEnumeration(); } + + navigator.permissions.query({ name: 'microphone' as PermissionName }).then((microphonePermissionStatus) => { + if (microphonePermissionStatus.state !== 'granted') { + const handleStateChange = () => { + this._updateAvailableDevices(); + microphonePermissionStatus.removeEventListener('change', this._updateAvailableDevices); + }; + microphonePermissionStatus.addEventListener('change', handleStateChange); + } + }).catch((reason) => this._log.warn(`Warning: unable to listen for microphone permission changes. ${reason}`)); } /** diff --git a/tests/audiohelper.js b/tests/audiohelper.js index fafa8c9d2..8bcc79160 100644 --- a/tests/audiohelper.js +++ b/tests/audiohelper.js @@ -7,6 +7,19 @@ function getUserMedia() { return Promise.resolve({ id: 'default', getTracks: () => [] }); } +const defaultNavigator = { + permissions: { + query: function() { + return Promise.resolve('prompt'); + } + }, + mediaDevices: { + enumerateDevices: function(){ + return Promise.resolve([]); + } + } +}; + describe('AudioHelper', () => { context('when enumerateDevices is not supported', () => { const noop = () => {}; @@ -34,7 +47,7 @@ describe('AudioHelper', () => { ? navigator : undefined; HTMLAudioElement = undefined; - navigator = { }; + navigator = defaultNavigator; }); after(() => { @@ -68,6 +81,8 @@ describe('AudioHelper', () => { let availableDevices; let handlers; let mediaDevices; + let oldHTMLAudioElement; + let oldNavigator; beforeEach(() => { eventObserver = new AudioProcessorEventObserver(); @@ -100,6 +115,23 @@ describe('AudioHelper', () => { }); }); + before(() => { + oldHTMLAudioElement = typeof HTMLAudioElement !== 'undefined' + ? HTMLAudioElement + : undefined; + oldNavigator = typeof navigator !== 'undefined' + ? navigator + : undefined; + HTMLAudioElement = undefined; + navigator = defaultNavigator + }); + + after(() => { + HTMLAudioElement = oldHTMLAudioElement; + navigator = oldNavigator; + }); + + describe('constructor', () => { it('should set .isOutputSelectionSupported to true', () => { assert.equal(audio.isOutputSelectionSupported, true); @@ -124,6 +156,24 @@ describe('AudioHelper', () => { }); }); + describe('navigator.permissions', () => { + it('should listen for microphone state changes', () => { + let onChangeHandler = sinon.stub(); + let microphonePermissionStatus = { + state: 'prompt', + addEventListener: (eventName, listener) => { + if (eventName === 'change') { + listener() + } + } + }; + navigator.permissions.query = () => Promise.resolve(microphonePermissionStatus); + audio = new AudioHelper(onActiveOutputsChanged, onActiveInputChanged, {}); + microphonePermissionStatus.addEventListener('change', onChangeHandler); + sinon.assert.calledOnce(onChangeHandler); + }) + }) + describe('._destroy', () => { it('should properly dispose the audio instance', () => { audio._stopDefaultInputDeviceStream = sinon.stub(); diff --git a/tests/index.ts b/tests/index.ts index 6188f58b6..5057f38d4 100644 --- a/tests/index.ts +++ b/tests/index.ts @@ -73,6 +73,11 @@ root.navigator = { }, platform: 'platform', userAgent: 'userAgent', + permissions: { + query: function() { + return Promise.resolve('prompt'); + } + } }; root.RTCPeerConnection = root.window.RTCPeerConnection = function() { }; From fa8970002bd6eafaeb678996424349cc9941455c Mon Sep 17 00:00:00 2001 From: Kevin Date: Fri, 2 Aug 2024 14:58:33 -0700 Subject: [PATCH 2/7] fix: remove typecast; destroy listener --- lib/twilio/audiohelper.ts | 25 ++++++++++++++++++++++++- tests/audiohelper.js | 3 ++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/twilio/audiohelper.ts b/lib/twilio/audiohelper.ts index 221e56d12..166302835 100644 --- a/lib/twilio/audiohelper.ts +++ b/lib/twilio/audiohelper.ts @@ -163,11 +163,21 @@ class AudioHelper extends EventEmitter { */ private _mediaDevices: AudioHelper.MediaDevicesLike | null; + /** + * The microphone permission status + */ + private _microphonePermissionStatus: PermissionStatus; + /** * Called with the new input stream when the active input is changed. */ private _onActiveInputChanged: (stream: MediaStream | null) => Promise; + /** + * Handler for microphone permission status change + */ + private _onMicrophonePermissionStatusChanged: () => void; + /** * Internal reference to the processed stream */ @@ -276,13 +286,17 @@ class AudioHelper extends EventEmitter { this._initializeEnumeration(); } - navigator.permissions.query({ name: 'microphone' as PermissionName }).then((microphonePermissionStatus) => { + // NOTE (kchoy): Currently microphone permissions are not supported in firefox. + // https://github.com/mozilla/standards-positions/issues/19#issuecomment-370158947 + navigator.permissions.query({ name: 'microphone' }).then((microphonePermissionStatus) => { if (microphonePermissionStatus.state !== 'granted') { const handleStateChange = () => { this._updateAvailableDevices(); microphonePermissionStatus.removeEventListener('change', this._updateAvailableDevices); }; microphonePermissionStatus.addEventListener('change', handleStateChange); + this._microphonePermissionStatus = microphonePermissionStatus; + this._onMicrophonePermissionStatusChanged = handleStateChange; } }).catch((reason) => this._log.warn(`Warning: unable to listen for microphone permission changes. ${reason}`)); } @@ -297,6 +311,7 @@ class AudioHelper extends EventEmitter { this._destroyProcessedStream(); this._maybeStopPollingVolume(); this.removeAllListeners(); + this._stopMicrophonePermissionListener(); this._unbind(); } @@ -392,6 +407,14 @@ class AudioHelper extends EventEmitter { } } + /** + * Remove event listener for microphone permissions + * @private + */ + _stopMicrophonePermissionListener(): void { + this._microphonePermissionStatus.removeEventListener('change', this._onMicrophonePermissionStatusChanged); + } + /** * Unbind the listeners from mediaDevices. * @private diff --git a/tests/audiohelper.js b/tests/audiohelper.js index 8bcc79160..7448b9fd3 100644 --- a/tests/audiohelper.js +++ b/tests/audiohelper.js @@ -180,6 +180,7 @@ describe('AudioHelper', () => { audio._stopSelectedInputDeviceStream = sinon.stub(); audio._destroyProcessedStream = sinon.stub(); audio._maybeStopPollingVolume = sinon.stub(); + audio._stopMicrophonePermissionListener = sinon.stub(); audio._destroy(); assert.strictEqual(audio.eventNames().length, 0); sinon.assert.calledOnce(audio._stopDefaultInputDeviceStream); @@ -187,7 +188,7 @@ describe('AudioHelper', () => { sinon.assert.calledOnce(audio._destroyProcessedStream); sinon.assert.calledOnce(audio._maybeStopPollingVolume); sinon.assert.calledOnce(mediaDevices.removeEventListener); - + sinon.assert.calledOnce(audio._stopMicrophonePermissionListener); }); }); From 5fb1902bfdd134de752da18dbb4a01a5f361beba Mon Sep 17 00:00:00 2001 From: Kevin Date: Fri, 2 Aug 2024 15:34:15 -0700 Subject: [PATCH 3/7] fix: null permissions --- lib/twilio/audiohelper.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/twilio/audiohelper.ts b/lib/twilio/audiohelper.ts index 166302835..67a2e2256 100644 --- a/lib/twilio/audiohelper.ts +++ b/lib/twilio/audiohelper.ts @@ -166,7 +166,7 @@ class AudioHelper extends EventEmitter { /** * The microphone permission status */ - private _microphonePermissionStatus: PermissionStatus; + private _microphonePermissionStatus: PermissionStatus | null; /** * Called with the new input stream when the active input is changed. @@ -412,7 +412,9 @@ class AudioHelper extends EventEmitter { * @private */ _stopMicrophonePermissionListener(): void { - this._microphonePermissionStatus.removeEventListener('change', this._onMicrophonePermissionStatusChanged); + if (this._microphonePermissionStatus?.removeEventListener) { + this._microphonePermissionStatus.removeEventListener('change', this._onMicrophonePermissionStatusChanged); + } } /** From da790471b3cb46b6292f39b85812fcbe022ab33d Mon Sep 17 00:00:00 2001 From: Kevin Date: Fri, 2 Aug 2024 15:39:30 -0700 Subject: [PATCH 4/7] chore: update changelog --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2482ce59e..d804efe44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ :warning: **Important**: If you are upgrading to version 2.3.0 or later and have firewall rules or network configuration that blocks any unknown traffic by default, you need to update your configuration to allow connections to the new DNS names and IP addresses. Please refer to this [changelog](#230-january-23-2023) for more details. +2.11.3 (August 2, 2024) +====================== + +Improvements +------------ + +- The SDK now updates its internal device list when the microphone permission changes + 2.11.2 (June 26, 2024) ====================== From b02f550bc367a118d9a85cf5de22888fcc6cd35f Mon Sep 17 00:00:00 2001 From: Kevin Date: Mon, 12 Aug 2024 10:45:10 -0700 Subject: [PATCH 5/7] refactor: remove listener in function; nit: changelog --- CHANGELOG.md | 2 +- lib/twilio/audiohelper.ts | 20 +++++++++----------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 937eea5fe..c1c692e6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ Bug Fixes Improvements ------------ -- The SDK now updates its internal device list when the microphone permission changes +- The SDK now updates its internal device list when the microphone permission changes. 2.11.2 (June 26, 2024) ====================== diff --git a/lib/twilio/audiohelper.ts b/lib/twilio/audiohelper.ts index 67a2e2256..7bc417f64 100644 --- a/lib/twilio/audiohelper.ts +++ b/lib/twilio/audiohelper.ts @@ -292,7 +292,6 @@ class AudioHelper extends EventEmitter { if (microphonePermissionStatus.state !== 'granted') { const handleStateChange = () => { this._updateAvailableDevices(); - microphonePermissionStatus.removeEventListener('change', this._updateAvailableDevices); }; microphonePermissionStatus.addEventListener('change', handleStateChange); this._microphonePermissionStatus = microphonePermissionStatus; @@ -407,16 +406,6 @@ class AudioHelper extends EventEmitter { } } - /** - * Remove event listener for microphone permissions - * @private - */ - _stopMicrophonePermissionListener(): void { - if (this._microphonePermissionStatus?.removeEventListener) { - this._microphonePermissionStatus.removeEventListener('change', this._onMicrophonePermissionStatusChanged); - } - } - /** * Unbind the listeners from mediaDevices. * @private @@ -834,6 +823,15 @@ class AudioHelper extends EventEmitter { }); } + /** + * Remove event listener for microphone permissions + */ + private _stopMicrophonePermissionListener(): void { + if (this._microphonePermissionStatus?.removeEventListener) { + this._microphonePermissionStatus.removeEventListener('change', this._onMicrophonePermissionStatusChanged); + } + } + /** * Stop the selected audio stream */ From b103d0443216196d68ee1eac840ebb7ceb076e51 Mon Sep 17 00:00:00 2001 From: charliesantos Date: Tue, 13 Aug 2024 11:11:56 -0700 Subject: [PATCH 6/7] Tests --- lib/twilio/audiohelper.ts | 1 + tests/audiohelper.js | 102 ++++++++++++++++++++------------------ tests/index.ts | 4 +- 3 files changed, 58 insertions(+), 49 deletions(-) diff --git a/lib/twilio/audiohelper.ts b/lib/twilio/audiohelper.ts index 7bc417f64..ecb37940e 100644 --- a/lib/twilio/audiohelper.ts +++ b/lib/twilio/audiohelper.ts @@ -292,6 +292,7 @@ class AudioHelper extends EventEmitter { if (microphonePermissionStatus.state !== 'granted') { const handleStateChange = () => { this._updateAvailableDevices(); + this._stopMicrophonePermissionListener(); }; microphonePermissionStatus.addEventListener('change', handleStateChange); this._microphonePermissionStatus = microphonePermissionStatus; diff --git a/tests/audiohelper.js b/tests/audiohelper.js index 7448b9fd3..12e6aa841 100644 --- a/tests/audiohelper.js +++ b/tests/audiohelper.js @@ -1,5 +1,6 @@ const assert = require('assert'); const sinon = require('sinon'); +const EventTarget = require('./eventtarget'); const AudioHelper = require('../lib/twilio/audiohelper').default; const { AudioProcessorEventObserver } = require('../lib/twilio/audioprocessoreventobserver'); @@ -7,20 +8,9 @@ function getUserMedia() { return Promise.resolve({ id: 'default', getTracks: () => [] }); } -const defaultNavigator = { - permissions: { - query: function() { - return Promise.resolve('prompt'); - } - }, - mediaDevices: { - enumerateDevices: function(){ - return Promise.resolve([]); - } - } -}; - describe('AudioHelper', () => { + const wait = () => new Promise(res => res()); + context('when enumerateDevices is not supported', () => { const noop = () => {}; @@ -47,7 +37,7 @@ describe('AudioHelper', () => { ? navigator : undefined; HTMLAudioElement = undefined; - navigator = defaultNavigator; + navigator = { }; }); after(() => { @@ -81,8 +71,6 @@ describe('AudioHelper', () => { let availableDevices; let handlers; let mediaDevices; - let oldHTMLAudioElement; - let oldNavigator; beforeEach(() => { eventObserver = new AudioProcessorEventObserver(); @@ -115,23 +103,6 @@ describe('AudioHelper', () => { }); }); - before(() => { - oldHTMLAudioElement = typeof HTMLAudioElement !== 'undefined' - ? HTMLAudioElement - : undefined; - oldNavigator = typeof navigator !== 'undefined' - ? navigator - : undefined; - HTMLAudioElement = undefined; - navigator = defaultNavigator - }); - - after(() => { - HTMLAudioElement = oldHTMLAudioElement; - navigator = oldNavigator; - }); - - describe('constructor', () => { it('should set .isOutputSelectionSupported to true', () => { assert.equal(audio.isOutputSelectionSupported, true); @@ -157,22 +128,58 @@ describe('AudioHelper', () => { }); describe('navigator.permissions', () => { - it('should listen for microphone state changes', () => { - let onChangeHandler = sinon.stub(); - let microphonePermissionStatus = { - state: 'prompt', - addEventListener: (eventName, listener) => { - if (eventName === 'change') { - listener() - } + let oldMicPerm; + let mockMicPerm; + let mockEventTarget; + let enumerateDevices; + + beforeEach(() => { + enumerateDevices = sinon.stub().returns(new Promise(res => res([]))); + oldMicPerm = navigator.permissions; + mockEventTarget = new EventTarget(); + mockMicPerm = { + query: function() { + return Promise.resolve(mockEventTarget); } }; - navigator.permissions.query = () => Promise.resolve(microphonePermissionStatus); - audio = new AudioHelper(onActiveOutputsChanged, onActiveInputChanged, {}); - microphonePermissionStatus.addEventListener('change', onChangeHandler); - sinon.assert.calledOnce(onChangeHandler); - }) - }) + navigator.permissions = mockMicPerm; + }); + + afterEach(() => { + navigator.permissions = oldMicPerm; + }); + + it('should update list of devices when microphone state is not granted', async () => { + audio = new AudioHelper(onActiveOutputsChanged, onActiveInputChanged, { enumerateDevices }); + await wait(); + mockEventTarget.dispatchEvent({ type: 'change' }); + sinon.assert.calledTwice(enumerateDevices); + }); + + it('should update list of devices only once', async () => { + audio = new AudioHelper(onActiveOutputsChanged, onActiveInputChanged, { enumerateDevices }); + await wait(); + mockEventTarget.dispatchEvent({ type: 'change' }); + mockEventTarget.dispatchEvent({ type: 'change' }); + sinon.assert.calledTwice(enumerateDevices); + }); + + it('should not update list of devices when microphone state is granted', async () => { + mockEventTarget.state = 'granted'; + audio = new AudioHelper(onActiveOutputsChanged, onActiveInputChanged, { enumerateDevices }); + await wait(); + mockEventTarget.dispatchEvent({ type: 'change' }); + sinon.assert.calledOnce(enumerateDevices); + }); + + it('should remove the onchange handler on destroy', async () => { + audio = new AudioHelper(onActiveOutputsChanged, onActiveInputChanged, { enumerateDevices }); + await wait(); + audio._destroy(); + mockEventTarget.dispatchEvent({ type: 'change' }); + sinon.assert.calledOnce(enumerateDevices); + }); + }); describe('._destroy', () => { it('should properly dispose the audio instance', () => { @@ -188,7 +195,6 @@ describe('AudioHelper', () => { sinon.assert.calledOnce(audio._destroyProcessedStream); sinon.assert.calledOnce(audio._maybeStopPollingVolume); sinon.assert.calledOnce(mediaDevices.removeEventListener); - sinon.assert.calledOnce(audio._stopMicrophonePermissionListener); }); }); diff --git a/tests/index.ts b/tests/index.ts index 5057f38d4..a0fedfa95 100644 --- a/tests/index.ts +++ b/tests/index.ts @@ -1,5 +1,7 @@ import * as WebSocket from 'ws'; +const EventTarget = require('./eventtarget'); + const root = global as any; let handlers = {}; @@ -75,7 +77,7 @@ root.navigator = { userAgent: 'userAgent', permissions: { query: function() { - return Promise.resolve('prompt'); + return Promise.resolve(new EventTarget()); } } }; From a16a0f84c2c08c8924f98c4ffbbff43d795d0509 Mon Sep 17 00:00:00 2001 From: charliesantos Date: Tue, 13 Aug 2024 11:24:14 -0700 Subject: [PATCH 7/7] Tests --- tests/audiohelper.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/audiohelper.js b/tests/audiohelper.js index 12e6aa841..69568abcc 100644 --- a/tests/audiohelper.js +++ b/tests/audiohelper.js @@ -16,7 +16,7 @@ describe('AudioHelper', () => { let audio; let oldHTMLAudioElement; - let oldNavigator; + let oldMediaDevices; beforeEach(() => { audio = new AudioHelper(noop, noop, { @@ -33,16 +33,15 @@ describe('AudioHelper', () => { oldHTMLAudioElement = typeof HTMLAudioElement !== 'undefined' ? HTMLAudioElement : undefined; - oldNavigator = typeof navigator !== 'undefined' - ? navigator - : undefined; HTMLAudioElement = undefined; - navigator = { }; + + oldMediaDevices = navigator.mediaDevices; + navigator.mediaDevices = undefined; }); after(() => { HTMLAudioElement = oldHTMLAudioElement; - navigator = oldNavigator; + navigator.mediaDevices = oldMediaDevices; }); describe('constructor', () => { @@ -187,7 +186,6 @@ describe('AudioHelper', () => { audio._stopSelectedInputDeviceStream = sinon.stub(); audio._destroyProcessedStream = sinon.stub(); audio._maybeStopPollingVolume = sinon.stub(); - audio._stopMicrophonePermissionListener = sinon.stub(); audio._destroy(); assert.strictEqual(audio.eventNames().length, 0); sinon.assert.calledOnce(audio._stopDefaultInputDeviceStream);