Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(110)

Unified Diff: net/socket/client_socket_pool_base_unittest.cc

Issue 257044: This is a second attempt at submitting this changelist. The original one was... (Closed) Base URL: svn://chrome-svn/chrome/trunk/src/
Patch Set: '' Created 11 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/test/automation/automation_proxy_uitest.cc ('k') | net/socket/socket_test_util.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/socket/client_socket_pool_base_unittest.cc
===================================================================
--- net/socket/client_socket_pool_base_unittest.cc (revision 28729)
+++ net/socket/client_socket_pool_base_unittest.cc (working copy)
@@ -143,31 +143,46 @@
return DoConnect(false /* error */, false /* sync */);
case kMockPendingJob:
set_load_state(LOAD_STATE_CONNECTING);
- MessageLoop::current()->PostTask(
+
+ // Depending on execution timings, posting a delayed task can result
+ // in the task getting executed the at the earliest possible
+ // opportunity or only after returning once from the message loop and
+ // then a second call into the message loop. In order to make behavior
+ // more deterministic, we change the default delay to 2ms. This should
+ // always require us to wait for the second call into the message loop.
+ //
+ // N.B. The correct fix for this and similar timing problems is to
+ // abstract time for the purpose of unittests. Unfortunately, we have
+ // a lot of third-party components that directly call the various
+ // time functions, so this change would be rather invasive.
+ MessageLoop::current()->PostDelayedTask(
FROM_HERE,
method_factory_.NewRunnableMethod(
- &TestConnectJob::DoConnect,
- true /* successful */,
- true /* async */));
+ &TestConnectJob::DoConnect,
+ true /* successful */,
+ true /* async */),
+ 2);
return ERR_IO_PENDING;
case kMockPendingFailingJob:
set_load_state(LOAD_STATE_CONNECTING);
- MessageLoop::current()->PostTask(
+ MessageLoop::current()->PostDelayedTask(
FROM_HERE,
method_factory_.NewRunnableMethod(
- &TestConnectJob::DoConnect,
- false /* error */,
- true /* async */));
+ &TestConnectJob::DoConnect,
+ false /* error */,
+ true /* async */),
+ 2);
return ERR_IO_PENDING;
case kMockWaitingJob:
client_socket_factory_->WaitForSignal(this);
waiting_success_ = true;
return ERR_IO_PENDING;
case kMockAdvancingLoadStateJob:
- MessageLoop::current()->PostTask(
+ MessageLoop::current()->PostDelayedTask(
FROM_HERE,
method_factory_.NewRunnableMethod(
- &TestConnectJob::AdvanceLoadState, load_state_));
+ &TestConnectJob::AdvanceLoadState, load_state_),
+ 2);
return ERR_IO_PENDING;
default:
NOTREACHED();
@@ -396,6 +411,13 @@
}
virtual void TearDown() {
+ // We post all of our delayed tasks with a 2ms delay. I.e. they don't
+ // actually become pending until 2ms after they have been created. In order
+ // to flush all tasks, we need to wait so that we know there are no
+ // soon-to-be-pending tasks waiting.
+ PlatformThread::Sleep(10);
+ MessageLoop::current()->RunAllPending();
+
// Need to delete |pool_| before we turn late binding back off. We also need
// to delete |requests_| because the pool is reference counted and requests
// keep reference to it.
@@ -715,6 +737,13 @@
connect_job_factory_->set_job_type(TestConnectJob::kMockPendingJob);
EXPECT_EQ(ERR_IO_PENDING, StartRequest("d", kDefaultPriority));
+ // We post all of our delayed tasks with a 2ms delay. I.e. they don't
+ // actually become pending until 2ms after they have been created. In order
+ // to flush all tasks, we need to wait so that we know there are no
+ // soon-to-be-pending tasks waiting.
+ PlatformThread::Sleep(10);
+ MessageLoop::current()->RunAllPending();
+
// The next synchronous request should wait for its turn.
connect_job_factory_->set_job_type(TestConnectJob::kMockJob);
EXPECT_EQ(ERR_IO_PENDING, StartRequest("e", kDefaultPriority));
@@ -950,14 +979,26 @@
test_connect_job_factory_->set_job_type(next_job_type_);
handle_->Reset();
within_callback_ = true;
+ TestCompletionCallback next_job_callback;
int rv = InitHandle(
- handle_, "a", kDefaultPriority, this, pool_.get(), NULL);
+ handle_, "a", kDefaultPriority, &next_job_callback, pool_.get(),
+ NULL);
switch (next_job_type_) {
case TestConnectJob::kMockJob:
EXPECT_EQ(OK, rv);
break;
case TestConnectJob::kMockPendingJob:
EXPECT_EQ(ERR_IO_PENDING, rv);
+
+ // For pending jobs, wait for new socket to be created. This makes
+ // sure there are no more pending operations nor any unclosed sockets
+ // when the test finishes.
+ // We need to give it a little bit of time to run, so that all the
+ // operations that happen on timers (e.g. cleanup of idle
+ // connections) can execute.
+ MessageLoop::current()->SetNestableTasksAllowed(true);
+ PlatformThread::Sleep(10);
+ EXPECT_EQ(OK, next_job_callback.WaitForResult());
break;
default:
FAIL() << "Unexpected job type: " << next_job_type_;
@@ -1812,6 +1853,12 @@
req.handle()->Reset();
EXPECT_EQ(OK, req2.WaitForResult());
req2.handle()->Reset();
+
+ // We post all of our delayed tasks with a 2ms delay. I.e. they don't
+ // actually become pending until 2ms after they have been created. In order
+ // to flush all tasks, we need to wait so that we know there are no
+ // soon-to-be-pending tasks waiting.
+ PlatformThread::Sleep(10);
MessageLoop::current()->RunAllPending();
ASSERT_EQ(2, pool_->IdleSocketCount());
« no previous file with comments | « chrome/test/automation/automation_proxy_uitest.cc ('k') | net/socket/socket_test_util.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698