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

Revert of Changed P2PTestConductor to use a separate WorkerThread. (patchset #1 id:1 of https://codereview.webrtc.org/1859933002/ )

Browse Source 
Reason for revert:
Causes P2PTestConductor.LocalP2PTestDtlsTransferCaller to fail on Win dbg.

https://build.chromium.org/p/client.webrtc/builders/Win32%20Debug/builds/7469/steps/peerconnection_unittests/logs/stdio

e:\b\build\slave\win\build\src\webrtc\api\peerconnection_unittest.cc(1221): error: Value of: initiating_client_->ice_connection_state()
  Actual: 2
Expected: webrtc::PeerConnectionInterface::kIceConnectionCompleted
Which is: 3

Original issue's description:
> Changed P2PTestConductor to use a separate WorkerThread.
>
> P2PTestConductor currently use the current thread both as a signaling thread and a worker thread. Although convenient while debugging, it can also hide real bugs. An example is https://codereview.webrtc.org/1766653002/#ps420001 where the worker thread is deadlocked in the track proxy due to that the worker thread waits for the signaling thread but the proxy in turns invokes the worker thread..... That bug was only discovered on Android. I suggest we let the P2PTestConductor use a separate thread as a worker thread to better cover how PeerConnections are used in reality.
>
> BUG=webrtc:5426
>
> Committed: https://crrev.com/6172401972c54813698d73580779d675d99178b4
> Cr-Commit-Position: refs/heads/master@{#12252}

TBR=nisse@webrtc.org,pthatcher@webrtc.org,deadbeef@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5426

Review URL: https://codereview.webrtc.org/1866503003

Cr-Commit-Position: refs/heads/master@{#12255}
This commit is contained in:
perkj
2016-04-06 01:28:30 -07:00
committed by Commit bot
parent efc38584b7
commit 05255b0e8a
1 changed files with 21 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
webrtc/api/peerconnection_unittest.cc
View File

@ -155,11 +155,10 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
const MediaConstraintsInterface* constraints,
const PeerConnectionFactory::Options* options,
rtc::scoped_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store,
bool prefer_constraint_apis,
rtc::Thread* worker_thread) {
bool prefer_constraint_apis) {
PeerConnectionTestClient* client(new PeerConnectionTestClient(id));
if (!client->Init(constraints, options, std::move(dtls_identity_store),
prefer_constraint_apis, worker_thread)) {
prefer_constraint_apis)) {
delete client;
return nullptr;
}
@ -169,28 +168,24 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
static PeerConnectionTestClient* CreateClient(
const std::string& id,
const MediaConstraintsInterface* constraints,
const PeerConnectionFactory::Options* options,
rtc::Thread* worker_thread) {
const PeerConnectionFactory::Options* options) {
rtc::scoped_ptr<FakeDtlsIdentityStore> dtls_identity_store(
rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore()
: nullptr);
return CreateClientWithDtlsIdentityStore(id, constraints, options,
std::move(dtls_identity_store),
true, worker_thread);
return CreateClientWithDtlsIdentityStore(
id, constraints, options, std::move(dtls_identity_store), true);
}
static PeerConnectionTestClient* CreateClientPreferNoConstraints(
const std::string& id,
const PeerConnectionFactory::Options* options,
rtc::Thread* worker_thread) {
const PeerConnectionFactory::Options* options) {
rtc::scoped_ptr<FakeDtlsIdentityStore> dtls_identity_store(
rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore()
: nullptr);
return CreateClientWithDtlsIdentityStore(id, nullptr, options,
std::move(dtls_identity_store),
false, worker_thread);
return CreateClientWithDtlsIdentityStore(
id, nullptr, options, std::move(dtls_identity_store), false);
}
~PeerConnectionTestClient() {
@ -805,8 +800,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
const MediaConstraintsInterface* constraints,
const PeerConnectionFactory::Options* options,
rtc::scoped_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store,
bool prefer_constraint_apis,
rtc::Thread* worker_thread) {
bool prefer_constraint_apis) {
EXPECT_TRUE(!peer_connection_);
EXPECT_TRUE(!peer_connection_factory_);
if (!prefer_constraint_apis) {
@ -815,7 +809,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
prefer_constraint_apis_ = prefer_constraint_apis;
rtc::scoped_ptr<cricket::PortAllocator> port_allocator(
new cricket::FakePortAllocator(worker_thread, nullptr));
new cricket::FakePortAllocator(rtc::Thread::Current(), nullptr));
fake_audio_capture_module_ = FakeAudioCaptureModule::Create();
if (fake_audio_capture_module_ == nullptr) {
@ -824,8 +818,9 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
fake_video_decoder_factory_ = new FakeWebRtcVideoDecoderFactory();
fake_video_encoder_factory_ = new FakeWebRtcVideoEncoderFactory();
peer_connection_factory_ = webrtc::CreatePeerConnectionFactory(
worker_thread, rtc::Thread::Current(), fake_audio_capture_module_,
fake_video_encoder_factory_, fake_video_decoder_factory_);
rtc::Thread::Current(), rtc::Thread::Current(),
fake_audio_capture_module_, fake_video_encoder_factory_,
fake_video_decoder_factory_);
if (!peer_connection_factory_) {
return false;
}
@ -1024,9 +1019,7 @@ class P2PTestConductor : public testing::Test {
P2PTestConductor()
: pss_(new rtc::PhysicalSocketServer),
ss_(new rtc::VirtualSocketServer(pss_.get())),
ss_scope_(ss_.get()) {
RTC_CHECK(worker_thread_.Start());
}
ss_scope_(ss_.get()) {}
bool SessionActive() {
return initiating_client_->SessionActive() &&
@ -1134,11 +1127,11 @@ class P2PTestConductor : public testing::Test {
bool CreateTestClientsThatPreferNoConstraints() {
initiating_client_.reset(
PeerConnectionTestClient::CreateClientPreferNoConstraints(
"Caller: ", nullptr, &worker_thread_));
PeerConnectionTestClient::CreateClientPreferNoConstraints("Caller: ",
nullptr));
receiving_client_.reset(
PeerConnectionTestClient::CreateClientPreferNoConstraints(
"Callee: ", nullptr, &worker_thread_));
PeerConnectionTestClient::CreateClientPreferNoConstraints("Callee: ",
nullptr));
if (!initiating_client_ || !receiving_client_) {
return false;
}
@ -1158,9 +1151,9 @@ class P2PTestConductor : public testing::Test {
MediaConstraintsInterface* recv_constraints,
PeerConnectionFactory::Options* recv_options) {
initiating_client_.reset(PeerConnectionTestClient::CreateClient(
"Caller: ", init_constraints, init_options, &worker_thread_));
"Caller: ", init_constraints, init_options));
receiving_client_.reset(PeerConnectionTestClient::CreateClient(
"Callee: ", recv_constraints, recv_options, &worker_thread_));
"Callee: ", recv_constraints, recv_options));
if (!initiating_client_ || !receiving_client_) {
return false;
}
@ -1261,8 +1254,7 @@ class P2PTestConductor : public testing::Test {
// Make sure the new client is using a different certificate.
return PeerConnectionTestClient::CreateClientWithDtlsIdentityStore(
"New Peer: ", &setup_constraints, nullptr,
std::move(dtls_identity_store), prefer_constraint_apis_,
&worker_thread_);
std::move(dtls_identity_store), prefer_constraint_apis_);
}
void SendRtpData(webrtc::DataChannelInterface* dc, const std::string& data) {
@ -1302,9 +1294,6 @@ class P2PTestConductor : public testing::Test {
}
private:
// |worker_thread_| is used by both |initiating_client_| and
// |receiving_client_|. Must be destroyed last.
rtc::Thread worker_thread_;
rtc::scoped_ptr<rtc::PhysicalSocketServer> pss_;
rtc::scoped_ptr<rtc::VirtualSocketServer> ss_;
rtc::SocketServerScope ss_scope_;