diff options
7 files changed, 70 insertions, 62 deletions
diff --git a/remoting/protocol/jingle_session.cc b/remoting/protocol/jingle_session.cc index 3293396..713dc06 100644 --- a/remoting/protocol/jingle_session.cc +++ b/remoting/protocol/jingle_session.cc @@ -86,6 +86,7 @@ JingleSession::JingleSession( server_cert_(server_cert), state_(INITIALIZING), closed_(false), + closing_(false), cricket_session_(NULL), event_channel_(NULL), video_channel_(NULL), @@ -113,17 +114,16 @@ void JingleSession::Init(cricket::Session* cricket_session) { cert_verifier_.reset(new net::CertVerifier()); cricket_session_->SignalState.connect( this, &JingleSession::OnSessionState); + cricket_session_->SignalError.connect( + this, &JingleSession::OnSessionError); } -void JingleSession::CloseInternal(Task* closed_task, int result) { - if (MessageLoop::current() != jingle_session_manager_->message_loop()) { - jingle_session_manager_->message_loop()->PostTask( - FROM_HERE, NewRunnableMethod(this, &JingleSession::CloseInternal, - closed_task, result)); - return; - } +void JingleSession::CloseInternal(int result, bool failed) { + DCHECK_EQ(jingle_session_manager_->message_loop(), MessageLoop::current()); + + if (!closed_ && !closing_) { + closing_ = true; - if (!closed_) { if (control_ssl_socket_.get()) control_ssl_socket_.reset(); @@ -165,16 +165,14 @@ void JingleSession::CloseInternal(Task* closed_task, int result) { if (cricket_session_) cricket_session_->Terminate(); - SetState(CLOSED); + if (failed) + SetState(FAILED); + else + SetState(CLOSED); closed_ = true; } cert_verifier_.reset(); - - if (closed_task) { - closed_task->Run(); - delete closed_task; - } } bool JingleSession::HasSession(cricket::Session* cricket_session) { @@ -279,13 +277,29 @@ void JingleSession::set_receiver_token(const std::string& receiver_token) { } void JingleSession::Close(Task* closed_task) { - CloseInternal(closed_task, net::ERR_CONNECTION_CLOSED); + if (MessageLoop::current() != jingle_session_manager_->message_loop()) { + jingle_session_manager_->message_loop()->PostTask( + FROM_HERE, NewRunnableMethod(this, &JingleSession::Close, closed_task)); + return; + } + + CloseInternal(net::ERR_CONNECTION_CLOSED, false); + + if (closed_task) { + closed_task->Run(); + delete closed_task; + } } void JingleSession::OnSessionState( BaseSession* session, BaseSession::State state) { DCHECK_EQ(cricket_session_, session); + if (closed_) { + // Don't do anything if we already closed. + return; + } + switch (state) { case cricket::Session::STATE_SENTINITIATE: case cricket::Session::STATE_RECEIVEDINITIATE: @@ -315,6 +329,15 @@ void JingleSession::OnSessionState( } } +void JingleSession::OnSessionError( + BaseSession* session, BaseSession::Error error) { + DCHECK_EQ(cricket_session_, session); + + if (error != cricket::Session::ERROR_NONE) { + CloseInternal(net::ERR_CONNECTION_ABORTED, true); + } +} + void JingleSession::OnInitiate() { jid_ = cricket_session_->remote_name(); @@ -362,7 +385,10 @@ void JingleSession::OnInitiate() { if (!cricket_session_->initiator()) jingle_session_manager_->AcceptConnection(this, cricket_session_); - SetState(CONNECTING); + if (!closed_) { + // Set state to CONNECTING if the session is being accepted. + SetState(CONNECTING); + } } bool JingleSession::EstablishSSLConnection( @@ -450,44 +476,14 @@ void JingleSession::OnAccept() { } void JingleSession::OnTerminate() { - if (control_channel_adapter_.get()) - control_channel_adapter_->Close(net::ERR_CONNECTION_ABORTED); - if (control_channel_) { - control_channel_->OnSessionTerminate(cricket_session_); - control_channel_ = NULL; - } - - if (event_channel_adapter_.get()) - event_channel_adapter_->Close(net::ERR_CONNECTION_ABORTED); - if (event_channel_) { - event_channel_->OnSessionTerminate(cricket_session_); - event_channel_ = NULL; - } - - if (video_channel_adapter_.get()) - video_channel_adapter_->Close(net::ERR_CONNECTION_ABORTED); - if (video_channel_) { - video_channel_->OnSessionTerminate(cricket_session_); - video_channel_ = NULL; - } - - if (video_rtp_channel_.get()) - video_rtp_channel_->Close(net::ERR_CONNECTION_ABORTED); - if (video_rtcp_channel_.get()) - video_rtcp_channel_->Close(net::ERR_CONNECTION_ABORTED); - - SetState(CLOSED); - - closed_ = true; + CloseInternal(net::ERR_CONNECTION_ABORTED, false); } void JingleSession::OnSSLConnect(int result) { DCHECK(!closed_); if (result != net::OK) { LOG(ERROR) << "Error during SSL connection: " << result; - // TODO(hclam): Just setting the state is not enough. Need to invoke a - // callback to report failure. - CloseInternal(NULL, result); + CloseInternal(result, true); return; } @@ -503,6 +499,9 @@ void JingleSession::OnSSLConnect(int result) { void JingleSession::SetState(State new_state) { if (new_state != state_) { + DCHECK_NE(state_, CLOSED); + DCHECK_NE(state_, FAILED); + state_ = new_state; if (!closed_ && state_change_callback_.get()) state_change_callback_->Run(new_state); diff --git a/remoting/protocol/jingle_session.h b/remoting/protocol/jingle_session.h index 94c28f6..617ded8 100644 --- a/remoting/protocol/jingle_session.h +++ b/remoting/protocol/jingle_session.h @@ -92,7 +92,7 @@ class JingleSession : public protocol::Session, void Init(cricket::Session* cricket_session); // Close all the channels and terminate the session. - void CloseInternal(Task* closed_task, int result); + void CloseInternal(int result, bool failed); bool HasSession(cricket::Session* cricket_session); cricket::Session* ReleaseSession(); @@ -106,6 +106,9 @@ class JingleSession : public protocol::Session, // Used for Session.SignalState sigslot. void OnSessionState(cricket::BaseSession* session, cricket::BaseSession::State state); + // Used for Session.SignalError sigslot. + void OnSessionError(cricket::BaseSession* session, + cricket::BaseSession::Error error); void OnInitiate(); void OnAccept(); @@ -129,6 +132,7 @@ class JingleSession : public protocol::Session, scoped_ptr<StateChangeCallback> state_change_callback_; bool closed_; + bool closing_; // JID of the other side. Set when the connection is initialized, // and never changed after that. diff --git a/remoting/protocol/jingle_session_unittest.cc b/remoting/protocol/jingle_session_unittest.cc index 18bf2b0..692b2f0 100644 --- a/remoting/protocol/jingle_session_unittest.cc +++ b/remoting/protocol/jingle_session_unittest.cc @@ -587,15 +587,13 @@ TEST_F(JingleSessionTest, RejectConnection) { } // Verify that we can connect two endpoints. -// Disabled, http://crbug.com/70225. -TEST_F(JingleSessionTest, DISABLED_Connect) { +TEST_F(JingleSessionTest, Connect) { CreateServerPair(); ASSERT_TRUE(InitiateConnection()); } // Verify that data can be transmitted over the event channel. -// Disabled, http://crbug.com/70225. -TEST_F(JingleSessionTest, DISABLED_TestControlChannel) { +TEST_F(JingleSessionTest, TestControlChannel) { CreateServerPair(); ASSERT_TRUE(InitiateConnection()); scoped_refptr<TCPChannelTester> tester( @@ -625,8 +623,7 @@ TEST_F(JingleSessionTest, TestVideoChannel) { } // Verify that data can be transmitted over the event channel. -// Disabled, http://crbug.com/70225. -TEST_F(JingleSessionTest, DISABLED_TestEventChannel) { +TEST_F(JingleSessionTest, TestEventChannel) { CreateServerPair(); ASSERT_TRUE(InitiateConnection()); scoped_refptr<TCPChannelTester> tester( diff --git a/remoting/protocol/session.h b/remoting/protocol/session.h index 286dbe7..3051a68 100644 --- a/remoting/protocol/session.h +++ b/remoting/protocol/session.h @@ -82,8 +82,9 @@ class Session : public base::RefCountedThreadSafe<Session> { virtual const std::string& receiver_token() = 0; virtual void set_receiver_token(const std::string& receiver_token) = 0; - // Closes connection. Callbacks are guaranteed not to be called after - // |closed_task| is executed. + // Closes connection. Callbacks are guaranteed not to be called + // after |closed_task| is executed. Must be called before the object + // is destroyed, unless the state is set to FAILED or CLOSED. virtual void Close(Task* closed_task) = 0; protected: diff --git a/tools/valgrind/gtest_exclude/remoting_unittests.gtest-drmemory_win32.txt b/tools/valgrind/gtest_exclude/remoting_unittests.gtest-drmemory_win32.txt index 3b09ec1..4e0696a 100644 --- a/tools/valgrind/gtest_exclude/remoting_unittests.gtest-drmemory_win32.txt +++ b/tools/valgrind/gtest_exclude/remoting_unittests.gtest-drmemory_win32.txt @@ -1,10 +1,10 @@ -# These tests fail under Dr. Memory; see http://crbug.com/57832 +# Following tests create real libjingle connections, and libjingle has +# hardcoded timeouts, so these tests fail under TSan. +JingleSessionTest.Connect JingleSessionTest.TestControlChannel JingleSessionTest.TestEventChannel JingleSessionTest.TestVideoChannel JingleSessionTest.TestVideoRtpChannel -# http://crbug.com/70225 -JingleSessionTest.Connect # This test fails on an assertion, see http://crbug.com/57266 DecoderVp8Test.EncodeAndDecode diff --git a/tools/valgrind/gtest_exclude/remoting_unittests.gtest-tsan.txt b/tools/valgrind/gtest_exclude/remoting_unittests.gtest-tsan.txt index 0703323..5a8a7cb 100644 --- a/tools/valgrind/gtest_exclude/remoting_unittests.gtest-tsan.txt +++ b/tools/valgrind/gtest_exclude/remoting_unittests.gtest-tsan.txt @@ -1,4 +1,6 @@ -# Fail under TSan, see http://crbug.com/57832 +# Following tests create real libjingle connections, and libjingle has +# hardcoded timeouts, so these tests fail under TSan. +JingleSessionTest.Connect JingleSessionTest.TestControlChannel JingleSessionTest.TestEventChannel JingleSessionTest.TestVideoChannel diff --git a/tools/valgrind/gtest_exclude/remoting_unittests.gtest-tsan_win32.txt b/tools/valgrind/gtest_exclude/remoting_unittests.gtest-tsan_win32.txt index 6b0a66f..5a8a7cb 100644 --- a/tools/valgrind/gtest_exclude/remoting_unittests.gtest-tsan_win32.txt +++ b/tools/valgrind/gtest_exclude/remoting_unittests.gtest-tsan_win32.txt @@ -1,2 +1,7 @@ -# Fails under TSan/Win, see http://crbug.com/70225 +# Following tests create real libjingle connections, and libjingle has +# hardcoded timeouts, so these tests fail under TSan. JingleSessionTest.Connect +JingleSessionTest.TestControlChannel +JingleSessionTest.TestEventChannel +JingleSessionTest.TestVideoChannel +JingleSessionTest.TestVideoRtpChannel |