From 6b3da4a4a4f3ba82453836bec54846c6cac5a0f2 Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Mon, 21 May 2018 22:46:52 -0500 Subject: [PATCH 1/9] redux 4.0.0, react-redux 5.0.7 --- package-lock.json | 21 ++++++++++++--------- package.json | 4 ++-- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index e702ec49a..d7b835923 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10556,13 +10556,13 @@ } }, "react-redux": { - "version": "5.0.6", - "resolved": "https://registry.npmjs.org/react-redux/-/react-redux-5.0.6.tgz", - "integrity": "sha512-8taaaGu+J7PMJQDJrk/xiWEYQmdo3mkXw6wPr3K3LxvXis3Fymiq7c13S+Tpls/AyNUAsoONkU81AP0RA6y6Vw==", + "version": "5.0.7", + "resolved": "https://registry.npmjs.org/react-redux/-/react-redux-5.0.7.tgz", + "integrity": "sha512-5VI8EV5hdgNgyjfmWzBbdrqUkrVRKlyTKk1sGH3jzM2M2Mhj/seQgPXaz6gVAj2lz/nz688AdTqMO18Lr24Zhg==", "requires": { "hoist-non-react-statics": "2.5.0", "invariant": "2.2.3", - "lodash": "4.17.4", + "lodash": "4.17.10", "lodash-es": "4.17.5", "loose-envify": "1.3.1", "prop-types": "15.6.0" @@ -10572,6 +10572,11 @@ "version": "2.5.0", "resolved": "https://registry.npmjs.org/hoist-non-react-statics/-/hoist-non-react-statics-2.5.0.tgz", "integrity": "sha512-6Bl6XsDT1ntE0lHbIhr4Kp2PGcleGZ66qu5Jqk8lc0Xc/IeG6gVLmwUGs/K0Us+L8VWoKgj0uWdPMataOsm31w==" + }, + "lodash": { + "version": "4.17.10", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.10.tgz", + "integrity": "sha512-UejweD1pDoXu+AD825lWwp4ZGtSwgnpZxb3JDViD7StjQz+Nb/6l093lx4OQ0foGWNRoc19mWy7BzL+UAK2iVg==" } } }, @@ -10764,12 +10769,10 @@ } }, "redux": { - "version": "3.7.2", - "resolved": "https://registry.npmjs.org/redux/-/redux-3.7.2.tgz", - "integrity": "sha512-pNqnf9q1hI5HHZRBkj3bAngGZW/JMCmexDlOxw4XagXY2o1327nHH54LoTjiPJ0gizoqPDRqWyX/00g0hD6w+A==", + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/redux/-/redux-4.0.0.tgz", + "integrity": "sha512-NnnHF0h0WVE/hXyrB6OlX67LYRuaf/rJcbWvnHHEPCF/Xa/AZpwhs/20WyqzQae5x4SD2F9nPObgBh2rxAgLiA==", "requires": { - "lodash": "4.17.4", - "lodash-es": "4.17.5", "loose-envify": "1.3.1", "symbol-observable": "1.2.0" } diff --git a/package.json b/package.json index bce3d1331..4d456ffb0 100644 --- a/package.json +++ b/package.json @@ -67,8 +67,8 @@ "react-native-sound": "0.10.9", "react-native-vector-icons": "4.4.2", "react-native-webrtc": "github:jitsi/react-native-webrtc#52fe4646401408e0569e972cabf08f3c21b7a107", - "react-redux": "5.0.6", - "redux": "3.7.2", + "react-redux": "5.0.7", + "redux": "4.0.0", "redux-thunk": "2.2.0", "strophe.js": "github:jitsi/strophejs#1.2.14-1", "strophejs-plugin-disco": "0.0.2", From fcca15c8271d43a787afb0c32c56945a8c9ece9a Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Mon, 21 May 2018 22:42:22 -0500 Subject: [PATCH 2/9] Coding style --- react/features/app/components/AbstractApp.js | 5 +- .../features/base/participants/middleware.js | 187 +++++++++--------- 2 files changed, 100 insertions(+), 92 deletions(-) diff --git a/react/features/app/components/AbstractApp.js b/react/features/app/components/AbstractApp.js index 353cacbc5..9b3ebb698 100644 --- a/react/features/app/components/AbstractApp.js +++ b/react/features/app/components/AbstractApp.js @@ -386,14 +386,13 @@ export class AbstractApp extends Component { * @returns {Store} - The redux store to be used by this * {@code AbstractApp}. */ - _maybeCreateStore(props) { + _maybeCreateStore({ store }) { // The application Jitsi Meet is architected with redux. However, I do // not want consumers of the App React Component to be forced into // dealing with redux. If the consumer did not provide an external redux // store, utilize an internal redux store. - let store = props.store; - if (typeof store === 'undefined') { + // eslint-disable-next-line no-param-reassign store = this._createStore(); // This is temporary workaround to be able to dispatch actions from diff --git a/react/features/base/participants/middleware.js b/react/features/base/participants/middleware.js index 6c4c64505..6198667cd 100644 --- a/react/features/base/participants/middleware.js +++ b/react/features/base/participants/middleware.js @@ -1,13 +1,9 @@ // @flow -import UIEvents from '../../../../service/UI/UIEvents'; - import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../../app'; -import { - CONFERENCE_WILL_JOIN, - CONFERENCE_LEFT -} from '../conference'; +import { CONFERENCE_LEFT, CONFERENCE_WILL_JOIN } from '../conference'; import { MiddlewareRegistry } from '../redux'; +import UIEvents from '../../../../service/UI/UIEvents'; import { playSound, registerSound, unregisterSound } from '../sounds'; import { @@ -43,17 +39,10 @@ declare var APP: Object; * Middleware that captures CONFERENCE_JOINED and CONFERENCE_LEFT actions and * updates respectively ID of local participant. * - * @param {Store} store - Redux store. + * @param {Store} store - The redux store. * @returns {Function} */ MiddlewareRegistry.register(store => next => action => { - const { conference } = store.getState()['features/base/conference']; - - if (action.type === PARTICIPANT_JOINED - || action.type === PARTICIPANT_LEFT) { - _maybePlaySounds(store, action); - } - switch (action.type) { case APP_WILL_MOUNT: _registerSounds(store); @@ -77,29 +66,34 @@ MiddlewareRegistry.register(store => next => action => { const participant = getLocalParticipant(store.getState()); if (participant) { - const local = participant.id === action.participant.id; + const { id } = action.participant; store.dispatch(participantUpdated({ - id: action.participant.id, - local, + id, + local: participant.id === id, raisedHand: false })); } - if (typeof APP === 'object') { - APP.UI.markDominantSpeaker(action.participant.id); - } + typeof APP === 'object' + && APP.UI.markDominantSpeaker(action.participant.id); break; } - case KICK_PARTICIPANT: + case KICK_PARTICIPANT: { + const { conference } = store.getState()['features/base/conference']; + conference.kickParticipant(action.id); break; + } + + case MUTE_REMOTE_PARTICIPANT: { + const { conference } = store.getState()['features/base/conference']; - case MUTE_REMOTE_PARTICIPANT: conference.muteParticipant(action.id); break; + } // TODO Remove this middleware when the local display name update flow is // fully brought into redux. @@ -116,63 +110,16 @@ MiddlewareRegistry.register(store => next => action => { } case PARTICIPANT_JOINED: - case PARTICIPANT_UPDATED: { - const { participant } = action; - const { id, local, raisedHand } = participant; + _maybePlaySounds(store, action); - // Send an external update of the local participant's raised hand state - // if a new raised hand state is defined in the action. - if (typeof raisedHand !== 'undefined') { - if (local) { - conference.setLocalParticipantProperty( - 'raisedHand', - raisedHand); - } - - if (typeof APP === 'object') { - if (local) { - APP.UI.onLocalRaiseHandChanged(raisedHand); - APP.UI.setLocalRaisedHandStatus(raisedHand); - } else { - const remoteParticipant - = getParticipantById(store.getState(), id); - - remoteParticipant - && APP.UI.setRaisedHandStatus( - remoteParticipant.id, - remoteParticipant.name, - raisedHand); - } - } - } - - // Notify external listeners of potential avatarURL changes. - if (typeof APP === 'object') { - const preUpdateAvatarURL - = getAvatarURLByParticipantId(store.getState(), id); - - // Allow the redux update to go through and compare the old avatar - // to the new avatar and emit out change events if necessary. - const result = next(action); - - const postUpdateAvatarURL - = getAvatarURLByParticipantId(store.getState(), id); - - if (preUpdateAvatarURL !== postUpdateAvatarURL) { - const currentKnownId = local - ? APP.conference.getMyUserId() : id; - - APP.UI.refreshAvatarDisplay( - currentKnownId, postUpdateAvatarURL); - APP.API.notifyAvatarChanged( - currentKnownId, postUpdateAvatarURL); - } - - return result; - } + return _participantJoinedOrUpdated(store, next, action); + case PARTICIPANT_LEFT: + _maybePlaySounds(store, action); break; - } + + case PARTICIPANT_UPDATED: + return _participantJoinedOrUpdated(store, next, action); } return next(action); @@ -182,7 +129,7 @@ MiddlewareRegistry.register(store => next => action => { * Initializes the local participant and signals that it joined. * * @private - * @param {Store} store - The Redux store. + * @param {Store} store - The redux store. * @param {Dispatch} next - The redux dispatch function to dispatch the * specified action to the specified store. * @param {Action} action - The redux action which is being dispatched @@ -208,8 +155,8 @@ function _localParticipantJoined({ getState, dispatch }, next, action) { /** * Plays sounds when participants join/leave conference. * - * @param {Store} store - The Redux store. - * @param {Action} action - The Redux action. Should be either + * @param {Store} store - The redux store. + * @param {Action} action - The redux action. Should be either * {@link PARTICIPANT_JOINED} or {@link PARTICIPANT_LEFT}. * @private * @returns {void} @@ -223,8 +170,8 @@ function _maybePlaySounds({ getState, dispatch }, action) { // The intention there was to not play user joined notification in big // conferences where 100th person is joining. if (!action.participant.local - && (!startAudioMuted - || getParticipantCount(state) < startAudioMuted)) { + && (!startAudioMuted + || getParticipantCount(state) < startAudioMuted)) { if (action.type === PARTICIPANT_JOINED) { dispatch(playSound(PARTICIPANT_JOINED_SOUND_ID)); } else if (action.type === PARTICIPANT_LEFT) { @@ -233,30 +180,92 @@ function _maybePlaySounds({ getState, dispatch }, action) { } } +/** + * Notifies the feature base/participants that the action + * {@code PARTICIPANT_JOINED} or {@code PARTICIPANT_UPDATED} is being dispatched + * within a specific redux store. + * + * @param {Store} store - The redux store in which the specified {@code action} + * is being dispatched. + * @param {Dispatch} next - The redux {@code dispatch} function to dispatch the + * specified {@code action} in the specified {@code store}. + * @param {Action} action - The redux action {@code PARTICIPANT_JOINED} or + * {@code PARTICIPANT_UPDATED} which is being dispatched in the specified + * {@code store}. + * @private + * @returns {Object} The value returned by {@code next(action)}. + */ +function _participantJoinedOrUpdated({ getState }, next, action) { + const { participant: { id, local, raisedHand } } = action; + + // Send an external update of the local participant's raised hand state + // if a new raised hand state is defined in the action. + if (typeof raisedHand !== 'undefined') { + if (local) { + const { conference } = getState()['features/base/conference']; + + conference.setLocalParticipantProperty('raisedHand', raisedHand); + } + + if (typeof APP === 'object') { + if (local) { + APP.UI.onLocalRaiseHandChanged(raisedHand); + APP.UI.setLocalRaisedHandStatus(raisedHand); + } else { + const remoteParticipant = getParticipantById(getState(), id); + + remoteParticipant + && APP.UI.setRaisedHandStatus( + remoteParticipant.id, + remoteParticipant.name, + raisedHand); + } + } + } + + // Notify external listeners of potential avatarURL changes. + if (typeof APP === 'object') { + const oldAvatarURL = getAvatarURLByParticipantId(getState(), id); + + // Allow the redux update to go through and compare the old avatar + // to the new avatar and emit out change events if necessary. + const result = next(action); + const newAvatarURL = getAvatarURLByParticipantId(getState(), id); + + if (oldAvatarURL !== newAvatarURL) { + const currentKnownId = local ? APP.conference.getMyUserId() : id; + + APP.UI.refreshAvatarDisplay(currentKnownId, newAvatarURL); + APP.API.notifyAvatarChanged(currentKnownId, newAvatarURL); + } + + return result; + } + + return next(action); +} + /** * Registers sounds related with the participants feature. * - * @param {Store} store - The Redux store. + * @param {Store} store - The redux store. * @private * @returns {void} */ function _registerSounds({ dispatch }) { dispatch( registerSound(PARTICIPANT_JOINED_SOUND_ID, PARTICIPANT_JOINED_FILE)); - dispatch( - registerSound(PARTICIPANT_LEFT_SOUND_ID, PARTICIPANT_LEFT_FILE)); + dispatch(registerSound(PARTICIPANT_LEFT_SOUND_ID, PARTICIPANT_LEFT_FILE)); } /** * Unregisters sounds related with the participants feature. * - * @param {Store} store - The Redux store. + * @param {Store} store - The redux store. * @private * @returns {void} */ function _unregisterSounds({ dispatch }) { - dispatch( - unregisterSound(PARTICIPANT_JOINED_SOUND_ID)); - dispatch( - unregisterSound(PARTICIPANT_LEFT_SOUND_ID)); + dispatch(unregisterSound(PARTICIPANT_JOINED_SOUND_ID)); + dispatch(unregisterSound(PARTICIPANT_LEFT_SOUND_ID)); } From 983e62ffe98dae24d724ecb314f896bbb7db488f Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Tue, 22 May 2018 14:30:51 -0500 Subject: [PATCH 3/9] Associate remote participant w/ JitsiConference (_JOINED) The plan set in motion here is to associate remote participants with the JitsiConference instances that created them in order to be able to remove remote participants when a JitsiConference is no longer the primary focus of the jitsi-meet app. And that's supposed to alleviate a problem with multiplying remote thumbnails. Doing all of the above in a single commit is a bit of a high order. So I'm splitting the whole into multiple successive commits for the purposes of observability, comprehension. Each commit is supposed to be safe even if subsequent commits are not accepted, are reverted, whatever. Obviously, without the successive commits, a commit may be "unused". One of the important pieces of the multiplying remote thumbnails "fix" offered is removing remote participants who are no longer of interest i.e. PARTICIPANT_LEFT. But in order for _LEFT to be implemented, _JOINED must be implemented first. --- conference.js | 1 + modules/UI/shared_video/SharedVideo.js | 1 + react/features/base/conference/actions.js | 1 + react/features/base/participants/actions.js | 13 +- .../features/base/participants/middleware.js | 13 +- react/features/base/participants/reducer.js | 148 ++++++++++-------- 6 files changed, 99 insertions(+), 78 deletions(-) diff --git a/conference.js b/conference.js index 9968946f5..76a729eaa 100644 --- a/conference.js +++ b/conference.js @@ -1688,6 +1688,7 @@ export default { const displayName = user.getDisplayName(); APP.store.dispatch(participantJoined({ + conference: room, id, name: displayName, role: user.getRole() diff --git a/modules/UI/shared_video/SharedVideo.js b/modules/UI/shared_video/SharedVideo.js index 34c4f797e..c29e23015 100644 --- a/modules/UI/shared_video/SharedVideo.js +++ b/modules/UI/shared_video/SharedVideo.js @@ -306,6 +306,7 @@ export default class SharedVideoManager { SHARED_VIDEO_CONTAINER_TYPE, self.sharedVideo); APP.store.dispatch(participantJoined({ + conference: APP.conference, id: self.url, isBot: true, name: 'YouTube' diff --git a/react/features/base/conference/actions.js b/react/features/base/conference/actions.js index c2b56573d..5cae20a97 100644 --- a/react/features/base/conference/actions.js +++ b/react/features/base/conference/actions.js @@ -140,6 +140,7 @@ function _addConferenceListeners(conference, dispatch) { conference.on( JitsiConferenceEvents.USER_JOINED, (id, user) => dispatch(participantJoined({ + conference, id, name: user.getDisplayName(), role: user.getRole() diff --git a/react/features/base/participants/actions.js b/react/features/base/participants/actions.js index f3bbe98f0..a67f4ca20 100644 --- a/react/features/base/participants/actions.js +++ b/react/features/base/participants/actions.js @@ -214,11 +214,16 @@ export function participantDisplayNameChanged(id, displayName = '') { * * @param {Participant} participant - Information about participant. * @returns {{ - * type: PARTICIPANT_JOINED, - * participant: Participant + * type: PARTICIPANT_JOINED, + * participant: Participant * }} */ export function participantJoined(participant) { + if (!participant.local && !participant.conference) { + throw Error( + 'A remote participant must be associated with a JitsiConference!'); + } + return { type: PARTICIPANT_JOINED, participant @@ -393,7 +398,5 @@ export function showParticipantJoinedNotification(displayName) { joinedParticipantsNames.push( displayName || interfaceConfig.DEFAULT_REMOTE_DISPLAY_NAME); - return dispatch => { - _throttledNotifyParticipantConnected(dispatch); - }; + return dispatch => _throttledNotifyParticipantConnected(dispatch); } diff --git a/react/features/base/participants/middleware.js b/react/features/base/participants/middleware.js index 6198667cd..61735812d 100644 --- a/react/features/base/participants/middleware.js +++ b/react/features/base/participants/middleware.js @@ -139,15 +139,15 @@ MiddlewareRegistry.register(store => next => action => { */ function _localParticipantJoined({ getState, dispatch }, next, action) { const result = next(action); + const settings = getState()['features/base/settings']; - const localParticipant = { + + dispatch(localParticipantJoined({ avatarID: settings.avatarID, avatarURL: settings.avatarURL, email: settings.email, name: settings.displayName - }; - - dispatch(localParticipantJoined(localParticipant)); + })); return result; } @@ -204,7 +204,10 @@ function _participantJoinedOrUpdated({ getState }, next, action) { if (local) { const { conference } = getState()['features/base/conference']; - conference.setLocalParticipantProperty('raisedHand', raisedHand); + conference + && conference.setLocalParticipantProperty( + 'raisedHand', + raisedHand); } if (typeof APP === 'object') { diff --git a/react/features/base/participants/reducer.js b/react/features/base/participants/reducer.js index aa40e4449..5af3dc33a 100644 --- a/react/features/base/participants/reducer.js +++ b/react/features/base/participants/reducer.js @@ -39,6 +39,35 @@ declare var APP: Object; const PARTICIPANT_PROPS_TO_OMIT_WHEN_UPDATE = [ 'dominantSpeaker', 'id', 'local', 'pinned' ]; +/** + * Listen for actions which add, remove, or update the set of participants in + * the conference. + * + * @param {Participant[]} state - List of participants to be modified. + * @param {Object} action - Action object. + * @param {string} action.type - Type of action. + * @param {Participant} action.participant - Information about participant to be + * added/removed/modified. + * @returns {Participant[]} + */ +ReducerRegistry.register('features/base/participants', (state = [], action) => { + switch (action.type) { + case DOMINANT_SPEAKER_CHANGED: + case PARTICIPANT_ID_CHANGED: + case PARTICIPANT_UPDATED: + case PIN_PARTICIPANT: + return state.map(p => _participant(p, action)); + + case PARTICIPANT_JOINED: + return [ ...state, _participantJoined(action) ]; + + case PARTICIPANT_LEFT: + return state.filter(p => p.id !== action.participant.id); + } + + return state; +}); + /** * Reducer function for a single participant. * @@ -67,52 +96,6 @@ function _participant(state: Object = {}, action) { } break; - case PARTICIPANT_JOINED: { - const { participant } = action; // eslint-disable-line no-shadow - const { - avatarURL, - connectionStatus, - dominantSpeaker, - email, - isBot, - local, - name, - pinned, - role - } = participant; - let { avatarID, id } = participant; - - // avatarID - // - // TODO Get the avatarID of the local participant from localStorage. - if (!avatarID && local) { - avatarID = randomHexString(32); - } - - // id - // - // XXX The situation of not having an ID for a remote participant should - // not happen. Maybe we should raise an error in this case or generate a - // random ID. - if (!id && local) { - id = LOCAL_PARTICIPANT_DEFAULT_ID; - } - - return { - avatarID, - avatarURL, - connectionStatus, - dominantSpeaker: dominantSpeaker || false, - email, - id, - isBot, - local: local || false, - name, - pinned: pinned || false, - role: role || PARTICIPANT_ROLE.NONE - }; - } - case PARTICIPANT_UPDATED: { const { participant } = action; // eslint-disable-line no-shadow let { id } = participant; @@ -147,31 +130,60 @@ function _participant(state: Object = {}, action) { } /** - * Listen for actions which add, remove, or update the set of participants in - * the conference. + * Reduces a specific redux action of type {@link PARTICIPANT_JOINED} in the + * feature base/participants. * - * @param {Participant[]} state - List of participants to be modified. - * @param {Object} action - Action object. - * @param {string} action.type - Type of action. - * @param {Participant} action.participant - Information about participant to be - * added/removed/modified. - * @returns {Participant[]} + * @param {Action} action - The redux action of type {@code PARTICIPANT_JOINED} + * to reduce. + * @private + * @returns {Object} The new participant derived from the payload of the + * specified {@code action} to be added into the redux state of the feature + * base/participants after the reduction of the specified + * {@code action}. */ -ReducerRegistry.register('features/base/participants', (state = [], action) => { - switch (action.type) { - case DOMINANT_SPEAKER_CHANGED: - case PARTICIPANT_ID_CHANGED: - case PARTICIPANT_UPDATED: - case PIN_PARTICIPANT: - return state.map(p => _participant(p, action)); +function _participantJoined({ participant }) { + const { + avatarURL, + connectionStatus, + dominantSpeaker, + email, + isBot, + local, + name, + pinned, + role + } = participant; + let { avatarID, conference, id } = participant; - case PARTICIPANT_JOINED: - return [ ...state, _participant(undefined, action) ]; + if (local) { + // avatarID + // + // TODO Get the avatarID of the local participant from localStorage. + avatarID || (avatarID = randomHexString(32)); - case PARTICIPANT_LEFT: - return state.filter(p => p.id !== action.participant.id); + // conference + // + // XXX The local participant is not identified in association with a + // JitsiConference because it is identified by the very fact that it is + // the local participant. + conference = undefined; - default: - return state; + // id + id || (id = LOCAL_PARTICIPANT_DEFAULT_ID); } -}); + + return { + avatarID, + avatarURL, + conference, + connectionStatus, + dominantSpeaker: dominantSpeaker || false, + email, + id, + isBot, + local: local || false, + name, + pinned: pinned || false, + role: role || PARTICIPANT_ROLE.NONE + }; +} From 37cd5bb5b9a61d0d15f60ba0c7bec03db8a50330 Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Tue, 22 May 2018 16:08:35 -0500 Subject: [PATCH 4/9] Associate remote participant w/ JitsiConference (_LEFT) The commit message of "Associate remote participant w/ JitsiConference (_JOINED)" explains the motivation for this commit. --- conference.js | 2 +- modules/UI/shared_video/SharedVideo.js | 2 +- react/features/base/conference/actions.js | 2 +- react/features/base/participants/actions.js | 22 +++++++++++++++++++-- react/features/base/participants/reducer.js | 16 +++++++++++++-- 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/conference.js b/conference.js index 76a729eaa..f30cf0ff5 100644 --- a/conference.js +++ b/conference.js @@ -1710,7 +1710,7 @@ export default { if (user.isHidden()) { return; } - APP.store.dispatch(participantLeft(id, user)); + APP.store.dispatch(participantLeft(id, room)); logger.log('USER %s LEFT', id, user); APP.API.notifyUserLeft(id); APP.UI.removeUser(id, user.getDisplayName()); diff --git a/modules/UI/shared_video/SharedVideo.js b/modules/UI/shared_video/SharedVideo.js index c29e23015..c43bcc216 100644 --- a/modules/UI/shared_video/SharedVideo.js +++ b/modules/UI/shared_video/SharedVideo.js @@ -517,7 +517,7 @@ export default class SharedVideoManager { UIEvents.UPDATE_SHARED_VIDEO, null, 'removed'); }); - APP.store.dispatch(participantLeft(this.url)); + APP.store.dispatch(participantLeft(this.url, APP.conference)); this.url = null; this.isSharedVideoShown = false; diff --git a/react/features/base/conference/actions.js b/react/features/base/conference/actions.js index 5cae20a97..b07714fac 100644 --- a/react/features/base/conference/actions.js +++ b/react/features/base/conference/actions.js @@ -147,7 +147,7 @@ function _addConferenceListeners(conference, dispatch) { }))); conference.on( JitsiConferenceEvents.USER_LEFT, - (...args) => dispatch(participantLeft(...args))); + id => dispatch(participantLeft(id, conference))); conference.on( JitsiConferenceEvents.USER_ROLE_CHANGED, (...args) => dispatch(participantRoleChanged(...args))); diff --git a/react/features/base/participants/actions.js b/react/features/base/participants/actions.js index a67f4ca20..b08cafeb9 100644 --- a/react/features/base/participants/actions.js +++ b/react/features/base/participants/actions.js @@ -123,7 +123,19 @@ export function localParticipantLeft() { const participant = getLocalParticipant(getState); if (participant) { - return dispatch(participantLeft(participant.id)); + return ( + dispatch( + participantLeft( + participant.id, + + // XXX Only the local participant is allowed to leave + // without stating the JitsiConference instance because + // the local participant is uniquely identified by the + // very fact that there is only one local participant + // (and the fact that the local participant "joins" at + // the beginning of the app and "leaves" at the end of + // the app). + undefined))); } }; } @@ -234,17 +246,23 @@ export function participantJoined(participant) { * Action to signal that a participant has left. * * @param {string} id - Participant's ID. + * @param {JitsiConference} conference - The {@code JitsiConference} associated + * with the participant identified by the specified {@code id}. Only the local + * participant is allowed to not specify an associated {@code JitsiConference} + * instance. * @returns {{ * type: PARTICIPANT_LEFT, * participant: { + * conference: JitsiConference, * id: string * } * }} */ -export function participantLeft(id) { +export function participantLeft(id, conference) { return { type: PARTICIPANT_LEFT, participant: { + conference, id } }; diff --git a/react/features/base/participants/reducer.js b/react/features/base/participants/reducer.js index 5af3dc33a..260432e52 100644 --- a/react/features/base/participants/reducer.js +++ b/react/features/base/participants/reducer.js @@ -61,8 +61,20 @@ ReducerRegistry.register('features/base/participants', (state = [], action) => { case PARTICIPANT_JOINED: return [ ...state, _participantJoined(action) ]; - case PARTICIPANT_LEFT: - return state.filter(p => p.id !== action.participant.id); + case PARTICIPANT_LEFT: { + // XXX A remote participant is uniquely identified by their id in a + // specific JitsiConference instance. The local participant is uniquely + // identified by the very fact that there is only one local participant + // (and the fact that the local participant "joins" at the beginning of + // the app and "leaves" at the end of the app). + const { conference, id } = action.participant; + + return state.filter(p => + !( + p.id === id + && (p.local + || (conference && p.conference === conference)))); + } } return state; From 771d60f95416a37ebde63dcafc685b086335cac2 Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Tue, 22 May 2018 16:47:43 -0500 Subject: [PATCH 5/9] Associate remote participant w/ JitsiConference (_UPDATED) The commit message of "Associate remote participant w/ JitsiConference (_JOINED)" explains the motivation for this commit. Practically, _JOINED and _LEFT combined with "Remove remote participants who are no longer of interest" should alleviate the problem with multiplying remote participants to an acceptable level of annoyance. Technically though, a remote participant cannot be identified by an ID only. The ID is (somewhat) "unique" in the context of a single JitsiConference instance. So in order to not have to scratch our heads over an obscure corner, racing case, it's better to always identify remote participants by the pair id-conference. Unfortunately, that's a bit of a high order given the existing source code. So I've implemented the cases which are the easiest so that new source code written with participantUpdated is more likely to identify a remote participant with the pair id-conference. Additionally, the commit "Reduce direct read access to the features/base/participants redux state" brings more control back to the functions of the feature base/participants so that one day we can (if we choose to) do something like, for example: If getParticipants is called with a conference, it returns the participants from features/base/participants who are associated with the specified conference. If no conference is specified in the function call, then default to the conference which is the primary focus of the app at the time of the function call. Added to the above, this should allow us to further reduce the cases in which we're identifying remote participants by id only and get us even closer to a more "predictable" behavior in corner, racing cases. --- conference.js | 29 +++++++++++++++++-- react/features/base/conference/actions.js | 6 +++- react/features/base/jwt/middleware.js | 10 +++++-- react/features/base/participants/actions.js | 8 ++++- react/features/base/participants/functions.js | 4 +-- .../features/base/participants/middleware.js | 13 ++++----- react/features/base/participants/reducer.js | 20 ++++++++++--- .../toolbox/components/web/Toolbox.js | 6 ++++ 8 files changed, 75 insertions(+), 21 deletions(-) diff --git a/conference.js b/conference.js index f30cf0ff5..d5ac0a340 100644 --- a/conference.js +++ b/conference.js @@ -1804,9 +1804,9 @@ export default { APP.UI.participantConnectionStatusChanged(id); }); - room.on(JitsiConferenceEvents.DOMINANT_SPEAKER_CHANGED, id => { - APP.store.dispatch(dominantSpeakerChanged(id)); - }); + room.on( + JitsiConferenceEvents.DOMINANT_SPEAKER_CHANGED, + id => APP.store.dispatch(dominantSpeakerChanged(id, room))); if (!interfaceConfig.filmStripOnly) { room.on(JitsiConferenceEvents.CONNECTION_INTERRUPTED, () => { @@ -1883,6 +1883,7 @@ export default { = displayName.substr(0, MAX_DISPLAY_NAME_LENGTH); APP.store.dispatch(participantUpdated({ + conference: room, id, name: formattedDisplayName })); @@ -1916,6 +1917,7 @@ export default { switch (name) { case 'raisedHand': APP.store.dispatch(participantUpdated({ + conference: room, id: participant.getId(), raisedHand: newValue === 'true' })); @@ -2015,6 +2017,7 @@ export default { APP.UI.addListener(UIEvents.EMAIL_CHANGED, this.changeLocalEmail); room.addCommandListener(this.commands.defaults.EMAIL, (data, from) => { APP.store.dispatch(participantUpdated({ + conference: room, id: from, email: data.value })); @@ -2026,6 +2029,7 @@ export default { (data, from) => { APP.store.dispatch( participantUpdated({ + conference: room, id: from, avatarURL: data.value })); @@ -2035,6 +2039,7 @@ export default { (data, from) => { APP.store.dispatch( participantUpdated({ + conference: room, id: from, avatarID: data.value })); @@ -2579,6 +2584,12 @@ export default { const localId = localParticipant.id; APP.store.dispatch(participantUpdated({ + // XXX Only the local participant is allowed to update without + // stating the JitsiConference instance (i.e. participant property + // `conference` for a remote participant) because the local + // participant is uniquely identified by the very fact that there is + // only one local participant. + id: localId, local: true, email: formattedEmail @@ -2606,6 +2617,12 @@ export default { } APP.store.dispatch(participantUpdated({ + // XXX Only the local participant is allowed to update without + // stating the JitsiConference instance (i.e. participant property + // `conference` for a remote participant) because the local + // participant is uniquely identified by the very fact that there is + // only one local participant. + id, local: true, avatarURL: formattedUrl @@ -2662,6 +2679,12 @@ export default { } APP.store.dispatch(participantUpdated({ + // XXX Only the local participant is allowed to update without + // stating the JitsiConference instance (i.e. participant property + // `conference` for a remote participant) because the local + // participant is uniquely identified by the very fact that there is + // only one local participant. + id, local: true, name: formattedNickname diff --git a/react/features/base/conference/actions.js b/react/features/base/conference/actions.js index b07714fac..8c18b7f1d 100644 --- a/react/features/base/conference/actions.js +++ b/react/features/base/conference/actions.js @@ -125,13 +125,14 @@ function _addConferenceListeners(conference, dispatch) { conference.on( JitsiConferenceEvents.DISPLAY_NAME_CHANGED, (id, displayName) => dispatch(participantUpdated({ + conference, id, name: displayName.substr(0, MAX_DISPLAY_NAME_LENGTH) }))); conference.on( JitsiConferenceEvents.DOMINANT_SPEAKER_CHANGED, - (...args) => dispatch(dominantSpeakerChanged(...args))); + id => dispatch(dominantSpeakerChanged(id, conference))); conference.on( JitsiConferenceEvents.PARTICIPANT_CONN_STATUS_CHANGED, @@ -155,18 +156,21 @@ function _addConferenceListeners(conference, dispatch) { conference.addCommandListener( AVATAR_ID_COMMAND, (data, id) => dispatch(participantUpdated({ + conference, id, avatarID: data.value }))); conference.addCommandListener( AVATAR_URL_COMMAND, (data, id) => dispatch(participantUpdated({ + conference, id, avatarURL: data.value }))); conference.addCommandListener( EMAIL_COMMAND, (data, id) => dispatch(participantUpdated({ + conference, id, email: data.value }))); diff --git a/react/features/base/jwt/middleware.js b/react/features/base/jwt/middleware.js index 75664a74d..ee1067741 100644 --- a/react/features/base/jwt/middleware.js +++ b/react/features/base/jwt/middleware.js @@ -143,7 +143,10 @@ function _overwriteLocalParticipant( if ((avatarURL || email || name) && (localParticipant = getLocalParticipant(getState))) { - const newProperties: Object = { id: localParticipant.id }; + const newProperties: Object = { + id: localParticipant.id, + local: true + }; if (avatarURL) { newProperties.avatarURL = avatarURL; @@ -264,7 +267,10 @@ function _undoOverwriteLocalParticipant( if ((avatarURL || name || email) && (localParticipant = getLocalParticipant(getState))) { - const newProperties: Object = { id: localParticipant.id }; + const newProperties: Object = { + id: localParticipant.id, + local: true + }; if (avatarURL === localParticipant.avatarURL) { newProperties.avatarURL = undefined; diff --git a/react/features/base/participants/actions.js b/react/features/base/participants/actions.js index b08cafeb9..59575e306 100644 --- a/react/features/base/participants/actions.js +++ b/react/features/base/participants/actions.js @@ -22,17 +22,23 @@ import { getLocalParticipant } from './functions'; * Create an action for when dominant speaker changes. * * @param {string} id - Participant's ID. + * @param {JitsiConference} conference - The {@code JitsiConference} associated + * with the participant identified by the specified {@code id}. Only the local + * participant is allowed to not specify an associated {@code JitsiConference} + * instance. * @returns {{ * type: DOMINANT_SPEAKER_CHANGED, * participant: { + * conference: JitsiConference, * id: string * } * }} */ -export function dominantSpeakerChanged(id) { +export function dominantSpeakerChanged(id, conference) { return { type: DOMINANT_SPEAKER_CHANGED, participant: { + conference, id } }; diff --git a/react/features/base/participants/functions.js b/react/features/base/participants/functions.js index 64957fca3..33fdda0c4 100644 --- a/react/features/base/participants/functions.js +++ b/react/features/base/participants/functions.js @@ -127,9 +127,7 @@ export function getLocalParticipant(stateful: Object | Function) { * @private * @returns {(Participant|undefined)} */ -export function getParticipantById( - stateful: Object | Function, - id: string) { +export function getParticipantById(stateful: Object | Function, id: string) { const participants = _getAllParticipants(stateful); return participants.find(p => p.id === id); diff --git a/react/features/base/participants/middleware.js b/react/features/base/participants/middleware.js index 61735812d..a4c99faf0 100644 --- a/react/features/base/participants/middleware.js +++ b/react/features/base/participants/middleware.js @@ -63,20 +63,19 @@ MiddlewareRegistry.register(store => next => action => { case DOMINANT_SPEAKER_CHANGED: { // Ensure the raised hand state is cleared for the dominant speaker. + + const { conference, id } = action.participant; const participant = getLocalParticipant(store.getState()); - if (participant) { - const { id } = action.participant; - - store.dispatch(participantUpdated({ + participant + && store.dispatch(participantUpdated({ + conference, id, local: participant.id === id, raisedHand: false })); - } - typeof APP === 'object' - && APP.UI.markDominantSpeaker(action.participant.id); + typeof APP === 'object' && APP.UI.markDominantSpeaker(id); break; } diff --git a/react/features/base/participants/reducer.js b/react/features/base/participants/reducer.js index 260432e52..20d86aadf 100644 --- a/react/features/base/participants/reducer.js +++ b/react/features/base/participants/reducer.js @@ -32,12 +32,24 @@ import { LOCAL_PARTICIPANT_DEFAULT_ID, PARTICIPANT_ROLE } from './constants'; declare var APP: Object; /** - * These properties should not be bulk assigned when updating a particular - * @see Participant. + * The participant properties which cannot be updated through + * {@link PARTICIPANT_UPDATED}. They either identify the participant or can only + * be modified through property-dedicated actions. + * * @type {string[]} */ -const PARTICIPANT_PROPS_TO_OMIT_WHEN_UPDATE - = [ 'dominantSpeaker', 'id', 'local', 'pinned' ]; +const PARTICIPANT_PROPS_TO_OMIT_WHEN_UPDATE = [ + + // The following properties identify the participant: + 'conference', + 'id', + 'local', + + // The following properties can only be modified through property-dedicated + // actions: + 'dominantSpeaker', + 'pinned' +]; /** * Listen for actions which add, remove, or update the set of participants in diff --git a/react/features/toolbox/components/web/Toolbox.js b/react/features/toolbox/components/web/Toolbox.js index d418b3076..d04cbe749 100644 --- a/react/features/toolbox/components/web/Toolbox.js +++ b/react/features/toolbox/components/web/Toolbox.js @@ -505,6 +505,12 @@ class Toolbox extends Component { const { _localParticipantID, _raisedHand } = this.props; this.props.dispatch(participantUpdated({ + // XXX Only the local participant is allowed to update without + // stating the JitsiConference instance (i.e. participant property + // `conference` for a remote participant) because the local + // participant is uniquely identified by the very fact that there is + // only one local participant. + id: _localParticipantID, local: true, raisedHand: !_raisedHand From 8cd2bd272be96f96303027328d888ca24a6e201c Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Tue, 22 May 2018 17:41:53 -0500 Subject: [PATCH 6/9] Reduce direct read access to the features/base/participants redux state As part of the work on fixing the problem with the multiplying thumbnails, we've associated remote participant w/ JitsiConference. However, there are periods of time when multiple JitsiConferences are in the redux state (and that period is going to be shorted by StateListenerRegistry). In order to give more control to the feature base/participants, reduce the occurrences of direct access to the features/base/participants redux state and utilize the feature's existing read access functions. Which will allow us in the future to enhance these functions to access participants which are relevant to the current conference of interest to the user only. --- .../components/ParticipantView.native.js | 5 +---- react/features/base/participants/functions.js | 11 ++++------- .../conference/components/Conference.native.js | 4 ++-- react/features/filmstrip/functions.js | 10 ++++++---- .../invite/components/InfoDialogButton.web.js | 8 ++++---- react/features/invite/middleware.any.js | 14 ++++++-------- .../presence-status/components/PresenceLabel.js | 4 +--- .../components/RemoteControlAuthorizationDialog.js | 5 +---- .../components/WelcomePageSideBar.native.js | 6 +++--- 9 files changed, 28 insertions(+), 39 deletions(-) diff --git a/react/features/base/participants/components/ParticipantView.native.js b/react/features/base/participants/components/ParticipantView.native.js index 3e9394b0c..21908aa5d 100644 --- a/react/features/base/participants/components/ParticipantView.native.js +++ b/react/features/base/participants/components/ParticipantView.native.js @@ -313,10 +313,7 @@ function _toBoolean(value, undefinedValue) { */ function _mapStateToProps(state, ownProps) { const { participantId } = ownProps; - const participant - = getParticipantById( - state['features/base/participants'], - participantId); + const participant = getParticipantById(state, participantId); let avatar; let connectionStatus; let participantName; diff --git a/react/features/base/participants/functions.js b/react/features/base/participants/functions.js index 33fdda0c4..f602aa2a1 100644 --- a/react/features/base/participants/functions.js +++ b/react/features/base/participants/functions.js @@ -240,11 +240,8 @@ export function isLocalParticipantModerator(stateful: Object | Function) { return false; } - const isModerator = localParticipant.role === PARTICIPANT_ROLE.MODERATOR; - - if (state['features/base/config'].enableUserRolesBasedOnToken) { - return isModerator && !state['features/base/jwt'].isGuest; - } - - return isModerator; + return ( + localParticipant.role === PARTICIPANT_ROLE.MODERATOR + && (!state['features/base/config'].enableUserRolesBasedOnToken + || !state['features/base/jwt'].isGuest)); } diff --git a/react/features/conference/components/Conference.native.js b/react/features/conference/components/Conference.native.js index c26b90d2b..00f3d2538 100644 --- a/react/features/conference/components/Conference.native.js +++ b/react/features/conference/components/Conference.native.js @@ -10,6 +10,7 @@ import { appNavigate } from '../../app'; import { connect, disconnect } from '../../base/connection'; import { DialogContainer } from '../../base/dialog'; import { CalleeInfoContainer } from '../../base/jwt'; +import { getParticipantCount } from '../../base/participants'; import { Container, LoadingIndicator, TintedView } from '../../base/react'; import { TestConnectionInfo } from '../../base/testing'; import { createDesiredLocalTracks } from '../../base/tracks'; @@ -383,7 +384,6 @@ function _mapStateToProps(state) { const { connecting, connection } = state['features/base/connection']; const { conference, joining, leaving } = state['features/base/conference']; const { reducedUI } = state['features/base/responsive-ui']; - const participants = state['features/base/participants']; // XXX There is a window of time between the successful establishment of the // XMPP connection and the subsequent commencement of joining the MUC during @@ -415,7 +415,7 @@ function _mapStateToProps(state) { * @private * @type {number} */ - _participantCount: participants.length, + _participantCount: getParticipantCount(state), /** * The indicator which determines whether the UI is reduced (to diff --git a/react/features/filmstrip/functions.js b/react/features/filmstrip/functions.js index 8656e4d69..d250f9cad 100644 --- a/react/features/filmstrip/functions.js +++ b/react/features/filmstrip/functions.js @@ -1,6 +1,9 @@ // @flow -import { getPinnedParticipant } from '../base/participants'; +import { + getParticipantCount, + getPinnedParticipant +} from '../base/participants'; declare var interfaceConfig: Object; @@ -13,8 +16,7 @@ declare var interfaceConfig: Object; * in the filmstrip, then {@code true}; otherwise, {@code false}. */ export function shouldRemoteVideosBeVisible(state: Object) { - const participants = state['features/base/participants']; - const participantCount = participants.length; + const participantCount = getParticipantCount(state); let pinnedParticipant; return Boolean( @@ -26,7 +28,7 @@ export function shouldRemoteVideosBeVisible(state: Object) { || (participantCount > 1 && (state['features/filmstrip'].hovered || state['features/toolbox'].visible - || ((pinnedParticipant = getPinnedParticipant(participants)) + || ((pinnedParticipant = getPinnedParticipant(state)) && pinnedParticipant.local))) || (typeof interfaceConfig === 'object' diff --git a/react/features/invite/components/InfoDialogButton.web.js b/react/features/invite/components/InfoDialogButton.web.js index 30c1f785a..0df0acb4b 100644 --- a/react/features/invite/components/InfoDialogButton.web.js +++ b/react/features/invite/components/InfoDialogButton.web.js @@ -236,10 +236,10 @@ function _mapStateToProps(state) { return { _dialIn: state['features/invite'], _disableAutoShow: state['features/base/config'].iAmRecorder, - _liveStreamViewURL: currentLiveStreamingSession - && currentLiveStreamingSession.liveStreamViewURL, - _participantCount: - getParticipantCount(state['features/base/participants']), + _liveStreamViewURL: + currentLiveStreamingSession + && currentLiveStreamingSession.liveStreamViewURL, + _participantCount: getParticipantCount(state), _toolboxVisible: state['features/toolbox'].visible }; } diff --git a/react/features/invite/middleware.any.js b/react/features/invite/middleware.any.js index a83061ca9..d51c3b3da 100644 --- a/react/features/invite/middleware.any.js +++ b/react/features/invite/middleware.any.js @@ -118,15 +118,13 @@ MiddlewareRegistry.register(store => next => action => { * @returns {string} - The presence status. */ function _getParticipantPresence(state, id) { - if (!id) { - return undefined; - } - const participants = state['features/base/participants']; - const participantById = getParticipantById(participants, id); + if (id) { + const participantById = getParticipantById(state, id); - if (!participantById) { - return undefined; + if (participantById) { + return participantById.presence; + } } - return participantById.presence; + return undefined; } diff --git a/react/features/presence-status/components/PresenceLabel.js b/react/features/presence-status/components/PresenceLabel.js index 5282da3b1..0140d2079 100644 --- a/react/features/presence-status/components/PresenceLabel.js +++ b/react/features/presence-status/components/PresenceLabel.js @@ -99,9 +99,7 @@ class PresenceLabel extends Component { * }} */ function _mapStateToProps(state, ownProps) { - const participant - = getParticipantById( - state['features/base/participants'], ownProps.participantID); + const participant = getParticipantById(state, ownProps.participantID); return { _presence: participant && participant.presence diff --git a/react/features/remote-control/components/RemoteControlAuthorizationDialog.js b/react/features/remote-control/components/RemoteControlAuthorizationDialog.js index 8bda33bca..77e5b39b7 100644 --- a/react/features/remote-control/components/RemoteControlAuthorizationDialog.js +++ b/react/features/remote-control/components/RemoteControlAuthorizationDialog.js @@ -159,10 +159,7 @@ class RemoteControlAuthorizationDialog extends Component<*> { */ function _mapStateToProps(state, ownProps) { const { _displayName, participantId } = ownProps; - const participant - = getParticipantById( - state['features/base/participants'], - participantId); + const participant = getParticipantById(state, participantId); return { _displayName: participant ? participant.name : _displayName diff --git a/react/features/welcome/components/WelcomePageSideBar.native.js b/react/features/welcome/components/WelcomePageSideBar.native.js index be75282c5..c76a10ef3 100644 --- a/react/features/welcome/components/WelcomePageSideBar.native.js +++ b/react/features/welcome/components/WelcomePageSideBar.native.js @@ -156,11 +156,11 @@ class WelcomePageSideBar extends Component { * @returns {Object} */ function _mapStateToProps(state: Object) { - const _localParticipant = getLocalParticipant(state); + const localParticipant = getLocalParticipant(state); return { - _avatar: getAvatarURL(_localParticipant), - _displayName: getParticipantDisplayName(state, _localParticipant.id), + _avatar: getAvatarURL(localParticipant), + _displayName: getParticipantDisplayName(state, localParticipant.id), _visible: state['features/welcome'].sideBarVisible }; } From db21e97c1959075549aab9de9c620aadb09b6e3d Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Mon, 21 May 2018 22:44:00 -0500 Subject: [PATCH 7/9] StateListenerRegistry "Middleware" redux state changes, not actions. --- react/features/app/components/AbstractApp.js | 9 +- .../base/redux/StateListenerRegistry.js | 159 ++++++++++++++++++ react/features/base/redux/index.js | 1 + 3 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 react/features/base/redux/StateListenerRegistry.js diff --git a/react/features/app/components/AbstractApp.js b/react/features/app/components/AbstractApp.js index 9b3ebb698..af7991cbb 100644 --- a/react/features/app/components/AbstractApp.js +++ b/react/features/app/components/AbstractApp.js @@ -10,7 +10,11 @@ import Thunk from 'redux-thunk'; import { i18next } from '../../base/i18n'; import { localParticipantLeft } from '../../base/participants'; import { Fragment, RouteRegistry } from '../../base/react'; -import { MiddlewareRegistry, ReducerRegistry } from '../../base/redux'; +import { + MiddlewareRegistry, + ReducerRegistry, + StateListenerRegistry +} from '../../base/redux'; import { SoundCollection } from '../../base/sounds'; import { PersistenceRegistry } from '../../base/storage'; import { toURLString } from '../../base/util'; @@ -404,6 +408,9 @@ export class AbstractApp extends Component { } } + // StateListenerRegistry + store && StateListenerRegistry.subscribe(store); + return store; } diff --git a/react/features/base/redux/StateListenerRegistry.js b/react/features/base/redux/StateListenerRegistry.js new file mode 100644 index 000000000..7bfa273eb --- /dev/null +++ b/react/features/base/redux/StateListenerRegistry.js @@ -0,0 +1,159 @@ +// @flow + +const logger = require('jitsi-meet-logger').getLogger(__filename); + +/** + * The type listener supported for registration with + * {@link StateListenerRegistry} in association with a {@link Selector}. + * + * @param {any} selection - The value derived from the redux store/state by the + * associated {@code Selector}. Immutable! + * @param {Store} store - The redux store. Provided in case the {@code Listener} + * needs to {@code dispatch} or {@code getState}. The latter is advisable only + * if the {@code Listener} is not to respond to changes to that state. + * @param {any} prevSelection - The value previously derived from the redux + * store/state by the associated {@code Selector}. The {@code Listener} is + * invoked only if {@code prevSelection} and {@code selection} are different. + * Immutable! + */ +type Listener = (selection: any, store: Store, prevSelection: any) => void; + +/** + * The type selector supported for registration with + * {@link StateListenerRegistry} in association with a {@link Listener}. + * + * @param {Object} state - The redux state from which the {@code Selector} is to + * derive data. + * @param {any} prevSelection - The value previously derived from the redux + * store/state by the {@code Selector}. Provided in case the {@code Selector} + * needs to derive the returned value from the specified {@code state} and + * {@code prevSelection}. Immutable! + * @returns {any} The value derived from the specified {@code state} and/or + * {@code prevSelection}. The associated {@code Listener} will only be invoked + * if the returned value is other than {@code prevSelection}. + */ +type Selector = (state: Object, prevSelection: any) => any; + +/** + * A type of a {@link Selector}-{@link Listener} association in which the + * {@code Listener} listens to changes in the values derived from a redux + * store/state by the {@code Selector}. + */ +type SelectorListener = { + + /** + * The {@code Listener} which listens to changes in the values selected by + * {@link selector}. + */ + listener: Listener, + + /** + * The {@code Selector} which selects values whose changes are listened to + * by {@link listener}. + */ + selector: Selector +}; + +/** + * A registry listeners which listen to changes in a redux store/state. + */ +class StateListenerRegistry { + /** + * The {@link Listener}s registered with this {@code StateListenerRegistry} + * to be notified when the values derived by associated {@link Selector}s + * from a redux store/state change. + */ + _selectorListeners: Set = new Set(); + + _listener: (Store) => void; + + /** + * Invoked by a specific redux store any time an action is dispatched, and + * some part of the state (tree) may potentially have changed. + * + * @param {Object} context - The redux store invoking the listener and the + * private state of this {@code StateListenerRegistry} associated with the + * redux store. + * @returns {void} + */ + _listener({ prevSelections, store }: { + prevSelections: Map, + store: Store + }) { + for (const selectorListener of this._selectorListeners) { + const prevSelection = prevSelections.get(selectorListener); + + try { + const selection + = selectorListener.selector( + store.getState(), + prevSelection); + + if (prevSelection !== selection) { + prevSelections.set(selectorListener, selection); + selectorListener.listener(selection, store, prevSelection); + } + } catch (e) { + // Don't let one faulty listener prevent other listeners from + // being notified about their associated changes. + logger.error(e); + } + } + } + + /** + * Registers a specific listener to be notified when the value derived by a + * specific {@code selector} from a redux store/state changes. + * + * @param {Function} selector - The pure {@code Function} of the redux + * store/state (and the previous selection of made by {@code selector}) + * which selects the value listened to by the specified {@code listener}. + * @param {Function} listener - The listener to register with this + * {@code StateListenerRegistry} so that it gets invoked when the value + * returned by the specified {@code selector} changes. + * @returns {void} + */ + register(selector: Selector, listener: Listener) { + this._selectorListeners.add({ + listener, + selector + }); + } + + /** + * Subscribes to a specific redux store (so that this instance gets notified + * any time an action is dispatched, and some part of the state (tree) of + * the specified redux store may potentially have changed). + * + * @param {Store} store - The redux store to which this + * {@code StateListenerRegistry} is to {@code subscribe}. + * @returns {void} + */ + subscribe(store: Store) { + // XXX If StateListenerRegistry is not utilized by the app to listen to + // state changes, do not bother subscribing to the store at all. + if (this._selectorListeners.size) { + store.subscribe( + this._listener.bind( + this, + { + /** + * The previous selections of the {@code Selector}s + * registered with this {@code StateListenerRegistry}. + * + * @type Map + */ + prevSelections: new Map(), + + /** + * The redux store. + * + * @type Store + */ + store + })); + } + } +} + +export default new StateListenerRegistry(); diff --git a/react/features/base/redux/index.js b/react/features/base/redux/index.js index 9658a8706..1643344a8 100644 --- a/react/features/base/redux/index.js +++ b/react/features/base/redux/index.js @@ -1,3 +1,4 @@ export * from './functions'; export { default as MiddlewareRegistry } from './MiddlewareRegistry'; export { default as ReducerRegistry } from './ReducerRegistry'; +export { default as StateListenerRegistry } from './StateListenerRegistry'; From ee9fcbb7355dae55821515685ba4b1380d73686b Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Mon, 21 May 2018 22:48:51 -0500 Subject: [PATCH 8/9] Remove remote participants who are no longer of interest The Jitsi Meet app always has at most 1 conference of primary interest. It may have to juggle with 2 JitsiConference instances at the same time if 1 is in the process of being left and one is joining/joined. But the one which is joining or joined (which we call conference in the features/base/conference redux state) is the one "of interest", the other one is "clean up". Consequently, the remote participants of the conference "of interest" are the remote participants "of interest" and the others are "clean up". In order to reduce the time during which there may be multiplying remote thumbnails, clean the remote participants who are no longer "of interest" up. --- .../features/base/participants/middleware.js | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/react/features/base/participants/middleware.js b/react/features/base/participants/middleware.js index a4c99faf0..ddd7f0a49 100644 --- a/react/features/base/participants/middleware.js +++ b/react/features/base/participants/middleware.js @@ -2,13 +2,14 @@ import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../../app'; import { CONFERENCE_LEFT, CONFERENCE_WILL_JOIN } from '../conference'; -import { MiddlewareRegistry } from '../redux'; +import { MiddlewareRegistry, StateListenerRegistry } from '../redux'; import UIEvents from '../../../../service/UI/UIEvents'; import { playSound, registerSound, unregisterSound } from '../sounds'; import { localParticipantIdChanged, localParticipantJoined, + participantLeft, participantUpdated } from './actions'; import { @@ -124,6 +125,26 @@ MiddlewareRegistry.register(store => next => action => { return next(action); }); +/** + * Syncs the redux state features/base/participants up with the redux state + * features/base/conference by ensuring that the former does not contain remote + * participants no longer relevant to the latter. Introduced to address an issue + * with multiplying thumbnails in the filmstrip. + */ +StateListenerRegistry.register( + /* selector */ state => { + const { conference, joining } = state['features/base/conference']; + + return conference || joining; + }, + /* listener */ (conference, { dispatch, getState }) => { + for (const p of getState()['features/base/participants']) { + !p.local + && (!conference || p.conference !== conference) + && dispatch(participantLeft(p.id, p.conference)); + } + }); + /** * Initializes the local participant and signals that it joined. * From 2ecacf6c3ea6b558b96aac1f7a5efe0faf4c4ad3 Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Wed, 23 May 2018 00:12:44 -0500 Subject: [PATCH 9/9] [RN] Fix a "TypeError: Cannot read property handlers of undefined" in ImageCache With some of the preceding commits in the "multiplying remote thumbnails" story line, I started hitting this error with 100% reproducibility: 1. Have a remote participant prepared in conferenceA. Web will do as well. 2. On iOS prepare to join conferenceB in Safari and use the same device for step 3. 3. Join conferenceA on the iOS device from step 2 with audio-only. The audio-only is so that avatars are always visible. Wait for the remote participant prepared in step 1 to appear. 4. Switch to Safari and hit "Continue in the app" to have the app leave conferenceA and join conferenceB. What happens: After the iOS device joins conferenceB in the Jitsi Meet app, the local participant is on the large video (as expected) but the avatar of the local participant is the default audo-generated auto-colored placeholder. That's because this error was hit and the avatar couldn't be "fetched". --- react/features/mobile/image-cache/middleware.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/react/features/mobile/image-cache/middleware.js b/react/features/mobile/image-cache/middleware.js index 97b207bef..dd9d18b4f 100644 --- a/react/features/mobile/image-cache/middleware.js +++ b/react/features/mobile/image-cache/middleware.js @@ -1,7 +1,6 @@ /* @flow */ import { APP_WILL_MOUNT } from '../../app'; -import { CONFERENCE_FAILED, CONFERENCE_LEFT } from '../../base/conference'; import { getAvatarURL, getLocalParticipant, @@ -37,8 +36,15 @@ const _PREFETCH_AVATAR_URLS = false; MiddlewareRegistry.register(({ getState }) => next => action => { switch (action.type) { case APP_WILL_MOUNT: - case CONFERENCE_FAILED: - case CONFERENCE_LEFT: + // XXX CONFERENCE_FAILED/LEFT are no longer used here because they + // are tricky to get right as detectors of the moments in time at which + // CachedImage is not used. Anyway, if ImageCache is to be cleared from + // time to time, SET_LOCATION_URL is a much easier detector of such + // opportune times. Fixes at least one 100%-reproducible case of + // "TypeError: Cannot read property handlers of undefined." Anyway, in + // order to reduce the re-downloading of the same avatars, eventually we + // decided to not clear during the runtime of the app (other that at the + // beginning that is). ImageCache && ImageCache.get().clear(); break;