Commit 606c65ca authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot

ServiceWorker: Enable WebEmbeddedWorkerImplTest

The test was disabled because a task bound with an unretained object may run
after the object gets freed. To avoid that, this CL adds test::RunPendingTasks()
call to drain the queued tasks before the object deallocation.

For Sheriff, feel free to revert this CL if the flakiness still happens.

Bug: 1007616
Change-Id: If7382250336a0f3820795b8a31e4429c6b64353f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824761Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699671}
parent d93018d7
...@@ -220,6 +220,13 @@ class WebEmbeddedWorkerImplTest : public testing::Test { ...@@ -220,6 +220,13 @@ class WebEmbeddedWorkerImplTest : public testing::Test {
} }
void TearDown() override { void TearDown() override {
// Drain queued tasks posted from the worker thread in order to avoid tasks
// bound with unretained objects from running after tear down. Worker
// termination may post such tasks (see https://crbug,com/1007616).
// TODO(nhiroki): Stop using synchronous WaitableEvent, and instead use
// QuitClosure to wait until all the tasks run before test completion.
test::RunPendingTasks();
Platform::Current() Platform::Current()
->GetURLLoaderMockFactory() ->GetURLLoaderMockFactory()
->UnregisterAllURLsAndClearMemoryCache(); ->UnregisterAllURLsAndClearMemoryCache();
...@@ -232,8 +239,7 @@ class WebEmbeddedWorkerImplTest : public testing::Test { ...@@ -232,8 +239,7 @@ class WebEmbeddedWorkerImplTest : public testing::Test {
} // namespace } // namespace
// Disabled due to flakiness: https://crbug.com/1007616 TEST_F(WebEmbeddedWorkerImplTest, TerminateSoonAfterStart) {
TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateSoonAfterStart) {
worker_->StartWorkerContext( worker_->StartWorkerContext(
CreateStartData(), CreateStartData(),
/*installed_scripts_manager_params=*/nullptr, /*installed_scripts_manager_params=*/nullptr,
...@@ -241,13 +247,12 @@ TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateSoonAfterStart) { ...@@ -241,13 +247,12 @@ TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateSoonAfterStart) {
Thread::Current()->GetTaskRunner()); Thread::Current()->GetTaskRunner());
testing::Mock::VerifyAndClearExpectations(mock_client_.get()); testing::Mock::VerifyAndClearExpectations(mock_client_.get());
// Terminate the worker immediately after start.
worker_->TerminateWorkerContext(); worker_->TerminateWorkerContext();
// The worker thread was started. Wait for shutdown tasks to finish.
worker_->WaitForShutdownForTesting(); worker_->WaitForShutdownForTesting();
} }
// Disabled due to flakiness: https://crbug.com/1007616 TEST_F(WebEmbeddedWorkerImplTest, TerminateWhileWaitingForDebugger) {
TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateWhileWaitingForDebugger) {
std::unique_ptr<WebEmbeddedWorkerStartData> start_data = CreateStartData(); std::unique_ptr<WebEmbeddedWorkerStartData> start_data = CreateStartData();
start_data->wait_for_debugger_mode = start_data->wait_for_debugger_mode =
WebEmbeddedWorkerStartData::kWaitForDebugger; WebEmbeddedWorkerStartData::kWaitForDebugger;
...@@ -258,13 +263,15 @@ TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateWhileWaitingForDebugger) { ...@@ -258,13 +263,15 @@ TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateWhileWaitingForDebugger) {
Thread::Current()->GetTaskRunner()); Thread::Current()->GetTaskRunner());
testing::Mock::VerifyAndClearExpectations(mock_client_.get()); testing::Mock::VerifyAndClearExpectations(mock_client_.get());
// Terminate the worker while waiting for the debugger.
worker_->TerminateWorkerContext(); worker_->TerminateWorkerContext();
worker_->WaitForShutdownForTesting(); worker_->WaitForShutdownForTesting();
} }
// Disabled due to flakiness: https://crbug.com/1007616 // TODO(nhiroki): Remove this. This test is the same with
TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateWhileLoadingScript) { // TerminateSoonAfterStart() because off-the-main-thread worker script loading
// Load the shadow page. // got enabled by default.
TEST_F(WebEmbeddedWorkerImplTest, TerminateWhileLoadingScript) {
worker_->StartWorkerContext( worker_->StartWorkerContext(
CreateStartData(), CreateStartData(),
/*installed_scripts_manager_params=*/nullptr, /*installed_scripts_manager_params=*/nullptr,
...@@ -274,13 +281,10 @@ TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateWhileLoadingScript) { ...@@ -274,13 +281,10 @@ TEST_F(WebEmbeddedWorkerImplTest, DISABLED_TerminateWhileLoadingScript) {
// Terminate before finishing the script load. // Terminate before finishing the script load.
worker_->TerminateWorkerContext(); worker_->TerminateWorkerContext();
// The worker thread was started. Wait for shutdown tasks to finish.
worker_->WaitForShutdownForTesting(); worker_->WaitForShutdownForTesting();
} }
// Disabled due to flakiness: https://crbug.com/1007616 TEST_F(WebEmbeddedWorkerImplTest, ScriptNotFound) {
TEST_F(WebEmbeddedWorkerImplTest, DISABLED_ScriptNotFound) {
// Load the shadow page.
WebURL script_url = url_test_helpers::ToKURL(kNotFoundScriptURL); WebURL script_url = url_test_helpers::ToKURL(kNotFoundScriptURL);
WebURLResponse response; WebURLResponse response;
response.SetMimeType("text/javascript"); response.SetMimeType("text/javascript");
...@@ -301,8 +305,7 @@ TEST_F(WebEmbeddedWorkerImplTest, DISABLED_ScriptNotFound) { ...@@ -301,8 +305,7 @@ TEST_F(WebEmbeddedWorkerImplTest, DISABLED_ScriptNotFound) {
mock_client_->WaitUntilFailedToLoadClassicScript(); mock_client_->WaitUntilFailedToLoadClassicScript();
// The worker thread was started. Ask to shutdown and wait for shutdown // Terminate the worker for cleanup.
// tasks to finish.
worker_->TerminateWorkerContext(); worker_->TerminateWorkerContext();
worker_->WaitForShutdownForTesting(); worker_->WaitForShutdownForTesting();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment