Remove posting of ICE messages from WebRTCSession in PeerConnection to signaling thread.
These callbacks are called from signal thread already. There is no point
in posting messages on the same thread again.
BUG=2922
R=fischman@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/9219004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@5626 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java b/talk/app/webrtc/javatests/src/org/webrtc/PeerConnectionTest.java
index b4c96b1..fddb503 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 @@
@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 b404ec4..72f3717 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -71,17 +71,6 @@
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<const webrtc::JsepIceCandidate> candidate;
};
struct SetSessionDescriptionMsg : public talk_base::MessageData {
@@ -669,24 +658,6 @@
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<CandidateMsg*>(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::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 64acad0..50f9df0 100644
--- a/talk/app/webrtc/webrtcsession_unittest.cc
+++ b/talk/app/webrtc/webrtcsession_unittest.cc
@@ -856,6 +856,7 @@
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 @@
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 6d449b3..f79fccc 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.TestDataChannel
PeerConnectionInterfaceTest.TestSendBinaryOnRtpDataChannel
PeerConnectionInterfaceTest.TestSendOnlyDataChannel
-# Tracked in https://code.google.com/p/webrtc/issues/detail?id=2924.
-WebRtcSessionTest.TestIceStatesBundle