Commit 1698e0ae authored by Gabriel Charette's avatar Gabriel Charette Committed by Commit Bot

[base] Don't implicitly allow nesting in microtasks

This allowance seems like an unintentional overlook of the fact that
DidRunTask() runs microtasks. Microtasks should be treated the same
as regular application tasks and not be allowed to implicitly nest
application tasks.

R=altimin@chromium.org

Change-Id: Ie3f9d41d7525e465f813c7e2456cce3f4394ad57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537002
Commit-Queue: Gabriel Charette <gab@chromium.org>
Auto-Submit: Gabriel Charette <gab@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827943}
parent ce4f1b69
......@@ -183,15 +183,12 @@ void ThreadControllerImpl::DoWork(WorkType work_type) {
// [OnTaskStarted(), OnTaskEnded()] must outscope all other tracing calls
// so that the "ThreadController active" trace event lives on top of all
// "run task" events. It must also encompass DidRunTask() to cover
// microtasks.
// "run task" events.
DCHECK_GT(main_sequence_only().run_level_tracker.num_run_levels(), 0U);
main_sequence_only().run_level_tracker.OnTaskStarted();
{
// Trace-parsing tools (DevTools, Lighthouse, etc) consume this event
// to determine long tasks.
// The event scope must span across DidRunTask call below to make sure
// it covers RunMicrotasks event.
// See https://crbug.com/681863 and https://crbug.com/874982
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "RunTask");
......@@ -204,6 +201,8 @@ void ThreadControllerImpl::DoWork(WorkType work_type) {
return;
}
// This processes microtasks, hence all scoped operations above must end
// after it.
sequence_->DidRunTask();
}
main_sequence_only().run_level_tracker.OnTaskEnded();
......
......@@ -336,17 +336,15 @@ TimeDelta ThreadControllerWithMessagePumpImpl::DoWorkImpl(
// [OnTaskStarted(), OnTaskEnded()] must outscope all other tracing calls
// so that the "ThreadController active" trace event lives on top of all
// "run task" events. It must also encompass DidRunTask() to cover
// microtasks.
// "run task" events.
main_thread_only().run_level_tracker.OnTaskStarted();
{
// Execute the task and assume the worst: it is probably not reentrant.
main_thread_only().task_execution_allowed = false;
AutoReset<bool> ban_nested_application_tasks(
&main_thread_only().task_execution_allowed, false);
// Trace-parsing tools (DevTools, Lighthouse, etc) consume this event
// to determine long tasks.
// The event scope must span across DidRunTask call below to make sure
// it covers RunMicrotasks event.
// See https://crbug.com/681863 and https://crbug.com/874982
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "RunTask");
......@@ -365,7 +363,8 @@ TimeDelta ThreadControllerWithMessagePumpImpl::DoWorkImpl(
}
#endif
main_thread_only().task_execution_allowed = true;
// This processes microtasks, hence all scoped operations above must end
// after it.
main_thread_only().task_source->DidRunTask();
}
main_thread_only().run_level_tracker.OnTaskEnded();
......
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