Worker: Always return default task runners of the main thread when a frame is not given
Before this CL, ParentFrameTaskRunners::Create() with null-LocalFrame returns
default task runners of the current thread. Service workers call it in such a
manner because they don't have an associated frame. That creation function is
supposed to be called on the main thread, so the returned task runners should
always be default task runners of the main thread.
After this CL, the creation function is separated into two functions:
Create(LocalFrame&) and Create(). The former is equivalent to the previous
Create() with non-null-LocalFrame and always returns task runners associated
with the given frame. The latter is equivalent to the previous Create() with
null-LocalFrame and always returns default task runners of the main thread
regardless of the calling thread.
This enables service workers to create ParentFrameTaskRunners on
non-main-threads, and is useful for supporting off-main-thread service worker
start because it bypasses the main thread on startup and doesn't give a chance
to create the task runners on the main thread.
Bug: 692909
Change-Id: I1d5ac4e4515c5abddacb0b156957e04ca74a5f2e
Reviewed-on: https://chromium-review.googlesource.com/558761
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484208}diff --git a/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp b/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp
index 3f07a02..52997f68 100644
--- a/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp
+++ b/third_party/WebKit/Source/core/exported/WebSharedWorkerImpl.cpp
@@ -369,12 +369,11 @@
// SharedWorker can sometimes run tasks that are initiated by/associated with
// a document's frame but these documents can be from a different process. So
- // we intentionally populate the task runners with null document in order to
- // use the thread's default task runner. Note that |m_document| should not be
- // used as it's a dummy document for loading that doesn't represent the frame
- // of any associated document.
- ParentFrameTaskRunners* task_runners =
- ParentFrameTaskRunners::Create(nullptr);
+ // we intentionally populate the task runners with default task runners of the
+ // main thread. Note that |m_document| should not be used as it's a dummy
+ // document for loading that doesn't represent the frame of any associated
+ // document.
+ ParentFrameTaskRunners* task_runners = ParentFrameTaskRunners::Create();
reporting_proxy_ = new SharedWorkerReportingProxy(this, task_runners);
worker_thread_ = WTF::MakeUnique<SharedWorkerThread>(
diff --git a/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp b/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
index 75a3ac6..5b0b0a9 100644
--- a/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
+++ b/third_party/WebKit/Source/core/html/canvas/CanvasAsyncBlobCreator.cpp
@@ -194,7 +194,7 @@
num_rows_completed_ = 0;
if (document) {
parent_frame_task_runner_ =
- ParentFrameTaskRunners::Create(document->GetFrame());
+ ParentFrameTaskRunners::Create(*document->GetFrame());
}
if (script_promise_resolver_) {
function_type_ = kOffscreenCanvasToBlobPromise;
diff --git a/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp b/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp
index fcd6e6c..108af2e 100644
--- a/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp
+++ b/third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp
@@ -235,7 +235,7 @@
reporting_proxy_ = WTF::MakeUnique<WorkerReportingProxy>();
security_origin_ = GetDocument().GetSecurityOrigin();
parent_frame_task_runners_ =
- ParentFrameTaskRunners::Create(&dummy_page_holder_->GetFrame());
+ ParentFrameTaskRunners::Create(dummy_page_holder_->GetFrame());
worker_thread_ = WTF::MakeUnique<WorkerThreadForTest>(
ThreadableLoadingContext::Create(GetDocument()), *reporting_proxy_);
diff --git a/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.cpp b/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.cpp
index 02f5085..350f6c8 100644
--- a/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.cpp
+++ b/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.cpp
@@ -12,17 +12,28 @@
namespace blink {
+ParentFrameTaskRunners* ParentFrameTaskRunners::Create(LocalFrame& frame) {
+ DCHECK(frame.GetDocument());
+ DCHECK(frame.GetDocument()->IsContextThread());
+ DCHECK(IsMainThread());
+ return new ParentFrameTaskRunners(&frame);
+}
+
+ParentFrameTaskRunners* ParentFrameTaskRunners::Create() {
+ return new ParentFrameTaskRunners(nullptr);
+}
+
ParentFrameTaskRunners::ParentFrameTaskRunners(LocalFrame* frame)
: ContextLifecycleObserver(frame ? frame->GetDocument() : nullptr) {
- if (frame && frame->GetDocument())
- DCHECK(frame->GetDocument()->IsContextThread());
-
// For now we only support very limited task types.
for (auto type :
{TaskType::kUnspecedTimer, TaskType::kUnspecedLoading,
TaskType::kNetworking, TaskType::kPostedMessage,
TaskType::kCanvasBlobSerialization, TaskType::kUnthrottled}) {
- task_runners_.insert(type, TaskRunnerHelper::Get(type, frame));
+ auto task_runner =
+ frame ? TaskRunnerHelper::Get(type, frame)
+ : Platform::Current()->MainThread()->GetWebTaskRunner();
+ task_runners_.insert(type, std::move(task_runner));
}
}
diff --git a/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.h b/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.h
index 9e5da927..398a8403 100644
--- a/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.h
+++ b/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.h
@@ -20,12 +20,9 @@
class WebTaskRunner;
// Represents a set of task runners of the parent (or associated) document's
-// frame. This could be accessed from worker thread(s) and must be initialized
-// on the parent context thread (i.e. MainThread) on construction time, rather
-// than being done lazily.
+// frame, or default task runners of the main thread.
//
-// This observes LocalFrame lifecycle only for in-process worker cases (i.e.
-// only when a non-null LocalFrame is given).
+// This observes LocalFrame lifecycle only when this is created with LocalFrame.
class CORE_EXPORT ParentFrameTaskRunners final
: public GarbageCollectedFinalized<ParentFrameTaskRunners>,
public ContextLifecycleObserver {
@@ -33,11 +30,18 @@
WTF_MAKE_NONCOPYABLE(ParentFrameTaskRunners);
public:
- static ParentFrameTaskRunners* Create(LocalFrame* frame) {
- return new ParentFrameTaskRunners(frame);
- }
+ // Returns task runners associated with a given frame. This must be called on
+ // the frame's context thread, that is, the main thread. The given frame must
+ // have a valid execution context.
+ static ParentFrameTaskRunners* Create(LocalFrame&);
- // Might return nullptr for unsupported task types.
+ // Returns default task runners of the main thread. This can be called from
+ // any threads. This must be used only for shared workers, service workers and
+ // tests that don't have a parent frame.
+ static ParentFrameTaskRunners* Create();
+
+ // Might return nullptr for unsupported task types. This can be called from
+ // any threads.
RefPtr<WebTaskRunner> Get(TaskType);
DECLARE_VIRTUAL_TRACE();
diff --git a/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp b/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp
index 7cba209..505b177 100644
--- a/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp
+++ b/third_party/WebKit/Source/core/workers/ThreadedMessagingProxyBase.cpp
@@ -34,7 +34,7 @@
worker_clients_(worker_clients),
worker_inspector_proxy_(WorkerInspectorProxy::Create()),
parent_frame_task_runners_(ParentFrameTaskRunners::Create(
- ToDocument(execution_context_.Get())->GetFrame())),
+ *ToDocument(execution_context_.Get())->GetFrame())),
asked_to_terminate_(false),
keep_alive_(this) {
DCHECK(IsParentContextThread());
diff --git a/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp b/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp
index 45e2f87..29fc3ee 100644
--- a/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp
+++ b/third_party/WebKit/Source/core/workers/WorkerThreadTest.cpp
@@ -79,17 +79,17 @@
void TearDown() override {}
void Start() {
- worker_thread_->StartWithSourceCode(
- security_origin_.Get(), "//fake source code",
- ParentFrameTaskRunners::Create(nullptr));
+ worker_thread_->StartWithSourceCode(security_origin_.Get(),
+ "//fake source code",
+ ParentFrameTaskRunners::Create());
}
void StartWithSourceCodeNotToFinish() {
// Use a JavaScript source code that makes an infinite loop so that we
// can catch some kind of issues as a timeout.
- worker_thread_->StartWithSourceCode(
- security_origin_.Get(), "while(true) {}",
- ParentFrameTaskRunners::Create(nullptr));
+ worker_thread_->StartWithSourceCode(security_origin_.Get(),
+ "while(true) {}",
+ ParentFrameTaskRunners::Create());
}
void SetForcibleTerminationDelayInMs(
@@ -308,7 +308,7 @@
kWebAddressSpaceLocal, nullptr /* originTrialToken */,
nullptr /* WorkerSettings */, WorkerV8Settings::Default());
worker_thread_->Start(std::move(startup_data),
- ParentFrameTaskRunners::Create(nullptr));
+ ParentFrameTaskRunners::Create());
// Used to wait for worker thread termination in a debugger task on the
// worker thread.
diff --git a/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScopeTest.cpp b/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScopeTest.cpp
index 276ffba..e1da93f9 100644
--- a/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScopeTest.cpp
+++ b/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScopeTest.cpp
@@ -51,7 +51,7 @@
nullptr, kDontPauseWorkerGlobalScopeOnStart, nullptr, "",
security_origin_.Get(), clients, kWebAddressSpaceLocal, nullptr,
nullptr, WorkerV8Settings::Default()),
- ParentFrameTaskRunners::Create(nullptr));
+ ParentFrameTaskRunners::Create());
return thread;
}
diff --git a/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThreadTest.cpp b/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThreadTest.cpp
index c35fa6b0..3897dd4 100644
--- a/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThreadTest.cpp
+++ b/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThreadTest.cpp
@@ -87,7 +87,7 @@
nullptr, kDontPauseWorkerGlobalScopeOnStart, nullptr, "",
security_origin_.Get(), clients, kWebAddressSpaceLocal, nullptr,
nullptr, WorkerV8Settings::Default()),
- ParentFrameTaskRunners::Create(nullptr));
+ ParentFrameTaskRunners::Create());
return thread;
}
diff --git a/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp b/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
index 6622674..1fee025 100644
--- a/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
+++ b/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
@@ -96,7 +96,7 @@
public:
void SetUp() override {
CompositorWorkerThread::CreateSharedBackingThreadForTest();
- parent_frame_task_runners_ = ParentFrameTaskRunners::Create(nullptr);
+ parent_frame_task_runners_ = ParentFrameTaskRunners::Create();
object_proxy_ = TestCompositorWorkerObjectProxy::Create(
parent_frame_task_runners_.Get());
security_origin_ =
diff --git a/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp b/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp
index cd8100c..48ea22b2 100644
--- a/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp
+++ b/third_party/WebKit/Source/modules/exported/WebEmbeddedWorkerImpl.cpp
@@ -507,10 +507,9 @@
// We have a dummy document here for loading but it doesn't really represent
// the document/frame of associated document(s) for this worker. Here we
- // populate the task runners with null document not to confuse the frame
- // scheduler (which will end up using the thread's default task runner).
+ // populate the task runners with default task runners of the main thread.
worker_thread_->Start(std::move(startup_data),
- ParentFrameTaskRunners::Create(nullptr));
+ ParentFrameTaskRunners::Create());
worker_inspector_proxy_->WorkerThreadCreated(document, worker_thread_.get(),
worker_start_data_.script_url);
diff --git a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeProxy.cpp b/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeProxy.cpp
index 8cf550e6..e29973f 100644
--- a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeProxy.cpp
+++ b/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScopeProxy.cpp
@@ -584,9 +584,9 @@
worker_global_scope_(nullptr) {
// ServiceWorker can sometimes run tasks that are initiated by/associated with
// a document's frame but these documents can be from a different process. So
- // we intentionally populate the task runners with null document in order to
- // use the thread's default task runner.
- parent_frame_task_runners_ = ParentFrameTaskRunners::Create(nullptr);
+ // we intentionally populate the task runners with default task runners of the
+ // main thread.
+ parent_frame_task_runners_ = ParentFrameTaskRunners::Create();
}
void ServiceWorkerGlobalScopeProxy::Detach() {
diff --git a/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScopeTest.cpp b/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScopeTest.cpp
index d002990..8964619 100644
--- a/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScopeTest.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/AudioWorkletGlobalScopeTest.cpp
@@ -57,7 +57,7 @@
nullptr, kDontPauseWorkerGlobalScopeOnStart, nullptr, "",
security_origin_.Get(), nullptr, kWebAddressSpaceLocal, nullptr,
nullptr, WorkerV8Settings::Default()),
- ParentFrameTaskRunners::Create(nullptr));
+ ParentFrameTaskRunners::Create());
return thread;
}
diff --git a/third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp b/third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
index 9723af46..f494b603 100644
--- a/third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
+++ b/third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp
@@ -47,7 +47,7 @@
nullptr, kDontPauseWorkerGlobalScopeOnStart, nullptr, "",
security_origin_.Get(), nullptr, kWebAddressSpaceLocal, nullptr,
nullptr, WorkerV8Settings::Default()),
- ParentFrameTaskRunners::Create(nullptr));
+ ParentFrameTaskRunners::Create());
return thread;
}
