Skip to content

Commit b4ed7c2

Browse files
authored
fix(client): align device preference persistence with permission and track end events (#2196)
### 💡 Overview This PR fixes a bug where device preferences were applied when the browser had not yet granted permissions, which could trigger unexpected permission prompts. Now applyPersistedPreferences is gated behind a `browserPermissionState === 'granted' `check, so preferences are only restored when the user has already granted access. Additionally, when a track ends unexpectedly (tab close, device disconnect), we no longer store that state to device preferences, preventing incorrect muted/disabled preferences from being persisted. ### 📝 Implementation notes 🎫 Ticket: https://linear.app/stream/issue/REACT-944/align-device-preferences-with-browser-permissions 📑 Docs: https://github.com/GetStream/docs-content/pull/<id> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Camera, microphone, and speaker preferences now respect browser permission state: persisted preferences are applied and stored only when permission is explicitly "granted"; otherwise they are not applied or saved. * **Tests** * Added and updated unit tests to cover permission-gated apply/store behavior and track-end persistence scenarios across camera, microphone, speaker, and device manager logic. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 34ad675 commit b4ed7c2

File tree

8 files changed

+200
-10
lines changed

8 files changed

+200
-10
lines changed

packages/client/src/devices/CameraManager.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Observable } from 'rxjs';
1+
import { firstValueFrom, Observable } from 'rxjs';
22
import { Call } from '../Call';
33
import { CameraDirection, CameraManagerState } from './CameraManagerState';
44
import { DeviceManager } from './DeviceManager';
@@ -140,7 +140,14 @@ export class CameraManager extends DeviceManager<CameraManagerState> {
140140
this.state.status === undefined &&
141141
this.state.optimisticStatus === undefined;
142142
let persistedPreferencesApplied = false;
143-
if (shouldApplyDefaults && this.devicePersistence.enabled) {
143+
const permissionState = await firstValueFrom(
144+
this.state.browserPermissionState$,
145+
);
146+
if (
147+
shouldApplyDefaults &&
148+
this.devicePersistence.enabled &&
149+
permissionState === 'granted'
150+
) {
144151
persistedPreferencesApplied =
145152
await this.applyPersistedPreferences(enabledInCallType);
146153
}

packages/client/src/devices/DeviceManager.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,19 @@ export abstract class DeviceManager<
8989
if (this.devicePersistence.enabled) {
9090
this.subscriptions.push(
9191
createSubscription(
92-
combineLatest([this.state.selectedDevice$, this.state.status$]),
93-
([selectedDevice, status]) => {
94-
if (!status) return;
92+
combineLatest([
93+
this.state.selectedDevice$,
94+
this.state.status$,
95+
this.state.browserPermissionState$,
96+
]),
97+
([selectedDevice, status, browserPermissionState]) => {
98+
if (
99+
!status ||
100+
(this.isTrackStoppedDueToTrackEnd && status === 'disabled') ||
101+
browserPermissionState !== 'granted'
102+
)
103+
return;
104+
95105
this.persistPreference(selectedDevice, status);
96106
},
97107
),

packages/client/src/devices/MicrophoneManager.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,14 @@ export class MicrophoneManager extends AudioDeviceManager<MicrophoneManagerState
356356
this.state.status === undefined &&
357357
this.state.optimisticStatus === undefined;
358358
let persistedPreferencesApplied = false;
359-
if (shouldApplyDefaults && this.devicePersistence.enabled) {
359+
const permissionState = await firstValueFrom(
360+
this.state.browserPermissionState$,
361+
);
362+
if (
363+
shouldApplyDefaults &&
364+
this.devicePersistence.enabled &&
365+
permissionState === 'granted'
366+
) {
360367
persistedPreferencesApplied = await this.applyPersistedPreferences(true);
361368
}
362369

packages/client/src/devices/SpeakerManager.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import { combineLatest } from 'rxjs';
22
import { Call } from '../Call';
33
import { isReactNative } from '../helpers/platforms';
44
import { SpeakerState } from './SpeakerState';
5-
import { deviceIds$, getAudioOutputDevices } from './devices';
5+
import {
6+
deviceIds$,
7+
getAudioBrowserPermission,
8+
getAudioOutputDevices,
9+
} from './devices';
610
import {
711
AudioSettingsRequestDefaultDeviceEnum,
812
CallSettingsResponse,
@@ -111,9 +115,17 @@ export class SpeakerManager {
111115

112116
if (!isReactNative() && this.devicePersistence.enabled) {
113117
this.subscriptions.push(
114-
createSubscription(this.state.selectedDevice$, (selectedDevice) => {
115-
this.persistSpeakerDevicePreference(selectedDevice);
116-
}),
118+
createSubscription(
119+
combineLatest([
120+
this.state.selectedDevice$,
121+
getAudioBrowserPermission(this.call.tracer).asStateObservable(),
122+
]),
123+
([selectedDevice, browserPermissionState]) => {
124+
if (!selectedDevice || browserPermissionState !== 'granted') return;
125+
126+
this.persistSpeakerDevicePreference(selectedDevice);
127+
},
128+
),
117129
);
118130
}
119131
}

packages/client/src/devices/__tests__/CameraManager.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,9 @@ describe('CameraManager', () => {
306306
});
307307

308308
it('should skip defaults when preferences are applied', async () => {
309+
vi.spyOn(mockBrowserPermission, 'asStateObservable').mockReturnValue(
310+
of('granted'),
311+
);
309312
const devicePersistence = { enabled: true, storageKey: '' };
310313
const persistedManager = new CameraManager(call, devicePersistence);
311314
const applySpy = vi
@@ -329,6 +332,32 @@ describe('CameraManager', () => {
329332
expect(enableSpy).not.toHaveBeenCalled();
330333
});
331334

335+
it('should skip persisted preferences when permission is not granted', async () => {
336+
vi.spyOn(mockBrowserPermission, 'asStateObservable').mockReturnValue(
337+
of('prompt'),
338+
);
339+
const devicePersistence = { enabled: true, storageKey: '' };
340+
const persistedManager = new CameraManager(call, devicePersistence);
341+
const applySpy = vi.spyOn(
342+
persistedManager as never,
343+
'applyPersistedPreferences',
344+
);
345+
const enableSpy = vi.spyOn(persistedManager, 'enable');
346+
347+
await persistedManager.apply(
348+
fromPartial({
349+
enabled: true,
350+
target_resolution: { width: 640, height: 480 },
351+
camera_facing: 'front',
352+
camera_default_on: true,
353+
}),
354+
true,
355+
);
356+
357+
expect(applySpy).not.toHaveBeenCalled();
358+
expect(enableSpy).toHaveBeenCalled();
359+
});
360+
332361
it('should not apply defaults when device is not pristine', async () => {
333362
manager.state.setStatus('enabled');
334363
const selectDirectionSpy = vi.spyOn(manager, 'selectDirection');
@@ -445,6 +474,9 @@ describe('CameraManager', () => {
445474
createVideoStreamForDevice(selectedDevice.deviceId),
446475
);
447476
});
477+
vi.spyOn(mockBrowserPermission, 'asStateObservable').mockReturnValue(
478+
of('granted'),
479+
);
448480

449481
const stressManager = new CameraManager(call, {
450482
enabled: true,

packages/client/src/devices/__tests__/DeviceManager.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ describe('Device Manager', () => {
7676
beforeEach(() => {
7777
storageKey = '@test/device-preferences';
7878
localStorageMock = createLocalStorageMock();
79+
vi.spyOn(mockBrowserPermission, 'asStateObservable').mockReturnValue(
80+
of('granted'),
81+
);
7982
Object.defineProperty(window, 'localStorage', {
8083
configurable: true,
8184
value: localStorageMock,
@@ -455,6 +458,74 @@ describe('Device Manager', () => {
455458
},
456459
]);
457460
});
461+
462+
it('stores preferences when permission is granted', async () => {
463+
const persistenceEnabledManager = new TestInputMediaDeviceManager(
464+
manager['call'],
465+
{ enabled: true, storageKey },
466+
);
467+
const listDevicesSpy = vi.spyOn(persistenceEnabledManager, 'listDevices');
468+
469+
emitDeviceIds(mockVideoDevices);
470+
persistenceEnabledManager.state.setDevice(mockVideoDevices[0].deviceId);
471+
persistenceEnabledManager.state.setStatus('enabled');
472+
473+
expect(readPreferences(storageKey).camera).toBeDefined();
474+
expect(listDevicesSpy).toHaveBeenCalled();
475+
expect(readPreferences(storageKey).camera).toEqual([
476+
{
477+
selectedDeviceId: mockVideoDevices[0].deviceId,
478+
selectedDeviceLabel: mockVideoDevices[0].label,
479+
muted: false,
480+
},
481+
]);
482+
});
483+
484+
it('does not store preferences when permission is not granted', async () => {
485+
vi.spyOn(mockBrowserPermission, 'asStateObservable').mockReturnValue(
486+
of('prompt'),
487+
);
488+
const persistenceEnabledManager = new TestInputMediaDeviceManager(
489+
manager['call'],
490+
{ enabled: true, storageKey },
491+
);
492+
const listDevicesSpy = vi.spyOn(persistenceEnabledManager, 'listDevices');
493+
494+
emitDeviceIds(mockVideoDevices);
495+
persistenceEnabledManager.state.setDevice(mockVideoDevices[0].deviceId);
496+
persistenceEnabledManager.state.setStatus('enabled');
497+
498+
expect(readPreferences(storageKey).camera).toBeUndefined();
499+
expect(listDevicesSpy).not.toHaveBeenCalled();
500+
});
501+
502+
it('does not overwrite preferences when track ends unexpectedly', async () => {
503+
const persistenceEnabledManager = new TestInputMediaDeviceManager(
504+
manager['call'],
505+
{ enabled: true, storageKey },
506+
);
507+
508+
await persistenceEnabledManager.enable();
509+
510+
expect(readPreferences(storageKey).camera).toEqual([
511+
{
512+
selectedDeviceId: mockVideoDevices[0].deviceId,
513+
selectedDeviceLabel: mockVideoDevices[0].label,
514+
muted: false,
515+
},
516+
]);
517+
518+
const [track] = persistenceEnabledManager.state.mediaStream!.getTracks();
519+
await ((track as MockTrack).eventHandlers['ended'] as Function)();
520+
521+
expect(readPreferences(storageKey).camera).toEqual([
522+
{
523+
selectedDeviceId: mockVideoDevices[0].deviceId,
524+
selectedDeviceLabel: mockVideoDevices[0].label,
525+
muted: false,
526+
},
527+
]);
528+
});
458529
});
459530

460531
describe('applyPersistedPreferences', () => {

packages/client/src/devices/__tests__/MicrophoneManager.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,29 @@ describe('MicrophoneManager', () => {
479479
expect(enableSpy).not.toHaveBeenCalled();
480480
});
481481

482+
it('should skip persisted preferences when permission is not granted', async () => {
483+
vi.spyOn(mockBrowserPermission, 'asStateObservable').mockReturnValue(
484+
of('prompt'),
485+
);
486+
const devicePersistence = { enabled: true, storageKey: '' };
487+
const persistedManager = new MicrophoneManager(
488+
call,
489+
devicePersistence,
490+
'disable-tracks',
491+
);
492+
const applySpy = vi.spyOn(
493+
persistedManager as never,
494+
'applyPersistedPreferences',
495+
);
496+
const enableSpy = vi.spyOn(persistedManager, 'enable');
497+
498+
// @ts-expect-error - partial data
499+
await persistedManager.apply({ mic_default_on: true }, true);
500+
501+
expect(applySpy).not.toHaveBeenCalled();
502+
expect(enableSpy).toHaveBeenCalled();
503+
});
504+
482505
it('should not apply defaults when mic is not pristine', async () => {
483506
manager.state.setStatus('enabled');
484507
const applySpy = vi.spyOn(manager as never, 'applyPersistedPreferences');

packages/client/src/devices/__tests__/SpeakerManager.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ describe('SpeakerManager.test', () => {
3737
beforeEach(() => {
3838
storageKey = '@test/speaker-preferences';
3939
localStorageMock = createLocalStorageMock();
40+
vi.spyOn(mockBrowserPermission, 'asStateObservable').mockReturnValue(
41+
of('granted'),
42+
);
4043
Object.defineProperty(window, 'localStorage', {
4144
configurable: true,
4245
value: localStorageMock,
@@ -125,6 +128,31 @@ describe('SpeakerManager.test', () => {
125128
expect(manager.state.selectedDevice).toBe('');
126129
});
127130

131+
it('persists speaker selection when permission is granted', async () => {
132+
const persistedManager = new SpeakerManager(
133+
new Call({
134+
id: '',
135+
type: '',
136+
streamClient: new StreamClient('abc123'),
137+
clientStore: new StreamVideoWriteableStateStore(),
138+
}),
139+
{ enabled: true, storageKey },
140+
);
141+
const listDevicesSpy = vi.spyOn(persistedManager, 'listDevices');
142+
const audioOutputDevice = {
143+
deviceId: 'speaker-1',
144+
kind: 'audiooutput',
145+
label: 'Speaker 1',
146+
groupId: 'speaker-group',
147+
} as MediaDeviceInfo;
148+
149+
emitDeviceIds([audioOutputDevice]);
150+
persistedManager.select(audioOutputDevice.deviceId);
151+
152+
expect(listDevicesSpy).toHaveBeenCalled();
153+
expect(persistedManager.state.selectedDevice).toBe('speaker-1');
154+
});
155+
128156
describe('apply (web)', () => {
129157
it('does nothing when persistence is disabled', () => {
130158
const selectSpy = vi.spyOn(manager, 'select');

0 commit comments

Comments
 (0)