Browse Source

fix(base/participants): ensure default local id outside of conference

Makes sure that whenever a conference is left or switched, the local
participant's id will be equal to the default value.

The problem fixed by this commit is a situation where the local
participant may end up sharing the same ID with it's "ghost" when
rejoining a disconnected conference. The most important and easiest to
hit case is when the conference is left after the CONFERENCE_FAILED
event.

Another rare and harder to encounter in the real world issue is
where CONFERENCE_LEFT may come with the delay due to it's asynchronous
nature. The step by step scenario is as follows: trying to leave a
conference, but the network is not doing well, so it takes time,
requests are timing out. After getting back to the welcome page the
the CONFERENCE_LEFT has not arrived yet. The same conference is joined
again and the load config may timeout, but it will be read from the
cache. Now the network gets better and conference is joining which
results in our ghost participant added to the redux state. At this point
there's the root issue: two participants with the same id, because the
local one was neither cleared nor set to the new one yet
(PARTICIPANT_JOINED come, before CONFERENCE_JOINED where we adjust the
id). Then comes CONFERENCE_JOINED and we try to update our local id.
We're updating the ID of both ghost and local participant. It could be
also that the delayed CONFERENCE_LEFT comes for the old conference, but
it's too late and it would update the id for both participants.

The approach here reasons that the ID of the local participant
may be reset as soon as the local participant and, respectively, her ID
is no longer involved in a recoverable JitsiConference of interest to
the user and, consequently, the app.

Co-authored-by: Pawel Domas <pawel.domas@jitsi.org>
Co-authored-by: Lyubo Marinov <lmarinov@atlassian.com>
master
paweldomas 7 years ago
parent
commit
7186a9c79c

+ 41
- 1
react/features/base/conference/functions.js View File

7
 import {
7
 import {
8
     AVATAR_ID_COMMAND,
8
     AVATAR_ID_COMMAND,
9
     AVATAR_URL_COMMAND,
9
     AVATAR_URL_COMMAND,
10
-    EMAIL_COMMAND
10
+    EMAIL_COMMAND,
11
+    JITSI_CONFERENCE_URL_KEY
11
 } from './constants';
12
 } from './constants';
12
 
13
 
