Selaa lähdekoodia

fix(conference): Make sure join waits for confernce.init.

It was possible that join can be executed before conference.init have even started or we haven't reached the point ot create the initialGUMPromise. This was causing the following issues:
 - users stuck on the prejoin screen
 - participants join 2+ times in the call (we have been creating more than 1 local participants from a single page).
factor2
Hristo Terezov 11 kuukautta sitten
vanhempi
commit
960a08c066

+ 38
- 14
conference.js Näytä tiedosto

@@ -560,10 +560,10 @@ export default {
560 560
      * If prejoin page is enabled open an new connection in the background
561 561
      * and create local tracks.
562 562
      *
563
-     * @param {{ roomName: string }} options
563
+     * @param {{ roomName: string, shouldDispatchConnect }} options
564 564
      * @returns {Promise}
565 565
      */
566
-    async init({ roomName }) {
566
+    async init({ roomName, shouldDispatchConnect }) {
567 567
         const state = APP.store.getState();
568 568
         const initialOptions = {
569 569
             startAudioOnly: config.startAudioOnly,
@@ -607,27 +607,49 @@ export default {
607 607
         const { dispatch, getState } = APP.store;
608 608
         const { tryCreateLocalTracks, errors } = this.createInitialLocalTracks(initialOptions);
609 609
 
610
-        dispatch(setInitialGUMPromise(tryCreateLocalTracks.then(async tr => {
610
+        tryCreateLocalTracks.then(async tr => {
611 611
             const tracks = handleInitialTracks(initialOptions, tr);
612 612
 
613 613
             this._initDeviceList(true);
614 614
 
615
+            const { initialGUMPromise } = getState()['features/base/media'];
616
+
615 617
             if (isPrejoinPageVisible(getState())) {
616 618
                 dispatch(gumPending([ MEDIA_TYPE.AUDIO, MEDIA_TYPE.VIDEO ], IGUMPendingState.NONE));
617
-                dispatch(setInitialGUMPromise());
619
+
620
+                // Since the conference is not yet created in redux this function will execute synchronous
621
+                // which will guarantee us that the local tracks are added to redux before we proceed.
618 622
                 initPrejoin(tracks, errors, dispatch);
623
+
624
+                // resolve the initialGUMPromise in case connect have finished so that we can proceed to join.
625
+                if (initialGUMPromise) {
626
+                    logger.debug('Resolving the initialGUM promise! (prejoinVisible=true)');
627
+                    initialGUMPromise.resolve({
628
+                        tracks,
629
+                        errors
630
+                    });
631
+                }
632
+
633
+                logger.debug('Clear the initialGUM promise! (prejoinVisible=true)');
634
+
635
+                // For prejoin we don't need the initial GUM promise since the tracks are already added to the store
636
+                // via initPrejoin
637
+                dispatch(setInitialGUMPromise());
619 638
             } else {
620 639
                 APP.store.dispatch(displayErrorsForCreateInitialLocalTracks(errors));
621 640
                 setGUMPendingStateOnFailedTracks(tracks, APP.store.dispatch);
622
-            }
623 641
 
624
-            return {
625
-                tracks,
626
-                errors
627
-            };
628
-        })));
642
+                if (initialGUMPromise) {
643
+                    logger.debug('Resolving the initialGUM promise!');
644
+                    initialGUMPromise.resolve({
645
+                        tracks,
646
+                        errors
647
+                    });
648
+                }
649
+            }
650
+        });
629 651
 
630
-        if (!isPrejoinPageVisible(getState())) {
652
+        if (shouldDispatchConnect) {
631 653
             logger.info('Dispatching connect from init since prejoin is not visible.');
632 654
             dispatch(connect());
633 655
         }
@@ -2047,8 +2069,9 @@ export default {
2047 2069
             const { dispatch } = APP.store;
2048 2070
 
2049 2071
             return dispatch(getAvailableDevices())
2050
-                .then(devices => {
2051
-                    APP.UI.onAvailableDevicesChanged(devices);
2072
+                .then(() => {
2073
+                    this.updateAudioIconEnabled();
2074
+                    this.updateVideoIconEnabled();
2052 2075
                 });
2053 2076
         }
2054 2077
 
@@ -2213,7 +2236,8 @@ export default {
2213 2236
 
2214 2237
         return Promise.all(promises)
2215 2238
             .then(() => {
2216
-                APP.UI.onAvailableDevicesChanged(filteredDevices);
2239
+                this.updateAudioIconEnabled();
2240
+                this.updateVideoIconEnabled();
2217 2241
             });
2218 2242
     },
2219 2243
 

+ 0
- 8
modules/UI/UI.js Näytä tiedosto

@@ -211,14 +211,6 @@ UI.handleLastNEndpoints = function(leavingIds, enteringIds) {
211 211
  */
212 212
 UI.setAudioLevel = (id, lvl) => VideoLayout.setAudioLevel(id, lvl);
213 213
 
214
-/**
215
- * Update list of available physical devices.
216
- */
217
-UI.onAvailableDevicesChanged = function() {
218
-    APP.conference.updateAudioIconEnabled();
219
-    APP.conference.updateVideoIconEnabled();
220
-};
221
-
222 214
 /**
223 215
  * Returns the id of the current video shown on large.
224 216
  * Currently used by tests (torture).

+ 1
- 1
react/features/base/app/components/BaseApp.tsx Näytä tiedosto

@@ -79,7 +79,7 @@ export default class BaseApp<P> extends Component<P, IState> {
79 79
          * @see {@link #_initStorage}
80 80
          * @type {Promise}
81 81
          */
82
-        this._init = createDeferred();
82
+        this._init = createDeferred<void>();
83 83
 
84 84
         try {
85 85
             await this._initStorage();

+ 22
- 34
react/features/base/conference/middleware.any.ts Näytä tiedosto

@@ -614,37 +614,6 @@ function _setRoom({ dispatch, getState }: IStore, next: Function, action: AnyAct
614 614
     return next(action);
615 615
 }
616 616
 
617
-/**
618
- * Synchronizes local tracks from state with local tracks in JitsiConference
619
- * instance.
620
- *
621
- * @param {Store} store - The redux store.
622
- * @param {Object} action - Action object.
623
- * @private
624
- * @returns {Promise}
625
- */
626
-function _syncConferenceLocalTracksWithState({ getState }: IStore, action: AnyAction) {
627
-    const state = getState();
628
-    const conference = getCurrentConference(state);
629
-    let promise;
630
-
631
-    if (conference) {
632
-        const track = action.track.jitsiTrack;
633
-
634
-        if (action.type === TRACK_ADDED) {
635
-            // If gUM is slow and tracks are created after the user has already joined the conference, avoid
636
-            // adding the tracks to the conference if the user is a visitor.
637
-            if (!iAmVisitor(state)) {
638
-                promise = _addLocalTracksToConference(conference, [ track ]);
639
-            }
640
-        } else {
641
-            promise = _removeLocalTracksFromConference(conference, [ track ]);
642
-        }
643
-    }
644
-
645
-    return promise || Promise.resolve();
646
-}
647
-
648 617
 /**
649 618
  * Notifies the feature base/conference that the action {@code TRACK_ADDED}
650 619
  * or {@code TRACK_REMOVED} is being dispatched within a specific redux store.
@@ -664,9 +633,28 @@ function _trackAddedOrRemoved(store: IStore, next: Function, action: AnyAction)
664 633
 
665 634
     // TODO All track swapping should happen here instead of conference.js.
666 635
     if (track?.local) {
667
-        return (
668
-            _syncConferenceLocalTracksWithState(store, action)
669
-                .then(() => next(action)));
636
+        const { getState } = store;
637
+        const state = getState();
638
+        const conference = getCurrentConference(state);
639
+        let promise;
640
+
641
+        if (conference) {
642
+            const jitsiTrack = action.track.jitsiTrack;
643
+
644
+            if (action.type === TRACK_ADDED) {
645
+                // If gUM is slow and tracks are created after the user has already joined the conference, avoid
646
+                // adding the tracks to the conference if the user is a visitor.
647
+                if (!iAmVisitor(state)) {
648
+                    promise = _addLocalTracksToConference(conference, [ jitsiTrack ]);
649
+                }
650
+            } else {
651
+                promise = _removeLocalTracksFromConference(conference, [ jitsiTrack ]);
652
+            }
653
+
654
+            if (promise) {
655
+                return promise.then(() => next(action));
656
+            }
657
+        }
670 658
     }
671 659
 
672 660
     return next(action);

+ 8
- 9
react/features/base/conference/middleware.web.ts Näytä tiedosto

@@ -149,12 +149,15 @@ MiddlewareRegistry.register(store => next => action => {
149 149
         break;
150 150
     }
151 151
     case CONNECTION_ESTABLISHED: {
152
-        if (isPrejoinPageVisible(getState())) {
153
-            let { initialGUMPromise } = getState()['features/base/media'];
152
+        const { initialGUMPromise } = getState()['features/base/media'];
153
+        const promise = initialGUMPromise ? initialGUMPromise.promise : Promise.resolve({ tracks: [] });
154
+        const prejoinVisible = isPrejoinPageVisible(getState());
154 155
 
155
-            initialGUMPromise = initialGUMPromise || Promise.resolve({ tracks: [] });
156
+        logger.debug(`On connection established: prejoinVisible: ${prejoinVisible}, initialGUMPromiseExists=${
157
+            Boolean(initialGUMPromise)}, promiseExists=${Boolean(promise)}`);
156 158
 
157
-            initialGUMPromise.then(() => {
159
+        if (prejoinVisible) {
160
+            promise.then(() => {
158 161
                 const state = getState();
159 162
                 let localTracks = getLocalTracks(state['features/base/tracks']);
160 163
                 const trackReplacePromises = [];
@@ -186,11 +189,7 @@ MiddlewareRegistry.register(store => next => action => {
186 189
                 });
187 190
             });
188 191
         } else {
189
-            let { initialGUMPromise } = getState()['features/base/media'];
190
-
191
-            initialGUMPromise = initialGUMPromise || Promise.resolve({ tracks: [] });
192
-
193
-            initialGUMPromise.then(({ tracks }) => {
192
+            promise.then(({ tracks }) => {
194 193
                 let tracksToUse = tracks ?? [];
195 194
 
196 195
                 if (iAmVisitor(getState())) {

+ 14
- 6
react/features/base/media/reducer.ts Näytä tiedosto

@@ -3,6 +3,7 @@ import { AnyAction, combineReducers } from 'redux';
3 3
 import { CONFERENCE_FAILED, CONFERENCE_LEFT } from '../conference/actionTypes';
4 4
 import ReducerRegistry from '../redux/ReducerRegistry';
5 5
 import { TRACK_REMOVED } from '../tracks/actionTypes';
6
+import { DefferedPromise, createDeferred } from '../util/helpers';
6 7
 
7 8
 import {
8 9
     GUM_PENDING,
@@ -88,6 +89,12 @@ function _audio(state: IAudioState = _AUDIO_INITIAL_MEDIA_STATE, action: AnyActi
88 89
     }
89 90
 }
90 91
 
92
+// Using a deferred promise here to make sure that once the connection is established even if conference.init and the
93
+// initial track creation haven't been started we would wait for it to finish before starting to join the room.
94
+// NOTE: The previous implementation was using the GUM promise from conference.init. But it turned out that connect
95
+// may finish even before conference.init is executed.
96
+const DEFAULT_INITIAL_PROMISE_STATE = createDeferred<IInitialGUMPromiseResult>();
97
+
91 98
 /**
92 99
  * Reducer fot the common properties in media state.
93 100
  *
@@ -96,7 +103,8 @@ function _audio(state: IAudioState = _AUDIO_INITIAL_MEDIA_STATE, action: AnyActi
96 103
  * @param {string} action.type - Type of action.
97 104
  * @returns {ICommonState}
98 105
  */
99
-function _initialGUMPromise(state: initialGUMPromise | null = null, action: AnyAction) {
106
+function _initialGUMPromise(state: DefferedPromise<IInitialGUMPromiseResult> | null = DEFAULT_INITIAL_PROMISE_STATE,
107
+        action: AnyAction) {
100 108
     if (action.type === SET_INITIAL_GUM_PROMISE) {
101 109
         return action.promise ?? null;
102 110
     }
@@ -264,10 +272,10 @@ interface IAudioState {
264 272
     unmuteBlocked: boolean;
265 273
 }
266 274
 
267
-type initialGUMPromise = Promise<{
268
-        errors?: any;
269
-        tracks: Array<any>;
270
-    }> | null;
275
+interface IInitialGUMPromiseResult {
276
+    errors?: any;
277
+    tracks: Array<any>;
278
+}
271 279
 
272 280
 interface IScreenshareState {
273 281
     available: boolean;
@@ -286,7 +294,7 @@ interface IVideoState {
286 294
 
287 295
 export interface IMediaState {
288 296
     audio: IAudioState;
289
-    initialGUMPromise: initialGUMPromise;
297
+    initialGUMPromise: DefferedPromise<IInitialGUMPromiseResult> | null;
290 298
     screenshare: IScreenshareState;
291 299
     video: IVideoState;
292 300
 }

+ 8
- 3
react/features/base/util/helpers.ts Näytä tiedosto

@@ -22,16 +22,21 @@ export function assignIfDefined(target: Object, source: Object) {
22 22
     return to;
23 23
 }
24 24
 
25
+export type DefferedPromise<T> = {
26
+    promise: Promise<T>;
27
+    reject: (reason?: any) => void;
28
+    resolve: (value: T) => void;
29
+};
25 30
 
26 31
 /**
27 32
  * Creates a deferred object.
28 33
  *
29 34
  * @returns {{promise, resolve, reject}}
30 35
  */
31
-export function createDeferred() {
32
-    const deferred: any = {};
36
+export function createDeferred<T>() {
37
+    const deferred = {} as DefferedPromise<T>;
33 38
 
34
-    deferred.promise = new Promise((resolve, reject) => {
39
+    deferred.promise = new Promise<T>((resolve, reject) => {
35 40
         deferred.resolve = resolve;
36 41
         deferred.reject = reject;
37 42
     });

+ 5
- 2
react/features/conference/actions.web.ts Näytä tiedosto

@@ -49,9 +49,11 @@ export function setupInitialDevices() {
49 49
 /**
50 50
  * Init.
51 51
  *
52
+ * @param {boolean} shouldDispatchConnect - Whether or not connect should be dispatched. This should be false only when
53
+ * prejoin is enabled.
52 54
  * @returns {Promise<JitsiConnection>}
53 55
  */
54
-export function init() {
56
+export function init(shouldDispatchConnect: boolean) {
55 57
     return (dispatch: IStore['dispatch'], getState: IStore['getState']) => {
56 58
         const room = getBackendSafeRoomName(getState()['features/base/conference'].room);
57 59
 
@@ -59,7 +61,8 @@ export function init() {
59 61
         // from the old app (at the moment of writing).
60 62
         return dispatch(setupInitialDevices()).then(
61 63
             () => APP.conference.init({
62
-                roomName: room
64
+                roomName: room,
65
+                shouldDispatchConnect
63 66
             }).catch((error: Error) => {
64 67
                 APP.API.notifyConferenceLeft(APP.conference.roomName);
65 68
                 logger.error(error);

+ 16
- 2
react/features/conference/components/web/Conference.tsx Näytä tiedosto

@@ -104,12 +104,24 @@ interface IProps extends AbstractProps, WithTranslation {
104 104
 
105 105
     /**
106 106
      * If visitors queue page is visible or not.
107
+     * NOTE: This should be set to true once we received an error on connect. Before the first connect this will always
108
+     * be false.
107 109
      */
108 110
     _showVisitorsQueue: boolean;
109 111
 
110 112
     dispatch: IStore['dispatch'];
111 113
 }
112 114
 
115
+/**
116
+ * Returns true if the prejoin screen should be displayed and false otherwise.
117
+ *
118
+ * @param {IProps} props - The props object.
119
+ * @returns {boolean} - True if the prejoin screen should be displayed and false otherwise.
120
+ */
121
+function shouldShowPrejoin({ _showPrejoin, _showVisitorsQueue }: IProps) {
122
+    return _showPrejoin && !_showVisitorsQueue;
123
+}
124
+
113 125
 /**
114 126
  * The conference page of the Web application.
115 127
  */
@@ -265,7 +277,7 @@ class Conference extends AbstractConference<IProps, any> {
265 277
 
266 278
                     <CalleeInfoContainer />
267 279
 
268
-                    { (_showPrejoin && !_showVisitorsQueue) && <Prejoin />}
280
+                    { shouldShowPrejoin(this.props) && <Prejoin />}
269 281
                     { (_showLobby && !_showVisitorsQueue) && <LobbyScreen />}
270 282
                     { _showVisitorsQueue && <VisitorsQueue />}
271 283
                 </div>
@@ -384,7 +396,9 @@ class Conference extends AbstractConference<IProps, any> {
384 396
 
385 397
         const { dispatch, t } = this.props;
386 398
 
387
-        dispatch(init());
399
+        // if we will be showing prejoin we don't want to call connect from init.
400
+        // Connect will be dispatched from prejoin screen.
401
+        dispatch(init(!shouldShowPrejoin(this.props)));
388 402
 
389 403
         maybeShowSuboptimalExperienceNotification(dispatch, t);
390 404
     }

Loading…
Peruuta
Tallenna