React-native-webrtc: We don't handle `channelId` correctly

[xilin]

Expected behavior

RTCPeerConnection pc = xxx;
const channel1 = pc.createDataChannel("label1", {maxRetransmits: 0, ordered: false});
channel1.onopen = xxx; // should be called
const channel2 = pc.createDataChannel("label2", {maxRetransmits: 0, ordered: false});
channel2.onopen = xxx; // should be called

Observerd behavior

Only first channel can receive state change event.

Steps to reproduce the problem

Just create multiple data channels in iOS, with react-native-webrtc >= 1.75.3

Platform information

• React Native version: 0.63.3
• Plugin version: 1.75.3
• OS: iOS
• OS version: 10

There is already a PR try to resolve this issue: https://github.com/react-native-webrtc/react-native-webrtc/pull/726 .
However it didn't fix the problem completely.
Based on the discussion, I found that in the PR, aka commit f3736ed, those guys introduced checking for negotiated. If negotiated == false, id will be reset to -1. And in the following method AllocateSid, it will try to run potential_sid += 2;.
So when - (void)dataChannelDidChangeState:(RTCDataChannel*)channel is called, channel.channelId had been changed, and different from what we recorded in peerConnection.dataChannels which is done in createDataChannel:(nonnull NSNumber *)peerConnectionId label:(NSString *)label config:(RTCDataChannelConfiguration *)config.
This negotiated logic still exists in the newest master branch of webrtc.

Previously, our team used [email protected], which rely on [M69](https://github.com/jitsi/webrtc/tree/cb536cf7a368e77ec29a6779de7fbebf2c300b70), checking the datachannel.cc in that commit, there is not negotiated logic, so everything is happy back on that time.

This PR seems to work for one channel, because it changes the id generation to start from 1 instead of 0, which coincidently matches the id generated in webrtc sdk.

I don't have enough knowledge on webrtc and negotiated flag. What should be a correct fix for this?

Currently, in order to be able to use multiple channels, you can try to manually assign ids to your data channels, start from 1, and +2 for each. Which means:

const channel1 = pc.createDataChannel("label1", {id: 1, maxRetransmits: 0, ordered: false});
const channel2 = pc.createDataChannel("label1", {id: 3, maxRetransmits: 0, ordered: false});
const channel3 = pc.createDataChannel("label1", {id: 5, maxRetransmits: 0, ordered: false});

[xilin]

Maybe we can change the way to get channelId in delegate method? just like android version, stored mId. Then the lib can work correctly. However, it is not a clean fix.

Here is android: https://github.com/react-native-webrtc/react-native-webrtc/blob/master/android/src/main/java/com/oney/WebRTCModule/DataChannelObserver.java#L82

Here is iOS: https://github.com/react-native-webrtc/react-native-webrtc/blob/master/ios/RCTWebRTC/WebRTCModule+RTCDataChannel.m#L81

[saghul]

Your suggestions sounds good to me. Thanks for investigating! Any chance you can send a PR?

[xilin]

@saghul I'll try to submit a PR in the weekend.

[Minishlink]

Hello, after this PR I have a crash when a connection is made. Specifically the crash occurs in dataChannelDidChangeState in RTCDataChannel.m : a NSDictionnary is created with the channel id but channel.originDataChannelId is null.
Note that it doesn't always happen so it looks like a race condition. Do I create a new issue or we can follow it here?

This PR was needed only when you're using multiple data channel for one peer right? If so I'll revert the changes locally in the meantime

[saghul]

Yes please open a new issue.

[Minishlink]

Here it is https://github.com/react-native-webrtc/react-native-webrtc/issues/962, maybe you can take a look @xilin? Thanks