Patch 1/3 to get WebScheduler via WebThread

content/child/scheduler/worker_web_scheduler_impl.cc

Index: content/child/scheduler/worker_web_scheduler_impl.cc
diff --git a/content/child/scheduler/worker_web_scheduler_impl.cc b/content/child/scheduler/worker_web_scheduler_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..20c862838098b5f4c04607947c2a5a999df7a8cb
--- /dev/null
+++ b/content/child/scheduler/worker_web_scheduler_impl.cc
@@ -0,0 +1,103 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/child/scheduler/worker_web_scheduler_impl.h"
+
+#include "base/bind.h"
+#include "base/single_thread_task_runner.h"
+#include "content/child/scheduler/worker_scheduler.h"
+#include "third_party/WebKit/public/platform/WebTraceLocation.h"
+
+namespace content {
+
+WorkerWebSchedulerImpl::WorkerWebSchedulerImpl(

[rmcilroy, 2015/04/15 12:27:28]

Do we need a separate one of these for Worker and RendererScheduler? Could we
create a BaseScheduler interface which both RendererScheduler and
WorkerScheduler implement and use that here?

[alex clarke (OOO till 29th), 2015/04/15 14:25:46]

There are enough common bits to do that, and I think it improves readability.

[rmcilroy, 2015/04/15 17:58:15]

This wasn't actually what I meant. I meant that we could have a single
WebSchedulerImpl class which is used by the renderer_web_thread_impl and the
worker_web_thread_impl, with the only difference being the scheduler passed in
its constructer (RendererScheduler for the former and WorkerScheduler for the
latter). This would mean we would need to define a common BaseScheduler
interface which both RenderScheduler and WorkerScheduler inherit from (I realise
we dropped this from your original refractor CL but it seems like it would be
useful here). 

Would this be possible?

[alex clarke (OOO till 29th), 2015/04/16 09:11:55]

I suspect we'd end up with diamond inheritance problems if we introduced a
common BaseScheduler. Avoiding that will almost certainly snowball into quite a
big refactor and as such it's not appropriate to do that in this patch.

[Sami, 2015/04/16 10:44:01]

My view is that the WebScheduler adapters should be modeled after the family
tree of {Render,Worker}Scheduler. Right now there's no inheritance in the
former, so following that pattern here seems fine to me.

EDIT: based on offline discussion Ross will try to add a pure virtual
BaseScheduler that both RendererScheduler and WorkerScheduler implement.

[rmcilroy, 2015/04/16 11:21:05]

CL adds the common base scheduler (which I've named ChildScheduler) up at
https://codereview.chromium.org/1089833002

+    WorkerScheduler* worker_scheduler)
+    : worker_scheduler_(worker_scheduler),
+      idle_task_runner_(worker_scheduler_->IdleTaskRunner()),
+      task_runner_(worker_scheduler_->DefaultTaskRunner()) {
+}
+
+WorkerWebSchedulerImpl::~WorkerWebSchedulerImpl() {
+}
+
+bool WorkerWebSchedulerImpl::shouldYieldForHighPriorityWork() {
+  return false;
+}
+
+bool WorkerWebSchedulerImpl::canExceedIdleDeadlineIfRequired() {
+  return worker_scheduler_->CanExceedIdleDeadlineIfRequired();
+}
+
+void WorkerWebSchedulerImpl::runIdleTask(
+    scoped_ptr<blink::WebThread::IdleTask> task,
+    base::TimeTicks deadline) {
+  task->run((deadline - base::TimeTicks()).InSecondsF());
+}
+
+void WorkerWebSchedulerImpl::runTask(scoped_ptr<blink::WebThread::Task> task) {
+  task->run();
+}
+
+void WorkerWebSchedulerImpl::postIdleTask(
+    const blink::WebTraceLocation& web_location,
+    blink::WebThread::IdleTask* task) {
+  DCHECK(idle_task_runner_);
+  scoped_ptr<blink::WebThread::IdleTask> scoped_task(task);
+  tracked_objects::Location location(web_location.functionName(),

[Sami, 2015/04/15 12:37:13]

Is it worth writing a (possibly templated) helper to cut down on the repetition
here?

[alex clarke (OOO till 29th), 2015/04/15 14:25:46]

I was unable to get one template that can do it all because it needs to choose
between runIdleTask and runTask.  I suppose partial template specialization or
passing in a templatized function pointer would do it but really I think it was
getting way too complicated for little benefit.

A macro based approach would be easier but that's icky.

[Sami, 2015/04/16 10:44:01]

Okay, thanks for giving it a try anyway!

+                                     web_location.fileName(), -1, nullptr);
+  idle_task_runner_->PostIdleTask(
+      location, base::Bind(&WorkerWebSchedulerImpl::runIdleTask,
+                           base::Passed(&scoped_task)));
+}
+
+void WorkerWebSchedulerImpl::postNonNestableIdleTask(
+    const blink::WebTraceLocation& web_location,
+    blink::WebThread::IdleTask* task) {
+  DCHECK(idle_task_runner_);
+  scoped_ptr<blink::WebThread::IdleTask> scoped_task(task);
+  tracked_objects::Location location(web_location.functionName(),
+                                     web_location.fileName(), -1, nullptr);
+  idle_task_runner_->PostNonNestableIdleTask(
+      location, base::Bind(&WorkerWebSchedulerImpl::runIdleTask,
+                           base::Passed(&scoped_task)));
+}
+
+void WorkerWebSchedulerImpl::postIdleTaskAfterWakeup(
+    const blink::WebTraceLocation& web_location,
+    blink::WebThread::IdleTask* task) {
+  DCHECK(idle_task_runner_);
+  scoped_ptr<blink::WebThread::IdleTask> scoped_task(task);
+  tracked_objects::Location location(web_location.functionName(),
+                                     web_location.fileName(), -1, nullptr);
+  idle_task_runner_->PostIdleTaskAfterWakeup(
+      location, base::Bind(&WorkerWebSchedulerImpl::runIdleTask,
+                           base::Passed(&scoped_task)));
+}
+
+void WorkerWebSchedulerImpl::postLoadingTask(
+    const blink::WebTraceLocation& web_location,
+    blink::WebThread::Task* task) {
+  DCHECK(task_runner_);
+  scoped_ptr<blink::WebThread::Task> scoped_task(task);
+  tracked_objects::Location location(web_location.functionName(),
+                                     web_location.fileName(), -1, nullptr);
+  task_runner_->PostTask(location, base::Bind(&WorkerWebSchedulerImpl::runTask,
+                                              base::Passed(&scoped_task)));
+}
+
+void WorkerWebSchedulerImpl::postTimerTask(
+    const blink::WebTraceLocation& web_location,
+    blink::WebThread::Task* task,
+    long long delayMs) {
+  DCHECK(task_runner_);
+  scoped_ptr<blink::WebThread::Task> scoped_task(task);
+  tracked_objects::Location location(web_location.functionName(),
+                                     web_location.fileName(), -1, nullptr);
+  task_runner_->PostDelayedTask(
+      location,
+      base::Bind(&WorkerWebSchedulerImpl::runTask, base::Passed(&scoped_task)),
+      base::TimeDelta::FromMilliseconds(delayMs));
+}
+
+}  // namespace content