fix(base/tracks): handle GUM in progress

This commit adds extra actions/Redux state to be able to deal with
the GUM operation being in progress. There will be early local track
stub in the Redux state for any a local track for which GUM has been
called, but not completed yet.

Local track is considered valid only after TRACK_ADDED event when it
will have JitsiLocalTrack instance set.

react/features/base/conference/actions.js

@@ -12,7 +12,7 @@ import {
     participantRoleChanged,
     participantUpdated
 } from '../participants';
-import { trackAdded, trackRemoved } from '../tracks';
+import { getLocalTracks, trackAdded, trackRemoved } from '../tracks';
 
 import {
     CONFERENCE_FAILED,
@@ -222,8 +222,7 @@ export function conferenceLeft(conference: Object) {
 function _conferenceWillJoin(conference: Object) {
     return (dispatch: Dispatch<*>, getState: Function) => {
         const localTracks
-            = getState()['features/base/tracks']
-                .filter(t => t.local)
+            = getLocalTracks(getState()['features/base/tracks'])
                 .map(t => t.jitsiTrack);
 
         if (localTracks.length) {

react/features/base/tracks/actionTypes.js

@@ -10,15 +10,45 @@
 export const TRACK_ADDED = Symbol('TRACK_ADDED');
 
 /**
- * Action for when a track cannot be created because permissions have not been
- * granted.
+ * Action triggered when a local track starts being created through the WebRTC
+ * getUserMedia call. It will include extra 'gumProcess' field which is
+ * a Promise with extra 'cancel' method which can be used to cancel the process.
+ * Canceling will result in disposing any JitsiLocalTrack returned by the GUM
+ * callback. There will be TRACK_CREATE_CANCELED event instead of track
+ * added/gum failed events.
  *
  * {
- *     type: TRACK_PERMISSION_ERROR,
- *     trackType: string
+ *     type: TRACK_BEING_CREATED
+ *     track: {
+ *         local: true,
+ *         gumProcess: Promise with cancel() method to abort,
+ *         mediaType: MEDIA_TYPE
+ *     }
  * }
  */
-export const TRACK_PERMISSION_ERROR = Symbol('TRACK_PERMISSION_ERROR');
+export const TRACK_BEING_CREATED = Symbol('TRACK_BEING_CREATED');
+
+/**
+ * Action sent when canceled GUM process completes either successfully or with
+ * an error (error is ignored and track is immediately disposed if created).
+ *
+ * {
+ *     type: TRACK_CREATE_CANCELED,
+ *     trackType: MEDIA_TYPE
+ * }
+ */
+export const TRACK_CREATE_CANCELED = Symbol('TRACK_CREATE_CANCELED');
+
+/**
+ * Action sent when GUM fails with an error other than permission denied.
+ *
+ * {
+ *     type: TRACK_CREATE_ERROR,
+ *     permissionDenied: Boolean,
+ *     trackType: MEDIA_TYPE
+ * }
+ */
+export const TRACK_CREATE_ERROR = Symbol('TRACK_CREATE_ERROR');
 
 /**
  * Action for when a track has been removed from the conference,

react/features/base/tracks/actions.js

@@ -10,7 +10,9 @@ import { getLocalParticipant } from '../participants';
 
 import {
     TRACK_ADDED,
-    TRACK_PERMISSION_ERROR,
+    TRACK_BEING_CREATED,
+    TRACK_CREATE_CANCELED,
+    TRACK_CREATE_ERROR,
     TRACK_REMOVED,
     TRACK_UPDATED
 } from './actionTypes';
@@ -79,7 +81,11 @@ export function createLocalTracksA(options = {}) {
         // to implement them) and the right thing to do is to ask for each
         // device separately.
         for (const device of devices) {
-            createLocalTracksF(
+            if (getState()['features/base/tracks']
+                    .find(t => t.local && t.mediaType === device)) {
+                throw new Error(`Local track for ${device} already exists`);
+            }
+            const gumProcess = createLocalTracksF(
                 {
                     cameraDeviceId: options.cameraDeviceId,
                     devices: [ device ],
@@ -89,9 +95,48 @@ export function createLocalTracksA(options = {}) {
                 /* firePermissionPromptIsShownEvent */ false,
                 store)
             .then(
-                localTracks => dispatch(_updateLocalTracks(localTracks)),
+                localTracks => {
+                    // Because GUM is called for 1 device (which is actually
+                    // a media type 'audio','video', 'screen' etc.) we should
+                    // not get more than one JitsiTrack.
+                    if (localTracks.length !== 1) {
+                        throw new Error(
+                            'Expected exactly 1 track, but was '
+                                + `given ${localTracks.length} tracks`
+                                + `for device: ${device}.`);
+                    }
+
+                    if (gumProcess.canceled) {
+                        return _disposeTracks(localTracks)
+                                    .then(
+                                        () =>
+                                            dispatch(
+                                                _trackCreateCanceled(device)));
+                    }
+
+                    return dispatch(trackAdded(localTracks[0]));
+                },
+                // eslint-disable-next-line no-confusing-arrow
                 reason =>
-                    dispatch(_onCreateLocalTracksRejected(reason, device)));
+                    dispatch(
+                        gumProcess.canceled
+                            ? _trackCreateCanceled(device)
+                            : _onCreateLocalTracksRejected(reason, device)));
+
+            gumProcess.cancel = () => {
+                gumProcess.canceled = true;
+
+                return gumProcess;
+            };
+
+            dispatch({
+                type: TRACK_BEING_CREATED,
+                track: {
+                    local: true,
+                    gumProcess,
+                    mediaType: device
+                }
+            });
         }
     };
 }
