From b7d03138ed53f63bd706910e71869e3071a72ce2 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Fri, 23 Aug 2019 12:21:46 -0700 Subject: [PATCH] Remove deprecated ThreadTest::GetThreadTaskRunner and use the newer CreateNewThread API. (#11395) We will end up creating fewer threads in tests. --- lib/ui/painting/image_decoder_unittests.cc | 74 ++++++++++--------- runtime/dart_isolate_unittests.cc | 8 +- runtime/dart_lifecycle_unittests.cc | 3 +- shell/common/shell_unittests.cc | 10 +-- .../embedder/tests/embedder_unittests.cc | 2 +- testing/thread_test.cc | 9 --- testing/thread_test.h | 18 ----- 7 files changed, 50 insertions(+), 74 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 52a77760f..7045a248c 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -105,11 +105,12 @@ static sk_sp OpenFixtureAsSkData(const char* name) { TEST_F(ImageDecoderFixtureTest, CanCreateImageDecoder) { auto loop = fml::ConcurrentMessageLoop::Create(); - TaskRunners runners(GetCurrentTestName(), // label - GetThreadTaskRunner(), // platform - GetThreadTaskRunner(), // gpu - GetThreadTaskRunner(), // ui - GetThreadTaskRunner() // io + auto thread_task_runner = CreateNewThread(); + TaskRunners runners(GetCurrentTestName(), // label + thread_task_runner, // platform + thread_task_runner, // gpu + thread_task_runner, // ui + thread_task_runner // io ); @@ -125,15 +126,16 @@ TEST_F(ImageDecoderFixtureTest, CanCreateImageDecoder) { TEST_F(ImageDecoderFixtureTest, InvalidImageResultsError) { auto loop = fml::ConcurrentMessageLoop::Create(); - TaskRunners runners(GetCurrentTestName(), // label - GetThreadTaskRunner(), // platform - GetThreadTaskRunner(), // gpu - GetThreadTaskRunner(), // ui - GetThreadTaskRunner() // io + auto thread_task_runner = CreateNewThread(); + TaskRunners runners(GetCurrentTestName(), // label + thread_task_runner, // platform + thread_task_runner, // gpu + thread_task_runner, // ui + thread_task_runner // io ); fml::AutoResetWaitableEvent latch; - GetThreadTaskRunner()->PostTask([&]() { + thread_task_runner->PostTask([&]() { TestIOManager manager(runners.GetIOTaskRunner()); ImageDecoder decoder(runners, loop->GetTaskRunner(), manager.GetWeakIOManager()); @@ -155,11 +157,11 @@ TEST_F(ImageDecoderFixtureTest, InvalidImageResultsError) { TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) { auto loop = fml::ConcurrentMessageLoop::Create(); - TaskRunners runners(GetCurrentTestName(), // label - GetThreadTaskRunner(), // platform - CreateNewThread("gpu"), // gpu - CreateNewThread("ui"), // ui - CreateNewThread("io") // io + TaskRunners runners(GetCurrentTestName(), // label + CreateNewThread("platform"), // platform + CreateNewThread("gpu"), // gpu + CreateNewThread("ui"), // ui + CreateNewThread("io") // io ); fml::AutoResetWaitableEvent latch; @@ -197,11 +199,11 @@ TEST_F(ImageDecoderFixtureTest, ValidImageResultsInSuccess) { TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) { auto loop = fml::ConcurrentMessageLoop::Create(); - TaskRunners runners(GetCurrentTestName(), // label - GetThreadTaskRunner(), // platform - CreateNewThread("gpu"), // gpu - CreateNewThread("ui"), // ui - CreateNewThread("io") // io + TaskRunners runners(GetCurrentTestName(), // label + CreateNewThread("platform"), // platform + CreateNewThread("gpu"), // gpu + CreateNewThread("ui"), // ui + CreateNewThread("io") // io ); fml::AutoResetWaitableEvent latch; @@ -244,11 +246,11 @@ TEST_F(ImageDecoderFixtureTest, ExifDataIsRespectedOnDecode) { TEST_F(ImageDecoderFixtureTest, CanDecodeWithoutAGPUContext) { auto loop = fml::ConcurrentMessageLoop::Create(); - TaskRunners runners(GetCurrentTestName(), // label - GetThreadTaskRunner(), // platform - CreateNewThread("gpu"), // gpu - CreateNewThread("ui"), // ui - CreateNewThread("io") // io + TaskRunners runners(GetCurrentTestName(), // label + CreateNewThread("platform"), // platform + CreateNewThread("gpu"), // gpu + CreateNewThread("ui"), // ui + CreateNewThread("io") // io ); fml::AutoResetWaitableEvent latch; @@ -295,11 +297,11 @@ TEST_F(ImageDecoderFixtureTest, CanDecodeWithResizes) { ASSERT_NE(image_dimensions.width(), image_dimensions.height()); auto loop = fml::ConcurrentMessageLoop::Create(); - TaskRunners runners(GetCurrentTestName(), // label - GetThreadTaskRunner(), // platform - CreateNewThread("gpu"), // gpu - CreateNewThread("ui"), // ui - CreateNewThread("io") // io + TaskRunners runners(GetCurrentTestName(), // label + CreateNewThread("platform"), // platform + CreateNewThread("gpu"), // gpu + CreateNewThread("ui"), // ui + CreateNewThread("io") // io ); fml::AutoResetWaitableEvent latch; @@ -380,11 +382,11 @@ TEST_F(ImageDecoderFixtureTest, CanResizeWithoutDecode) { ASSERT_NE(image_dimensions.width(), image_dimensions.height()); auto loop = fml::ConcurrentMessageLoop::Create(); - TaskRunners runners(GetCurrentTestName(), // label - GetThreadTaskRunner(), // platform - CreateNewThread("gpu"), // gpu - CreateNewThread("ui"), // ui - CreateNewThread("io") // io + TaskRunners runners(GetCurrentTestName(), // label + CreateNewThread("platform"), // platform + CreateNewThread("gpu"), // gpu + CreateNewThread("ui"), // ui + CreateNewThread("io") // io ); fml::AutoResetWaitableEvent latch; diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 2a693f86e..73da3c6a9 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -327,7 +327,7 @@ TEST_F(DartIsolateTest, CanRegisterNativeCallback) { }))); const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(), + auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(), "canRegisterNativeCallback", {}); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -350,7 +350,7 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) { const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(), + auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(), "testCanSaveCompilationTrace", {}); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -373,7 +373,7 @@ TEST_F(DartIsolateTest, CanLaunchSecondaryIsolates) { }))); const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(), + auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(), "testCanLaunchSecondaryIsolate", {}); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -392,7 +392,7 @@ TEST_F(DartIsolateTest, CanRecieveArguments) { const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetThreadTaskRunner(), + auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(), "testCanRecieveArguments", {"arg1"}); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); diff --git a/runtime/dart_lifecycle_unittests.cc b/runtime/dart_lifecycle_unittests.cc index 7ec1606fc..7a607b7be 100644 --- a/runtime/dart_lifecycle_unittests.cc +++ b/runtime/dart_lifecycle_unittests.cc @@ -113,6 +113,8 @@ TEST_F(DartLifecycleTest, DISABLED_ShuttingDownTheVMShutsDownAllIsolates) { // Make sure the service protocol launches settings.enable_observatory = true; + auto thread_task_runner = CreateNewThread(); + for (size_t i = 0; i < 3; i++) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); @@ -127,7 +129,6 @@ TEST_F(DartLifecycleTest, DISABLED_ShuttingDownTheVMShutsDownAllIsolates) { fml::CountDownLatch latch(isolate_count); auto vm_data = vm_ref.GetVMData(); - auto thread_task_runner = GetThreadTaskRunner(); for (size_t i = 0; i < isolate_count; ++i) { thread_task_runner->PostTask( [vm_data, &settings, &latch, thread_task_runner]() { diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index d7c1473d7..1f85facaf 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -577,7 +577,7 @@ TEST_F(ShellTest, WaitForFirstFrameMultiple) { /// single-thread setup. TEST_F(ShellTest, WaitForFirstFrameInlined) { Settings settings = CreateSettingsForFixture(); - auto task_runner = GetThreadTaskRunner(); + auto task_runner = CreateNewThread(); TaskRunners task_runners("test", task_runner, task_runner, task_runner, task_runner); std::unique_ptr shell = @@ -617,7 +617,7 @@ static size_t GetRasterizerResourceCacheBytesSync(Shell& shell) { TEST_F(ShellTest, SetResourceCacheSize) { Settings settings = CreateSettingsForFixture(); - auto task_runner = GetThreadTaskRunner(); + auto task_runner = CreateNewThread(); TaskRunners task_runners("test", task_runner, task_runner, task_runner, task_runner); std::unique_ptr shell = @@ -667,7 +667,7 @@ TEST_F(ShellTest, SetResourceCacheSize) { TEST_F(ShellTest, SetResourceCacheSizeEarly) { Settings settings = CreateSettingsForFixture(); - auto task_runner = GetThreadTaskRunner(); + auto task_runner = CreateNewThread(); TaskRunners task_runners("test", task_runner, task_runner, task_runner, task_runner); std::unique_ptr shell = @@ -695,7 +695,7 @@ TEST_F(ShellTest, SetResourceCacheSizeEarly) { TEST_F(ShellTest, SetResourceCacheSizeNotifiesDart) { Settings settings = CreateSettingsForFixture(); - auto task_runner = GetThreadTaskRunner(); + auto task_runner = CreateNewThread(); TaskRunners task_runners("test", task_runner, task_runner, task_runner, task_runner); std::unique_ptr shell = @@ -733,7 +733,7 @@ TEST_F(ShellTest, SetResourceCacheSizeNotifiesDart) { TEST_F(ShellTest, CanCreateImagefromDecompressedBytes) { Settings settings = CreateSettingsForFixture(); - auto task_runner = GetThreadTaskRunner(); + auto task_runner = CreateNewThread(); TaskRunners task_runners("test", task_runner, task_runner, task_runner, task_runner); diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 607c2c264..db1ee72cd 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -313,7 +313,7 @@ TEST_F(EmbedderTest, PlatformMessagesCanReceiveResponse) { }; Captures captures; - GetThreadTaskRunner()->PostTask([&]() { + CreateNewThread()->PostTask([&]() { captures.thread_id = std::this_thread::get_id(); auto& context = GetEmbedderContext(); EmbedderConfigBuilder builder(context); diff --git a/testing/thread_test.cc b/testing/thread_test.cc index deb371e63..88415169a 100644 --- a/testing/thread_test.cc +++ b/testing/thread_test.cc @@ -11,17 +11,12 @@ namespace testing { // |testing::Test| void ThreadTest::SetUp() { - thread_ = std::make_unique(); - thread_task_runner_ = thread_->GetTaskRunner(); - fml::MessageLoop::EnsureInitializedForCurrentThread(); current_task_runner_ = fml::MessageLoop::GetCurrent().GetTaskRunner(); } // |testing::Test| void ThreadTest::TearDown() { - thread_task_runner_ = nullptr; - thread_ = nullptr; current_task_runner_ = nullptr; extra_threads_.clear(); } @@ -30,10 +25,6 @@ fml::RefPtr ThreadTest::GetCurrentTaskRunner() { return current_task_runner_; } -fml::RefPtr ThreadTest::GetThreadTaskRunner() { - return thread_task_runner_; -} - fml::RefPtr ThreadTest::CreateNewThread(std::string name) { auto thread = std::make_unique(name); auto runner = thread->GetTaskRunner(); diff --git a/testing/thread_test.h b/testing/thread_test.h index c2d754543..8c55dbf80 100644 --- a/testing/thread_test.h +++ b/testing/thread_test.h @@ -44,22 +44,6 @@ class ThreadTest : public ::testing::Test { /// fml::RefPtr GetCurrentTaskRunner(); - //---------------------------------------------------------------------------- - /// @brief Get the task runner for a dedicated thread created for this - /// test instance by this fixture. This threads message loop will - /// be terminated and the thread joined when the test is done. - /// - /// @attention Prefer using the `CreateNewThread` call. - /// - /// @bug This is an older call that should probably be deprecated and - /// eventually removed from use. It is redundant now that the - /// `CreateNewThread` call allows tests to create new threads (and - /// name them to boot) as necessary. - /// - /// @return The task runner for a dedicated thread created for the test. - /// - fml::RefPtr GetThreadTaskRunner(); - //---------------------------------------------------------------------------- /// @brief Creates a new thread, initializes a message loop on it, and, /// returns its task runner to the unit-test. The message loop is @@ -80,8 +64,6 @@ class ThreadTest : public ::testing::Test { void TearDown() override; private: - std::unique_ptr thread_; - fml::RefPtr thread_task_runner_; fml::RefPtr current_task_runner_; std::vector> extra_threads_; }; -- GitLab