content/browser/service_worker/service_worker_browsertest.cc - Issue 2204873002: ServiceWorker: Run functions in the browsertest on appropriate thread

Unified Diff: content/browser/service_worker/service_worker_browsertest.cc

Issue 2204873002: ServiceWorker: Run functions in the browsertest on appropriate thread (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 4 years, 4 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

Index: content/browser/service_worker/service_worker_browsertest.cc

diff --git a/content/browser/service_worker/service_worker_browsertest.cc b/content/browser/service_worker/service_worker_browsertest.cc

index 4d73b52bba36bcc6c5170679cf6898a6c2cf6e56..25902d00b731ae03a234ca5bfa24320cb818a366 100644

--- a/content/browser/service_worker/service_worker_browsertest.cc

+++ b/content/browser/service_worker/service_worker_browsertest.cc

@@ -7,6 +7,7 @@

#include <map>

#include <memory>

+#include <mutex>

#include <utility>

#include "base/bind.h"

@@ -383,6 +384,7 @@ class ConsoleListener : public EmbeddedWorkerInstance::Listener {

const base::string16& message,

int line_number,

const GURL& source_url) override {

nhiroki 2016/08/02 09:15:43 Can you add DCHECK_CURRENTLY_ON(BrowserThread::IO)

shimazu 2016/08/02 23:34:49 Done.

+ std::lock_guard<std::mutex> lock(mutex_);

messages_.push_back(message);

if (!quit_.is_null() && messages_.size() == expected_message_count_) {

BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, quit_);

@@ -391,15 +393,22 @@ class ConsoleListener : public EmbeddedWorkerInstance::Listener {

}

void WaitForConsoleMessages(size_t expected_message_count) {

- if (messages_.size() >= expected_message_count)

- return;

- expected_message_count_ = expected_message_count;

base::RunLoop console_run_loop;

- quit_ = console_run_loop.QuitClosure();

+ {

+ std::lock_guard<std::mutex> lock(mutex_);

+ if (messages_.size() >= expected_message_count)

+ return;

+ expected_message_count_ = expected_message_count;

+ quit_ = console_run_loop.QuitClosure();

+ }

console_run_loop.Run();

- ASSERT_EQ(messages_.size(), expected_message_count);

+ {

+ std::lock_guard<std::mutex> lock(mutex_);

+ ASSERT_EQ(messages_.size(), expected_message_count);

+ }

}

bool OnMessageReceived(const IPC::Message& message) override { return false; }

@@ -407,6 +416,7 @@ class ConsoleListener : public EmbeddedWorkerInstance::Listener {

private:

std::vector<base::string16> messages_;

+ std::mutex mutex_;

size_t expected_message_count_;

base::Closure quit_;

nhiroki 2016/08/02 09:15:42 We may want to add a comment like "// These must b

shimazu 2016/08/02 23:34:49 Done.

};

@@ -930,21 +940,15 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED);

}

-// Has errors under TSan. See https://crbug.com/631323.

-#if defined(THREAD_SANITIZER)

-#define MAYBE_InstallWithWaitUntil_RejectConsoleMessage \

- DISABLED_InstallWithWaitUntil_RejectConsoleMessage

-#else

-#define MAYBE_InstallWithWaitUntil_RejectConsoleMessage \

- InstallWithWaitUntil_RejectConsoleMessage

-#endif

IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

- MAYBE_InstallWithWaitUntil_RejectConsoleMessage) {

+ InstallWithWaitUntil_RejectConsoleMessage) {

RunOnIOThread(base::Bind(&self::SetUpRegistrationOnIOThread, this,

"/service_worker/worker_install_rejected.js"));

ConsoleListener console_listener;

- version_->embedded_worker()->AddListener(&console_listener);

+ RunOnIOThread(base::Bind(&EmbeddedWorkerInstance::AddListener,

+ base::Unretained(version_->embedded_worker()),

+ &console_listener));

// Dispatch install on a worker.

ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;

@@ -960,7 +964,9 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

console_listener.WaitForConsoleMessages(1);

ASSERT_NE(base::string16::npos,

console_listener.messages()[0].find(expected));

- version_->embedded_worker()->RemoveListener(&console_listener);

+ RunOnIOThread(base::Bind(&EmbeddedWorkerInstance::RemoveListener,

+ base::Unretained(version_->embedded_worker()),

+ &console_listener));

}

