Added FSM to protect peerConnection use in basestack #1387
Conversation
| const PeerConnectionFsm = StateMachine.factory({ | ||
| init: 'initial', | ||
| transitions: [ | ||
| { name: 'create-offer', from: activeStates, to: 'stable' }, |
There was a problem hiding this comment.
👍 I really like this pattern, which will help us tracking issues in the future.
| }, 0); | ||
| return Promise.resolve(); | ||
| }, | ||
| protectedRemoveStream: (stream) => { |
There was a problem hiding this comment.
I'd keep a blank line between functions in the protectedCalls and queuedCalls, to improve readability a bit
| firstLocalDescriptionQueue.stopEnqueuing(); | ||
| firstLocalDescriptionQueue.dequeueAll(); | ||
| setTimeout(() => { | ||
| negotiationQueue.stopEnqueuing(); |
There was a problem hiding this comment.
why in general we protect calls to next items in negotiationQueue and not to firstLocalDescriptionQueue?
There was a problem hiding this comment.
Because... They should also be protected, I'll fix it 👍
| }, | ||
| }); | ||
|
|
||
| export default PeerConnectionFsm; |
| getHistory: function getHistory() { | ||
| return this.history; | ||
| }, | ||
| onBeforeClose: function onBeforeClose(lifecycle) { |
There was a problem hiding this comment.
I'd leave a blank line between functions here too.
Also, is it possible to have lambdas here?
methods: {
getHistory: () => {
},
onBeforeClose: () => {
},
...
};Possibly not due to the use of this, but I'm not sure.
There was a problem hiding this comment.
Yes, it's because the use of this in the state-machine functions. Agree on the use of blank lines 👍
Description
This PR adds a finite state machine that tracks and protects all calls from baseStack to peerConnection.
This PR also reorganizes many of the functions in baseStack. There are now two structures
enqueuedCallsandprotectedCalls.protectedCallsare only called fromPeerConnectionFsmenqueuedCallscan be called from anywhere else and they are protected by aFunctionQueue.[] It needs and includes Unit Tests
Changes in Client or Server public APIs
[] It includes documentation for these changes in
/doc.