diff --git a/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java b/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java index b4c96b1701..fddb503450 100644 --- a/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java +++ b/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java @@ -175,17 +175,7 @@ public class PeerConnectionTest extends TestCase { @Override public synchronized void onIceConnectionChange( IceConnectionState newState) { - // This is a bit crazy. The offerer goes CHECKING->CONNECTED->COMPLETED - // mostly, but sometimes the middle CONNECTED is delivered as COMPLETED. - // Assuming a bug in underlying libjingle but compensating for it here to - // green the tree. - // TODO(fischman): either remove the craxy logic below when libjingle is - // fixed or rewrite the comment above if what libjingle is doing is - // actually legit. - assertTrue( - expectedIceConnectionChanges.remove(newState) || - (newState == IceConnectionState.COMPLETED && - expectedIceConnectionChanges.remove(IceConnectionState.CONNECTED))); + assertEquals(expectedIceConnectionChanges.removeFirst(), newState); } public synchronized void expectIceGatheringChange( diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc index b404ec4a9e..72f37174be 100644 --- a/talk/app/webrtc/peerconnection.cc +++ b/talk/app/webrtc/peerconnection.cc @@ -71,17 +71,6 @@ enum { MSG_SET_SESSIONDESCRIPTION_SUCCESS = 0, MSG_SET_SESSIONDESCRIPTION_FAILED, MSG_GETSTATS, - MSG_ICECONNECTIONCHANGE, - MSG_ICEGATHERINGCHANGE, - MSG_ICECANDIDATE, - MSG_ICECOMPLETE, -}; - -struct CandidateMsg : public talk_base::MessageData { - explicit CandidateMsg(const webrtc::JsepIceCandidate* candidate) - : candidate(candidate) { - } - talk_base::scoped_ptr candidate; }; struct SetSessionDescriptionMsg : public talk_base::MessageData { @@ -669,24 +658,6 @@ void PeerConnection::OnMessage(talk_base::Message* msg) { delete param; break; } - case MSG_ICECONNECTIONCHANGE: { - observer_->OnIceConnectionChange(ice_connection_state_); - break; - } - case MSG_ICEGATHERINGCHANGE: { - observer_->OnIceGatheringChange(ice_gathering_state_); - break; - } - case MSG_ICECANDIDATE: { - CandidateMsg* data = static_cast(msg->pdata); - observer_->OnIceCandidate(data->candidate.get()); - delete data; - break; - } - case MSG_ICECOMPLETE: { - observer_->OnIceComplete(); - break; - } default: ASSERT(false && "Not implemented"); break; @@ -758,35 +729,29 @@ void PeerConnection::OnRemoveLocalStream(MediaStreamInterface* stream) { void PeerConnection::OnIceConnectionChange( PeerConnectionInterface::IceConnectionState new_state) { + ASSERT(signaling_thread()->IsCurrent()); ice_connection_state_ = new_state; - signaling_thread()->Post(this, MSG_ICECONNECTIONCHANGE); + observer_->OnIceConnectionChange(ice_connection_state_); } void PeerConnection::OnIceGatheringChange( PeerConnectionInterface::IceGatheringState new_state) { + ASSERT(signaling_thread()->IsCurrent()); if (IsClosed()) { return; } ice_gathering_state_ = new_state; - signaling_thread()->Post(this, MSG_ICEGATHERINGCHANGE); + observer_->OnIceGatheringChange(ice_gathering_state_); } void PeerConnection::OnIceCandidate(const IceCandidateInterface* candidate) { - JsepIceCandidate* candidate_copy = NULL; - if (candidate) { - // TODO(ronghuawu): Make IceCandidateInterface reference counted instead - // of making a copy. - candidate_copy = new JsepIceCandidate(candidate->sdp_mid(), - candidate->sdp_mline_index(), - candidate->candidate()); - } - // The Post takes the ownership of the |candidate_copy|. - signaling_thread()->Post(this, MSG_ICECANDIDATE, - new CandidateMsg(candidate_copy)); + ASSERT(signaling_thread()->IsCurrent()); + observer_->OnIceCandidate(candidate); } void PeerConnection::OnIceComplete() { - signaling_thread()->Post(this, MSG_ICECOMPLETE); + ASSERT(signaling_thread()->IsCurrent()); + observer_->OnIceComplete(); } void PeerConnection::ChangeSignalingState( diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 64acad0e18..50f9df01fc 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -856,6 +856,7 @@ class WebRtcSessionTest : public testing::Test { EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionChecking, observer_.ice_connection_state_, kIceCandidatesTimeout); + // The ice connection state is "Connected" too briefly to catch in a test. EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted, observer_.ice_connection_state_, @@ -2649,10 +2650,11 @@ TEST_F(WebRtcSessionTest, TestIceStatesBasic) { TestLoopbackCall(); } -// Runs the loopback call test with BUNDLE, STUN, and TCP enabled. +// Runs the loopback call test with BUNDLE and STUN enabled. TEST_F(WebRtcSessionTest, TestIceStatesBundle) { allocator_.set_flags(cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | cricket::PORTALLOCATOR_ENABLE_BUNDLE | + cricket::PORTALLOCATOR_DISABLE_TCP | cricket::PORTALLOCATOR_DISABLE_RELAY); TestLoopbackCall(); } diff --git a/tools/valgrind-webrtc/gtest_exclude/libjingle_peerconnection_unittest.gtest-memcheck.txt b/tools/valgrind-webrtc/gtest_exclude/libjingle_peerconnection_unittest.gtest-memcheck.txt index 6d449b3999..f79fccc282 100644 --- a/tools/valgrind-webrtc/gtest_exclude/libjingle_peerconnection_unittest.gtest-memcheck.txt +++ b/tools/valgrind-webrtc/gtest_exclude/libjingle_peerconnection_unittest.gtest-memcheck.txt @@ -28,5 +28,3 @@ PeerConnectionInterfaceTest.DataChannelCloseWhenPeerConnectionClose PeerConnectionInterfaceTest.TestDataChannel PeerConnectionInterfaceTest.TestSendBinaryOnRtpDataChannel PeerConnectionInterfaceTest.TestSendOnlyDataChannel -# Tracked in https://code.google.com/p/webrtc/issues/detail?id=2924. -WebRtcSessionTest.TestIceStatesBundle