From 54d19441b064ae06604c6a26c3863a091a8bb8bb Mon Sep 17 00:00:00 2001 From: ssid Date: Wed, 6 May 2015 07:42:03 -0700 Subject: [PATCH] Adding task runner/ message loop to tests that use IsolateHolder. For adding v8 heap memory dump provider, the gin::IsolateHolder needs to have a task runner at initialization. The tests which don't initiallize task runner when initialization are changed in this CL to have either the message loop initializaed earlier or a dummy task runner. Refer crrev.com/1088683003 for the dump provider. BUG=476013 Review URL: https://codereview.chromium.org/1130433002 Cr-Commit-Position: refs/heads/master@{#328531} --- .../test/test_blink_web_unit_test_support.cc | 50 +++++++++++++++++++ gin/modules/module_registry_unittest.cc | 2 - gin/modules/timer_unittest.cc | 22 ++++---- gin/shell_runner_unittest.cc | 2 + gin/test/v8_test.h | 2 + 5 files changed, 65 insertions(+), 13 deletions(-) diff --git a/content/test/test_blink_web_unit_test_support.cc b/content/test/test_blink_web_unit_test_support.cc index 7405be6f417d04..8aa66187af66a4 100644 --- a/content/test/test_blink_web_unit_test_support.cc +++ b/content/test/test_blink_web_unit_test_support.cc @@ -8,7 +8,10 @@ #include "base/files/file_util.h" #include "base/files/scoped_temp_dir.h" #include "base/path_service.h" +#include "base/single_thread_task_runner.h" #include "base/strings/utf_string_conversions.h" +#include "base/thread_task_runner_handle.h" +#include "base/threading/platform_thread.h" #include "components/scheduler/renderer/renderer_scheduler.h" #include "components/scheduler/renderer/webthread_impl_for_renderer_scheduler.h" #include "content/test/mock_webclipboard_impl.h" @@ -41,6 +44,40 @@ #include "gin/v8_initializer.h" #endif +namespace { + +class DummyTaskRunner : public base::SingleThreadTaskRunner { + public: + DummyTaskRunner() : thread_id_(base::PlatformThread::CurrentId()) {} + + bool PostDelayedTask(const tracked_objects::Location& from_here, + const base::Closure& task, + base::TimeDelta delay) override { + NOTREACHED(); + return false; + } + + bool PostNonNestableDelayedTask(const tracked_objects::Location& from_here, + const base::Closure& task, + base::TimeDelta delay) override { + NOTREACHED(); + return false; + } + + bool RunsTasksOnCurrentThread() const override { + return thread_id_ == base::PlatformThread::CurrentId(); + } + + protected: + ~DummyTaskRunner() override {} + + base::PlatformThreadId thread_id_; + + DISALLOW_COPY_AND_ASSIGN(DummyTaskRunner); +}; + +} // namespace + namespace content { TestBlinkWebUnitTestSupport::TestBlinkWebUnitTestSupport() { @@ -55,10 +92,23 @@ TestBlinkWebUnitTestSupport::TestBlinkWebUnitTestSupport() { gin::V8Initializer::LoadV8Snapshot(); #endif + scoped_refptr dummy_task_runner; + scoped_ptr dummy_task_runner_handle; if (base::MessageLoopProxy::current()) { renderer_scheduler_ = scheduler::RendererScheduler::Create(); web_thread_.reset(new scheduler::WebThreadImplForRendererScheduler( renderer_scheduler_.get())); + } else { + // Dummy task runner is initialized here because the blink::initialize + // creates IsolateHolder which needs the current task runner handle. There + // should be no task posted to this task runner. The message loop is not + // created before this initialization because some tests need specific kinds + // of message loops, and their types are not known upfront. Some tests also + // create their own thread bundles or message loops, and doing the same in + // TestBlinkWebUnitTestSupport would introduce a conflict. + dummy_task_runner = make_scoped_refptr(new DummyTaskRunner()); + dummy_task_runner_handle.reset( + new base::ThreadTaskRunnerHandle(dummy_task_runner)); } blink::initialize(this); diff --git a/gin/modules/module_registry_unittest.cc b/gin/modules/module_registry_unittest.cc index 2d6e6daf83eb1e..00c6a94e1939ae 100644 --- a/gin/modules/module_registry_unittest.cc +++ b/gin/modules/module_registry_unittest.cc @@ -5,7 +5,6 @@ #include "gin/modules/module_registry.h" #include "base/bind.h" -#include "base/message_loop/message_loop.h" #include "gin/modules/module_registry_observer.h" #include "gin/modules/module_runner_delegate.h" #include "gin/public/context_holder.h" @@ -25,7 +24,6 @@ struct TestHelper { scope(runner.get()) { } - base::MessageLoop message_loop; ModuleRunnerDelegate delegate; scoped_ptr runner; Runner::Scope scope; diff --git a/gin/modules/timer_unittest.cc b/gin/modules/timer_unittest.cc index 705bdc5697864c..612275e4729bec 100644 --- a/gin/modules/timer_unittest.cc +++ b/gin/modules/timer_unittest.cc @@ -64,9 +64,10 @@ struct TestHelper { result->GetWrapper(isolate)); } - void QuitSoon() { - loop.PostDelayedTask(FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(), - base::TimeDelta::FromMilliseconds(0)); + void QuitSoon(base::MessageLoop* message_loop) { + message_loop->PostDelayedTask(FROM_HERE, + base::MessageLoop::QuitWhenIdleClosure(), + base::TimeDelta::FromMilliseconds(0)); } ShellRunnerDelegate delegate; @@ -74,7 +75,6 @@ struct TestHelper { Runner::Scope scope; Handle timer_module; Handle result; - base::MessageLoop loop; }; } // namespace @@ -91,8 +91,8 @@ TEST_F(TimerUnittest, OneShot) { helper.runner->Run(source, "script"); EXPECT_EQ(0, helper.result->count()); - helper.QuitSoon(); - helper.loop.Run(); + helper.QuitSoon(&message_loop_); + message_loop_.Run(); EXPECT_EQ(1, helper.result->count()); } @@ -107,8 +107,8 @@ TEST_F(TimerUnittest, OneShotCancel) { helper.runner->Run(source, "script"); EXPECT_EQ(0, helper.result->count()); - helper.QuitSoon(); - helper.loop.Run(); + helper.QuitSoon(&message_loop_); + message_loop_.Run(); EXPECT_EQ(0, helper.result->count()); } @@ -128,7 +128,7 @@ TEST_F(TimerUnittest, Repeating) { helper.runner->Run(source, "script"); EXPECT_EQ(0, helper.result->count()); - helper.loop.Run(); + message_loop_.Run(); EXPECT_EQ(3, helper.result->count()); } @@ -143,9 +143,9 @@ TEST_F(TimerUnittest, TimerCallbackToDestroyedRunner) { EXPECT_EQ(0, helper.result->count()); // Destroy runner, which should destroy the timer object we created. - helper.QuitSoon(); + helper.QuitSoon(&message_loop_); helper.runner.reset(NULL); - helper.loop.Run(); + message_loop_.Run(); // Timer should not have run because it was deleted. EXPECT_EQ(0, helper.result->count()); diff --git a/gin/shell_runner_unittest.cc b/gin/shell_runner_unittest.cc index 340b673362f44d..4e4b9c2fa9cff5 100644 --- a/gin/shell_runner_unittest.cc +++ b/gin/shell_runner_unittest.cc @@ -5,6 +5,7 @@ #include "gin/shell_runner.h" #include "base/compiler_specific.h" +#include "base/message_loop/message_loop.h" #include "gin/array_buffer.h" #include "gin/converter.h" #include "gin/public/isolate_holder.h" @@ -23,6 +24,7 @@ using v8::String; namespace gin { TEST(RunnerTest, Run) { + base::MessageLoop message_loop; std::string source = "this.result = 'PASS';\n"; #ifdef V8_USE_EXTERNAL_STARTUP_DATA diff --git a/gin/test/v8_test.h b/gin/test/v8_test.h index e9209ce97921fb..592b88904035a0 100644 --- a/gin/test/v8_test.h +++ b/gin/test/v8_test.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" #include "testing/gtest/include/gtest/gtest.h" #include "v8/include/v8.h" @@ -26,6 +27,7 @@ class V8Test : public testing::Test { void TearDown() override; protected: + base::MessageLoop message_loop_; scoped_ptr instance_; v8::Persistent context_;