@@ -103,12 +148,17 @@ export function createLocalTracksA(options = {}) {
  * @returns {Function}
  */
 export function destroyLocalTracks() {
-    return (dispatch, getState) =>
-        dispatch(
-            _disposeAndRemoveTracks(
-                getState()['features/base/tracks']
-                    .filter(t => t.local)
-                    .map(t => t.jitsiTrack)));
+    return (dispatch, getState) => {
+        // First wait until any getUserMedia in progress is settled and then get
+        // rid of all local tracks.
+        _cancelAllGumInProgress(getState)
+            .then(
+                () => dispatch(
+                    _disposeAndRemoveTracks(
+                        getState()['features/base/tracks']
+                            .filter(t => t.local)
+                            .map(t => t.jitsiTrack))));
+    };
 }
 
 /**
@@ -316,81 +366,85 @@ function _addTracks(tracks) {
 }
 
 /**
- * Disposes passed tracks and signals them to be removed.
+ * Signals that track create operation for given media track has been canceled.
+ * Will clean up local track stub from the Redux state which holds the
+ * 'gumProcess' reference.
  *
- * @param {(JitsiLocalTrack|JitsiRemoteTrack)[]} tracks - List of tracks.
- * @protected
- * @returns {Function}
+ * @param {MEDIA_TYPE} mediaType - The type of the media for which the track was
+ * being created.
+ * @returns {{
+ *      type,
+ *      trackType: MEDIA_TYPE
+ * }}
+ * @private
  */
-export function _disposeAndRemoveTracks(tracks) {
-    return dispatch =>
-        Promise.all(
-            tracks.map(t =>
-                t.dispose()
-                    .catch(err => {
-                        // Track might be already disposed so ignore such an
-                        // error. Of course, re-throw any other error(s).
-                        if (err.name !== JitsiTrackErrors.TRACK_IS_DISPOSED) {
-                            throw err;
-                        }
-                    })
-            ))
-            .then(Promise.all(tracks.map(t => dispatch(trackRemoved(t)))));
+function _trackCreateCanceled(mediaType) {
+    return {
+        type: TRACK_CREATE_CANCELED,
+        trackType: mediaType
+    };
 }
 
 /**
- * Finds the first {@code JitsiLocalTrack} in a specific array/list of
- * {@code JitsiTrack}s which is of a specific {@code MEDIA_TYPE}.
+ * Cancels and waits for any get user media operations currently in progress to
+ * complete.
  *
- * @param {JitsiTrack[]} tracks - The array/list of {@code JitsiTrack}s to look
- * through.
- * @param {MEDIA_TYPE} mediaType - The {@code MEDIA_TYPE} of the first
- * {@code JitsiLocalTrack} to be returned.
+ * @param {Function} getState - The Redux store {@code getState} method used to
+ * obtain the state.
+ * @returns {Promise} - A Promise resolved once all {@code gumProcess.cancel}
+ * Promises are settled. That is when they are either resolved or rejected,
+ * because all we care about here is to be sure that get user media callbacks
+ * have completed (returned from the native side).
  * @private
- * @returns {JitsiLocalTrack} The first {@code JitsiLocalTrack}, if any, in the
- * specified {@code tracks} of the specified {@code mediaType}.
  */
-function _getLocalTrack(tracks, mediaType) {
-    return tracks.find(track =>
-        track.isLocal()
-
-            // XXX JitsiTrack#getType() returns a MEDIA_TYPE value in the terms
-            // of lib-jitsi-meet while mediaType is in the terms of jitsi-meet.
-            && track.getType() === mediaType);
+function _cancelAllGumInProgress(getState) {
+    // FIXME use logger
+    const logError
+        = error =>
+            console.error('gumProcess.cancel failed', JSON.stringify(error));
+
+    return Promise.all(
+        getState()['features/base/tracks']
+            .filter(t => t.local)
+            .map(
+                t => t.gumProcess
+                    && t.gumProcess.cancel().catch(logError)));
 }
 
 /**
- * Determines which local media tracks should be added and which removed.
+ * Disposes passed tracks and signals them to be removed.
  *
- * @param {(JitsiLocalTrack|JitsiRemoteTrack)[]} currentTracks - List of
- * current/existing media tracks.
- * @param {(JitsiLocalTrack|JitsiRemoteTrack)[]} newTracks - List of new media
- * tracks.
- * @private
- * @returns {{
- *     tracksToAdd: JitsiLocalTrack[],
- *     tracksToRemove: JitsiLocalTrack[]
- * }}
+ * @param {(JitsiLocalTrack|JitsiRemoteTrack)[]} tracks - List of tracks.
+ * @protected
+ * @returns {Function}
  */
-function _getLocalTracksToChange(currentTracks, newTracks) {
-    const tracksToAdd = [];
-    const tracksToRemove = [];
-
-    for (const mediaType of [ MEDIA_TYPE.AUDIO, MEDIA_TYPE.VIDEO ]) {
-        const newTrack = _getLocalTrack(newTracks, mediaType);
-
-        if (newTrack) {
-            const currentTrack = _getLocalTrack(currentTracks, mediaType);
-
-            tracksToAdd.push(newTrack);
-            currentTrack && tracksToRemove.push(currentTrack);
-        }
-    }
+export function _disposeAndRemoveTracks(tracks) {
+    return dispatch =>
+        _disposeTracks(tracks)
+            .then(
+                () => Promise.all(tracks.map(t => dispatch(trackRemoved(t)))));
+}
 
-    return {
-        tracksToAdd,
-        tracksToRemove
-    };
+/**
+ * Disposes passed tracks.
+ *
+ * @param {(JitsiLocalTrack|JitsiRemoteTrack)[]} tracks - List of tracks.
+ * @protected
+ * @returns {Promise} - A Promise resolved once {@link JitsiTrack.dispose()} is
+ * done for every track from the list.
+ */
+function _disposeTracks(tracks) {
+    return Promise.all(
+        tracks.map(t =>
+            t.dispose()
+                .catch(err => {
+                    // Track might be already disposed so ignore such an
+                    // error. Of course, re-throw any other error(s).
+                    if (err.name !== JitsiTrackErrors.TRACK_IS_DISPOSED) {
+                        throw err;
+                    }
+                })
+        ));
 }
 
 /**
@@ -430,8 +484,10 @@ function _onCreateLocalTracksRejected({ gum }, device) {
                     trackPermissionError = error instanceof DOMException;
                     break;
                 }
-                trackPermissionError && dispatch({
-                    type: TRACK_PERMISSION_ERROR,
+
+                dispatch({
+                    type: TRACK_CREATE_ERROR,
+                    permissionDenied: trackPermissionError,
                     trackType: device
                 });
             }
@@ -468,24 +524,3 @@ function _shouldMirror(track) {
             // but that may not be the case tomorrow.
             && track.getCameraFacingMode() === CAMERA_FACING_MODE.USER);
 }
-
-/**
- * Set new local tracks replacing any existing tracks that were previously
- * available. Currently only one audio and one video local tracks are allowed.
- *
- * @param {(JitsiLocalTrack|JitsiRemoteTrack)[]} [newTracks=[]] - List of new
- * media tracks.
- * @private
- * @returns {Function}
- */
-function _updateLocalTracks(newTracks = []) {
-    return (dispatch, getState) => {
-        const tracks
-            = getState()['features/base/tracks'].map(t => t.jitsiTrack);
-        const { tracksToAdd, tracksToRemove }
-            = _getLocalTracksToChange(tracks, newTracks);
-
-        return dispatch(_disposeAndRemoveTracks(tracksToRemove))
-            .then(() => dispatch(_addTracks(tracksToAdd)));
-    };
-}

react/features/base/tracks/functions.js

@@ -106,7 +106,26 @@ export function getLocalAudioTrack(tracks) {
  * @returns {(Track|undefined)}
  */
 export function getLocalTrack(tracks, mediaType) {
-    return tracks.find(t => t.local && t.mediaType === mediaType);
+    return getLocalTracks(tracks).find(t => t.mediaType === mediaType);
+}
+
+/**
+ * Returns an array containing local tracks. Local tracks without valid
+ * JitsiTrack will not be included in the list.
+ *
+ * @param {Track[]} tracks - An array of all local tracks.
+ * @returns {Track[]}
+ */
+export function getLocalTracks(tracks) {
+
+    // XXX A local track is considered ready only once it has 'jitsiTrack' field
+    // set by the TRACK_ADDED action. Until then there is a stub added just
+    // before get user media call with a cancellable 'gumInProgress' field which
+    // then can be used to destroy the track that has not yet been added to
+    // the Redux store. Once GUM is cancelled it will never make it to the store
+    // nor there will be any TRACK_ADDED/TRACK_REMOVED related events fired for
+    // it.
+    return tracks.filter(t => t.local && t.jitsiTrack);
 }
 
 /**

react/features/base/tracks/reducer.js

@@ -3,15 +3,22 @@ import { ReducerRegistry } from '../redux';
 
 import {
     TRACK_ADDED,
+    TRACK_BEING_CREATED,
+    TRACK_CREATE_CANCELED,
+    TRACK_CREATE_ERROR,
     TRACK_REMOVED,
     TRACK_UPDATED
 } from './actionTypes';
 
 /**
  * @typedef {Object} Track
- * @property {(JitsiLocalTrack|JitsiRemoteTrack)} jitsiTrack - JitsiTrack
- * instance.
+ * @property {(JitsiLocalTrack|JitsiRemoteTrack)} [jitsiTrack] - JitsiTrack
+ * instance. Optional for local tracks if those are being created (GUM in
+ * progress).
  * @property {boolean} local=false - If track is local.
+ * @property {Promise} [gumProcess] - if local track is being created it
+ * will have no JitsiTrack, but a 'gumProcess' set to a Promise with and extra
+ * cancel().
  * @property {MEDIA_TYPE} mediaType=false - Media type of track.
  * @property {boolean} mirror=false - The indicator which determines whether the
  * display/rendering of the track should be mirrored. It only makes sense in the
@@ -81,11 +88,25 @@ ReducerRegistry.register('features/base/tracks', (state = [], action) => {
     case TRACK_UPDATED:
         return state.map(t => track(t, action));
 
-    case TRACK_ADDED:
-        return [
-            ...state,
-            action.track
-        ];
+    case TRACK_ADDED: {
+        let withoutTrackStub = state;
+
+        if (action.track.local) {
+            withoutTrackStub
+                = state.filter(
+                    t => !t.local || t.mediaType !== action.track.mediaType);
+        }
+
+        return [ ...withoutTrackStub, action.track ];
+    }
+
+    case TRACK_BEING_CREATED:
+        return [ ...state, action.track ];
+
+    case TRACK_CREATE_CANCELED:
+    case TRACK_CREATE_ERROR: {
+        return state.filter(t => !t.local || t.mediaType !== action.trackType);
+    }
 
     case TRACK_REMOVED:
         return state.filter(t => t.jitsiTrack !== action.track.jitsiTrack);

react/features/mobile/permissions/middleware.js

@@ -5,7 +5,7 @@ import { Alert, Linking, NativeModules } from 'react-native';
 import { isRoomValid } from '../../base/conference';
 import { Platform } from '../../base/react';
 import { MiddlewareRegistry } from '../../base/redux';
-import { TRACK_PERMISSION_ERROR } from '../../base/tracks';
+import { TRACK_CREATE_ERROR } from '../../base/tracks';
 
 /**
  * Middleware that captures track permission errors and alerts the user so they
@@ -18,14 +18,16 @@ MiddlewareRegistry.register(store => next => action => {
     const result = next(action);
 
     switch (action.type) {
-    case TRACK_PERMISSION_ERROR:
+    case TRACK_CREATE_ERROR:
         // XXX We do not currently have user interface outside of a conference
         // which the user may tap and cause a permission-related error. If we
         // alert whenever we (intend to) ask for a permission, the scenario of
         // entering the WelcomePage, being asked for the camera permission, me
         // denying it, and being alerted that there is an error is overwhelming
         // me.
-        if (isRoomValid(store.getState()['features/base/conference'].room)) {
+        if (action.permissionDenied
+                && isRoomValid(
+                        store.getState()['features/base/conference'].room)) {
             _alertPermissionErrorWithSettings(action.trackType);
         }
         break;