Skip to content

Commit

Permalink
fix: leaking listeners while waiting on auth dialog (jitsi#11288)
Browse files Browse the repository at this point in the history
* fix: Drop duplicate call of wait for owner.

* fix: Fixes leaking listeners while waiting for host to join.

While waiting for the host to join on the dialog we attempt to join over and over again till we are admitted to enter the meeting. While doing that authRequired flag is on, and we were adding listeners on and on.

* feat: Introduces conference join in progress action.

This event is coming from lib-jitsi-meet and is fired when we receive the first presence of series when joining. It is always fired before joined event.

* fix: Moves testing middleware to use CONFERENCE_JOIN_IN_PROGRESS.

* fix: Moves follow-me middleware to use CONFERENCE_JOIN_IN_PROGRESS.

* fix: Moves some polls logic to middleware and use CONFERENCE_JOIN_IN_PROGRESS.

* fix: Moves reactions middleware to use CONFERENCE_JOIN_IN_PROGRESS.

* fix: Moves recordings middleware to use CONFERENCE_JOIN_IN_PROGRESS.

* fix: Moves shared-video middleware to use CONFERENCE_JOIN_IN_PROGRESS.

* fix: Moves videosipgw middleware to use CONFERENCE_JOIN_IN_PROGRESS.

* squash: Fix comments.

* fix: Fixes join in progress on web.

* fix: Moves variable extraction inside handlers.

* fix: Moves variable extraction inside handlers again.

* fix: Moves etherpad middleware to use CONFERENCE_JOIN_IN_PROGRESS.
  • Loading branch information
damencho authored Apr 6, 2022
1 parent 33db511 commit a99532b
Show file tree
Hide file tree
Showing 17 changed files with 312 additions and 251 deletions.
10 changes: 5 additions & 5 deletions conference.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
commonUserLeftHandling,
conferenceFailed,
conferenceJoined,
conferenceJoinInProgress,
conferenceLeft,
conferenceSubjectChanged,
conferenceTimestampChanged,
Expand Down Expand Up @@ -142,8 +143,7 @@ import {
initPrejoin,
isPrejoinPageVisible,
makePrecallTest,
setJoiningInProgress,
setPrejoinPageVisibility
setJoiningInProgress
} from './react/features/prejoin';
import { disableReceiver, stopReceiver } from './react/features/remote-control';
import { setScreenAudioShareState, isScreenAudioShared } from './react/features/screen-share/';
Expand Down Expand Up @@ -2068,9 +2068,9 @@ export default {
room.on(JitsiConferenceEvents.CONFERENCE_JOINED, () => {
this._onConferenceJoined();
});
room.on(JitsiConferenceEvents.CONFERENCE_JOIN_IN_PROGRESS, () => {
APP.store.dispatch(setPrejoinPageVisibility(false));
});
room.on(
JitsiConferenceEvents.CONFERENCE_JOIN_IN_PROGRESS,
() => APP.store.dispatch(conferenceJoinInProgress(room)));

room.on(
JitsiConferenceEvents.CONFERENCE_LEFT,
Expand Down
11 changes: 7 additions & 4 deletions modules/UI/authentication/AuthHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { openConnection } from '../../../connection';
import {
openAuthDialog,
openLoginDialog } from '../../../react/features/authentication/actions.web';
import { WaitForOwnerDialog } from '../../../react/features/authentication/components';
import {
LoginDialog,
WaitForOwnerDialog
} from '../../../react/features/authentication/components';
import {
isTokenAuthEnabled,
getTokenAuthUrl
Expand All @@ -16,7 +19,7 @@ import { isDialogOpen } from '../../../react/features/base/dialog';
import { setJWT } from '../../../react/features/base/jwt';
import UIUtil from '../util/UIUtil';

import LoginDialog from './LoginDialog';
import ExternalLoginDialog from './LoginDialog';


let externalAuthWindow;
Expand Down Expand Up @@ -51,7 +54,7 @@ function doExternalAuth(room, lockPassword) {
getUrl = room.getExternalAuthUrl(true);
}
getUrl.then(url => {
externalAuthWindow = LoginDialog.showExternalAuthDialog(
externalAuthWindow = ExternalLoginDialog.showExternalAuthDialog(
url,
() => {
externalAuthWindow = null;
Expand Down Expand Up @@ -187,7 +190,7 @@ function authenticate(room: Object, lockPassword: string) {
* @param {string} [lockPassword] password to use if the conference is locked
*/
function requireAuth(room: Object, lockPassword: string) {
if (!isDialogOpen(APP.store, WaitForOwnerDialog)) {
if (isDialogOpen(APP.store, WaitForOwnerDialog) || isDialogOpen(APP.store, LoginDialog)) {
return;
}

Expand Down
1 change: 0 additions & 1 deletion react/features/app/middlewares.any.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import '../lobby/middleware';
import '../notifications/middleware';
import '../overlay/middleware';
import '../polls/middleware';
import '../polls/subscriber';
import '../reactions/middleware';
import '../recent-list/middleware';
import '../recording/middleware';
Expand Down
9 changes: 6 additions & 3 deletions react/features/authentication/middleware.web.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import {
import {
hideLoginDialog,
openWaitForOwnerDialog,
stopWaitForOwner,
waitForOwner
stopWaitForOwner
} from './actions.web';
import { LoginDialog, WaitForOwnerDialog } from './components';

Expand Down Expand Up @@ -72,7 +71,11 @@ MiddlewareRegistry.register(store => next => action => {
recoverable = error.recoverable;
}
if (recoverable) {
store.dispatch(waitForOwner());
// we haven't migrated all the code from AuthHandler, and we need for now conference.js to trigger
// the dialog to pass all required parameters to WaitForOwnerDialog
// keep it commented, so we do not trigger sending iqs to jicofo twice
// and showing the broken dialog with no handler
// store.dispatch(waitForOwner());
} else {
store.dispatch(stopWaitForOwner());
}
Expand Down
11 changes: 11 additions & 0 deletions react/features/base/conference/actionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@ export const CONFERENCE_FAILED = 'CONFERENCE_FAILED';
*/
export const CONFERENCE_JOINED = 'CONFERENCE_JOINED';

/**
* The type of (redux) action which signals that a specific conference joining is in progress.
* A CONFERENCE_JOINED is guaranteed to follow.
*
* {
* type: CONFERENCE_JOIN_IN_PROGRESS,
* conference: JitsiConference
* }
*/
export const CONFERENCE_JOIN_IN_PROGRESS = 'CONFERENCE_JOIN_IN_PROGRESS';

/**
* The type of (redux) action which signals that a specific conference was left.
*
Expand Down
21 changes: 21 additions & 0 deletions react/features/base/conference/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { getBackendSafeRoomName } from '../util';
import {
AUTH_STATUS_CHANGED,
CONFERENCE_FAILED,
CONFERENCE_JOIN_IN_PROGRESS,
CONFERENCE_JOINED,
CONFERENCE_LEFT,
CONFERENCE_LOCAL_SUBJECT_CHANGED,
Expand Down Expand Up @@ -105,6 +106,9 @@ function _addConferenceListeners(conference, dispatch, state) {
conference.on(
JitsiConferenceEvents.CONFERENCE_JOINED,
(...args) => dispatch(conferenceJoined(conference, ...args)));
conference.on(
JitsiConferenceEvents.CONFERENCE_JOIN_IN_PROGRESS,
(...args) => dispatch(conferenceJoinInProgress(conference, ...args)));
conference.on(
JitsiConferenceEvents.CONFERENCE_LEFT,
(...args) => {
Expand Down Expand Up @@ -350,6 +354,23 @@ export function conferenceJoined(conference: Object) {
};
}

/**
* Signals that a specific conference join is in progress.
*
* @param {JitsiConference} conference - The JitsiConference instance for which join by the local participant
* is in progress.
* @returns {{
* type: CONFERENCE_JOIN_IN_PROGRESS,
* conference: JitsiConference
* }}
*/
export function conferenceJoinInProgress(conference: Object) {
return {
type: CONFERENCE_JOIN_IN_PROGRESS,
conference
};
}

/**
* Signals that a specific conference has been left.
*
Expand Down
12 changes: 10 additions & 2 deletions react/features/base/conference/middleware.web.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

import { AUDIO_ONLY_SCREEN_SHARE_NO_TRACK } from '../../../../modules/UI/UIErrors';
import { showNotification, NOTIFICATION_TIMEOUT_TYPE } from '../../notifications';
import { setSkipPrejoinOnReload } from '../../prejoin';
import {
setPrejoinPageVisibility,
setSkipPrejoinOnReload
} from '../../prejoin';
import { setScreenAudioShareState, setScreenshareAudioTrack } from '../../screen-share';
import { AudioMixerEffect } from '../../stream-effects/audio-mixer/AudioMixerEffect';
import { setAudioOnly } from '../audio-only';
Expand All @@ -19,7 +22,7 @@ import {
TOGGLE_SCREENSHARING
} from '../tracks';

import { CONFERENCE_FAILED, CONFERENCE_JOINED } from './actionTypes';
import { CONFERENCE_FAILED, CONFERENCE_JOIN_IN_PROGRESS, CONFERENCE_JOINED } from './actionTypes';
import { getCurrentConference } from './functions';
import './middleware.any';

Expand All @@ -28,6 +31,11 @@ MiddlewareRegistry.register(store => next => action => {
const { enableForcedReload } = getState()['features/base/config'];

switch (action.type) {
case CONFERENCE_JOIN_IN_PROGRESS: {
dispatch(setPrejoinPageVisibility(false));

break;
}
case CONFERENCE_JOINED: {
if (enableForcedReload) {
dispatch(setSkipPrejoinOnReload(false));
Expand Down
4 changes: 2 additions & 2 deletions react/features/base/testing/middleware.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow

import { CONFERENCE_WILL_JOIN } from '../conference';
import { CONFERENCE_JOIN_IN_PROGRESS } from '../conference/actionTypes';
import { SET_CONFIG } from '../config';
import { JitsiConferenceEvents } from '../lib-jitsi-meet';
import { MiddlewareRegistry } from '../redux';
Expand All @@ -24,7 +24,7 @@ import logger from './logger';
*/
MiddlewareRegistry.register(store => next => action => {
switch (action.type) {
case CONFERENCE_WILL_JOIN:
case CONFERENCE_JOIN_IN_PROGRESS:
_bindConferenceConnectionListener(action.conference, store);
break;
case SET_CONFIG: {
Expand Down
37 changes: 19 additions & 18 deletions react/features/etherpad/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import UIEvents from '../../../service/UI/UIEvents';
import { getCurrentConference } from '../base/conference';
import { CONFERENCE_JOIN_IN_PROGRESS } from '../base/conference/actionTypes';
import { MiddlewareRegistry, StateListenerRegistry } from '../base/redux';

import { TOGGLE_DOCUMENT_EDITING } from './actionTypes';
Expand All @@ -21,6 +22,23 @@ const ETHERPAD_COMMAND = 'etherpad';
// eslint-disable-next-line no-unused-vars
MiddlewareRegistry.register(({ dispatch, getState }) => next => action => {
switch (action.type) {
case CONFERENCE_JOIN_IN_PROGRESS: {
const { conference } = action;

conference.addCommandListener(ETHERPAD_COMMAND,
({ value }) => {
let url;
const { etherpad_base: etherpadBase } = getState()['features/base/config'];

if (etherpadBase) {
url = new URL(value, etherpadBase).toString();
}

dispatch(setDocumentUrl(url));
}
);
break;
}
case TOGGLE_DOCUMENT_EDITING: {
if (typeof APP !== 'undefined') {
APP.UI.emitEvent(UIEvents.ETHERPAD_CLICKED);
Expand All @@ -39,24 +57,7 @@ MiddlewareRegistry.register(({ dispatch, getState }) => next => action => {
*/
StateListenerRegistry.register(
state => getCurrentConference(state),
(conference, { dispatch, getState }, previousConference) => {
if (conference) {
conference.addCommandListener(ETHERPAD_COMMAND,
({ value }) => {
let url;
const { etherpad_base: etherpadBase } = getState()['features/base/config'];

if (etherpadBase) {
const u = new URL(value, etherpadBase);

url = u.toString();
}

dispatch(setDocumentUrl(url));
}
);
}

(conference, { dispatch }, previousConference) => {
if (previousConference) {
dispatch(setDocumentUrl(undefined));
}
Expand Down
4 changes: 2 additions & 2 deletions react/features/follow-me/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import _ from 'lodash';

import { CONFERENCE_WILL_JOIN } from '../base/conference/actionTypes';
import { CONFERENCE_JOIN_IN_PROGRESS } from '../base/conference/actionTypes';
import {
getParticipantById,
getPinnedParticipant,
Expand Down Expand Up @@ -61,7 +61,7 @@ let nextOnStageTimer = 0;
*/
MiddlewareRegistry.register(store => next => action => {
switch (action.type) {
case CONFERENCE_WILL_JOIN: {
case CONFERENCE_JOIN_IN_PROGRESS: {
const { conference } = action;

conference.addCommandListener(
Expand Down
Loading

0 comments on commit a99532b

Please sign in to comment.