haskal
/
jiti-meet
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

rn: fix losing audio if call is hangup too quickly 

This PR changes the logic for connecting / disconnecting conferences. Instead of
doing it in mount / unmount events from the Conference component, it moves the
logic to the appNavigatee action.

This fixes a regression introduced in 774c5ecd when trying to make sure the
conference terminated event is always sent.

By moving the logic to appNavigate we no longer depend on side-effects for
connecting / disconnecting, and the code should be more maintainable moving
forward.

An improvement to this is the concept of sessions, which, while not tackled
here, was taken into consideration.
This commit is contained in:
Saúl Ibarra Corretgé 2019-05-22 12:01:58 +02:00 committed by Saúl Ibarra Corretgé
parent 9352517705
commit a4cf79c161
5 changed files with 52 additions and 145 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
react/features/app/actions.js
View File

@ -10,8 +10,9 @@ import {
setConfig,
storeConfig
} from '../base/config';
import { setLocationURL } from '../base/connection';
import { connect, disconnect, setLocationURL } from '../base/connection';
import { loadConfig } from '../base/lib-jitsi-meet';
import { createDesiredLocalTracks } from '../base/tracks';
import { parseURIString, toURLString } from '../base/util';
import { setFatalError } from '../overlay';
@ -58,6 +59,12 @@ export function appNavigate(uri: ?string) {
const { contextRoot, host, room } = location;
const locationURL = new URL(location.toString());
// Disconnect from any current conference.
// FIXME: unify with web.
if (navigator.product === 'ReactNative') {
dispatch(disconnect());
}
dispatch(configWillLoad(locationURL, room));
let protocol = location.protocol.toLowerCase();
@ -87,12 +94,20 @@ export function appNavigate(uri: ?string) {
}
}
if (getState()['features/base/config'].locationURL === locationURL) {
dispatch(setLocationURL(locationURL));
dispatch(setConfig(config));
dispatch(setRoom(room));
} else {
if (getState()['features/base/config'].locationURL !== locationURL) {
dispatch(loadConfigError(new Error('Config no longer needed!'), locationURL));
return;
}
dispatch(setLocationURL(locationURL));
dispatch(setConfig(config));
dispatch(setRoom(room));
// FIXME: unify with web, currently the connection and track creation happens in conference.js.
if (room && navigator.product === 'ReactNative') {
dispatch(createDesiredLocalTracks());
dispatch(connect());
}
};
}

52
react/features/base/conference/middleware.js
View File

