Make base::MessageLoop support external task sources |
|||||
Issue descriptionThis is a prerequisite to support ScopedTaskEnvironment::MOCK_TIME on top of custom MessageLoops (i.e. ForUI/ForIO). This will also enable mocking the TaskScheduler's service thread (which uses a MessageLoopForIO) and as such having MOCK_TIME also affect TaskScheduler tasks. Then this will allow MOCK_TIME on TestBrowserThreadBundle (which uses a MessageLoopForUI); nicely rounding out the MOCK_TIME APIs. Which will in turn allow fixing base::Timer's non-thread-safe test patterns (i.e. getting rid of Timer::SetTaskRunner() for the most part -- issue 587199 ), adding checks that it's strictly used from a single sequence. Which will in turn allow a new design for base::Timer and solve major issues with accumulation of delayed tasks in MessageLoop's DelayedQueue (https://bugs.chromium.org/p/chromium/issues/detail?id=786597#c5). Woohoo! The reason this is a prerequisite is that mocking time applies to application tasks, but when out of tasks in a non-default MessageLoop, control must be left in the hands of the MessagePump, not the mocked-time application task source, as the next message might be coming in from the platform (e.g. UI or overlapped IO event). Doing so means there needs to be a notion of MessageLoop being notified of new application tasks. There also needs to be a way for MessageLoop to tell its task source when it is idle (as MOCK_TIME dictates that time should advance when reaching idle && !quit_when_idle). This is where the SequencedTaskSource API comes in (see go/jolly-jumper and upcoming CL @ https://chromium-review.googlesource.com/c/chromium/src/+/1088762). The current MessageLoop design is as follows: MessagePump(Type) -> asks MessageLoop to do work through the MessagePump::Delegate interface MessageLoop -> takes tasks from IncomingTaskQueue's queues on MessagePump's request (TriageQueue, DelayedQueue, DeferredQueue) IncomingTaskQueue -> Ref-counted API which accumulates posted tasks (and delayed/deferred tasks), manages thread-safe queue Also notifies MessageLoop of available work (ScheduleWork) MessageLoopTaskRunner -> Ref-counted API which receives tasks and immediately forwards them to IncomingTaskQueue on shutdown: IncomingTaskQueue is detached from MessageLoop (IncomingTaskQueue::WillDestroyCurrentMesssageLoop()). Only MessageLoopTaskRunner is left with a reference to it (and callers may indefinitely hold references to this bundle through a SingleThreadTaskRunner obtained in various ways -- PostTask() will return false from this point on). Proposal: MessagePump(Type) -> Same. MessageLoop -> takes tasks from IncomingTaskQueue still (diff: now uniquely owns IncomingTaskQueue and MessageLoopTaskRunner is constructed before it as the default SequencedTaskSource if none was provided) IncomingTaskQueue -> Thread-hostile (bound to MessageLoop thread) queues for tasks, feeding from a SequencedTaskSource No-longer ref-counted (uniquely owned by MessageLoop), now pull (from SequencedTaskSource) instead of push model (from MessageLoopTaskRunner). MessageLoopTaskRunner -> thread-safe queue (moved from IncomingTaskQueue -- keeps optimization of only reloading from thread-safe incoming queue once out of tasks), is-a SequencedTaskSource (and isn't created if an external SequencedTaskSource was provided) Isn't aware of IncomingTaskQueue (its consumer) SequencedTaskSource -> Vends tasks to IncomingTaskQueue (pull) and notifies SequencedTaskSource::Observer when a task is queued (push) MessageLoop::Controller -> is-a SequencedTaskSource::Observer and manages ScheduleWork (moved from IncomingTaskQueue). Created by MessageLoop and handed-off to SequencedTaskSource in MessageLoop's constructor. on shutdown: MessageLoop::Controller is detached from its parent MessageLoop (queuing notifications now no-ops) IncomingTaskQueue is deleted along with MessageLoop MessageLoopTaskRunner is notified through SequencedTaskSource::Shutdown() returns false from PostTask(). bonus: Previously IncomingTaskQueue managed two Locks (|incoming_queue_lock_| for thread-safe task posting and |message_loop_lock_| for thread-safe/shutdown-safe MessageLoop scheduling). With this CL, the distinction is clearer : |incoming_queue_lock_| moves to MessageLoopTaskRunner and |message_loop_lock_| moves to MessageLoop::Controller. IncomingTaskQueue and its queues are now thread-hostile and strictly used on MessageLoop's thread. WIP CL : https://chromium-review.googlesource.com/c/chromium/src/+/1088762/7
,
Jul 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54b9310a85b7dcc93c1f2ea7d082ce4e0fc00add commit 54b9310a85b7dcc93c1f2ea7d082ce4e0fc00add Author: Gabriel Charette <gab@chromium.org> Date: Wed Jul 04 15:50:13 2018 [MessageLoop] Fix crash in TaskObserverPerfTest MessageLoop::AddTaskOserver now has a thread-affine DCHECK Caught while running base_perftests.exe locally, too bad waterfall doesn't run these (even if not analyzing actual perf results). R=danakj@chromium.org, kylechar@chromium.org Bug: 860252 Change-Id: I6d09f29bea7e56e1b315c1d5789b4c1cd65b9abb Reviewed-on: https://chromium-review.googlesource.com/1126320 Reviewed-by: danakj <danakj@chromium.org> Reviewed-by: kylechar <kylechar@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#572591} [modify] https://crrev.com/54b9310a85b7dcc93c1f2ea7d082ce4e0fc00add/base/threading/thread_perftest.cc
,
Jul 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3498afc6665fcd4731277e21052caffdd324b245 commit 3498afc6665fcd4731277e21052caffdd324b245 Author: Gabriel Charette <gab@chromium.org> Date: Thu Jul 05 18:08:35 2018 [MessageLoop] Improved MessageLoop PostTaskPerfTests Moved from message_pump_perftest.cc where it didn't really belong. Augmented the existing test to also use MessageLoopTaskRunner. Introduced a new test using the full MessageLoop/RunLoop stack. This is a prerequisite for the IncomingTaskQueue refactoring coming up @ https://chromium-review.googlesource.com/c/chromium/src/+/1088762 R=danakj@chromium.org, kylechar@chromium.org Bug: 860252 Change-Id: I7cabeaff2e3a28cef2f5b568c46c71d834ffd4f3 Reviewed-on: https://chromium-review.googlesource.com/1127143 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#572829} [modify] https://crrev.com/3498afc6665fcd4731277e21052caffdd324b245/base/BUILD.gn [modify] https://crrev.com/3498afc6665fcd4731277e21052caffdd324b245/base/message_loop/incoming_task_queue.h [add] https://crrev.com/3498afc6665fcd4731277e21052caffdd324b245/base/message_loop/message_loop_task_runner_perftest.cc [modify] https://crrev.com/3498afc6665fcd4731277e21052caffdd324b245/base/message_loop/message_pump_perftest.cc
,
Jul 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4036f8289df032ccc7bbf43e1a295d2a6db613ec commit 4036f8289df032ccc7bbf43e1a295d2a6db613ec Author: Gabriel Charette <gab@chromium.org> Date: Fri Jul 06 18:16:45 2018 Rename TaskAnnotator::DidQueueTask to better represent semantics Per the move-only nature of PendingTask (OnceClosure), DidQueueTask() already had to be invoked *before* queueing the task. Furthermore, it did modify metadata of the PendingTask and as such calling it after moving the task would have usually been wrong. (me aculpa : I'm the one that made task_backtrace mutable to enable this const& modification..!) This confused me when writing https://chromium-review.googlesource.com/c/chromium/src/+/1127262 and the upcoming task source observer interface will need a notion of WillQueueTask() vs DidQueueTask(). This change was extracted from it since this touches a lot of files. TBR=derat@chromium.org (alarm_timer_chromeos.cc side-effect) Bug: 860252 Change-Id: I41328fa25a929f602672e3c11176e62906a19d95 Reviewed-on: https://chromium-review.googlesource.com/1127263 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Reviewed-by: François Doray <fdoray@chromium.org> Reviewed-by: Alexander Timin <altimin@chromium.org> Cr-Commit-Position: refs/heads/master@{#573014} [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/debug/task_annotator.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/debug/task_annotator.h [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/debug/task_annotator_unittest.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/message_loop/incoming_task_queue.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/pending_task.h [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task/sequence_manager/sequence_manager_impl.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task/sequence_manager/sequence_manager_impl.h [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task/sequence_manager/task_queue_impl.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task/sequence_manager/thread_controller.h [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task/sequence_manager/thread_controller_impl.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task/sequence_manager/thread_controller_impl.h [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task_scheduler/scheduler_single_thread_task_runner_manager.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task_scheduler/scheduler_worker_pool.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task_scheduler/scheduler_worker_unittest.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task_scheduler/task_tracker.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task_scheduler/task_tracker.h [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task_scheduler/task_tracker_posix_unittest.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/base/task_scheduler/task_tracker_unittest.cc [modify] https://crrev.com/4036f8289df032ccc7bbf43e1a295d2a6db613ec/components/timers/alarm_timer_chromeos.cc
,
Jul 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4714a9fc4899c359fbe2b7be7c0269175b977e2b commit 4714a9fc4899c359fbe2b7be7c0269175b977e2b Author: Gabriel Charette <gab@chromium.org> Date: Wed Jul 11 02:46:45 2018 [MessageLoop] Refactor ScheduleWork and TaskAnnotator logic out of IncomingTaskQueue This was extracted from https://chromium-review.googlesource.com/c/chromium/src/+/1088762/17 in an attempt to simplify it (and is now a precursor to it). (MessageLoop::Controller was IncomingTaskQueue::MessageLoopController there but after grokking the resulting CL it's simpler under MessageLoop as done in this CL) This CL splits the logic in IncomingTaskQueue which took care of scheduling the MessageLoop into a dedicated class. In that follow-up CL: MessageLoopTaskRunner will interact directly with the task Observer and take IncomingTaskQueue completely out of the picture (merely a data structure holding various task queues at that point). IncomingTaskQueue also took care of detaching on MessageLoop shutdown which was moved to the new Controller class as well. message_loop.cc is the best place for this extracted logic as it all pertains precisely to MessageLoop's implementation detail (how ScheduleWork should be invoked). This CL simplifies locking as well by having a clear separation between the two locks instead of two locks in the same class used interchangibly. |incoming_queue_lock_| is now strictly for incoming tasks. |message_loop_lock_| is now strictly for ScheduleWork()/DisconnectFromParent(). Note: |message_loop_scheduled_| was dropped as it was redundant (always equal to |!was_empty|). Performance wise, the perf tests show that this change is a noop : * While BasicPostTaskPerfTest became simpler (executed less code) with this CL : The new BasicPostTaskPerfTest w/ MockObserverSimulatingOverhead reintroduces that overhead to show that it's still the same (or slightly in favor of this CL). * And the IntegratedPostTaskPerfTest are the same. * Augmented perf tests to 30 seconds which yields more reliable results. (and ran old ones under 30s mode too when comparing) * Results : https://docs.google.com/spreadsheets/d/100wYvbCI_dJ7gRnQiSsYaTb5OJnbF_muL6LyQWJLXSU/edit Bug: 860252 Change-Id: I22de2409d52414524cc125b0e2fe08e2c516fcbe Reviewed-on: https://chromium-review.googlesource.com/1127262 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Reviewed-by: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/heads/master@{#574048} [modify] https://crrev.com/4714a9fc4899c359fbe2b7be7c0269175b977e2b/base/message_loop/incoming_task_queue.cc [modify] https://crrev.com/4714a9fc4899c359fbe2b7be7c0269175b977e2b/base/message_loop/incoming_task_queue.h [modify] https://crrev.com/4714a9fc4899c359fbe2b7be7c0269175b977e2b/base/message_loop/message_loop.cc [modify] https://crrev.com/4714a9fc4899c359fbe2b7be7c0269175b977e2b/base/message_loop/message_loop.h [modify] https://crrev.com/4714a9fc4899c359fbe2b7be7c0269175b977e2b/base/message_loop/message_loop_task_runner_perftest.cc
,
Jul 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c19aaad0b9992edc7d6329023f2084a52edb7e15 commit c19aaad0b9992edc7d6329023f2084a52edb7e15 Author: Gabriel Charette <gab@chromium.org> Date: Sun Jul 15 03:53:35 2018 Revert "[MessageLoop] Refactor ScheduleWork and TaskAnnotator logic out of IncomingTaskQueue" This reverts commit 4714a9fc4899c359fbe2b7be7c0269175b977e2b. Reason for revert: perf regression (crbug.com/863125). I think I've identified the culprit but reverting to avoid compounding CLs if I'm wrong. Will reland with fix shortly. Original change's description: > [MessageLoop] Refactor ScheduleWork and TaskAnnotator logic out of IncomingTaskQueue > > This was extracted from > https://chromium-review.googlesource.com/c/chromium/src/+/1088762/17 > in an attempt to simplify it (and is now a precursor to it). > (MessageLoop::Controller was IncomingTaskQueue::MessageLoopController > there but after grokking the resulting CL it's simpler under MessageLoop > as done in this CL) > > This CL splits the logic in IncomingTaskQueue which took care of > scheduling the MessageLoop into a dedicated class. In that follow-up CL: > MessageLoopTaskRunner will interact directly with the task Observer and > take IncomingTaskQueue completely out of the picture (merely a data > structure holding various task queues at that point). > > IncomingTaskQueue also took care of detaching on MessageLoop shutdown > which was moved to the new Controller class as well. > > message_loop.cc is the best place for this extracted logic as it all > pertains precisely to MessageLoop's implementation detail (how > ScheduleWork should be invoked). > > This CL simplifies locking as well by having a clear separation between > the two locks instead of two locks in the same class used > interchangibly. |incoming_queue_lock_| is now strictly for incoming > tasks. |message_loop_lock_| is now strictly for > ScheduleWork()/DisconnectFromParent(). > > Note: |message_loop_scheduled_| was dropped as it was redundant (always > equal to |!was_empty|). > > Performance wise, the perf tests show that this change is a noop : > * While BasicPostTaskPerfTest became simpler (executed less code) with > this CL : > The new BasicPostTaskPerfTest w/ MockObserverSimulatingOverhead > reintroduces that overhead to show that it's still the same (or > slightly in favor of this CL). > * And the IntegratedPostTaskPerfTest are the same. > * Augmented perf tests to 30 seconds which yields more reliable results. > (and ran old ones under 30s mode too when comparing) > * Results : > https://docs.google.com/spreadsheets/d/100wYvbCI_dJ7gRnQiSsYaTb5OJnbF_muL6LyQWJLXSU/edit > > Bug: 860252 > Change-Id: I22de2409d52414524cc125b0e2fe08e2c516fcbe > Reviewed-on: https://chromium-review.googlesource.com/1127262 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: danakj <danakj@chromium.org> > Reviewed-by: kylechar <kylechar@chromium.org> > Cr-Commit-Position: refs/heads/master@{#574048} TBR=danakj@chromium.org,gab@chromium.org,kylechar@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 860252 , 863125 Change-Id: I871a9e87d799b7966558708826d97400162e0e82 Reviewed-on: https://chromium-review.googlesource.com/1137893 Reviewed-by: Gabriel Charette <gab@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#575173} [modify] https://crrev.com/c19aaad0b9992edc7d6329023f2084a52edb7e15/base/message_loop/incoming_task_queue.cc [modify] https://crrev.com/c19aaad0b9992edc7d6329023f2084a52edb7e15/base/message_loop/incoming_task_queue.h [modify] https://crrev.com/c19aaad0b9992edc7d6329023f2084a52edb7e15/base/message_loop/message_loop.cc [modify] https://crrev.com/c19aaad0b9992edc7d6329023f2084a52edb7e15/base/message_loop/message_loop.h [modify] https://crrev.com/c19aaad0b9992edc7d6329023f2084a52edb7e15/base/message_loop/message_loop_task_runner_perftest.cc
,
Jul 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/281621ef25c8b7fe829a6054e8785282aabc629a commit 281621ef25c8b7fe829a6054e8785282aabc629a Author: Gabriel Charette <gab@chromium.org> Date: Tue Jul 17 16:47:36 2018 Reland "[MessageLoop] Refactor ScheduleWork and TaskAnnotator logic out of IncomingTaskQueue" This is a reland of 4714a9fc4899c359fbe2b7be7c0269175b977e2b (PS1 is the original CL) Reverted @ https://chromium-review.googlesource.com/c/chromium/src/+/1137893 for a perf regression. Suspected cause : |IncomingTaskQueue::message_loop_scheduled_| wasn't equivalent to |was_empty| after all since |incoming_queue_| being empty doesn't mean |outgoing_queue_| is empty (hence setting a bit to false when both |outgoing_queue_| and |incoming_queue_| are empty is important to avoid scheduling more than necessary. Original change's description: > [MessageLoop] Refactor ScheduleWork and TaskAnnotator logic out of IncomingTaskQueue > > This was extracted from > https://chromium-review.googlesource.com/c/chromium/src/+/1088762/17 > in an attempt to simplify it (and is now a precursor to it). > (MessageLoop::Controller was IncomingTaskQueue::MessageLoopController > there but after grokking the resulting CL it's simpler under MessageLoop > as done in this CL) > > This CL splits the logic in IncomingTaskQueue which took care of > scheduling the MessageLoop into a dedicated class. In that follow-up CL: > MessageLoopTaskRunner will interact directly with the task Observer and > take IncomingTaskQueue completely out of the picture (merely a data > structure holding various task queues at that point). > > IncomingTaskQueue also took care of detaching on MessageLoop shutdown > which was moved to the new Controller class as well. > > message_loop.cc is the best place for this extracted logic as it all > pertains precisely to MessageLoop's implementation detail (how > ScheduleWork should be invoked). > > This CL simplifies locking as well by having a clear separation between > the two locks instead of two locks in the same class used > interchangibly. |incoming_queue_lock_| is now strictly for incoming > tasks. |message_loop_lock_| is now strictly for > ScheduleWork()/DisconnectFromParent(). > > Note: |message_loop_scheduled_| was dropped as it was redundant (always > equal to |!was_empty|). This last statement was incorrect. The equivalent of |message_loop_scheduled_| was incorporated into |was_empty| in PS2 of this reland. > > Performance wise, the perf tests show that this change is a noop : > * While BasicPostTaskPerfTest became simpler (executed less code) with > this CL : > The new BasicPostTaskPerfTest w/ MockObserverSimulatingOverhead > reintroduces that overhead to show that it's still the same (or > slightly in favor of this CL). > * And the IntegratedPostTaskPerfTest are the same. > * Augmented perf tests to 30 seconds which yields more reliable results. > (and ran old ones under 30s mode too when comparing) > * Results : > https://docs.google.com/spreadsheets/d/100wYvbCI_dJ7gRnQiSsYaTb5OJnbF_muL6LyQWJLXSU/edit > > Bug: 860252 > Change-Id: I22de2409d52414524cc125b0e2fe08e2c516fcbe > Reviewed-on: https://chromium-review.googlesource.com/1127262 > Commit-Queue: Gabriel Charette <gab@chromium.org> > Reviewed-by: danakj <danakj@chromium.org> > Reviewed-by: kylechar <kylechar@chromium.org> > Cr-Commit-Position: refs/heads/master@{#574048} Bug: 860252 Change-Id: Ibe52413366e9b3ff38c9cb51616f9b8938ca68ad Reviewed-on: https://chromium-review.googlesource.com/1137894 Reviewed-by: kylechar <kylechar@chromium.org> Commit-Queue: Gabriel Charette <gab@chromium.org> Cr-Commit-Position: refs/heads/master@{#575678} [modify] https://crrev.com/281621ef25c8b7fe829a6054e8785282aabc629a/base/message_loop/incoming_task_queue.cc [modify] https://crrev.com/281621ef25c8b7fe829a6054e8785282aabc629a/base/message_loop/incoming_task_queue.h [modify] https://crrev.com/281621ef25c8b7fe829a6054e8785282aabc629a/base/message_loop/message_loop.cc [modify] https://crrev.com/281621ef25c8b7fe829a6054e8785282aabc629a/base/message_loop/message_loop.h [modify] https://crrev.com/281621ef25c8b7fe829a6054e8785282aabc629a/base/message_loop/message_loop_task_runner_perftest.cc
,
Jul 18
,
Jul 18
Reland worked :) : https://crbug.com/863125#c9
,
Jul 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/02b1cd6abe987473248a3454a51efc5aefda0637 commit 02b1cd6abe987473248a3454a51efc5aefda0637 Author: Gabriel Charette <gab@chromium.org> Date: Fri Jul 27 21:37:08 2018 [MessageLoop] MessageLoopTaskRunner as multi-threaded incoming queue. Introduces the SequencedTaskSource interface to //base. This should ultimately be coalesced with a similar interface used by SequenceManager but this CL was complex enough in isolation that it was easier to introduce only the basic API for now. MessageLoop will as a follow-up expose an API that will allow it to be created with a custom SequencedTaskSource. This will enable ScopedTaskEnvironment::MOCK_TIME on custom MessageLoops (i.e. ForUI/ForIO) and soon after on all threads (not just main thread), since TaskScheduler's delayed tasks are managed on a MessageLoopForIO. Finally this will allow MOCK_TIME on TestBrowserThreadBundle (which uses a MessageLoopForUI); nicely rounding out the MOCK_TIME APIs. In this CL the following semantics have changed in MessageLoop : * MessageLoop::unbound_task_runner_ -> - MessageLoop::underlying_task_runner_ - MessageLoop::task_runner_ still initially set to it - but now const to clarify lifetime (and not set to null in ~MessageLoop() as this was pointless) - Implements SequencedTaskSource - Now has a Shutdown() phase so that PostTask() that happen-after ~MessageLoop() returns false (MessageLoop::Controller::DisconnectFromParent() still required, see comment in ~MessageLoop()) * IncomingTaskQueue (to be renamed to PendingTaskQueue in follow-up) : - no longer has a TriageQueue (MessageLoop communicates directly with SequencedTaskSource) - no longer ref-counted, strictly a member of MessageLoop - MessageLoop uses it as storage for tasks it must delay/defer - IncomingTaskQueue no longer has a Shutdown() phase, becomes dead storage on shutdown and dies with MessageLoop - Racy PostTask() on shutdown are now strictly handled between MessageLoop and its Controller * SequencedTaskSource : - the interface MessageLoop feeds from, initially only implemented by MessageLoopTaskRunner but will be made customizable as a follow-up * MessageLoop::SetTaskRunner() - still a thing to keep existing use cases alive but will ultimately die in favor of providing an external SequencedTaskSource (and killing all MessageLoop task_runner dances performed today :)) Note: most new logic in message_loop_task_runner.cc was moved as-is from incoming_task_queue.cc Bug: 708584, 860252 Change-Id: I23f40d293d2df6fe41374fad127745f2ff72fbe0 Reviewed-on: https://chromium-review.googlesource.com/1088762 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/heads/master@{#578809} [modify] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/BUILD.gn [modify] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/message_loop/incoming_task_queue.cc [modify] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/message_loop/incoming_task_queue.h [modify] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/message_loop/message_loop.cc [modify] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/message_loop/message_loop.h [modify] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/message_loop/message_loop_task_runner.cc [modify] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/message_loop/message_loop_task_runner.h [modify] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/message_loop/message_loop_task_runner_perftest.cc [add] https://crrev.com/02b1cd6abe987473248a3454a51efc5aefda0637/base/message_loop/sequenced_task_source.h
,
Jul 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/650ec6c171c55c91189a11fb88b8058ede8a0bad commit 650ec6c171c55c91189a11fb88b8058ede8a0bad Author: Gabriel Charette <gab@chromium.org> Date: Mon Jul 30 17:28:35 2018 [MessageLoop] Rename IncomingTaskQueue to PendingTaskQueue Rename-only follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/1088762 (made that CL's diff simpler to rename independently). R=kylechar@chromium.org Bug: 860252 , 708584 Change-Id: I3c5e2187b05a4966f7de718fe81c18b09d4e47ec Reviewed-on: https://chromium-review.googlesource.com/1153458 Commit-Queue: Gabriel Charette <gab@chromium.org> Reviewed-by: kylechar <kylechar@chromium.org> Cr-Commit-Position: refs/heads/master@{#579077} [modify] https://crrev.com/650ec6c171c55c91189a11fb88b8058ede8a0bad/base/BUILD.gn [modify] https://crrev.com/650ec6c171c55c91189a11fb88b8058ede8a0bad/base/message_loop/message_loop.h [modify] https://crrev.com/650ec6c171c55c91189a11fb88b8058ede8a0bad/base/message_loop/message_loop_task_runner.h [rename] https://crrev.com/650ec6c171c55c91189a11fb88b8058ede8a0bad/base/message_loop/pending_task_queue.cc [rename] https://crrev.com/650ec6c171c55c91189a11fb88b8058ede8a0bad/base/message_loop/pending_task_queue.h [modify] https://crrev.com/650ec6c171c55c91189a11fb88b8058ede8a0bad/base/test/scoped_task_environment.cc
,
Aug 23
,
Sep 24
We will most likely take another route to provide MOCK_TIME on all MainThreadTypes for ScopedTaskEnvironment and as such will likely drop this (and ultimately replace MessageLoop w/ SequenceManager so should stop making investments in ML). New approach : https://docs.google.com/document/d/1y08C6JQ9Yta3EQXzwIqqIIKHq9500WV6CWFZzZfDx7I/edit
,
Jan 2
While most of the refactoring here was useful, we opted to move towards base::SequenceManager as the basis of the per-thread task running infrastructure. As such ScopedTaskEnvironment is now based on it and we won't need to customize base::MessageLoop further (it is in fact being completely phased out). |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by gab@chromium.org
, Jul 4