diff --git a/talk/app/webrtc/dtlsidentitystore.cc b/talk/app/webrtc/dtlsidentitystore.cc index 16ed7989e..7beb68d13 100644 --- a/talk/app/webrtc/dtlsidentitystore.cc +++ b/talk/app/webrtc/dtlsidentitystore.cc @@ -44,48 +44,8 @@ enum { }; typedef rtc::ScopedMessageData IdentityResultMessageData; - } // namespace -// This class runs on the worker thread to generate the identity. It's necessary -// to separate this class from DtlsIdentityStore so that it can live on the -// worker thread after DtlsIdentityStore is destroyed. -class DtlsIdentityStore::WorkerTask : public sigslot::has_slots<>, - public rtc::MessageHandler { - public: - explicit WorkerTask(DtlsIdentityStore* store) : store_(store) { - store_->SignalDestroyed.connect(this, &WorkerTask::OnStoreDestroyed); - }; - - void GenerateIdentity() { - rtc::scoped_ptr identity( - rtc::SSLIdentity::Generate(DtlsIdentityStore::kIdentityName)); - - { - rtc::CritScope cs(&cs_); - if (store_) { - store_->PostGenerateIdentityResult_w(identity.Pass()); - } - } - } - - void OnMessage(rtc::Message* msg) override { - DCHECK(msg->message_id == MSG_GENERATE_IDENTITY); - GenerateIdentity(); - // Deleting msg->pdata will destroy the WorkerTask. - delete msg->pdata; - } - - private: - void OnStoreDestroyed() { - rtc::CritScope cs(&cs_); - store_ = NULL; - } - - rtc::CriticalSection cs_; - DtlsIdentityStore* store_; -}; - // Arbitrary constant used as common name for the identity. // Chosen to make the certificates more readable. const char DtlsIdentityStore::kIdentityName[] = "WebRTC"; @@ -96,16 +56,10 @@ DtlsIdentityStore::DtlsIdentityStore(rtc::Thread* signaling_thread, worker_thread_(worker_thread), pending_jobs_(0) {} -DtlsIdentityStore::~DtlsIdentityStore() { - SignalDestroyed(); -} +DtlsIdentityStore::~DtlsIdentityStore() {} void DtlsIdentityStore::Initialize() { - // Do not aggressively generate the free identity if the worker thread and the - // signaling thread are the same. - if (worker_thread_ != signaling_thread_) { - GenerateIdentity(); - } + GenerateIdentity(); } void DtlsIdentityStore::RequestIdentity(DTLSIdentityRequestObserver* observer) { @@ -125,6 +79,9 @@ void DtlsIdentityStore::RequestIdentity(DTLSIdentityRequestObserver* observer) { void DtlsIdentityStore::OnMessage(rtc::Message* msg) { switch (msg->message_id) { + case MSG_GENERATE_IDENTITY: + GenerateIdentity_w(); + break; case MSG_GENERATE_IDENTITY_RESULT: { rtc::scoped_ptr pdata( static_cast(msg->pdata)); @@ -148,12 +105,7 @@ void DtlsIdentityStore::GenerateIdentity() { pending_jobs_++; LOG(LS_VERBOSE) << "New DTLS identity generation is posted, " << "pending_identities=" << pending_jobs_; - - WorkerTask* task = new WorkerTask(this); - // The WorkerTask is owned by the message data to make sure it will not be - // leaked even if the task does not get run. - IdentityTaskMessageData* msg = new IdentityTaskMessageData(task); - worker_thread_->Post(task, MSG_GENERATE_IDENTITY, msg); + worker_thread_->Post(this, MSG_GENERATE_IDENTITY, NULL); } void DtlsIdentityStore::OnIdentityGenerated( @@ -191,22 +143,19 @@ void DtlsIdentityStore::ReturnIdentity( LOG(LS_WARNING) << "Failed to generate SSL identity"; } - // Do not aggressively generate the free identity if the worker thread and the - // signaling thread are the same. - if (worker_thread_ != signaling_thread_ && - pending_observers_.empty() && - pending_jobs_ == 0) { - // Generate a free identity in the background. - GenerateIdentity(); + if (pending_observers_.empty() && pending_jobs_ == 0) { + // Generate a free identity in the background. + GenerateIdentity(); } } -void DtlsIdentityStore::PostGenerateIdentityResult_w( - rtc::scoped_ptr identity) { +void DtlsIdentityStore::GenerateIdentity_w() { DCHECK(rtc::Thread::Current() == worker_thread_); - IdentityResultMessageData* msg = - new IdentityResultMessageData(identity.release()); + rtc::SSLIdentity* identity = rtc::SSLIdentity::Generate(kIdentityName); + + IdentityResultMessageData* msg = new IdentityResultMessageData(identity); signaling_thread_->Post(this, MSG_GENERATE_IDENTITY_RESULT, msg); } + } // namespace webrtc diff --git a/talk/app/webrtc/dtlsidentitystore.h b/talk/app/webrtc/dtlsidentitystore.h index ffe8067aa..2d52249bd 100644 --- a/talk/app/webrtc/dtlsidentitystore.h +++ b/talk/app/webrtc/dtlsidentitystore.h @@ -33,7 +33,6 @@ #include "talk/app/webrtc/peerconnectioninterface.h" #include "webrtc/base/messagehandler.h" -#include "webrtc/base/messagequeue.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/scoped_ref_ptr.h" @@ -52,7 +51,7 @@ class DtlsIdentityStore : public rtc::MessageHandler { DtlsIdentityStore(rtc::Thread* signaling_thread, rtc::Thread* worker_thread); - virtual ~DtlsIdentityStore(); + ~DtlsIdentityStore(); // Initialize will start generating the free identity in the background. void Initialize(); @@ -68,16 +67,11 @@ class DtlsIdentityStore : public rtc::MessageHandler { bool HasFreeIdentityForTesting() const; private: - sigslot::signal0 SignalDestroyed; - class WorkerTask; - typedef rtc::ScopedMessageData - IdentityTaskMessageData; - void GenerateIdentity(); void OnIdentityGenerated(rtc::scoped_ptr identity); void ReturnIdentity(rtc::scoped_ptr identity); - void PostGenerateIdentityResult_w(rtc::scoped_ptr identity); + void GenerateIdentity_w(); rtc::Thread* signaling_thread_; rtc::Thread* worker_thread_; diff --git a/talk/app/webrtc/dtlsidentitystore_unittest.cc b/talk/app/webrtc/dtlsidentitystore_unittest.cc index 262e7736f..67b7cb22a 100644 --- a/talk/app/webrtc/dtlsidentitystore_unittest.cc +++ b/talk/app/webrtc/dtlsidentitystore_unittest.cc @@ -80,12 +80,10 @@ class MockDtlsIdentityRequestObserver : class DtlsIdentityStoreTest : public testing::Test { protected: DtlsIdentityStoreTest() - : worker_thread_(new rtc::Thread()), - store_(new DtlsIdentityStore(rtc::Thread::Current(), - worker_thread_.get())), + : store_(new DtlsIdentityStore(rtc::Thread::Current(), + rtc::Thread::Current())), observer_( new rtc::RefCountedObject()) { - CHECK(worker_thread_->Start()); store_->Initialize(); } ~DtlsIdentityStoreTest() {} @@ -97,7 +95,6 @@ class DtlsIdentityStoreTest : public testing::Test { rtc::CleanupSSL(); } - rtc::scoped_ptr worker_thread_; rtc::scoped_ptr store_; rtc::scoped_refptr observer_; }; @@ -109,21 +106,4 @@ TEST_F(DtlsIdentityStoreTest, RequestIdentitySuccess) { EXPECT_TRUE_WAIT(observer_->LastRequestSucceeded(), kTimeoutMs); EXPECT_TRUE_WAIT(store_->HasFreeIdentityForTesting(), kTimeoutMs); - - observer_->Reset(); - - // Verifies that the callback is async when a free identity is ready. - store_->RequestIdentity(observer_.get()); - EXPECT_FALSE(observer_->call_back_called()); - EXPECT_TRUE_WAIT(observer_->LastRequestSucceeded(), kTimeoutMs); -} - -TEST_F(DtlsIdentityStoreTest, DeleteStoreEarlyNoCrash) { - EXPECT_FALSE(store_->HasFreeIdentityForTesting()); - - store_->RequestIdentity(observer_.get()); - store_.reset(); - - worker_thread_->Stop(); - EXPECT_FALSE(observer_->call_back_called()); } diff --git a/talk/app/webrtc/peerconnectionfactory.cc b/talk/app/webrtc/peerconnectionfactory.cc index 1c933764a..9f1e8588e 100644 --- a/talk/app/webrtc/peerconnectionfactory.cc +++ b/talk/app/webrtc/peerconnectionfactory.cc @@ -132,11 +132,6 @@ PeerConnectionFactory::~PeerConnectionFactory() { DCHECK(signaling_thread_->IsCurrent()); channel_manager_.reset(NULL); default_allocator_factory_ = NULL; - - // Make sure |worker_thread_| and |signaling_thread_| outlive - // |dtls_identity_store_|. - dtls_identity_store_.reset(NULL); - if (owns_ptrs_) { if (wraps_current_thread_) rtc::ThreadManager::Instance()->UnwrapCurrentThread();