@ -24,7 +24,6 @@ import { TRACK_ADDED, TRACK_REMOVED } from '../tracks';
import {
conferenceFailed,
conferenceLeft,
conferenceWillLeave,
createConference,
setLastN,
@ -37,8 +36,7 @@ import {
CONFERENCE_WILL_LEAVE,
DATA_CHANNEL_OPENED,
SET_AUDIO_ONLY,
SET_LASTN,
SET_ROOM
SET_LASTN
} from './actionTypes';
import {
_addLocalTracksToConference,
@ -99,9 +97,6 @@ MiddlewareRegistry.register(store => next => action => {
case SET_LASTN:
return _setLastN(store, next, action);
case SET_ROOM:
return _setRoom(store, next, action);
case TRACK_ADDED:
case TRACK_REMOVED:
return _trackAddedOrRemoved(store, next, action);
@ -566,51 +561,6 @@ function _setReceiverVideoConstraint(conference, preferred, max) {
}
}
/**
* Notifies the feature {@code base/conference} that the redix action
* {@link SET_ROOM} 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} to the specified {@code store}.
* @param {Action} action - The redux action {@code SET_ROOM} which is being
* dispatched in the specified {@code store}.
* @private
* @returns {Object} The value returned by {@code next(action)}.
*/
function _setRoom({ dispatch, getState }, next, action) {
const result = next(action);
// By the time SET_ROOM is dispatched, base/connection's locationURL and
// base/conference's leaving should be the only conference-related sources
// of truth.
const state = getState();
const { leaving } = state['features/base/conference'];
const { locationURL } = state['features/base/connection'];
const dispatchConferenceLeft = new Set();
// Figure out which of the JitsiConferences referenced by base/conference
// have not dispatched or are not likely to dispatch CONFERENCE_FAILED and
// CONFERENCE_LEFT.
forEachConference(state, (conference, url) => {
if (conference !== leaving && url && url !== locationURL) {
dispatchConferenceLeft.add(conference);
}
return true; // All JitsiConference instances are to be examined.
});
// Dispatch CONFERENCE_LEFT for the JitsiConferences referenced by
// base/conference which have not dispatched or are not likely to dispatch
// CONFERENCE_FAILED or CONFERENCE_LEFT.
for (const conference of dispatchConferenceLeft) {
dispatch(conferenceLeft(conference));
}
return result;
}
/**
* Synchronizes local tracks from state with local tracks in JitsiConference
* instance.

5
react/features/base/connection/actions.native.js
View File

@ -370,10 +370,7 @@ export function disconnect() {
if (connection_) {
promise = promise.then(() => connection_.disconnect());
} else {
// FIXME: We have no connection! Fake a disconnect. Because of how the current disconnec is implemented
// (by doing the diconnect() in the Conference component unmount) we have lost the location URL already.
// Oh well, at least send the event.
promise.then(() => dispatch(_connectionDisconnected({}, '')));
logger.info('No connection found while disconnecting.');
}
return promise;

80
react/features/conference/components/native/Conference.js
View File

@ -5,7 +5,6 @@ import React from 'react';
import { BackHandler, SafeAreaView, StatusBar, View } from 'react-native';
import { appNavigate } from '../../../app';
import { connect, disconnect } from '../../../base/connection';
import { getParticipantCount } from '../../../base/participants';
import { Container, LoadingIndicator, TintedView } from '../../../base/react';
import { connect as reactReduxConnect } from '../../../base/redux';
@ -14,7 +13,6 @@ import {
makeAspectRatioAware
} from '../../../base/responsive-ui';
import { TestConnectionInfo } from '../../../base/testing';
import { createDesiredLocalTracks } from '../../../base/tracks';
import { ConferenceNotification } from '../../../calendar-sync';
import { Chat } from '../../../chat';
import { DisplayNameLabel } from '../../../display-name';
@ -66,29 +64,6 @@ type Props = AbstractProps & {
*/
_largeVideoParticipantId: string,
/**
* Current conference's full URL.
*
* @private
*/
_locationURL: URL,
/**
* The handler which dispatches the (redux) action connect.
*
* @private
* @returns {void}
*/
_onConnect: Function,
/**
* The handler which dispatches the (redux) action disconnect.
*
* @private
* @returns {void}
*/
_onDisconnect: Function,
/**
* Handles a hardware button press for back navigation. Leaves the
* associated {@code Conference}.
@ -166,8 +141,6 @@ class Conference extends AbstractConference<Props, *> {
* @returns {void}
*/
componentDidMount() {
this.props._onConnect();
BackHandler.addEventListener(
'hardwareBackPress',
this.props._onHardwareBackPress);
@ -184,26 +157,16 @@ class Conference extends AbstractConference<Props, *> {
*
* @inheritdoc
*/
componentDidUpdate(pevProps: Props) {
componentDidUpdate(prevProps: Props) {
const {
_locationURL: oldLocationURL,
_participantCount: oldParticipantCount,
_room: oldRoom
} = pevProps;
_participantCount: oldParticipantCount
} = prevProps;
const {
_locationURL: newLocationURL,
_participantCount: newParticipantCount,
_room: newRoom,
_setToolboxVisible,
_toolboxVisible
} = this.props;
// If the location URL changes we need to reconnect.
oldLocationURL !== newLocationURL && newRoom && this.props._onDisconnect();
// Start the connection process when there is a (valid) room.
oldRoom !== newRoom && newRoom && this.props._onConnect();
if (oldParticipantCount === 1
&& newParticipantCount > 1
&& _toolboxVisible) {
@ -228,8 +191,6 @@ class Conference extends AbstractConference<Props, *> {
BackHandler.removeEventListener(
'hardwareBackPress',
this.props._onHardwareBackPress);
this.props._onDisconnect();
}
/**
@ -396,36 +357,12 @@ class Conference extends AbstractConference<Props, *> {
* @param {Function} dispatch - Redux action dispatcher.
* @private
* @returns {{
* _onConnect: Function,
* _onDisconnect: Function,
* _onHardwareBackPress: Function,
* _setToolboxVisible: Function
* }}
*/
function _mapDispatchToProps(dispatch) {
return {
/**
* Dispatches actions to create the desired local tracks and for
* connecting to the conference.
*
* @private
* @returns {void}
*/
_onConnect() {
dispatch(createDesiredLocalTracks());
dispatch(connect());
},
/**
* Dispatches an action disconnecting from the conference.
*
* @private
* @returns {void}
*/
_onDisconnect() {
dispatch(disconnect());
},
/**
* Handles a hardware button press for back navigation. Leaves the
* associated {@code Conference}.
@ -462,8 +399,7 @@ function _mapDispatchToProps(dispatch) {
* @returns {Props}
*/
function _mapStateToProps(state) {
const { connecting, connection, locationURL }
= state['features/base/connection'];
const { connecting, connection } = state['features/base/connection'];
const {
conference,
joining,
@ -508,14 +444,6 @@ function _mapStateToProps(state) {
*/
_largeVideoParticipantId: state['features/large-video'].participantId,
/**
* Current conference's full URL.
*
* @private
* @type {URL}
*/
_locationURL: locationURL,
/**
* The number of participants in the conference.
*

33
react/features/toolbox/components/HangupButton.js
View File

@ -1,5 +1,6 @@
// @flow
import _ from 'lodash';
import { createToolbarEvent, sendAnalytics } from '../../analytics';
import { appNavigate } from '../../app';
@ -26,10 +27,33 @@ type Props = AbstractButtonProps & {
* @extends AbstractHangupButton
*/
class HangupButton extends AbstractHangupButton<Props, *> {
_hangup: Function;
accessibilityLabel = 'toolbar.accessibilityLabel.hangup';
label = 'toolbar.hangup';
tooltip = 'toolbar.hangup';
/**
* Initializes a new HangupButton instance.
*
* @param {Props} props - The read-only properties with which the new
* instance is to be initialized.
*/
constructor(props: Props) {
super(props);
this._hangup = _.once(() => {
sendAnalytics(createToolbarEvent('hangup'));
// FIXME: these should be unified.
if (navigator.product === 'ReactNative') {
this.props.dispatch(appNavigate(undefined));
} else {
this.props.dispatch(disconnect(true));
}
});
}
/**
* Helper function to perform the actual hangup action.
*
@ -38,14 +62,7 @@ class HangupButton extends AbstractHangupButton<Props, *> {
* @returns {void}
*/
_doHangup() {
sendAnalytics(createToolbarEvent('hangup'));
// FIXME: these should be unified.
if (navigator.product === 'ReactNative') {
this.props.dispatch(appNavigate(undefined));
} else {
this.props.dispatch(disconnect(true));
}
this._hangup();
}
}