New issue
Advanced search Search tips

Issue 730170 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 553459



Sign in to add a comment

Add Lazy(Sequenced|SingleThread|COMSTA)TaskRunner.

Project Member Reported by fdoray@chromium.org, Jun 6 2017

Issue description

A Lazy(Sequenced|SingleThread|COMSTA)TaskRunner will lazily create a TaskRunner.

A Lazy(Sequenced|SingleThread|COMSTA)TaskRunner is meant to be instantiated in an anonymous namespace and used to post tasks to the same sequence/thread from pieces of code that don't have a better way of sharing a TaskRunner. Adding a Lazy(Sequenced|SingleThread|COMSTA)TaskRunner to an anonymous namespace doesn't generate a static initializer.
 
Blocking: 553459
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 12 2017

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

commit a50035b77e0529cb0c916d19e18da91152c6dc69
Author: Francois Doray <fdoray@chromium.org>
Date: Mon Jun 12 19:45:49 2017

Extract code from LazyInstance::Pointer() to a helper function.

This will ease reuse of this code in other classes.

Bug:  730170 
Change-Id: I13b59fa864c0b1ddecef5e66d4ff0cf6a14d6fcb
Reviewed-on: https://chromium-review.googlesource.com/530984
Commit-Queue: Francois Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478723}
[modify] https://crrev.com/a50035b77e0529cb0c916d19e18da91152c6dc69/base/lazy_instance.cc
[modify] https://crrev.com/a50035b77e0529cb0c916d19e18da91152c6dc69/base/lazy_instance.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 12 2017

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

commit 1eb7c8cc54c9648277d10d113ed8a1b0cd1e9a01
Author: Francois Doray <fdoray@chromium.org>
Date: Mon Jun 12 21:50:51 2017

Add Lazy(Sequenced|SingleThread|COMSTA)TaskRunner.

A Lazy(Sequenced|SingleThread|COMSTA)TaskRunner lazily creates a TaskRunner.

A Lazy(Sequenced|SingleThread|COMSTA)TaskRunner is meant to be instantiated in
an anonymous namespace and used to post tasks to the same sequence/thread
from pieces of code that don't have a better way of sharing a TaskRunner.
Adding a Lazy(Sequenced|SingleThread|COMSTA)TaskRunner to an anonymous
namespace doesn't generate a static initializer.

Bug:  730170 
Change-Id: Idb35cfe9a5ca5501b7057d7b90b9df3f702faff1
Reviewed-on: https://chromium-review.googlesource.com/524141
Commit-Queue: Francois Doray <fdoray@chromium.org>
Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478779}
[modify] https://crrev.com/1eb7c8cc54c9648277d10d113ed8a1b0cd1e9a01/base/BUILD.gn
[add] https://crrev.com/1eb7c8cc54c9648277d10d113ed8a1b0cd1e9a01/base/task_scheduler/lazy_task_runner.cc
[add] https://crrev.com/1eb7c8cc54c9648277d10d113ed8a1b0cd1e9a01/base/task_scheduler/lazy_task_runner.h
[add] https://crrev.com/1eb7c8cc54c9648277d10d113ed8a1b0cd1e9a01/base/task_scheduler/lazy_task_runner_unittest.cc
[modify] https://crrev.com/1eb7c8cc54c9648277d10d113ed8a1b0cd1e9a01/base/test/scoped_task_environment.h
[modify] https://crrev.com/1eb7c8cc54c9648277d10d113ed8a1b0cd1e9a01/docs/task_scheduler_migration.md

Comment 4 by fdoray@chromium.org, Jun 16 2017

Status: Fixed (was: Started)

Comment 5 by gab@chromium.org, Jun 21 2017

As discussed offline, the proposed {} syntax for initializers doesn't work with multiple traits, MSVC complains, e.g.:

