base::SequencedTaskRunner::GetCurrentDefault() in the renderer

[Daniel Cheng]

While reviewing a CL, I noticed that there are a few instances of base::SequencedTaskRunner::GetCurrentDefault() and/or base::BindPostTaskToCurrentDefault() in Blink.

I thought that this was disallowed in Blink, but when I checked https://chromium.googlesource.com/chromium/src/+/main/third_party/blink/renderer/platform/scheduler/TaskSchedulingInBlink.md#Thread_global-task-runners, it actually doesn't mention anything about SequencedTaskRunner.

1. Should we update the docs to clarify the restrictions (if any) around the current default SequencedTaskRunner?
2. If we want to generally disallow GetCurrentDefault(), are we evaluating and/or prompting people to fix uses in the renderer?
3. Should we try to make the restrictions a bit more fine-grained? audit_non_blink_usage.py is a rather jumbled mess (sorry, this is my fault; I wrote the original without any real thought about scaling the list to hundreds of lines), but the current allowlist generally allows things per-directory. I was thinking that maybe we should have (currently hypothetical) base::DisallowCurrentDefaultScope at the top-level in RendererMain, and then base::AllowCurrentDefaultScope to opt-in individual uses. This would, of course, just be a DCHECK, but it might be more fine-grained than what we currently have?

Daniel

[Wez]

Could we arrange for this to be statically checked, e.g. by moving that (and other non-Renderer things) into distinct GN targets and using assert-no-deps + #include validation to guard?

--
You received this message because you are subscribed to the Google Groups "scheduler-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scheduler-de...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/scheduler-dev/CAF3XrKp8oV%2B6h9RguOA%2BDW_7DgzV6ttE0L3nusLv_%3DKsSvkeZQ%40mail.gmail.com.

[Dave Tapuska]

I prefer Daniel's approach with the DisallowCurrentDefaultScope...

Things like: https://source.chromium.org/chromium/chromium/src/+/main:mojo/public/cpp/bindings/lib/task_runner_helper.cc;drc=ac83a5a2d3c04763d86ce16d92f3904cc9566d3a;l=13 can get called with blink code... even though the SequencedTaskRunner isn't inside blink code. If blink provides a null task runner for any mojo receiver it will end up calling this code.

dave.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/scheduler-dev/CALekkJczJEH3OFc0SSGT_Cd-0vE_YVoUQNCzrr1uV5JSDSHW6A%40mail.gmail.com.

[François Doray]

base::SequencedTaskRunner::GetCurrentDefault() is disallowed in //blink via presubmit [code] but some sub-directories have exceptions [example code]. Like @Dave Tapuska, I like the idea of a runtime check because we can enforce requirements even when //third_party/blink/ calls non-Blink code.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/scheduler-dev/CAHgVhZXJT64hW%3DMyKHJTSACAoe%3D8xpX%2Bkqq_bYc84eafBQDAFw%40mail.gmail.com.