We relied on the default destructor of RefCountedChannel to destroy its
members in reverse initialization order (deleting the DTLS wrapper
before the underlying ICE channel).
However, std::vector also may use the default assignment operator, which
performs a member-wise copy in initialization order. Which results in
deleting the ICE channel before the DTLS one. This CL fixes this by
using a vector of pointers instead of structures, and uses RefCountedObject
to handle ref-counting.
BUG=chromium:672951
Review-Url: https://codereview.webrtc.org/2571683004
Cr-Commit-Position: refs/heads/master@{#15583}
"Crypto required" is a property of the PeerConnection of construction
time; it has nothing to do with SDP. So I'm moving it out of
MediaContentDescription and putting it in the BaseChannel constructor
instead. This is more intuitive, and provides the added assurance that
"secure_required_" can't be flipped from "true" to "false".
BUG=None
Review-Url: https://codereview.webrtc.org/2537343003
Cr-Commit-Position: refs/heads/master@{#15579}
Also make supported protocols explicit in check.
Fix inconsistency where TLS_PROTOCOL_NAME was not exported.
BUG=webrtc:6885
Review-Url: https://codereview.webrtc.org/2570803003
Cr-Commit-Position: refs/heads/master@{#15577}
This CL doesn't start *using* a=bundle-only; it just adds support for
parsing it. We need to do this first, because otherwise old versions of
WebRTC will interpret a zero port value as a rejected m= section.
BUG=webrtc:4674
Review-Url: https://codereview.webrtc.org/2562183002
Cr-Commit-Position: refs/heads/master@{#15558}
Changing the configuration will cause subsequently generated offers to change
the ufrag/pwd as necessary, so that a new round of gathering is started that
uses the new configuration.
This CL also makes some minor unrelated changes: changing the reference SDP in
the PC tests to more match what we generate, and relaxing the network thread
requirement for JsepTransport (since there's no reason the "needs-ice-restart"
flag can't be accessed from the signaling thread).
BUG=webrtc:6714
Review-Url: https://codereview.webrtc.org/2563153002
Cr-Commit-Position: refs/heads/master@{#15527}
Their base class, Transport, still exists, but it now has a more specific
role: a helper class that applies TransportDescriptions. And is renamed
to JsepTransport as a result.
TransportController is now the entity primarily responsible for managing
TransportChannels. It also starts storing pointers to the DTLS and ICE
chanels separately, which will make it easier to remove
TransportChannel/TransportChannelImpl in a subsequent CL.
BUG=None
Review-Url: https://codereview.webrtc.org/2517883002
Cr-Commit-Position: refs/heads/master@{#15453}
Reason for revert:
Deletion of transport.h broke downstream builds.
Going to reland with transport.h containing enums/etc.
Original issue's description:
> Refactoring that removes P2PTransport and DtlsTransport classes.
>
> Their base class, Transport, still exists, but it now has a more specific
> role: a helper class that applies TransportDescriptions. And is renamed
> to JsepTransport as a result.
>
> TransportController is now the entity primarily responsible for managing
> TransportChannels. It also starts storing pointers to the DTLS and ICE
> chanels separately, which will make it easier to remove
> TransportChannel/TransportChannelImpl in a subsequent CL.
>
> BUG=None
>
> Committed: https://crrev.com/bd28681d02dee8c185aeb39207e8154f0ad14a37
> Cr-Commit-Position: refs/heads/master@{#15450}
TBR=pthatcher@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None
Review-Url: https://codereview.webrtc.org/2553043004
Cr-Commit-Position: refs/heads/master@{#15452}
Their base class, Transport, still exists, but it now has a more specific
role: a helper class that applies TransportDescriptions. And is renamed
to JsepTransport as a result.
TransportController is now the entity primarily responsible for managing
TransportChannels. It also starts storing pointers to the DTLS and ICE
chanels separately, which will make it easier to remove
TransportChannel/TransportChannelImpl in a subsequent CL.
BUG=None
Review-Url: https://codereview.webrtc.org/2517883002
Cr-Commit-Position: refs/heads/master@{#15450}
Some applications explicitly require RFC3550 style RTP without ICE.
Port number requirement of RFC3550 section 11 will be addressed in a follow-up CL.
BUG=webrtc:6436
Review-Url: https://codereview.webrtc.org/2377883003
Cr-Commit-Position: refs/heads/master@{#15005}
When the selected connection becomes not receiving and there are many connections,
If we use a round-robin fashion to ping all connections, none of the connections will
be in receiving state for sufficient long time to ensure switching connections promptly.
Triggered check will help in this situation to some extent but it may still fail to switch promptly when there are a lot of connections.
With this CL, if the selected connection is weak, once we find a writable connection on a network we start to ping it with a higher priority to keep it in receiving state.
Plus, if the selected connection is weak, we choose a shorter ping interval (900ms) for all writable connections.
BUG=b/32022719
Review-Url: https://codereview.webrtc.org/2369963004
Cr-Commit-Position: refs/heads/master@{#14991}
Writable() and the related signal are already part of rtc::PacketTransportInterface. Sense of code symmetry aesthetics dictates that receiving() and the related signal should be declared in the same place.
BUG=webrtc:6531
Review-Url: https://codereview.webrtc.org/2444793003
Cr-Commit-Position: refs/heads/master@{#14865}
Introduce a convention on categorizing GN targets:
1. Production code
2. Tests
3. Examples
4. Tools
The first two have targets spread out all over the tree,
while the latter are isolated to examples/ and tools/ directories.
Another new convention: Each directory's BUILD.gn file shall contain
a target named similar to the directory name. This target shall
contain the 'most common' production code, i.e. so that most
consumers of the directory can depend on only the directory
(which implicitly means that target in GN).
//webrtc:webrtc_tests is changed to depend on all WebRTC tests.
From now on, it's necessary to add new test targets to this dependency
tree in order to get them compiled.
Two new group targets are created:
//webrtc/modules/audio_coding:audio_coding_tests
//webrtc/modules/audio_processing:audio_processing_tests
to reduce the long list of tests in //webrtc:webrtc_tests.
Visibility on //webrtc:webrtc and //webrtc:webrtc_tests is restricted
to the root target, to avoid circular dependencies due to the monolithic
property of these targets (a problem we've had in the past).
The 'root' target at the top level is renamed to 'default', which means GN will
build this target instead of _all_ generated targets
(see https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/faq.md#Can-I-control-what-targets-are-built-by-default).
This target now depends on everything we want to build, meaning all targets now
explicitly needs to be wired up from the root target in order to get build.
Having this, the number of compiled objects on Android is decreased from 8855 to 6276. It also gives us better control over our build.
BUG=webrtc:6440
TESTED=git cl try --clobber
NOTRY=True
Review-Url: https://codereview.webrtc.org/2441383002
Cr-Commit-Position: refs/heads/master@{#14821}
The loopback range is 127.0.0.0/8, which is everything from 127.0.0.0 to
127.255.255.255.
BUG=chromium:649118
Review-Url: https://codereview.webrtc.org/2445933003
Cr-Commit-Position: refs/heads/master@{#14807}
Introduce rtc::PacketTransportInterface. Refactor cricket::TransportChannel.
Fix signal slots parameter types in all related code.
BUG=webrtc:6531
Review-Url: https://codereview.webrtc.org/2416023002
Cr-Commit-Position: refs/heads/master@{#14778}
media/ and p2p/ doesn't actually depend on these anymore.
BUG=webrtc:5539
NOTRY=True
Review-Url: https://codereview.webrtc.org/2447533003
Cr-Commit-Position: refs/heads/master@{#14761}
In the swarming test, the machines sometimes were blocked for 1-2 seconds without processing anything.
This CL makes sure that 1 second timeout is only used with fake clock.
BUG=webrtc:6500
Review-Url: https://codereview.webrtc.org/2442813002
Cr-Commit-Position: refs/heads/master@{#14756}
This can be used for a certain security exploit, and doesn't have any
other practical applications we know of.
BUG=chromium:649118
Review-Url: https://codereview.webrtc.org/2440043004
Cr-Commit-Position: refs/heads/master@{#14751}
A rtc::PacketTransportInterface typedef is introduced to allow preparing
downstream projects for the upcoming refactoring of
cricket::Transport. This refactoring will introduce
rtc::PacketTransportInterface in https://codereview.webrtc.org/2416023002/ .
BUG=webrtc:6531
Review-Url: https://codereview.webrtc.org/2429803002
Cr-Commit-Position: refs/heads/master@{#14672}
Reason for revert:
Breaks upstream code.
Original issue's description:
> Prune connections based on network name.
> Previously we prune connections on the same network pointer.
> So if an IPv6 and an IPv4 network are on the same network interface, IPv4 connection won't be pruned even if an IPv6 connection with higher priority becomes writable.
>
> With this change, as long as one connection becomes writable, all connections having lower priority with the same network name will be pruned.
>
> Also simplify the implementation.
>
> BUG=webrtc:6512
>
> Committed: https://crrev.com/aae2784c1fab9d1510393dec15d76caa574e2da8
> Cr-Commit-Position: refs/heads/master@{#14593}
TBR=skvlad@webrtc.org,honghaiz@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6512
Review-Url: https://codereview.webrtc.org/2412433003
Cr-Commit-Position: refs/heads/master@{#14601}
Previously we prune connections on the same network pointer.
So if an IPv6 and an IPv4 network are on the same network interface, IPv4 connection won't be pruned even if an IPv6 connection with higher priority becomes writable.
With this change, as long as one connection becomes writable, all connections having lower priority with the same network name will be pruned.
Also simplify the implementation.
BUG=webrtc:6512
Review-Url: https://codereview.webrtc.org/2395243005
Cr-Commit-Position: refs/heads/master@{#14593}
The Connection class will now blindly forward SignalReadyToSend, and
P2PTransportChannel will decide whether to forward it further (which
it was already doing).
BUG=webrtc:6448
Review-Url: https://codereview.webrtc.org/2374183005
Cr-Commit-Position: refs/heads/master@{#14462}
This means the DTLS handshake can make progress while the SDP answer
containing the fingerprint is still in transit. If the signaling path
if significantly slower than the media path, this can have a moderate
impact on call setup time.
Of course, until the fingerprint is verified no media can be sent. Any
attempted write will result in SR_BLOCK.
This essentially fulfills the requirements of RFC 4572, Section 6.2:
Note that when the offer/answer model is being used, it is possible
for a media connection to outrace the answer back to the offerer.
Thus, if the offerer has offered a 'setup:passive' or 'setup:actpass'
role, it MUST (as specified in RFC 4145 [2]) begin listening for an
incoming connection as soon as it sends its offer. However, it MUST
NOT assume that the data transmitted over the TLS connection is valid
until it has received a matching fingerprint in an SDP answer. If
the fingerprint, once it arrives, does not match the client's
certificate, the server endpoint MUST terminate the media connection
with a bad_certificate error, as stated in the previous paragraph.
BUG=webrtc:6387
Review-Url: https://codereview.webrtc.org/2163683003
Cr-Commit-Position: refs/heads/master@{#14461}
Reason for revert:
It caused some tests in p2ptransportchannel flaky.
Original issue's description:
> Replace RelayPort with TurnPort in p2ptransportchannel tests.
>
> Also remove the relay servers in the tests.
> Most of the code and the downstream apps are using TurnPort, not RelayPort. Most of the tests in this file are not using RelayPort anyway.
>
> BUG=None
> R=deadbeef@webrtc.org
>
> Committed: https://crrev.com/c8d21712dde64c7d613d1ea56c840438505a909f
> Cr-Commit-Position: refs/heads/master@{#14441}
TBR=deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None
Review-Url: https://codereview.webrtc.org/2385563002
Cr-Commit-Position: refs/heads/master@{#14443}
Also remove the relay servers in the tests.
Most of the code and the downstream apps are using TurnPort, not RelayPort. Most of the tests in this file are not using RelayPort anyway.
BUG=None
R=deadbeef@webrtc.org
Review URL: https://codereview.webrtc.org/2380923002 .
Cr-Commit-Position: refs/heads/master@{#14441}
This changes most non-test related rtc_source_set targets to be
rtc_static_library instead. Targets without any .cc files are excluded.
This should bring back the build behavior we used to have with GYP
(i.e. same symbols exported in the libjingle_peerconnection.a file, which
are used by some downstream projects).
After doing an Android build with these changes:
$ nm --defined-only -g -C out/Release/lib.unstripped/libjingle_peerconnection_so.so | grep -i createpeerconnectionf
00077c51 T Java_org_webrtc_PeerConnectionFactory_nativeCreatePeerConnectionFactory
$ nm --defined-only -g -C out/Release/obj/webrtc/api/libjingle_peerconnection.a | grep -i createpeerconnectionf
00000001 T webrtc::CreatePeerConnectionFactory(rtc::Thread*, rtc::Thread*, rtc::Thread*, webrtc::AudioDeviceModule*, cricket::WebRtcVideoEncoderFactory*, cricket::WebRtcVideoDecoderFactory*)
00000001 T webrtc::CreatePeerConnectionFactory()
See https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/cookbook.md#Note-on-static-libraries
for more details on this.
NOTICE: This should be further cleaned up in the future, to reduce
binary bloat and unnecessary linking time. Right now it's more
important to restore the desired build output though.
BUG=webrtc:6410, chromium:630755
Review-Url: https://codereview.webrtc.org/2361623004
Cr-Commit-Position: refs/heads/master@{#14364}
Reason for revert:
Broke a downstream user of SSLStreamAdapter. Need to add the new interface (returning error code instead of bool) in a backwards compatible way.
Original issue's description:
> Allow the DTLS fingerprint verification to occur after the handshake.
>
> This means the DTLS handshake can make progress while the SDP answer
> containing the fingerprint is still in transit. If the signaling path
> if significantly slower than the media path, this can have a moderate
> impact on call setup time.
>
> Of course, until the fingerprint is verified no media can be sent. Any
> attempted write will result in SR_BLOCK.
>
> This essentially fulfills the requirements of RFC 4572, Section 6.2:
>
> Note that when the offer/answer model is being used, it is possible
> for a media connection to outrace the answer back to the offerer.
> Thus, if the offerer has offered a 'setup:passive' or 'setup:actpass'
> role, it MUST (as specified in RFC 4145 [2]) begin listening for an
> incoming connection as soon as it sends its offer. However, it MUST
> NOT assume that the data transmitted over the TLS connection is valid
> until it has received a matching fingerprint in an SDP answer. If
> the fingerprint, once it arrives, does not match the client's
> certificate, the server endpoint MUST terminate the media connection
> with a bad_certificate error, as stated in the previous paragraph.
>
> BUG=webrtc:6387
> R=mattdr@webrtc.org, pthatcher@webrtc.org
>
> Committed: https://crrev.com/042041bf9585f92e962387c59ca805f1218338f9
> Cr-Commit-Position: refs/heads/master@{#14296}
TBR=pthatcher@webrtc.org,mattdr@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6387
Review-Url: https://codereview.webrtc.org/2352863003
Cr-Commit-Position: refs/heads/master@{#14298}
Previously when a Turn port is pruned, if its candidate has been sent to the remote side, the remote side will keep the candidate and use that to create connections.
We now signal the remote side to remove the candidates so that at least no new connection will be created using the removed candidates.
Also updated the virtual socket server to better support our test cases.
1. Allow the virtual socket server to set transit delay for packets sent from a given IP address.
2. Ensure the ordered packet delivery for each socket (Previously the delivery order is enforced on the whole test case, so if a udp packet gets delayed based on its IP address, all TCP packets sent after the UDP packet will be delayed at least until the UDP packet is received).
BUG=webrtc:6380
R=deadbeef@webrtc.org, pthatcher@webrtc.org, skvlad@webrtc.org
Review URL: https://codereview.webrtc.org/2261523004 .
Cr-Commit-Position: refs/heads/master@{#14297}
This means the DTLS handshake can make progress while the SDP answer
containing the fingerprint is still in transit. If the signaling path
if significantly slower than the media path, this can have a moderate
impact on call setup time.
Of course, until the fingerprint is verified no media can be sent. Any
attempted write will result in SR_BLOCK.
This essentially fulfills the requirements of RFC 4572, Section 6.2:
Note that when the offer/answer model is being used, it is possible
for a media connection to outrace the answer back to the offerer.
Thus, if the offerer has offered a 'setup:passive' or 'setup:actpass'
role, it MUST (as specified in RFC 4145 [2]) begin listening for an
incoming connection as soon as it sends its offer. However, it MUST
NOT assume that the data transmitted over the TLS connection is valid
until it has received a matching fingerprint in an SDP answer. If
the fingerprint, once it arrives, does not match the client's
certificate, the server endpoint MUST terminate the media connection
with a bad_certificate error, as stated in the previous paragraph.
BUG=webrtc:6387
R=mattdr@webrtc.org, pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/2163683003 .
Cr-Commit-Position: refs/heads/master@{#14296}
During GN vs GYP auditing it was discovered that some
GN targets that had public_configs were not exposing them
to dependents where the dependent depended on a group, which
in turn included that target as a dependency. Instead of
changing those public_configs to all_dependent_configs
(which would be a change from GYP), it's better to just change
those group targets to use public_deps instead.
BUG=webrtc:6323
NOTRY=True
TESTED=Generated GYP and GN project files on Mac and ran the
tools/gyp_flag_compare.py script before and after this patch was
applied. The file in question used for inspection was the
webrtc/api/webrtcsessiondescriptionfactory.cc
which is a part of the libjingle_peerconnection target.
Review-Url: https://codereview.webrtc.org/2344623002
Cr-Commit-Position: refs/heads/master@{#14222}
It may happen in some legitimate scenarios.
For example a turn port may have had a refresh request timeout, so it won't create a new connection for a ping from an unknown address.
R=deadbeef@webrtc.org
Review URL: https://codereview.webrtc.org/2327233002 .
Cr-Commit-Position: refs/heads/master@{#14173}
