Commit a7fe515e authored by ksakamoto's avatar ksakamoto Committed by Commit bot

ServiceWorker: Use scheduler's default task queue for posting tasks on main thread

This fixes a task ordering bug between WebMessagePortChannelImpl and
ServiceWorkerScriptContext. The former posts tasks via the Blink
scheduler's default task queue, and the latter was using the MessageLoop
directly. This patch makes tasks from service worker to the main thread
go through the scheduler.

BUG=460833
TEST=http/tests/serviceworker/postmessage-msgport-to-client.html

Review URL: https://codereview.chromium.org/958523002

Cr-Commit-Position: refs/heads/master@{#318022}
parent 4653a504
...@@ -106,7 +106,7 @@ EmbeddedWorkerContextClient::EmbeddedWorkerContextClient( ...@@ -106,7 +106,7 @@ EmbeddedWorkerContextClient::EmbeddedWorkerContextClient(
script_url_(script_url), script_url_(script_url),
worker_devtools_agent_route_id_(worker_devtools_agent_route_id), worker_devtools_agent_route_id_(worker_devtools_agent_route_id),
sender_(ChildThreadImpl::current()->thread_safe_sender()), sender_(ChildThreadImpl::current()->thread_safe_sender()),
main_thread_proxy_(base::MessageLoopProxy::current()), main_thread_task_runner_(RenderThreadImpl::current()->GetTaskRunner()),
weak_factory_(this) { weak_factory_(this) {
TRACE_EVENT_ASYNC_BEGIN0("ServiceWorker", TRACE_EVENT_ASYNC_BEGIN0("ServiceWorker",
"EmbeddedWorkerContextClient::StartingWorkerContext", "EmbeddedWorkerContextClient::StartingWorkerContext",
...@@ -180,7 +180,7 @@ void EmbeddedWorkerContextClient::workerReadyForInspection() { ...@@ -180,7 +180,7 @@ void EmbeddedWorkerContextClient::workerReadyForInspection() {
} }
void EmbeddedWorkerContextClient::workerContextFailedToStart() { void EmbeddedWorkerContextClient::workerContextFailedToStart() {
DCHECK(main_thread_proxy_->RunsTasksOnCurrentThread()); DCHECK(main_thread_task_runner_->RunsTasksOnCurrentThread());
DCHECK(!script_context_); DCHECK(!script_context_);
Send(new EmbeddedWorkerHostMsg_WorkerScriptLoadFailed(embedded_worker_id_)); Send(new EmbeddedWorkerHostMsg_WorkerScriptLoadFailed(embedded_worker_id_));
...@@ -243,7 +243,7 @@ void EmbeddedWorkerContextClient::workerContextDestroyed() { ...@@ -243,7 +243,7 @@ void EmbeddedWorkerContextClient::workerContextDestroyed() {
// Now we should be able to free the WebEmbeddedWorker container on the // Now we should be able to free the WebEmbeddedWorker container on the
// main thread. // main thread.
main_thread_proxy_->PostTask( main_thread_task_runner_->PostTask(
FROM_HERE, FROM_HERE,
base::Bind(&CallWorkerContextDestroyedOnMainThread, base::Bind(&CallWorkerContextDestroyedOnMainThread,
embedded_worker_id_)); embedded_worker_id_));
...@@ -355,7 +355,7 @@ void EmbeddedWorkerContextClient::didHandleCrossOriginConnectEvent( ...@@ -355,7 +355,7 @@ void EmbeddedWorkerContextClient::didHandleCrossOriginConnectEvent(
blink::WebServiceWorkerNetworkProvider* blink::WebServiceWorkerNetworkProvider*
EmbeddedWorkerContextClient::createServiceWorkerNetworkProvider( EmbeddedWorkerContextClient::createServiceWorkerNetworkProvider(
blink::WebDataSource* data_source) { blink::WebDataSource* data_source) {
DCHECK(main_thread_proxy_->RunsTasksOnCurrentThread()); DCHECK(main_thread_task_runner_->RunsTasksOnCurrentThread());
// Create a content::ServiceWorkerNetworkProvider for this data source so // Create a content::ServiceWorkerNetworkProvider for this data source so
// we can observe its requests. // we can observe its requests.
...@@ -380,7 +380,7 @@ EmbeddedWorkerContextClient::createServiceWorkerNetworkProvider( ...@@ -380,7 +380,7 @@ EmbeddedWorkerContextClient::createServiceWorkerNetworkProvider(
blink::WebServiceWorkerProvider* blink::WebServiceWorkerProvider*
EmbeddedWorkerContextClient::createServiceWorkerProvider() { EmbeddedWorkerContextClient::createServiceWorkerProvider() {
DCHECK(main_thread_proxy_->RunsTasksOnCurrentThread()); DCHECK(main_thread_task_runner_->RunsTasksOnCurrentThread());
DCHECK(provider_context_); DCHECK(provider_context_);
// Blink is responsible for deleting the returned object. // Blink is responsible for deleting the returned object.
......
...@@ -14,7 +14,7 @@ ...@@ -14,7 +14,7 @@
#include "url/gurl.h" #include "url/gurl.h"
namespace base { namespace base {
class MessageLoopProxy; class SingleThreadTaskRunner;
class TaskRunner; class TaskRunner;
} }
...@@ -130,8 +130,8 @@ class EmbeddedWorkerContextClient ...@@ -130,8 +130,8 @@ class EmbeddedWorkerContextClient
// TODO: Implement DevTools related method overrides. // TODO: Implement DevTools related method overrides.
int embedded_worker_id() const { return embedded_worker_id_; } int embedded_worker_id() const { return embedded_worker_id_; }
base::MessageLoopProxy* main_thread_proxy() const { base::SingleThreadTaskRunner* main_thread_task_runner() const {
return main_thread_proxy_.get(); return main_thread_task_runner_.get();
} }
ThreadSafeSender* thread_safe_sender() { return sender_.get(); } ThreadSafeSender* thread_safe_sender() { return sender_.get(); }
...@@ -148,7 +148,7 @@ class EmbeddedWorkerContextClient ...@@ -148,7 +148,7 @@ class EmbeddedWorkerContextClient
const GURL script_url_; const GURL script_url_;
const int worker_devtools_agent_route_id_; const int worker_devtools_agent_route_id_;
scoped_refptr<ThreadSafeSender> sender_; scoped_refptr<ThreadSafeSender> sender_;
scoped_refptr<base::MessageLoopProxy> main_thread_proxy_; scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
scoped_refptr<base::TaskRunner> worker_task_runner_; scoped_refptr<base::TaskRunner> worker_task_runner_;
scoped_ptr<ServiceWorkerScriptContext> script_context_; scoped_ptr<ServiceWorkerScriptContext> script_context_;
......
...@@ -253,7 +253,7 @@ void ServiceWorkerScriptContext::PostMessageToDocument( ...@@ -253,7 +253,7 @@ void ServiceWorkerScriptContext::PostMessageToDocument(
// messages for MessagePort (e.g. QueueMessages) are sent from main thread // messages for MessagePort (e.g. QueueMessages) are sent from main thread
// (with thread hopping), so we need to do the same thread hopping here not // (with thread hopping), so we need to do the same thread hopping here not
// to overtake those messages. // to overtake those messages.
embedded_context_->main_thread_proxy()->PostTask( embedded_context_->main_thread_task_runner()->PostTask(
FROM_HERE, FROM_HERE,
base::Bind(&SendPostMessageToDocumentOnMainThread, base::Bind(&SendPostMessageToDocumentOnMainThread,
make_scoped_refptr(embedded_context_->thread_safe_sender()), make_scoped_refptr(embedded_context_->thread_safe_sender()),
...@@ -268,7 +268,7 @@ void ServiceWorkerScriptContext::PostCrossOriginMessageToClient( ...@@ -268,7 +268,7 @@ void ServiceWorkerScriptContext::PostCrossOriginMessageToClient(
// messages for MessagePort (e.g. QueueMessages) are sent from main thread // messages for MessagePort (e.g. QueueMessages) are sent from main thread
// (with thread hopping), so we need to do the same thread hopping here not // (with thread hopping), so we need to do the same thread hopping here not
// to overtake those messages. // to overtake those messages.
embedded_context_->main_thread_proxy()->PostTask( embedded_context_->main_thread_task_runner()->PostTask(
FROM_HERE, FROM_HERE,
base::Bind(&SendCrossOriginMessageToClientOnMainThread, base::Bind(&SendCrossOriginMessageToClientOnMainThread,
make_scoped_refptr(embedded_context_->thread_safe_sender()), make_scoped_refptr(embedded_context_->thread_safe_sender()),
...@@ -412,11 +412,12 @@ void ServiceWorkerScriptContext::OnPostMessage( ...@@ -412,11 +412,12 @@ void ServiceWorkerScriptContext::OnPostMessage(
"ServiceWorkerScriptContext::OnPostEvent"); "ServiceWorkerScriptContext::OnPostEvent");
std::vector<WebMessagePortChannelImpl*> ports; std::vector<WebMessagePortChannelImpl*> ports;
if (!sent_message_port_ids.empty()) { if (!sent_message_port_ids.empty()) {
base::MessageLoopProxy* loop_proxy = embedded_context_->main_thread_proxy(); base::SingleThreadTaskRunner* task_runner =
embedded_context_->main_thread_task_runner();
ports.resize(sent_message_port_ids.size()); ports.resize(sent_message_port_ids.size());
for (size_t i = 0; i < sent_message_port_ids.size(); ++i) { for (size_t i = 0; i < sent_message_port_ids.size(); ++i) {
ports[i] = new WebMessagePortChannelImpl( ports[i] = new WebMessagePortChannelImpl(
new_routing_ids[i], sent_message_port_ids[i], loop_proxy); new_routing_ids[i], sent_message_port_ids[i], task_runner);
} }
} }
...@@ -438,11 +439,12 @@ void ServiceWorkerScriptContext::OnCrossOriginMessageToWorker( ...@@ -438,11 +439,12 @@ void ServiceWorkerScriptContext::OnCrossOriginMessageToWorker(
"ServiceWorkerScriptContext::OnCrossOriginMessageToWorker"); "ServiceWorkerScriptContext::OnCrossOriginMessageToWorker");
std::vector<WebMessagePortChannelImpl*> ports; std::vector<WebMessagePortChannelImpl*> ports;
if (!sent_message_port_ids.empty()) { if (!sent_message_port_ids.empty()) {
base::MessageLoopProxy* loop_proxy = embedded_context_->main_thread_proxy(); base::SingleThreadTaskRunner* task_runner =
embedded_context_->main_thread_task_runner();
ports.resize(sent_message_port_ids.size()); ports.resize(sent_message_port_ids.size());
for (size_t i = 0; i < sent_message_port_ids.size(); ++i) { for (size_t i = 0; i < sent_message_port_ids.size(); ++i) {
ports[i] = new WebMessagePortChannelImpl( ports[i] = new WebMessagePortChannelImpl(
new_routing_ids[i], sent_message_port_ids[i], loop_proxy); new_routing_ids[i], sent_message_port_ids[i], task_runner);
} }
} }
......
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