-
Gabriel Charette authored
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}
b030a4a0