Update RTCPeerConnection-helper.js

[youennf]

Take RTCPeerConnection-helper.js changes from #13499
This contains changes that should not affect existing tests but might be required to land new tests or modified tests from #13499.

Compared to #13499, no change have been made yet to createDataChannelPair since this seems to change behavior of existing tests.
A separate PR could be envisioned to land these changes separately.

[youennf]

@lgrahl, PTAL

[lgrahl]

Looks good to me.

One thing: Considering the amount of effort put into this work, I would prefer the commit in the three PRs being authored by myself (git commit --author "Lennart Grahl <[email protected]>") and having you as a co-author if desired. Please also include the original PR number in the commit message. Otherwise the context is lost. Thanks!

[alvestrand]

@jan-ivar will suggest a better way to handle early candidates at callee than the one proposed here.

[lgrahl]

@jan-ivar I would like to get my tests finally merged and a breaking change to this (which has been carefully crafted to not break anything else) may need further refactoring of my tests which I would really like to avoid. Unless this would radically improve things, I'd suggest to merge this for now and do the refactoring later.

[youennf]

This PR no longer changes any ICE candidate processing.
@jan-ivar, does this address all your concerns?

[foolip]

This PR is blocked on the required "Travis CI - Pull Request" check after #14499. In order to trigger it, I will close and reopen this PR.

[jan-ivar]

Code is not broken, but note my reservations.

[jan-ivar]

Ok, but I dislike helpers options when they turn into replacement APIs.

doSignalingHandshake(pc1, pc2, {modfiyOffer: offer => myRegExp(offer)});

The above produces a false positive, due to my typo (did you catch it?), a problem the spec API does not have:

await pc1.setLocalDescription(myRegExp(await pc1.createOffer()));
await pc2.setRemoteDescription(pc1.localDescription);
await pc2.setLocalDescription(await pc2.createAnswer());
await pc1.setRemoteDescription(pc2.localDescription);

The above is appropriate for the (few) who need to mangle SDP IMHO. If not, I'd prefer the more typo-safe:

doSignalingHandshakeModifyOfferAnswer(pc1, pc2, offer => myRegExp(offer), answer => answer);

As to plain doSignalingHandshake, once crbug 740501 is fixed, could we use the spec-provided API instead?

[youennf]

doSignalingHandshakeModifyOfferAnswer seems better to me as well.
Let's merge this PR for now, that will allow making progress on merging the data channel tests.
We can improve on it later as a refactoring PR.

[youennf]

Filed #14633 to track the refactoring PR.