Commit ed98e25e authored by Fernando Serboncini's avatar Fernando Serboncini Committed by Commit Bot

Only execute Worker.RAF inside PostMessage taskrunner

If a RAF takes too long, OnBeginFrame may flood the task runner with
tasks and not allow postMessages to pass. This makes sure that RAFs are
executed in the same task queue as postMessages. We also change
BeginFrameProvider logic to only disable setNeedsBeginFrame with one
frame delay, to minimize the number of mojo calls.

Bug: 863962
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iec4b21d9e41571a9fc2d9880aa26ce45704d8d0c
Reviewed-on: https://chromium-review.googlesource.com/1139188
Commit-Queue: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Reviewed-by: default avatarKen Rockot <rockot@chromium.org>
Reviewed-by: default avatarJustin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577308}
parent d84bb42a
...@@ -124,6 +124,9 @@ enum class TaskType : unsigned { ...@@ -124,6 +124,9 @@ enum class TaskType : unsigned {
// The task runner may be throttled. // The task runner may be throttled.
kMiscPlatformAPI = 22, kMiscPlatformAPI = 22,
// Tasks used for DedicatedWorker's requestAnimationFrame.
kWorkerAnimation = 51,
/////////////////////////////////////// ///////////////////////////////////////
// Not-speced tasks should use one of the following task types // Not-speced tasks should use one of the following task types
/////////////////////////////////////// ///////////////////////////////////////
...@@ -194,7 +197,7 @@ enum class TaskType : unsigned { ...@@ -194,7 +197,7 @@ enum class TaskType : unsigned {
kWorkerThreadTaskQueueV8 = 47, kWorkerThreadTaskQueueV8 = 47,
kWorkerThreadTaskQueueCompositor = 48, kWorkerThreadTaskQueueCompositor = 48,
kCount = 51, kCount = 52,
}; };
} // namespace blink } // namespace blink
......
...@@ -30,9 +30,10 @@ ParentExecutionContextTaskRunners::ParentExecutionContextTaskRunners( ...@@ -30,9 +30,10 @@ ParentExecutionContextTaskRunners::ParentExecutionContextTaskRunners(
// For now we only support very limited task types. Sort in the TaskType enum // For now we only support very limited task types. Sort in the TaskType enum
// value order. // value order.
for (auto type : {TaskType::kNetworking, TaskType::kPostedMessage, for (auto type : {TaskType::kNetworking, TaskType::kPostedMessage,
TaskType::kInternalDefault, TaskType::kInternalLoading, TaskType::kWorkerAnimation, TaskType::kInternalDefault,
TaskType::kInternalTest, TaskType::kInternalMedia, TaskType::kInternalLoading, TaskType::kInternalTest,
TaskType::kInternalInspector, TaskType::kInternalWorker}) { TaskType::kInternalMedia, TaskType::kInternalInspector,
TaskType::kInternalWorker}) {
auto task_runner = auto task_runner =
context ? context->GetTaskRunner(type) context ? context->GetTaskRunner(type)
: Platform::Current()->CurrentThread()->GetTaskRunner(); : Platform::Current()->CurrentThread()->GetTaskRunner();
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/core/workers/worker_animation_frame_provider.h" #include "third_party/blink/renderer/core/workers/worker_animation_frame_provider.h"
#include "third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h" #include "third_party/blink/renderer/core/offscreencanvas/offscreen_canvas.h"
#include "third_party/blink/renderer/platform/cross_thread_functional.h"
#include "third_party/blink/renderer/platform/wtf/time.h" #include "third_party/blink/renderer/platform/wtf/time.h"
namespace blink { namespace blink {
...@@ -15,7 +16,9 @@ WorkerAnimationFrameProvider::WorkerAnimationFrameProvider( ...@@ -15,7 +16,9 @@ WorkerAnimationFrameProvider::WorkerAnimationFrameProvider(
: begin_frame_provider_( : begin_frame_provider_(
std::make_unique<BeginFrameProvider>(begin_frame_provider_params, std::make_unique<BeginFrameProvider>(begin_frame_provider_params,
this)), this)),
callback_collection_(context) {} callback_collection_(context),
context_(context),
weak_factory_(this) {}
int WorkerAnimationFrameProvider::RegisterCallback( int WorkerAnimationFrameProvider::RegisterCallback(
FrameRequestCallbackCollection::FrameCallback* callback) { FrameRequestCallbackCollection::FrameCallback* callback) {
...@@ -30,15 +33,23 @@ void WorkerAnimationFrameProvider::CancelCallback(int id) { ...@@ -30,15 +33,23 @@ void WorkerAnimationFrameProvider::CancelCallback(int id) {
} }
void WorkerAnimationFrameProvider::BeginFrame() { void WorkerAnimationFrameProvider::BeginFrame() {
double time = WTF::CurrentTimeTicksInMilliseconds(); // TODO(fserb): Remove this once the Mojo changes for scheduling are in.
// We don't want to expose microseconds residues to users. context_->GetTaskRunner(TaskType::kWorkerAnimation)
time = round(time * 60) / 60; ->PostTask(
FROM_HERE,
WTF::Bind(
[](base::WeakPtr<WorkerAnimationFrameProvider> provider) {
double time = WTF::CurrentTimeTicksInMilliseconds();
// We don't want to expose microseconds residues to users.
time = round(time * 60) / 60;
callback_collection_.ExecuteCallbacks(time, time); provider->callback_collection_.ExecuteCallbacks(time, time);
for (auto& offscreen_canvas : offscreen_canvases_) { for (auto& offscreen_canvas : provider->offscreen_canvases_) {
offscreen_canvas->PushFrameIfNeeded(); offscreen_canvas->PushFrameIfNeeded();
} }
},
weak_factory_.GetWeakPtr()));
} }
void WorkerAnimationFrameProvider::RegisterOffscreenCanvas( void WorkerAnimationFrameProvider::RegisterOffscreenCanvas(
...@@ -57,6 +68,7 @@ void WorkerAnimationFrameProvider::DeregisterOffscreenCanvas( ...@@ -57,6 +68,7 @@ void WorkerAnimationFrameProvider::DeregisterOffscreenCanvas(
void WorkerAnimationFrameProvider::Trace(blink::Visitor* visitor) { void WorkerAnimationFrameProvider::Trace(blink::Visitor* visitor) {
visitor->Trace(callback_collection_); visitor->Trace(callback_collection_);
visitor->Trace(context_);
} }
} // namespace blink } // namespace blink
...@@ -62,6 +62,10 @@ class CORE_EXPORT WorkerAnimationFrameProvider ...@@ -62,6 +62,10 @@ class CORE_EXPORT WorkerAnimationFrameProvider
// not hold strong references. // not hold strong references.
Vector<UntracedMember<OffscreenCanvas>> offscreen_canvases_; Vector<UntracedMember<OffscreenCanvas>> offscreen_canvases_;
bool in_begin_frame_; bool in_begin_frame_;
Member<ExecutionContext> context_;
base::WeakPtrFactory<WorkerAnimationFrameProvider> weak_factory_;
}; };
} // namespace blink } // namespace blink
......
...@@ -76,8 +76,9 @@ void BeginFrameProvider::CreateCompositorFrameSinkIfNeeded() { ...@@ -76,8 +76,9 @@ void BeginFrameProvider::CreateCompositorFrameSinkIfNeeded() {
void BeginFrameProvider::RequestBeginFrame() { void BeginFrameProvider::RequestBeginFrame() {
requested_needs_begin_frame_ = true; requested_needs_begin_frame_ = true;
if (needs_begin_frame_) if (needs_begin_frame_) {
return; return;
}
CreateCompositorFrameSinkIfNeeded(); CreateCompositorFrameSinkIfNeeded();
...@@ -87,11 +88,10 @@ void BeginFrameProvider::RequestBeginFrame() { ...@@ -87,11 +88,10 @@ void BeginFrameProvider::RequestBeginFrame() {
void BeginFrameProvider::OnBeginFrame(const viz::BeginFrameArgs& args) { void BeginFrameProvider::OnBeginFrame(const viz::BeginFrameArgs& args) {
// If there was no need for a BeginFrame, just skip it. // If there was no need for a BeginFrame, just skip it.
if (needs_begin_frame_) { if (needs_begin_frame_ && requested_needs_begin_frame_) {
requested_needs_begin_frame_ = false; requested_needs_begin_frame_ = false;
begin_frame_client_->BeginFrame(); begin_frame_client_->BeginFrame();
} else {
if (!requested_needs_begin_frame_) { if (!requested_needs_begin_frame_) {
needs_begin_frame_ = false; needs_begin_frame_ = false;
compositor_frame_sink_->SetNeedsBeginFrame(false); compositor_frame_sink_->SetNeedsBeginFrame(false);
......
...@@ -333,6 +333,7 @@ scoped_refptr<base::SingleThreadTaskRunner> FrameSchedulerImpl::GetTaskRunner( ...@@ -333,6 +333,7 @@ scoped_refptr<base::SingleThreadTaskRunner> FrameSchedulerImpl::GetTaskRunner(
// PostedMessage can be used for navigation, so we shouldn't defer it // PostedMessage can be used for navigation, so we shouldn't defer it
// when expecting a user gesture. // when expecting a user gesture.
case TaskType::kPostedMessage: case TaskType::kPostedMessage:
case TaskType::kWorkerAnimation:
// UserInteraction tasks should be run even when expecting a user gesture. // UserInteraction tasks should be run even when expecting a user gesture.
case TaskType::kUserInteraction: case TaskType::kUserInteraction:
// Media events should not be deferred to ensure that media playback is // Media events should not be deferred to ensure that media playback is
......
...@@ -219,6 +219,8 @@ const char* TaskTypeToString(TaskType task_type) { ...@@ -219,6 +219,8 @@ const char* TaskTypeToString(TaskType task_type) {
return "WorkerThreadTaskQueueV8"; return "WorkerThreadTaskQueueV8";
case TaskType::kWorkerThreadTaskQueueCompositor: case TaskType::kWorkerThreadTaskQueueCompositor:
return "WorkerThreadTaskQueueCompositor"; return "WorkerThreadTaskQueueCompositor";
case TaskType::kWorkerAnimation:
return "WorkerAnimation";
case TaskType::kCount: case TaskType::kCount:
return "Count"; return "Count";
} }
......
...@@ -92,6 +92,7 @@ scoped_refptr<base::SingleThreadTaskRunner> WorkerScheduler::GetTaskRunner( ...@@ -92,6 +92,7 @@ scoped_refptr<base::SingleThreadTaskRunner> WorkerScheduler::GetTaskRunner(
switch (type) { switch (type) {
case TaskType::kJavascriptTimer: case TaskType::kJavascriptTimer:
case TaskType::kPostedMessage: case TaskType::kPostedMessage:
case TaskType::kWorkerAnimation:
return TaskQueueWithTaskType::Create(throttleable_task_queue_, type); return TaskQueueWithTaskType::Create(throttleable_task_queue_, type);
case TaskType::kDeprecatedNone: case TaskType::kDeprecatedNone:
case TaskType::kDOMManipulation: case TaskType::kDOMManipulation:
......
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