zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Set the usage pattern bits for adding remote ICE candidates from SDP."

Browse Source 
This reverts commit 7c6f74ab0344e9c6201de711d54026e9990b8e6c.

Reason for revert: Need to merge with stacked changes on bits in a single patch to avoid disruption.

Original change's description:
> Set the usage pattern bits for adding remote ICE candidates from SDP.
> 
> Currently these bits are only set when a remote ICE candidate is
> successfully added via addIceCandidate. For non-trickled sessions in
> which the remote candidates are added via the remote description, these
> bits are lost. This also happens for trickled sessions, though a rare
> case, when addIceCandidate does not succeed because the peer connection
> is not ready to add any remote candidate.
> 
> Bug: webrtc:10868
> Change-Id: Ib2f199f9ffc936060473934d25ba397ef31131a3
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/148880
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28844}

TBR=hta@webrtc.org,qingsi@webrtc.org

Change-Id: Ia0d24b345f04e6c83199d7692bb55a440e6ff464
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10868
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149023
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28845}
This commit is contained in:
Qingsi Wang
2019-08-13 18:29:32 +00:00
committed by Commit Bot
parent 7c6f74ab03
commit d419808e45
2 changed files with 7 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
pc/peer_connection.cc
View File

@ -3695,6 +3695,13 @@ bool PeerConnection::AddIceCandidate(
if (ready) {
bool result = UseCandidate(ice_candidate);
if (result) {
NoteUsageEvent(UsageEvent::REMOTE_CANDIDATE_ADDED);
if (ice_candidate->candidate().address().IsUnresolvedIP()) {
NoteUsageEvent(UsageEvent::REMOTE_MDNS_CANDIDATE_ADDED);
}
if (ice_candidate->candidate().address().IsPrivateIP()) {
NoteUsageEvent(UsageEvent::REMOTE_PRIVATE_CANDIDATE_ADDED);
}
NoteAddIceCandidateResult(kAddIceCandidateSuccess);
} else {
NoteAddIceCandidateResult(kAddIceCandidateFailNotUsable);
@ -6337,13 +6344,6 @@ bool PeerConnection::UseCandidate(const IceCandidateInterface* candidate) {
RTCError error = transport_controller_->AddRemoteCandidates(
result.value()->name, candidates);
if (error.ok()) {
NoteUsageEvent(UsageEvent::REMOTE_CANDIDATE_ADDED);
if (candidate->candidate().address().IsUnresolvedIP()) {
NoteUsageEvent(UsageEvent::REMOTE_MDNS_CANDIDATE_ADDED);
}
if (candidate->candidate().address().IsPrivateIP()) {
NoteUsageEvent(UsageEvent::REMOTE_PRIVATE_CANDIDATE_ADDED);
}
// Candidates successfully submitted for checking.
if (ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew ||
ice_connection_state_ ==

77
pc/peer_connection_histogram_unittest.cc
View File

@ -18,7 +18,6 @@
#include "absl/types/optional.h"
#include "api/call/call_factory_interface.h"
#include "api/jsep.h"
#include "api/jsep_session_description.h"
#include "api/peer_connection_interface.h"
#include "api/peer_connection_proxy.h"
#include "api/rtc_error.h"
@ -33,7 +32,6 @@
#include "pc/peer_connection_wrapper.h"
#include "pc/sdp_utils.h"
#include "pc/test/mock_peer_connection_observers.h"
#include "pc/webrtc_sdp.h"
#include "rtc_base/arraysize.h"
#include "rtc_base/checks.h"
#include "rtc_base/fake_mdns_responder.h"
@ -208,10 +206,6 @@ class PeerConnectionWrapperForUsageHistogramTest
return true;
}
webrtc::PeerConnectionInterface::IceGatheringState ice_gathering_state() {
return pc()->ice_gathering_state();
}
private:
// Candidates that have been sent but not yet configured
std::vector<std::unique_ptr<webrtc::IceCandidateInterface>>
@ -669,77 +663,6 @@ TEST_F(PeerConnectionUsageHistogramTest, FingerprintWithPrivateIPCallee) {
#ifndef WEBRTC_ANDROID
#ifdef HAVE_SCTP
// Test that the usage pattern bits for adding remote private or mDNS candidates
// are set when the remote candidates are retrieved from Offer/Answer SDP
// instead of trickled ICE messages.
TEST_F(PeerConnectionUsageHistogramTest,
AddRemoteCandidatesFromRemoteDescription) {
// We construct the following data-channel-only scenario. The caller collects
// private local candidates and appends them in the Offer as in non-trickled
// sessions. The callee collects mDNS candidates. Only Offer is signaled to
// the callee and we expect a connection with prflx candidates.
auto caller = CreatePeerConnectionWithPrivateLocalAddresses();
auto callee = CreatePeerConnectionWithMdns(RTCConfiguration());
caller->CreateDataChannel("test_channel");
ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
// Wait until the gathering completes (or at least having gathered one
// candidate) so that the session description would have contained ICE
// candidates.
EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceGatheringComplete,
caller->ice_gathering_state(), kDefaultTimeout);
// Get the current offer that contains candidates and pass it to the callee.
//
// Note that we cannot use CloneSessionDescription on |cur_offer| to obtain an
// SDP with candidates. The method above does not strictly copy everything, in
// particular, not copying the ICE candidates.
// TODO(qingsi): Technically, this is a bug. Fix it.
auto cur_offer = caller->pc()->local_description();
ASSERT_TRUE(cur_offer);
std::string sdp_with_candidates_str;
cur_offer->ToString(&sdp_with_candidates_str);
auto offer = absl::make_unique<JsepSessionDescription>(SdpType::kOffer);
ASSERT_TRUE(SdpDeserialize(sdp_with_candidates_str, offer.get(),
nullptr /* error */));
ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
auto answer = callee->CreateAnswer();
callee->SetLocalDescription(CloneSessionDescription(answer.get()));
caller->SetRemoteDescription(std::move(answer));
EXPECT_TRUE_WAIT(caller->IsConnected(), kDefaultTimeout);
EXPECT_TRUE_WAIT(callee->IsConnected(), kDefaultTimeout);
// The callee needs to process the open message to have the data channel open.
EXPECT_TRUE_WAIT(callee->observer()->last_datachannel_ != nullptr,
kDefaultTimeout);
caller->pc()->Close();
callee->pc()->Close();
int expected_fingerprint_caller = MakeUsageFingerprint(
{PeerConnection::UsageEvent::DATA_ADDED,
PeerConnection::UsageEvent::SET_LOCAL_DESCRIPTION_CALLED,
PeerConnection::UsageEvent::SET_REMOTE_DESCRIPTION_CALLED,
PeerConnection::UsageEvent::CANDIDATE_COLLECTED,
PeerConnection::UsageEvent::PRIVATE_CANDIDATE_COLLECTED,
PeerConnection::UsageEvent::ICE_STATE_CONNECTED,
PeerConnection::UsageEvent::CLOSE_CALLED});
int expected_fingerprint_callee = MakeUsageFingerprint(
{PeerConnection::UsageEvent::DATA_ADDED,
PeerConnection::UsageEvent::SET_LOCAL_DESCRIPTION_CALLED,
PeerConnection::UsageEvent::SET_REMOTE_DESCRIPTION_CALLED,
PeerConnection::UsageEvent::CANDIDATE_COLLECTED,
PeerConnection::UsageEvent::MDNS_CANDIDATE_COLLECTED,
PeerConnection::UsageEvent::REMOTE_CANDIDATE_ADDED,
PeerConnection::UsageEvent::REMOTE_PRIVATE_CANDIDATE_ADDED,
PeerConnection::UsageEvent::ICE_STATE_CONNECTED,
PeerConnection::UsageEvent::CLOSE_CALLED});
EXPECT_EQ(2, webrtc::metrics::NumSamples(kUsagePatternMetric));
EXPECT_EQ(1, webrtc::metrics::NumEvents(kUsagePatternMetric,
expected_fingerprint_caller));
EXPECT_EQ(1, webrtc::metrics::NumEvents(kUsagePatternMetric,
expected_fingerprint_callee));
}
TEST_F(PeerConnectionUsageHistogramTest, NotableUsageNoted) {
auto caller = CreatePeerConnection();
caller->CreateDataChannel("foo");