Commit a817c133 authored by Seth Hampson's avatar Seth Hampson Committed by Commit Bot

Reland "Disallows reusing RTCIceTransport."

This is a reland of 1aaab3f0

Original change's description:
> Disallows reusing RTCIceTransport.
>
> Currently if an RTCIceTransport that comes from an RTCPeerConnection is
> reused to create and RTCQuicTransport there is a crash. This disallows
> this from happening by throwing an exception if the client attempts to
> do this.
>
> Bug: 10589
> Change-Id: Idcf538972516825fef3b275d062a19d0bd7a68c5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1593872
> Commit-Queue: Seth Hampson <shampson@chromium.org>
> Reviewed-by: Steve Anton <steveanton@chromium.org>
> Auto-Submit: Seth Hampson <shampson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#656241}

Bug: webrtc:10589
Change-Id: I879d3149e3fd3585e1efa395f63917bc94970efd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1594691Reviewed-by: default avatarSteve Anton <steveanton@chromium.org>
Commit-Queue: Seth Hampson <shampson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#656442}
parent 3e60ca69
...@@ -194,10 +194,15 @@ bool RTCIceTransport::HasConsumer() const { ...@@ -194,10 +194,15 @@ bool RTCIceTransport::HasConsumer() const {
return consumer_; return consumer_;
} }
bool RTCIceTransport::IsFromPeerConnection() const {
return peer_connection_;
}
IceTransportProxy* RTCIceTransport::ConnectConsumer( IceTransportProxy* RTCIceTransport::ConnectConsumer(
RTCQuicTransport* consumer) { RTCQuicTransport* consumer) {
DCHECK(consumer); DCHECK(consumer);
DCHECK(proxy_); DCHECK(proxy_);
DCHECK(!peer_connection_);
if (!consumer_) { if (!consumer_) {
consumer_ = consumer; consumer_ = consumer;
} else { } else {
......
...@@ -102,6 +102,21 @@ class MODULES_EXPORT RTCIceTransport final ...@@ -102,6 +102,21 @@ class MODULES_EXPORT RTCIceTransport final
// a QuicTransportProxy. It may be called repeatedly with the same // a QuicTransportProxy. It may be called repeatedly with the same
// RTCQuicTransport. // RTCQuicTransport.
bool HasConsumer() const; bool HasConsumer() const;
// If |this| was created from an RTCPeerConnection.
//
// Background: This is because we don't reuse an RTCIceTransport that has been
// created from an RTCPeerConnection for an RTCQuicTransport (see
// bugs.webrtc.org/10591). The core issue here is that the source of truth for
// connecting a consumer to ICE is at the P2PTransportChannel. In the case of
// RTCPeerConnection, the P2PTransportChannel is already connected and given
// to the RTCIceTransport. In the case of the RTCQuicTransport it uses the
// RTCIceTransport as the source of truth for enforcing just one connected
// consumer. Possible fixes to this issue could include: -Use the
// P2PTransportChannel as the source of truth directly (calling this
// synchronously from the main thread)
// -Asynchronously connect to the P2PTransport - if the count of connected
// transports to the P2PTransportChannel is > 1, then throw an exception.
bool IsFromPeerConnection() const;
IceTransportProxy* ConnectConsumer(RTCQuicTransport* consumer); IceTransportProxy* ConnectConsumer(RTCQuicTransport* consumer);
void DisconnectConsumer(RTCQuicTransport* consumer); void DisconnectConsumer(RTCQuicTransport* consumer);
......
...@@ -115,6 +115,14 @@ RTCQuicTransport* RTCQuicTransport::Create( ...@@ -115,6 +115,14 @@ RTCQuicTransport* RTCQuicTransport::Create(
"has a connected RTCQuicTransport."); "has a connected RTCQuicTransport.");
return nullptr; return nullptr;
} }
if (transport->IsFromPeerConnection()) {
exception_state.ThrowDOMException(
DOMExceptionCode::kInvalidStateError,
"Cannot construct an RTCQuicTransport "
"with an RTCIceTransport that came from an "
"RTCPeerConnection.");
return nullptr;
}
for (const auto& certificate : certificates) { for (const auto& certificate : certificates) {
if (certificate->expires() < ConvertSecondsToDOMTimeStamp(CurrentTime())) { if (certificate->expires() < ConvertSecondsToDOMTimeStamp(CurrentTime())) {
exception_state.ThrowTypeError( exception_state.ThrowTypeError(
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
<script src="/resources/testharness.js"></script> <script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script> <script src="/resources/testharnessreport.js"></script>
<script src="../webrtc/RTCIceTransport-extension-helper.js"></script> <script src="../webrtc/RTCIceTransport-extension-helper.js"></script>
<script src="../webrtc/RTCPeerConnection-helper.js"></script>
<script src="RTCQuicTransport-helper.js"></script> <script src="RTCQuicTransport-helper.js"></script>
<script src="../webrtc/dictionary-helper.js"></script> <script src="../webrtc/dictionary-helper.js"></script>
<script> <script>
...@@ -48,6 +49,21 @@ test(t => { ...@@ -48,6 +49,21 @@ test(t => {
}, 'RTCQuicTransport constructor throws if passed an RTCIceTransport that ' + }, 'RTCQuicTransport constructor throws if passed an RTCIceTransport that ' +
'already has an active RTCQuicTransport.'); 'already has an active RTCQuicTransport.');
promise_test(async t => {
const pc1 = new RTCPeerConnection();
t.add_cleanup(() => pc1.close());
const pc2 = new RTCPeerConnection();
t.add_cleanup(() => pc2.close());
pc1.createDataChannel('test');
await doSignalingHandshake(pc1, pc2);
const iceTransport = pc1.sctp.transport.iceTransport;
assert_throws('InvalidStateError',
() => makeQuicTransport(t, iceTransport));
}, 'RTCQuicTransport constructor throws if passed an RTCIceTransport that ' +
'came from an RTCPeerConnection.');
test(t => { test(t => {
const quicTransport = makeStandaloneQuicTransport(t); const quicTransport = makeStandaloneQuicTransport(t);
quicTransport.stop(); quicTransport.stop();
......
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