13
 /**
14
 /**
40
     return Promise.all(promises);
41
     return Promise.all(promises);
41
 }
42
 }
42
 
43
 
44
+/**
45
+ * Evaluates a specific predicate for each {@link JitsiConference} known to the
46
+ * redux state features/base/conference while it returns {@code true}.
47
+ *
48
+ * @param {Function | Object} stateful - The redux store, state, or
49
+ * {@code getState} function.
50
+ * @param {Function} predicate - The predicate to evaluate for each
51
+ * {@code JitsiConference} know to the redux state features/base/conference
52
+ * while it returns {@code true}.
53
+ * @returns {boolean} If the specified {@code predicate} returned {@code true}
54
+ * for all {@code JitsiConference} instances known to the redux state
55
+ * features/base/conference.
56
+ */
57
+export function forEachConference(
58
+        stateful: Function | Object,
59
+        predicate: (Object, URL) => boolean) {
60
+    const state = toState(stateful)['features/base/conference'];
61
+
62
+    for (const v of Object.values(state)) {
63
+        // Does the value of the base/conference's property look like a
64
+        // JitsiConference?
65
+        if (v && typeof v === 'object') {
66
+            // $FlowFixMe
67
+            const url: URL = v[JITSI_CONFERENCE_URL_KEY];
68
+
69
+            // XXX The Web version of Jitsi Meet does not utilize
70
+            // JITSI_CONFERENCE_URL_KEY at the time of this writing. An
71
+            // alternative is necessary then to recognize JitsiConference
72
+            // instances and myUserId is as good as any other property.
73
+            if ((url || typeof v.myUserId === 'function')
74
+                    && !predicate(v, url)) {
75
+                return false;
76
+            }
77
+        }
78
+    }
79
+
80
+    return true;
81
+}
82
+
43
 /**
83
 /**
44
  * Returns the current {@code JitsiConference} which is joining or joined and is
84
  * Returns the current {@code JitsiConference} which is joining or joined and is
45
  * not leaving. Please note the contrast with merely reading the
85
  * not leaving. Please note the contrast with merely reading the

+ 8
- 19
react/features/base/conference/middleware.js View File

34
     SET_RECEIVE_VIDEO_QUALITY,
34
     SET_RECEIVE_VIDEO_QUALITY,
35
     SET_ROOM
35
     SET_ROOM
36
 } from './actionTypes';
36
 } from './actionTypes';
37
-import { JITSI_CONFERENCE_URL_KEY } from './constants';
38
 import {
37
 import {
39
     _addLocalTracksToConference,
38
     _addLocalTracksToConference,
39
+    forEachConference,
40
     _handleParticipantError,
40
     _handleParticipantError,
41
     _removeLocalTracksFromConference
41
     _removeLocalTracksFromConference
42
 } from './functions';
42
 } from './functions';
354
     // base/conference's leaving should be the only conference-related sources
354
     // base/conference's leaving should be the only conference-related sources
355
     // of truth.
355
     // of truth.
356
     const state = getState();
356
     const state = getState();
357
-    const {
358
-        leaving,
359
-        ...stateFeaturesBaseConference
360
-    } = state['features/base/conference'];
357
+    const { leaving } = state['features/base/conference'];
361
     const { locationURL } = state['features/base/connection'];
358
     const { locationURL } = state['features/base/connection'];
362
     const dispatchConferenceLeft = new Set();
359
     const dispatchConferenceLeft = new Set();
363
 
360
 
364
     // Figure out which of the JitsiConferences referenced by base/conference
361
     // Figure out which of the JitsiConferences referenced by base/conference
365
     // have not dispatched or are not likely to dispatch CONFERENCE_FAILED and
362
     // have not dispatched or are not likely to dispatch CONFERENCE_FAILED and
366
     // CONFERENCE_LEFT.
363
     // CONFERENCE_LEFT.
367
-
368
-    // eslint-disable-next-line guard-for-in
369
-    for (const p in stateFeaturesBaseConference) {
370
-        const v = stateFeaturesBaseConference[p];
371
-
372
-        // Does the value of the base/conference's property look like a
373
-        // JitsiConference?
374
-        if (v && typeof v === 'object') {
375
-            const url = v[JITSI_CONFERENCE_URL_KEY];
376
-
377
-            if (url && v !== leaving && url !== locationURL) {
378
-                dispatchConferenceLeft.add(v);
379
-            }
364
+    forEachConference(state, (conference, url) => {
365
+        if (conference !== leaving && url && url !== locationURL) {
366
+            dispatchConferenceLeft.add(conference);
380
         }
367
         }
381
-    }
368
+
369
+        return true; // All JitsiConference instances are to be examined.
370
+    });
382
 
371
 
383
     // Dispatch CONFERENCE_LEFT for the JitsiConferences referenced by
372
     // Dispatch CONFERENCE_LEFT for the JitsiConferences referenced by
384
     // base/conference which have not dispatched or are not likely to dispatch
373
     // base/conference which have not dispatched or are not likely to dispatch

+ 36
- 5
react/features/base/participants/middleware.js View File

1
 // @flow
1
 // @flow
2
 
2
 
3
 import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../../app';
3
 import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from '../../app';
4
-import { CONFERENCE_LEFT, CONFERENCE_WILL_JOIN } from '../conference';
4
+import { CONFERENCE_WILL_JOIN, forEachConference } from '../conference';
5
 import { CALLING, INVITED } from '../../presence-status';
5
 import { CALLING, INVITED } from '../../presence-status';
6
 import { MiddlewareRegistry, StateListenerRegistry } from '../redux';
6
 import { MiddlewareRegistry, StateListenerRegistry } from '../redux';
7
 import UIEvents from '../../../../service/UI/UIEvents';
7
 import UIEvents from '../../../../service/UI/UIEvents';
59
         store.dispatch(localParticipantIdChanged(action.conference.myUserId()));
59
         store.dispatch(localParticipantIdChanged(action.conference.myUserId()));
60
         break;
60
         break;
61
 
61
 
62
-    case CONFERENCE_LEFT:
63
-        store.dispatch(localParticipantIdChanged(LOCAL_PARTICIPANT_DEFAULT_ID));
64
-        break;
65
-
66
     case DOMINANT_SPEAKER_CHANGED: {
62
     case DOMINANT_SPEAKER_CHANGED: {
67
         // Ensure the raised hand state is cleared for the dominant speaker.
63
         // Ensure the raised hand state is cleared for the dominant speaker.
68
 
64
 
146
         }
142
         }
147
     });
143
     });
148
 
144
 
145
+/**
146
+ * Reset the ID of the local participant to
147
+ * {@link LOCAL_PARTICIPANT_DEFAULT_ID}. Such a reset is deemed possible only if
148
+ * the local participant and, respectively, her ID is not involved in a
149
+ * conference which is still of interest to the user and, consequently, the app.
150
+ * For example, a conference which is in the process of leaving is no longer of
151
+ * interest the user, is unrecoverable from the perspective of the user and,
152
+ * consequently, the app.
153
+ */
154
+StateListenerRegistry.register(
155
+    /* selector */ state => state['features/base/conference'],
156
+    /* listener */ ({ leaving }, { dispatch, getState }) => {
157
+        const state = getState();
158
+        const localParticipant = getLocalParticipant(state);
159
+        let id;
160
+
161
+        if (!localParticipant
162
+                || (id = localParticipant.id)
163
+                    === LOCAL_PARTICIPANT_DEFAULT_ID) {
164
+            // The ID of the local participant has been reset already.
165
+            return;
166
+        }
167
+
168
+        // The ID of the local may be reset only if it is not in use.
169
+        const dispatchLocalParticipantIdChanged
170
+            = forEachConference(
171
+                state,
172
+                conference =>
173
+                    conference === leaving || conference.myUserId() !== id);
174
+
175
+        dispatchLocalParticipantIdChanged
176
+            && dispatch(
177
+                localParticipantIdChanged(LOCAL_PARTICIPANT_DEFAULT_ID));
178
+    });
179
+
149
 /**
180
 /**
150
  * Initializes the local participant and signals that it joined.
181
  * Initializes the local participant and signals that it joined.
151
  *
182
  *

+ 10
- 19
react/features/mobile/external-api/middleware.js View File

11
     CONFERENCE_WILL_LEAVE,
11
     CONFERENCE_WILL_LEAVE,
12
     JITSI_CONFERENCE_URL_KEY,
12
     JITSI_CONFERENCE_URL_KEY,
13
     SET_ROOM,
13
     SET_ROOM,
14
+    forEachConference,
14
     isRoomValid
15
     isRoomValid
15
 } from '../../base/conference';
16
 } from '../../base/conference';
16
 import { LOAD_CONFIG_ERROR } from '../../base/config';
17
 import { LOAD_CONFIG_ERROR } from '../../base/config';
249
     // the same URL string multiple times) to try to send CONFERENCE_LEFT
250
     // the same URL string multiple times) to try to send CONFERENCE_LEFT
250
     // externally for a URL string which identifies a JitsiConference that the
251
     // externally for a URL string which identifies a JitsiConference that the
251
     // app is internally legitimately working with.
252
     // app is internally legitimately working with.
253
+    let swallowConferenceLeft = false;
252
 
254
 
253
-    if (url) {
254
-        const stateFeaturesBaseConference
255
-            = getState()['features/base/conference'];
256
-
257
-        // eslint-disable-next-line guard-for-in
258
-        for (const p in stateFeaturesBaseConference) {
259
-            const v = stateFeaturesBaseConference[p];
260
-
261
-            // Does the value of the base/conference's property look like a
262
-            // JitsiConference?
263
-            if (v && typeof v === 'object') {
264
-                const vURL = v[JITSI_CONFERENCE_URL_KEY];
265
-
266
-                if (vURL && vURL.toString() === url) {
267
-                    return true;
268
-                }
255
+    url
256
+        && forEachConference(getState, (conference, conferenceURL) => {
257
+            if (conferenceURL && conferenceURL.toString() === url) {
258
+                swallowConferenceLeft = true;
269
             }
259
             }
270
-        }
271
-    }
272
 
260
 
273
-    return false;
261
+            return !swallowConferenceLeft;
262
+        });
263
+
264
+    return swallowConferenceLeft;
274
 }
265
 }
275
 
266
 
276
 /**
267
 /**

Loading…
Cancel
Save