Commit b7c13214 authored by Dominik Röttsches's avatar Dominik Röttsches Committed by Commit Bot

Revert "Disallows reusing RTCIceTransport."

This reverts commit 1aaab3f0.

Reason for revert: Suspected of causing WebKit LayoutTests failures on 10.11, compare https://ci.chromium.org/p/chromium/builders/ci/Mac10.11%20Tests/37268 FindIt points out this CL.


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}

TBR=shampson@chromium.org,steveanton@chromium.org,amithi@chromium.org

Change-Id: I7eca98de36593304f7fbedeb9cfa2b12dc276254
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 10589
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1593379Reviewed-by: default avatarDominik Röttsches <drott@chromium.org>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Cr-Commit-Position: refs/heads/master@{#656325}
parent 39ae9a1d
...@@ -194,15 +194,10 @@ bool RTCIceTransport::HasConsumer() const { ...@@ -194,15 +194,10 @@ 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,21 +102,6 @@ class MODULES_EXPORT RTCIceTransport final ...@@ -102,21 +102,6 @@ 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,14 +115,6 @@ RTCQuicTransport* RTCQuicTransport::Create( ...@@ -115,14 +115,6 @@ 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,7 +4,6 @@ ...@@ -4,7 +4,6 @@
<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>
...@@ -49,23 +48,6 @@ test(t => { ...@@ -49,23 +48,6 @@ 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);
exchangeIceCandidates(pc1, pc2);
await listenToIceConnected(pc1);
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