Give NestableTasksAllowed() control back to MessageLoop.
While I still think RunLoop::Type::kNestableTasksAllowed is the right
API to allow nesting of application tasks in the usual case:
RunLoop::Delegate::Client::ProcessingTasksAllowed() wasn't the right
level of abstraction for the allowance (even without the bug it suffered).
When I hit https://bugs.chromium.org/p/chromium/issues/detail?id=754112#c6
I initially thought the solution was for Client::ProcessingTasksAllowed()
to return a scoped token to allow only a single application task at once
per RunLoop instance. But implementing it (see PS1) it became apparent
that this wasn't generally something most RunLoop::Delegates cared about.
Rather it was a fallout of some MessageLoops which permit reentrancy
without a nested RunLoop being involved (e.g. MessageLoop::Run() ->
MessageLoop::RunTask() can go through an OS-driven MessagePump which can
in some scenarios trigger reentrancy without a second call to Run()).
This CL keeps the API in RunLoop to declare allowance but then tells
RunLoop::Delegate::Run() via param whether it should allow application
tasks for that layer. Leaving it up to the impl to determine how to
control that.
This ends up getting rid of the duality of the previous check in
NestableTasksAllowed() as well. Now centralizing the logic in MessageLoop.
Much simpler than the alternative (again, see PS1).
While PS9 ended up keeping usage of a bool to control reentrancy (usage
of racing counters (allowed vs used) proved useless as they were either
equal or one off), it was renamed away from referring to "nestable tasks"
as this makes the nomenclature very confusing with "nested loops" which
are not the same thing at all. Instead we just name it
"task_execution_allowed_" and leave it up to MessageLoop logic
(DeferOrRunPendingTask()) to know to skip non-nestable tasks when inside
a nested loop.
For reference, see PS16-19 for attempts at making RunLoop be the sole
controller of nesting/nestable tasks allowance. This was dropped for now as
it proved to be ugly in conjunction with deprecated MessageLoop::*Nestable*
APIs that still need to be supported until callers are migrated.
Bug: 754112
Change-Id: I9f3a8431c00f69b0b5bb696e1e86e32bba6e6ffe
Reviewed-on: https://chromium-review.googlesource.com/730864
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511673}
CrOS-Libchrome-Original-Commit: b030a4a0884a87640ad65e51b20847c6213b8eab
8 files changed