New issue
Advanced search Search tips

Issue 750779 link

Starred by 1 user
Status: Fixed
Owner:
gab@chromium.org
Closed: Sep 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 781352
issue 703346



Sign in to add a comment

Migrate MessageLoop "nestable tasks allowed" concept to RunLoop

Project Member Reported by gab@chromium.org, Jul 31 2017 
Last blocker to making RunLoop support everything MessageLoop did as far as running tasks is concerned :)

MessageLoop::ScopedNestableTaskAllower class will be replaced by a RunLoop::Type enum param on RunLoop's constructor as the usage of the class is mostly to instantiate it before a RunLoop which is intended to allow nestable tasks.

It makes more sense to flag a given RunLoop to allow nestable tasks than it does to add scoped object in the scope of that same RunLoop.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ff403eecdd23a39853a4ebca52023fbba6c5d00

commit 3ff403eecdd23a39853a4ebca52023fbba6c5d00
Author: Gabriel Charette <gab@chromium.org>
Date: Mon Aug 07 04:22:48 2017

Introduce RunLoop::Type::NESTABLE_TASKS_ALLOWED to replace MessageLoop::ScopedNestableTaskAllower.

(as well as MessageLoop::SetNestableTasksAllowed())

Surveying usage: the scoped object is always instantiated right before
RunLoop().Run(). The intent is really to allow nestable tasks in that
RunLoop so it's better to explicitly label that RunLoop as such and it
allows us to break the last dependency that forced some RunLoop users
to use MessageLoop APIs.

There's also the odd case of allowing nestable tasks for loops that are
reentrant from a native task (without going through RunLoop), these
are the minority but will have to be handled (after cleaning up the
majority of cases that are RunLoop induced).
As highlighted by robliao@ in https://chromium-review.googlesource.com/c/600517
(which was merged in this CL).

R=danakj@chromium.org

Bug:  750779 
Change-Id: I43d122c93ec903cff3a6fe7b77ec461ea0656448
Reviewed-on: https://chromium-review.googlesource.com/594713
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492263}
[modify] https://crrev.com/3ff403eecdd23a39853a4ebca52023fbba6c5d00/base/message_loop/message_loop.cc
[modify] https://crrev.com/3ff403eecdd23a39853a4ebca52023fbba6c5d00/base/message_loop/message_loop.h
[modify] https://crrev.com/3ff403eecdd23a39853a4ebca52023fbba6c5d00/base/run_loop.cc
[modify] https://crrev.com/3ff403eecdd23a39853a4ebca52023fbba6c5d00/base/run_loop.h
[modify] https://crrev.com/3ff403eecdd23a39853a4ebca52023fbba6c5d00/base/run_loop_unittest.cc

Comment 2 by gab@chromium.org, Sep 15 2017

Status: Fixed (was: Started)

Comment 3 by gab@chromium.org, Nov 3 2017

Blocking: 781352
►

Sign in to add a comment