d:\src\chrome\src\chrome\installer\util\google_update_settings.cc(65): warning C4002: too many actual parameters for macro 'LAZY_SEQUENCED_TASK_RUNNER_INITIALIZER'
d:\src\chrome\src\chrome\installer\util\google_update_settings.cc(65): error C3329: syntax error: expected '}' not ')'
d:\src\chrome\src\chrome\installer\util\google_update_settings.cc(65): error C2143: syntax error: missing ')' before ';'
d:\src\chrome\src\chrome\installer\util\google_update_settings.cc(65): error C2059: syntax error: ';'

Should fix documentation to reflect that actual constructor has to be used.

Also, we discussed adding TaskRunner* LazyTaskRunner::GetRaw() as most callers merely make an inline call for which we should avoid the unnecessary refcount bump.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 22 2017

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

commit 7dac82ded18431d47b09634cdf3239e1ef310691
Author: Francois Doray <fdoray@chromium.org>
Date: Thu Jun 22 16:39:22 2017

Add ScopedLazyTaskRunnerListForTesting to ScopedAsyncTaskScheduler.

This ensures destruction of lazy TaskRunners when
ScopedAsyncTaskScheduler is destroyed.

ScopedAsyncTaskScheduler is deprecated, but it is used by
TestBrowserThreadBundle which is not.

Bug:  730170 
Change-Id: I0d77056555bb6fe20d5cea2d073e2defdeab27ca
Reviewed-on: https://chromium-review.googlesource.com/545095
Commit-Queue: Francois Doray <fdoray@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481557}
[modify] https://crrev.com/7dac82ded18431d47b09634cdf3239e1ef310691/base/test/scoped_async_task_scheduler.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 6 2017

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

commit f35d7feb5436027af2f931e5fa2932a1c24b9380
Author: Gabriel Charette <gab@chromium.org>
Date: Thu Jul 06 00:14:41 2017

Introduce GoogleUpdateSettings::CollectStatsConsentTaskRunner and get rid of FILE thread usage in callers.

An Android caller was also using BlockingPool() to make this call before
so this was potentially racy there.

R=asviktine@chromium.org, grt@chromium.org

Bug:  730170 ,  667892 ,  689520 
Change-Id: I04aacb46379f4e6e6fc27df5ddf09b05a860e223
Reviewed-on: https://chromium-review.googlesource.com/543560
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484389}
[modify] https://crrev.com/f35d7feb5436027af2f931e5fa2932a1c24b9380/chrome/browser/android/metrics/uma_session_stats.cc
[modify] https://crrev.com/f35d7feb5436027af2f931e5fa2932a1c24b9380/chrome/browser/browser_process_impl.cc
[modify] https://crrev.com/f35d7feb5436027af2f931e5fa2932a1c24b9380/chrome/browser/google/google_update_settings_posix.cc
[modify] https://crrev.com/f35d7feb5436027af2f931e5fa2932a1c24b9380/chrome/browser/metrics/metrics_reporting_state.cc
[modify] https://crrev.com/f35d7feb5436027af2f931e5fa2932a1c24b9380/chrome/browser/ui/views/session_crashed_bubble_view.cc
[modify] https://crrev.com/f35d7feb5436027af2f931e5fa2932a1c24b9380/chrome/installer/util/google_update_settings.cc
[modify] https://crrev.com/f35d7feb5436027af2f931e5fa2932a1c24b9380/chrome/installer/util/google_update_settings.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 21 2017

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

commit d542d2cff3966fdb9575bae8b538ede8b0805c0f
Author: Gabriel Charette <gab@chromium.org>
Date: Fri Jul 21 16:33:36 2017

Macros and {} variadic template don't play well together.

{} initialization of LAZY_TASK_RUNNERs doesn't work when providing
multiple traits. Update documentation to suggest explicit TaskTraits
initialization.

R=robliao@chromium.org

Bug:  730170 
Change-Id: Icbd93f47f9d3f36c344ea892ed4b7e11b00becd3
Reviewed-on: https://chromium-review.googlesource.com/580531
Reviewed-by: Robert Liao <robliao@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488676}
[modify] https://crrev.com/d542d2cff3966fdb9575bae8b538ede8b0805c0f/base/task_scheduler/lazy_task_runner.h

Sign in to add a comment