class WaitForLoaded : public EmbeddedWorkerInstance::Listener {

@@ -976,14 +982,7 @@ class WaitForLoaded : public EmbeddedWorkerInstance::Listener {

base::Closure quit_;

};

-// Has errors under TSan. See https://crbug.com/631323.

-#if defined(THREAD_SANITIZER)

-#define MAYBE_TimeoutStartingWorker DISABLED_TimeoutStartingWorker

-#else

-#define MAYBE_TimeoutStartingWorker TimeoutStartingWorker

-#endif

-IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

- MAYBE_TimeoutStartingWorker) {

+IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest, TimeoutStartingWorker) {

RunOnIOThread(base::Bind(&self::SetUpRegistrationOnIOThread, this,

"/service_worker/while_true_worker.js"));

@@ -992,12 +991,16 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

base::RunLoop start_run_loop;

base::RunLoop load_run_loop;

WaitForLoaded wait_for_load(load_run_loop.QuitClosure());

- version_->embedded_worker()->AddListener(&wait_for_load);

+ RunOnIOThread(base::Bind(&EmbeddedWorkerInstance::AddListener,

+ base::Unretained(version_->embedded_worker()),

+ &wait_for_load));

BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,

base::Bind(&self::StartOnIOThread, this,

start_run_loop.QuitClosure(), &status));

load_run_loop.Run();

- version_->embedded_worker()->RemoveListener(&wait_for_load);

+ RunOnIOThread(base::Bind(&EmbeddedWorkerInstance::RemoveListener,

+ base::Unretained(version_->embedded_worker()),

+ &wait_for_load));

// The script has loaded but start has not completed yet.

ASSERT_EQ(SERVICE_WORKER_ERROR_FAILED, status);

@@ -1094,15 +1097,8 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

EXPECT_EQ("cache_name", response2.cache_storage_cache_name);

}

-// Has errors under TSan. See https://crbug.com/631323.

-#if defined(THREAD_SANITIZER)

-#define MAYBE_FetchEvent_respondWithRejection \

- DISABLED_FetchEvent_respondWithRejection

-#else

-#define MAYBE_FetchEvent_respondWithRejection FetchEvent_respondWithRejection

-#endif

IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

- MAYBE_FetchEvent_respondWithRejection) {

+ FetchEvent_respondWithRejection) {

ServiceWorkerFetchEventResult result;

ServiceWorkerResponse response;

std::unique_ptr<storage::BlobDataHandle> blob_data_handle;

@@ -1111,7 +1107,9 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

"/service_worker/fetch_event_rejected.js"));

ConsoleListener console_listener;

- version_->embedded_worker()->AddListener(&console_listener);

+ RunOnIOThread(base::Bind(&EmbeddedWorkerInstance::AddListener,

+ base::Unretained(version_->embedded_worker()),

+ &console_listener));

FetchOnRegisteredWorker(&result, &response, &blob_data_handle);

const base::string16 expected1 = base::ASCIIToUTF16(

@@ -1122,7 +1120,9 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,

ASSERT_NE(base::string16::npos,

console_listener.messages()[0].find(expected1));

ASSERT_EQ(0u, console_listener.messages()[1].find(expected2));

- version_->embedded_worker()->RemoveListener(&console_listener);

+ RunOnIOThread(base::Bind(&EmbeddedWorkerInstance::RemoveListener,

+ base::Unretained(version_->embedded_worker()),

+ &console_listener));

ASSERT_EQ(SERVICE_WORKER_FETCH_EVENT_RESULT_RESPONSE, result);

EXPECT_EQ(0, response.status_code);

« no previous file with comments | « no previous file | no next file » | no next file with comments »