Commit 57862615 authored by Zhenyao Mo's avatar Zhenyao Mo Committed by Chromium LUCI CQ

Revert "Remove the enabling of RTP data channels."

This reverts commit c2dd88a3.

Reason for revert: broke Meet on Mac Canary, crbug.com/1168301

Original change's description:
> Remove the enabling of RTP data channels.
>
> This is the minimum change to prevent the use of RTP datachannels in Chrome,
> designed to be easily rolled back if needed.
>
> Feature link in Chrome dashboard:
> https://chromestatus.com/feature/6485681910054912
>
> Bug: webrtc:6625
> Change-Id: I19c78f703c8e322d8fa21c120c255942cd2bbd3c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2632953
> Reviewed-by: Henrik Boström <hbos@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Commit-Queue: Harald Alvestrand <hta@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#844489}

TBR=hta@chromium.org,hbos@chromium.org,guidou@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:6625
Change-Id: I3cea1636b68ee206c5107a1a8d2c55404236028e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2638598Reviewed-by: default avatarZhenyao Mo <zmo@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844999}
parent d69ae10a
...@@ -52,10 +52,31 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest, CanSetupLegacyCall) { ...@@ -52,10 +52,31 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest, CanSetupLegacyCall) {
MakeTypicalPeerConnectionCall("callWithLegacySdp();"); MakeTypicalPeerConnectionCall("callWithLegacySdp();");
} }
// This test will make a PeerConnection-based call and test an unreliable text
// dataChannel.
// TODO(mallinath) - Remove this test after rtp based data channel is disabled.
IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest, CallWithDataOnly) {
MakeTypicalPeerConnectionCall("callWithDataOnly();");
}
IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest, CallWithSctpDataOnly) { IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest, CallWithSctpDataOnly) {
MakeTypicalPeerConnectionCall("callWithSctpDataOnly();"); MakeTypicalPeerConnectionCall("callWithSctpDataOnly();");
} }
// This test will make a PeerConnection-based call and test an unreliable text
// dataChannel and audio and video tracks.
// TODO(mallinath) - Remove this test after rtp based data channel is disabled.
// Flaky. crbug.com/986872
#if defined(OS_LINUX) || defined(OS_CHROMEOS) || defined(OS_WIN)
#define MAYBE_CallWithDataAndMedia DISABLED_CallWithDataAndMedia
#else
#define MAYBE_CallWithDataAndMedia CallWithDataAndMedia
#endif
IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest,
MAYBE_CallWithDataAndMedia) {
MakeTypicalPeerConnectionCall("callWithDataAndMedia();");
}
#if defined(MEMORY_SANITIZER) #if defined(MEMORY_SANITIZER)
// Fails under MemorySanitizer: http://crbug.com/405951 // Fails under MemorySanitizer: http://crbug.com/405951
#define MAYBE_CallWithSctpDataAndMedia DISABLED_CallWithSctpDataAndMedia #define MAYBE_CallWithSctpDataAndMedia DISABLED_CallWithSctpDataAndMedia
...@@ -67,4 +88,12 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest, ...@@ -67,4 +88,12 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest,
MakeTypicalPeerConnectionCall("callWithSctpDataAndMedia();"); MakeTypicalPeerConnectionCall("callWithSctpDataAndMedia();");
} }
// This test will make a PeerConnection-based call and test an unreliable text
// dataChannel and later add an audio and video track.
// Doesn't work, therefore disabled: https://crbug.com/293252.
IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcDataBrowserTest,
DISABLED_CallWithDataAndLaterAddMedia) {
MakeTypicalPeerConnectionCall("callWithDataAndLaterAddMedia();");
}
} // namespace content } // namespace content
...@@ -36,18 +36,28 @@ ...@@ -36,18 +36,28 @@
return removeBundle(useExternalSdes(sdp)); return removeBundle(useExternalSdes(sdp));
}); });
createConnections({ createConnections({
'mandatory': {'DtlsSrtpKeyAgreement': false} 'mandatory': {'RtpDataChannels': true, 'DtlsSrtpKeyAgreement': false}
}); });
var hasExchanged = promiseDataChannelExchange({reliable: false});
navigator.mediaDevices.getUserMedia({audio: true, video: true}) navigator.mediaDevices.getUserMedia({audio: true, video: true})
.then(addStreamToBothConnectionsAndNegotiate) .then(addStreamToBothConnectionsAndNegotiate)
.catch(failTest); .catch(failTest);
Promise.all([ Promise.all([
hasExchanged,
detectVideoPlaying('remote-view-1'), detectVideoPlaying('remote-view-1'),
detectVideoPlaying('remote-view-2') detectVideoPlaying('remote-view-2')
]).then(reportTestSuccess); ]).then(reportTestSuccess);
} }
// Test only a data channel.
function callWithDataOnly() {
createConnections({optional:[{RtpDataChannels: true}]});
promiseDataChannelExchange({reliable: false})
.then(reportTestSuccess);
negotiate();
}
function callWithSctpDataOnly() { function callWithSctpDataOnly() {
createConnections({optional: [{DtlsSrtpKeyAgreement: true}]}); createConnections({optional: [{DtlsSrtpKeyAgreement: true}]});
promiseSctpDataChannelExchange({reliable: true}) promiseSctpDataChannelExchange({reliable: true})
...@@ -55,6 +65,21 @@ ...@@ -55,6 +65,21 @@
negotiate(); negotiate();
} }
// Test call with audio, video and a data channel.
function callWithDataAndMedia() {
createConnections({optional:[{RtpDataChannels: true}]});
var hasExchanged = promiseDataChannelExchange({reliable: false});
navigator.mediaDevices.getUserMedia({audio: true, video: true})
.then(addStreamToBothConnectionsAndNegotiate)
.catch(failTest);
Promise.all([
hasExchanged,
detectVideoPlaying('remote-view-1'),
detectVideoPlaying('remote-view-2')
]).then(reportTestSuccess);
}
function callWithSctpDataAndMedia() { function callWithSctpDataAndMedia() {
createConnections({optional: [{DtlsSrtpKeyAgreement: true}]}); createConnections({optional: [{DtlsSrtpKeyAgreement: true}]});
var hasExchanged = promiseSctpDataChannelExchange({reliable: true}); var hasExchanged = promiseSctpDataChannelExchange({reliable: true});
...@@ -69,6 +94,27 @@ ...@@ -69,6 +94,27 @@
]).then(reportTestSuccess); ]).then(reportTestSuccess);
} }
// Test call with a data channel and later add audio and video.
function callWithDataAndLaterAddMedia() {
createConnections({optional:[{RtpDataChannels: true}]});
var hasExchanged = promiseDataChannelExchange({reliable: false});
negotiate();
// Set an event handler for when the data channel has been closed.
hasExchanged.then(() => {
// When the video is flowing the test is done.
return navigator.mediaDevices.getUserMedia({audio: true, video: true})
.then(addStreamToBothConnectionsAndNegotiate);
}).then(() => {
return Promise.all([
detectVideoPlaying('remote-view-1'),
detectVideoPlaying('remote-view-2')
]);
})
.then(reportTestSuccess)
.catch(failTest);
}
// This function is used for setting up a test that: // This function is used for setting up a test that:
// 1. Creates a data channel on |gFirstConnection| and sends data to // 1. Creates a data channel on |gFirstConnection| and sends data to
// |gSecondConnection|. // |gSecondConnection|.
......
...@@ -112,6 +112,7 @@ const char kIceRestart[] = "IceRestart"; ...@@ -112,6 +112,7 @@ const char kIceRestart[] = "IceRestart";
const char kUseRtpMux[] = "googUseRtpMUX"; const char kUseRtpMux[] = "googUseRtpMUX";
// Below constraints should be used during PeerConnection construction. // Below constraints should be used during PeerConnection construction.
const char kEnableDtlsSrtp[] = "DtlsSrtpKeyAgreement"; const char kEnableDtlsSrtp[] = "DtlsSrtpKeyAgreement";
const char kEnableRtpDataChannels[] = "RtpDataChannels";
// Google-specific constraint keys. // Google-specific constraint keys.
// TODO(hta): These need to be made standard or deleted. crbug.com/605673 // TODO(hta): These need to be made standard or deleted. crbug.com/605673
const char kEnableDscp[] = "googDscp"; const char kEnableDscp[] = "googDscp";
...@@ -366,6 +367,16 @@ static void ParseOldStyleNames( ...@@ -366,6 +367,16 @@ static void ParseOldStyleNames(
WebFeature::kRTCConstraintEnableDtlsSrtpFalse); WebFeature::kRTCConstraintEnableDtlsSrtpFalse);
} }
result.enable_dtls_srtp.SetExact(ToBoolean(constraint.value_)); result.enable_dtls_srtp.SetExact(ToBoolean(constraint.value_));
} else if (constraint.name_.Equals(kEnableRtpDataChannels)) {
bool value = ToBoolean(constraint.value_);
if (value) {
Deprecation::CountDeprecation(
context, WebFeature::kRTCConstraintEnableRtpDataChannelsTrue);
} else {
UseCounter::Count(context,
WebFeature::kRTCConstraintEnableRtpDataChannelsFalse);
}
result.enable_rtp_data_channels.SetExact(ToBoolean(constraint.value_));
} else if (constraint.name_.Equals(kEnableDscp)) { } else if (constraint.name_.Equals(kEnableDscp)) {
result.enable_dscp.SetExact(ToBoolean(constraint.value_)); result.enable_dscp.SetExact(ToBoolean(constraint.value_));
} else if (constraint.name_.Equals(kEnableIPv6)) { } else if (constraint.name_.Equals(kEnableIPv6)) {
......
<!DOCTYPE html>
<html>
<head>
<title>RTCPeerConnection data channel type collison</title>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
</head>
</body>
<script>
promise_test(t => {
// This test specifically verifies fix to crbug.com/1030628
var mediaConstraints = {
optional: [{ RtpDataChannels: true }]
}
var pc = new RTCPeerConnection({}, mediaConstraints)
t.add_cleanup(() => pc.close());
pc.createDataChannel('')
var sd = new RTCSessionDescription({
type: 'offer',
sdp: 'v=0\r\no=- 2512342596970960666 2 IN IP4 127.0.0.1\r\ns=-\r\nt=0 0\r\na=group:BUNDLE data\r\na=msid-semantic: WMS\r\nm=application 9 UDP/DTLS/SCTP webrtc-datachannel\r\nc=IN IP4 0.0.0.0\r\na=ice-ufrag:xwIt\r\na=ice-pwd:WnJ0omUfzZhL3WX63jJH5mU8\r\na=ice-options:trickle\r\na=fingerprint:sha-256 E5:61:EC:E3:D4:04:55:16:E9:47:95:A7:4E:0C:82:10:A2:70:82:4F:59:29:99:B2:66:40:F2:46:86:A6:23:A3\r\na=setup:actpass\r\na=mid:data\r\na=sctp-port:5000\r\na=max-message-size:262144\r\n',
})
return promise_rejects_dom(t, "InvalidAccessError", pc.setRemoteDescription(sd));
}, 'Local RTP channel, remote offer of SCTP channel');
promise_test(async t => {
var pc = new RTCPeerConnection();
t.add_cleanup(() => pc.close());
pc.createDataChannel('');
offer = await pc.createOffer();
assert_regexp_match(offer.sdp, /m=application .* webrtc-datachannel/);
assert_equals(offer.sdp.match(/a=rtpmap:\d+ google-data/), null);
}, 'Asking for default Data Channels generates appropriate offer');
promise_test(async t => {
const rtpConstraints = { optional: [{ RtpDataChannels: true}]};
var pc = new RTCPeerConnection({}, rtpConstraints);
t.add_cleanup(() => pc.close());
pc.createDataChannel('');
offer = await pc.createOffer();
assert_equals(offer.sdp.match(/m=application .* webrtc-datachannel/), null);
assert_regexp_match(offer.sdp, /a=rtpmap:\d+ google-data/);
}, 'Asking for RTP Data Channels generates appropriate offer');
</script>
</body>
</html>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment