Compare commits

...

3 Commits

Author SHA1 Message Date
Saúl Ibarra Corretgé c589aeb60a app: avoid loading config when going back to the welcome page 2019-05-23 10:17:42 +02:00
Saúl Ibarra Corretgé e775c6d565 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.
2019-05-23 09:57:39 +02:00
Saúl Ibarra Corretgé ed2d4a8919 ios: always log delegate method calls 2019-05-23 09:45:53 +02:00
6 changed files with 67 additions and 153 deletions

View File

@ -42,11 +42,11 @@
- (void)_onJitsiMeetViewDelegateEvent:(NSString *)name
withData:(NSDictionary *)data {
#if DEBUG
NSLog(
@"[%s:%d] JitsiMeetViewDelegate %@ %@",
__FILE__, __LINE__, name, data);
#if DEBUG
NSAssert(
[NSThread isMainThread],
@"JitsiMeetViewDelegate %@ method invoked on a non-main thread",

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();
@ -74,25 +81,40 @@ export function appNavigate(uri: ?string) {
let config;
try {
config = await loadConfig(url);
dispatch(storeConfig(baseURL, config));
} catch (error) {
// Avoid (re)loading the config when there is no room.
if (!room) {
config = restoreConfig(baseURL);
}
if (!config) {
dispatch(loadConfigError(error, locationURL));
if (!config) {
try {
config = await loadConfig(url);
dispatch(storeConfig(baseURL, config));
} catch (error) {
config = restoreConfig(baseURL);
return;
if (!config) {
dispatch(loadConfigError(error, locationURL));
return;
}
}
}
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());
}
};
}

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.

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;

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.
*

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();
}
}