This moves the proxy TaskRunners into BrowserTaskExecutor, which
then exposes them to base/post_task.h.
Also replaces uses of LazyInstance with NoDestructor.
Bug: 878356
Change-Id: I4ca14d49404158da9f98953b3a725da47081881d
Reviewed-on: https://chromium-review.googlesource.com/1236437
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593868}
This removes BrowserThread::Post(NonNestable|)(Delayed|)Task and moves
GetTaskRunnerForThread into the impl (it's still used to vend
TaskRunners in BrowserTaskExecutor).
These methods have been replaced by base/post_task.h in conjunction
with content/public/browser/browser_task_traits.h
Bug: 878356
Change-Id: Ia122dc0921769f43da9271ddd6e1ce2f402df779
Reviewed-on: https://chromium-review.googlesource.com/1235728
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592879}
The two methods converged over the last year and are now equivalent.
Bug: 825054
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I4ec5f7afb5b7b21278af6c777efe9ad57d2aae68
Reviewed-on: https://chromium-review.googlesource.com/977063
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546529}
Sampling profiler highlights high contention on this lock:
crbug.com/821034.
Initializing/tear down was already single-threaded (tear down had to be moved
from BrowserProcessSubThread::Cleanup() to ~BrowserProcessSubThread() to
enforce this with a ThreadChecker however). Add dchecks to confirm/enforce
this and remove the lock which is usuless on data structures that are now (as
of series of changes over the last year) read-only after startup.
Use atomics to protect reads across the RUNNING=>SHUTDOWN boundary.
Bug: 821034
Change-Id: I7800048bff51ad79cb10ee89fd3a0a31534c393e
Reviewed-on: https://chromium-review.googlesource.com/973556
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546212}
This is a reland of d260e9cf66
It was reverted because of crbug.com/824716, these weren't new crashes
but known crashes mislabeled as a fallout of this change.
http://cl/190471699 fixes the crash backend to not rely on
"content::BrowserThreadImpl::IOThreadRun" being in the signature.
Only diff in this CL is to use base::debug::Alias() in methods that we
don't want optimized (i.e. IOThreadRun) out as CHECK_GT was seen as
optimized out in some of the reported crashes (even though the same
pattern as before was used by this CL..?)
Original change's description:
> Refactor BrowserThreadImpl, BrowserProcessSubThread, and BrowserMainLoop
>
> This brings back the invariant that BrowserThread::IO isn't available
> before BrowserMainLoop::CreateThreads(). This was broken to fix issue
> 729596 to bring up the thread earlier for ServiceManager but it is
> important that code that posts to BrowserThread::IO statically have an
> happens-after relationship to BrowserMainLoop::CreateThreads(). Exposing
> it statically earlier put that invariant at risk.
>
> Thankfully fixing issue 815225 resulted in finally reaching the long
> sought goal of only having BrowserThread::UI/IO. Now that the IO thread
> is also kicked off before it's named statically, BrowserThreadImpl no
> longer needs to be a base::Thread, hence this refactoring.
>
> Before this CL:
> * BrowserThreadImpl was a base::Thread
> (could be a fake thread if SetMessageLoop was used)
> * BrowserProcessSubThread was a BrowserThreadImpl
> (performed a bit more initialization)
> * BrowserProcessSubThread was only used in production (in
> BrowserMainLoop)
> * BrowserThreadImpl was used for fake threads (BrowserMainLoop for
> BrowserThread::UI) and for testing (TestBrowserThread(Impl)).
> * BrowserThreadImpl overrode Init/Run/CleanUp() from base::Thread to
> perform some sanity checks as well as drive IOThread's Delegate (ref.
> BrowserThread::SetIOThreadDelegate())
> * BrowserProcessSubThread re-overrode Init/Run/CleanUp() to perform
> per-thread //content initialization (tests missed out on that per
> TestBrowserThread bypassing BrowserProcessSubThread by directly
> subclassing BrowserThreadImpl).
>
> With this CL:
> * BrowserThreadImpl is merely a scoped object that binds a provided
> SingleThreadTaskRunner to a BrowserThread::ID.
> * BrowserProcessSubThread is a base::Thread and performs all of the
> initialization and cleanup specific to //content (this means it now
> also manages BrowserThread::SetIOThreadDelegate())
> * BrowserProcessSubThread can be brought up early before being bound to
> a BrowserThread::ID (BrowserMainLoop handles that through
> BrowserProcessSubThread ::RegisterAsBrowserThread())
>
> Unfortunate exceptions required for this CL:
> * IOThread::Init() (invoked through BrowserThreadDelegate) perfoms
> blocking operations this was previously performed before installed
> the ThreadRestrictions on BrowserThread::IO. But now that //content
> is initialized after bringing up the thread, a
> base::ScopedAllowBlocking is required in scope of IOThread::Init().
> * TestBrowserThread previously bypassing BrowserProcessSubThread by
> directly subclassing BrowserThreadImpl meant it wasn't subject to
> ThreadRestrictions (unfortunate becomes it denies allowance
> verification to product code running in unit tests). Adding it back
> causes DCHECKs, as such
> BrowserProcessSubThread::AllowBlockingForTesting was added to allow
> this CL to pass CQ.
>
> Of note:
> * BrowserProcessSubThread is still written as though it supports many
> BrowserThread::IDs but in practice it's mostly always
> BrowserThread::IO (except in ThreadWatcherTest I think). This change
> was big enough that I didn't bother also breaking that
> generalization.
> * BrowserThreadImpl's constructor was made private to ensure only
> BrowserProcessSubThread and a few select callers get to drive it (to
> avoid previous missed initialization issues)
> * Atomics to manage BrowserThread::SetIOThreadDelegate were removed.
> Restriction was instead added that this only be called before
> initialization and after shutdown (this was already the case).
>
> Follow-ups to this CL:
> * //ios duplicates this logic and will need to undergo the same change
> as a follow-up
> * Fixing ios will allow removal of base::Thread::SetMessageLoop hack :)
> * Removing BrowserThreadGlobals::lock_ to address crbug.com/821034 will
> be much easier
> * BrowserThread post APIs should DCHECK rather than no-op if using a
> BrowserThread::ID before it's registered.
>
> Bug: 815225, 821034, 729596
> Change-Id: If1038f23079df72203b1e95c7d26647f8824a726
> Reviewed-on: https://chromium-review.googlesource.com/969104
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544440}
TBR=jam@chromium.org
Bug: 815225, 821034, 729596, 824716
Change-Id: I9a180975c69a008f8519d1d3d44663aa58a74a92
Reviewed-on: https://chromium-review.googlesource.com/980793
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546104}
This reverts commit d260e9cf66.
Reason for revert: In Windows Canary versions since 67.0.3377.0, where this commit landed, IO thread hang reports have spiked dramatically. It is now the #1 browser crash report on Windows Canary at 33% of reports. I'm speculatively reverting this to see if the crash rate heals.
Original change's description:
> Refactor BrowserThreadImpl, BrowserProcessSubThread, and BrowserMainLoop
>
> This brings back the invariant that BrowserThread::IO isn't available
> before BrowserMainLoop::CreateThreads(). This was broken to fix issue
> 729596 to bring up the thread earlier for ServiceManager but it is
> important that code that posts to BrowserThread::IO statically have an
> happens-after relationship to BrowserMainLoop::CreateThreads(). Exposing
> it statically earlier put that invariant at risk.
>
> Thankfully fixing issue 815225 resulted in finally reaching the long
> sought goal of only having BrowserThread::UI/IO. Now that the IO thread
> is also kicked off before it's named statically, BrowserThreadImpl no
> longer needs to be a base::Thread, hence this refactoring.
>
> Before this CL:
> * BrowserThreadImpl was a base::Thread
> (could be a fake thread if SetMessageLoop was used)
> * BrowserProcessSubThread was a BrowserThreadImpl
> (performed a bit more initialization)
> * BrowserProcessSubThread was only used in production (in
> BrowserMainLoop)
> * BrowserThreadImpl was used for fake threads (BrowserMainLoop for
> BrowserThread::UI) and for testing (TestBrowserThread(Impl)).
> * BrowserThreadImpl overrode Init/Run/CleanUp() from base::Thread to
> perform some sanity checks as well as drive IOThread's Delegate (ref.
> BrowserThread::SetIOThreadDelegate())
> * BrowserProcessSubThread re-overrode Init/Run/CleanUp() to perform
> per-thread //content initialization (tests missed out on that per
> TestBrowserThread bypassing BrowserProcessSubThread by directly
> subclassing BrowserThreadImpl).
>
> With this CL:
> * BrowserThreadImpl is merely a scoped object that binds a provided
> SingleThreadTaskRunner to a BrowserThread::ID.
> * BrowserProcessSubThread is a base::Thread and performs all of the
> initialization and cleanup specific to //content (this means it now
> also manages BrowserThread::SetIOThreadDelegate())
> * BrowserProcessSubThread can be brought up early before being bound to
> a BrowserThread::ID (BrowserMainLoop handles that through
> BrowserProcessSubThread ::RegisterAsBrowserThread())
>
> Unfortunate exceptions required for this CL:
> * IOThread::Init() (invoked through BrowserThreadDelegate) perfoms
> blocking operations this was previously performed before installed
> the ThreadRestrictions on BrowserThread::IO. But now that //content
> is initialized after bringing up the thread, a
> base::ScopedAllowBlocking is required in scope of IOThread::Init().
> * TestBrowserThread previously bypassing BrowserProcessSubThread by
> directly subclassing BrowserThreadImpl meant it wasn't subject to
> ThreadRestrictions (unfortunate becomes it denies allowance
> verification to product code running in unit tests). Adding it back
> causes DCHECKs, as such
> BrowserProcessSubThread::AllowBlockingForTesting was added to allow
> this CL to pass CQ.
>
> Of note:
> * BrowserProcessSubThread is still written as though it supports many
> BrowserThread::IDs but in practice it's mostly always
> BrowserThread::IO (except in ThreadWatcherTest I think). This change
> was big enough that I didn't bother also breaking that
> generalization.
> * BrowserThreadImpl's constructor was made private to ensure only
> BrowserProcessSubThread and a few select callers get to drive it (to
> avoid previous missed initialization issues)
> * Atomics to manage BrowserThread::SetIOThreadDelegate were removed.
> Restriction was instead added that this only be called before
> initialization and after shutdown (this was already the case).
>
> Follow-ups to this CL:
> * //ios duplicates this logic and will need to undergo the same change
> as a follow-up
> * Fixing ios will allow removal of base::Thread::SetMessageLoop hack :)
> * Removing BrowserThreadGlobals::lock_ to address crbug.com/821034 will
> be much easier
> * BrowserThread post APIs should DCHECK rather than no-op if using a
> BrowserThread::ID before it's registered.
>
> Bug: 815225, 821034, 729596
> Change-Id: If1038f23079df72203b1e95c7d26647f8824a726
> Reviewed-on: https://chromium-review.googlesource.com/969104
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544440}
TBR=gab@chromium.org,jam@chromium.org
NOPRESUBMIT=true
# Not skipping CQ checks because original CL landed > 1 day ago.
# falken: Skipping presubmit to use deprecated ThreadResrictions::DisallowWaiting().
Bug: 815225, 821034, 729596
Change-Id: I2be97c5d8183497c005ab397c871f625b034d850
Reviewed-on: https://chromium-review.googlesource.com/979752
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545725}
Posting is still allowed in the SHUTDOWN phase in production (will no-op
when after the thread is torn down as usual) but it is again disallowed
after BrowserThreadImpl::ResetGlobalsForTesting (i.e. after
~TestBrowserThreadBundle).
content/browser/browser_thread_impl.cc :
- Core change to deny PostTask to uninitialized BrowserThreads
(shutdown is fine). In practice this prevents posting before
BrowserMainLoop::CreateThreads in production and outside the scope of
a TestBrowserThreadBundle in tests (and after
~TestBrowserThreadBundle() which fully uninitializes after shutdown).
chrome/browser/chrome_content_browser_client.cc :
- ChromeContentBrowserClient::SetApplicationLocale can happen
synchronously if IO thread is not yet initialized (change check to
reflect proper thread).
content/browser/loader/url_loader_factory_impl_unittest.cc :
- Make sure TestBrowserThreadBundle outlives members as well.
content/browser/media/capture/desktop_capture_device.cc
chrome/browser/media/router/discovery/dial/dial_url_fetcher.cc
content/browser/media/capture/desktop_capture_device_unittest.cc :
- crbug.com/823869
content/browser/net/network_quality_observer_impl_unittest.cc :
- NetworkQualityObserverImpl's constructor uses BrowserThreads, would
previously no-op and not break the test but this change brings it
closer to testing the same code as in production
content/browser/renderer_host/media/render_frame_audio_input_stream_factory_unittest.cc
chrome/browser/ssl/ssl_error_handler_unittest.cc
chrome/browser/chromeos/login/signin_partition_manager_unittest.cc
content/browser/renderer_host/render_widget_host_view_mac_editcommand_helper_unittest.mm :
- See inline comments
chrome/browser/component_updater/subresource_filter_component_installer_unittest.cc
chrome/browser/profiles/profile_shortcut_manager_unittest_win.cc :
- Trivial upgrade from ScopedTaskEnvironment as some logic does use
BrowserThreads.
Bug: 823797, 823869
Change-Id: Iab99220606714362959a00820ecd46334eae7c8a
Reviewed-on: https://chromium-review.googlesource.com/970861
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544845}
This brings back the invariant that BrowserThread::IO isn't available
before BrowserMainLoop::CreateThreads(). This was broken to fix issue
729596 to bring up the thread earlier for ServiceManager but it is
important that code that posts to BrowserThread::IO statically have an
happens-after relationship to BrowserMainLoop::CreateThreads(). Exposing
it statically earlier put that invariant at risk.
Thankfully fixing issue 815225 resulted in finally reaching the long
sought goal of only having BrowserThread::UI/IO. Now that the IO thread
is also kicked off before it's named statically, BrowserThreadImpl no
longer needs to be a base::Thread, hence this refactoring.
Before this CL:
* BrowserThreadImpl was a base::Thread
(could be a fake thread if SetMessageLoop was used)
* BrowserProcessSubThread was a BrowserThreadImpl
(performed a bit more initialization)
* BrowserProcessSubThread was only used in production (in
BrowserMainLoop)
* BrowserThreadImpl was used for fake threads (BrowserMainLoop for
BrowserThread::UI) and for testing (TestBrowserThread(Impl)).
* BrowserThreadImpl overrode Init/Run/CleanUp() from base::Thread to
perform some sanity checks as well as drive IOThread's Delegate (ref.
BrowserThread::SetIOThreadDelegate())
* BrowserProcessSubThread re-overrode Init/Run/CleanUp() to perform
per-thread //content initialization (tests missed out on that per
TestBrowserThread bypassing BrowserProcessSubThread by directly
subclassing BrowserThreadImpl).
With this CL:
* BrowserThreadImpl is merely a scoped object that binds a provided
SingleThreadTaskRunner to a BrowserThread::ID.
* BrowserProcessSubThread is a base::Thread and performs all of the
initialization and cleanup specific to //content (this means it now
also manages BrowserThread::SetIOThreadDelegate())
* BrowserProcessSubThread can be brought up early before being bound to
a BrowserThread::ID (BrowserMainLoop handles that through
BrowserProcessSubThread ::RegisterAsBrowserThread())
Unfortunate exceptions required for this CL:
* IOThread::Init() (invoked through BrowserThreadDelegate) perfoms
blocking operations this was previously performed before installed
the ThreadRestrictions on BrowserThread::IO. But now that //content
is initialized after bringing up the thread, a
base::ScopedAllowBlocking is required in scope of IOThread::Init().
* TestBrowserThread previously bypassing BrowserProcessSubThread by
directly subclassing BrowserThreadImpl meant it wasn't subject to
ThreadRestrictions (unfortunate becomes it denies allowance
verification to product code running in unit tests). Adding it back
causes DCHECKs, as such
BrowserProcessSubThread::AllowBlockingForTesting was added to allow
this CL to pass CQ.
Of note:
* BrowserProcessSubThread is still written as though it supports many
BrowserThread::IDs but in practice it's mostly always
BrowserThread::IO (except in ThreadWatcherTest I think). This change
was big enough that I didn't bother also breaking that
generalization.
* BrowserThreadImpl's constructor was made private to ensure only
BrowserProcessSubThread and a few select callers get to drive it (to
avoid previous missed initialization issues)
* Atomics to manage BrowserThread::SetIOThreadDelegate were removed.
Restriction was instead added that this only be called before
initialization and after shutdown (this was already the case).
Follow-ups to this CL:
* //ios duplicates this logic and will need to undergo the same change
as a follow-up
* Fixing ios will allow removal of base::Thread::SetMessageLoop hack :)
* Removing BrowserThreadGlobals::lock_ to address crbug.com/821034 will
be much easier
* BrowserThread post APIs should DCHECK rather than no-op if using a
BrowserThread::ID before it's registered.
Bug: 815225, 821034, 729596
Change-Id: If1038f23079df72203b1e95c7d26647f8824a726
Reviewed-on: https://chromium-review.googlesource.com/969104
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544440}
ServiceManager may need to launch child processes before a BrowserMainLoop
has been created, and therefore before any BrowserThreads exists. We
therefore replace the BrowserThread::PROCESS_LAUNCHER with a global
TaskScheduler sequence, created on-demand when the ServiceManager or
ChildProcessLauncherHelper first need it.
Under Windows we must use a single-thread TaskRunner, while on other
platforms we are able to use a normal TaskScheduler sequence. File
crbug.com/820200 to track that.
Bug: 815225
Change-Id: Ia0f46461fb9cc92fddacf81ee96b764de8477d11
Reviewed-on: https://chromium-review.googlesource.com/941264
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542212}
In this CL, we split the creation of the BrowserThread::IO thread and
initialization of its delegate:
1. The BrowserThread::IO is created in the
BrowserMainLoop::PostMainMessageLoopStart() after the UI thread
is created. Adjust the tracing events in the BrowserMainLoop.
2. Simplify the logic in BrowserMainLoop::CreateThreads() since only
the launcher thread is created there. Initializes the
delegate of the BrowserThread::IO there.
3. Update the logic in the BrowserThreadImpl when checking whether
the BrowserThread::IO has initialized.
4. Update BrowserMainLoopTest and BrowserProcessImplTest.
This is the first step of starting ServiceManager earlier.
Bug: 729596
Change-Id: I7a08fd7c7bf5548f8039c8c38152ecfde3bd20c4
Reviewed-on: https://chromium-review.googlesource.com/905424
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537318}
disk_cache users should only need to go through disk_cache.h;
the individual _impl.h files aren't public API.
Change-Id: Ie2e7f696ec81e03ff00dbfcfc5535fd7929acf83
Reviewed-on: https://chromium-review.googlesource.com/916742
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536713}
The LazyInstance::operator== removed by this CL is undesirable, because
1) it didn't have a corresponding operator!=
2) it was only used to compare against nullptr
(i.e. to check if the lazy instance was created already or not)
This CL removes LazyInstance::operator== and replaces it with
LazyInstance::IsCreated() method.
Bug: 789738
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Idfbb4a9f0699cb10f3f861860070b7f316a16ad6
Reviewed-on: https://chromium-review.googlesource.com/792124
Reviewed-by: Nick Carter <nick@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522172}
This reverts commit 6127a86f3a.
Reason for revert: Suspect this caused spike in ThreadWatcher hang reports: crbug.com/768886
Original change's description:
> Remove all remaining deprecated Web/BrowserThreads!!
>
> (except BrowserThread::FILE which has a handful use cases left still and
> will follow suit shortly)
>
> (BrowserThread::PROCESS_LAUNCHER migration was put on hold because of
> Android specific requirements around process launching)
>
> Remaining usage of TestBrowserThreadBundle::REAL_DB_THREAD could simply
> be removed as all of the product code was already migrated to TaskScheduler
> which always uses real threads in unit tests.
>
> Bug: 689520, 365909, 752144
> Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
> Change-Id: Id70b722aa35e8568795d5aac16947392ac1597be
>
> NOPRESUBMIT=True (for content/browser/browser_thread_impl.cc:211)
>
> Change-Id: Id70b722aa35e8568795d5aac16947392ac1597be
> Reviewed-on: https://chromium-review.googlesource.com/610880
> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#503732}
TBR=gab@chromium.org,jam@chromium.org,dpranke@chromium.org,asvitkine@chromium.org,sdefresne@chromium.org
# Not skipping CQ checks because original CL landed > 1 day ago.
NOPRESUBMIT=True (for restoring the deprecated browser threads)
Bug: 689520, 365909, 752144, 768886
Change-Id: I53d93572118303743e1dde71b2a473350717b215
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/689254
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504939}
(except BrowserThread::FILE which has a handful use cases left still and
will follow suit shortly)
(BrowserThread::PROCESS_LAUNCHER migration was put on hold because of
Android specific requirements around process launching)
Remaining usage of TestBrowserThreadBundle::REAL_DB_THREAD could simply
be removed as all of the product code was already migrated to TaskScheduler
which always uses real threads in unit tests.
Bug: 689520, 365909, 752144
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Id70b722aa35e8568795d5aac16947392ac1597be
NOPRESUBMIT=True (for content/browser/browser_thread_impl.cc:211)
Change-Id: Id70b722aa35e8568795d5aac16947392ac1597be
Reviewed-on: https://chromium-review.googlesource.com/610880
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503732}
These threads are deprecated and no longer used.
NOPRESUBMIT=true
Bug: 689520
Change-Id: I55b05b70626f91c8a71d56642c851bb786dbfcaa
Reviewed-on: https://chromium-review.googlesource.com/660597
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Francois Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501245}
Removes the ScopedTracker calls since we are deleting this infrastructure.
This saves about 25KB in an official build.
Design doc:
https://docs.google.com/document/d/1cibE5nAG9sdjVMfs-MkvrMtp43i9MLGqM_uNusLqiBY
Bug: 762311
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Id00f88d4b7c8c14d51b20d9b53dc233189bdb049
Reviewed-on: https://chromium-review.googlesource.com/651337
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500108}
This CL applies //tools/clang/base_bind_rewriters to //content/browser/{a,b,c,d,f,g}*
It rewrites base::Bind to base::BindOnce where the resulting base::Callback
is immediately converted to base::OnceCallback, which is considered safe
to use base::BindOnce.
E.g.:
base::PostTask(FROM_HERE, base::Bind([]{}));
base::OnceClosure cb = base::Bind([]{});
are converted to:
base::PostTask(FROM_HERE, base::BindOnce([]{}));
base::OnceClosure cb = base::BindOnce([]{});
Change-Id: I11ae9ac169c8e0ff9d9b95452fec22a8b7c91df7
NOPRESUBMIT=true
Change-Id: I11ae9ac169c8e0ff9d9b95452fec22a8b7c91df7
Reviewed-on: https://chromium-review.googlesource.com/628003
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496656}
TaskRunner::RunsTasksOnCurrentThread().
Make TaskRunner::RunsTasksOnCurrentThread() non-virtual and define
it in terms of TaskRunner::RunsTasksInCurrentSequence().
It's the first step to rename RunsTasksOnCurrentThread()
to RunsTasksInCurrentSequence().
BUG=665062
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2823103003
Cr-Commit-Position: refs/heads/master@{#470173}
First CL of a series to split inter-dependency between RunLoop and MessageLoop.
Also modernized bluetooth_socket_bluez_unittest.cc away from using deprecated MessageLoop Quit
methods on its fixture's MessageLoop member to ease transition.
Lastly, tried to add thread safety checks for documentation purposes but turns
out there are already improper usage of the API... those will have to be
addressed first through issue 715235.
https://codereview.chromium.org/2828913003 follows to
s/nested message loop/nested run loop/ in comments.
BUG=703346, 715235
Review-Url: https://codereview.chromium.org/2818533003
Cr-Commit-Position: refs/heads/master@{#469636}
After this CL, TaskRunner::PostTask and its family can take OnceClosure
in addition to Closure.
Most of the changes are mechanical replacement of Closure with OnceClosure
on TaskRunner family. Others are:
- Limit CriticalClosure from Closure to OnceClosure as no caller call
the resulting callback more than once
- Add several PostTaskAndReplyWithResult overloads for old Callback
version, for compatibility. (in base/task_scheduler/post_task.h)
- Update SequencedWorkerPool implementation for OnceClosure.
- Update task handling code in app_state.mm for OnceClosure, which is
needed to bring OnceClosure into a ObjC block.
BUG=704027
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2637843002
Cr-Commit-Position: refs/heads/master@{#462023}
This is a preparation CL for http://crrev.com/2637843002, which replaces
the Callback parameter of TaskRunner::PostTask with OnceCallback.
This one replaces the passed-by-const-ref Callback parameter of
TaskRunner::PostTask() with pass-by-value.
With the pass-by-const-ref manner as the old code does, we can't avoid
leaving a reference to the callback object on the original thread. That
is, the callback object may be destroyed either on the target thread or
the original thread. That's problematic when a non-thread-safe object is
bound to the callback.
Pass-by-value and move() in this CL mitigate the nondeterminism: if the
caller of TaskRunner::PostTask() passes the callback object as rvalue,
TaskRunner::PostTask() leaves no reference on the original thread.
I.e. the reference is not left if the callback is passed directly from
Bind(), or passed with std::move() as below.
task_runner->PostTask(FROM_HERE, base::Bind(&Foo));
base::Closure cb = base::Bind(&Foo);
task_runner->PostTask(FROM_HERE, std::move(cb));
Otherwise, if the caller passes the callback as lvalue, a reference to
the callback is left on the original thread as we do in the previous code.
I.e. a reference is left if the callback is passed from other non-temporary
variable.
base::Closure cb = base::Bind(&Foo);
task_runner->PostTask(FROM_HERE, cb);
This is less controversial part of http://crrev.com/2637843002. This CL
is mainly to land it incrementally.
TBR=shrike@chromium.org
BUG=704027
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2726523002
Cr-Commit-Position: refs/heads/master@{#460288}
This is a less controversial part of OnceCallback migration of PostTaskAndReply.
https://codereview.chromium.org/2657603004/
After this CL PostTaskAndReply family take the callback by value, and
move it to their storage. That reduces the chance to leave a reference
to the callback on the original thread.
Review-Url: https://codereview.chromium.org/2678303002
Cr-Commit-Position: refs/heads/master@{#448965}
This is part of the migration phase for TaskScheduler, design doc:
https://docs.google.com/document/d/1S2AAeoo1xa_vsLbDYBsDHCqhrkfiMgoIPlyRi6kxa5k/edit
Also generalizes BrowserThreadImpl away from the underlying thread's MessageLoop
and id in favor of TaskRunners and BrowserThreadState.
Cool side-effect: BrowserThreadImpl::StartWithOptions() no longer blocks on the
underlying thread starting (it used to via GetThreadId()), it now takes a reference
to its TaskRunner and lets the thread itself start on its own schedule :).
This wasn't previously intended to block (BrowserThreadImpl::StartAndWaitForTesting() is
supposed to be used for that) but had unintentionally regressed in http://crrev.com/408295
because GetThreadId() implicitly waits (http://crbug.com/672977).
Another nice improvement is that ~BrowserThreadImpl() now cleans the BrowserThreadGlobals
associated with it, this has the side-effect to force proper destruction order of
TestBrowserThread in tests which brought forth a few precursor cleanups
(https://crbug.com/653916#c24) which will from now on be enforced by this CL.
When redirection is disabled, the logic should be the exact same as before.
- Threads are brought up.
- Tasks are posted to their MessageLoop (albeit through their TaskRunner now).
- On shutdown, threads are joined one by one.
When redirection is enabled, we try to mimic this logic.
- Redirection TaskRunners are only live (|accepting_tasks|) for the same period that the
matching thread would be.
- On shutdown, we block on each TaskRunner's queue being flushed one-by-one
in the same order as in the no-redirection case (this almost identical to
real threads join % one slight difference documented in detail in
BrowserThreadImpl::StopRedirectionOfThreadID()).
BUG=653916, 672977
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng
TEST=
A) "out\Release\chrome.exe"
Runs/shuts down the exact same as before.
B) "out\Release\chrome.exe --force-fieldtrials=BrowserScheduler/Enabled/ --force-fieldtrial-params=BrowserScheduler.Enabled:Background/3;3;1;0;10000/BackgroundFileIO/3;3;1;0;10000/Foreground/3;3;1;0;10000/ForegroundFileIO/3;3;1;0;10000/RedirectSequencedWorkerPools/true/RedirectNonUINonIOBrowserThreads/true"
Runs/shuts down smoothly and chrome://tracing confirms that redirected BrowserThreads are running on TaskSchedulerWorkers.
Review-Url: https://codereview.chromium.org/2464233002
Cr-Commit-Position: refs/heads/master@{#439139}
The IO thread was already the only user and officially restricting users
of this API is required as BrowserThreadDelegates won't be compatible
with threads redirected to the TaskScheduler.
BUG=653916
Review-Url: https://codereview.chromium.org/2558943002
Cr-Commit-Position: refs/heads/master@{#437271}
Replacing one weird MessageLoop::IsIdleForTesting() loop in
chrome/browser/ui/app_list/speech_recognizer_browsertest.cc
with content::RunAllPendingInMessageLoop(content::BrowserThread::IO);
And all other use cases by BrowserThread::GetTaskRunnerForThread().
BUG=661344
Review-Url: https://codereview.chromium.org/2473823002
Cr-Commit-Position: refs/heads/master@{#429774}
This CL disallows task observers on the DB, FILE, FILE_USER_BLOCKING,
PROCESS_LAUNCHER and CACHE threads. Calling Add/RemoveTaskObserver
on these threads will result in a crash.
This is important because these threads will be migrated to
TaskScheduler which doesn't support task observers.
To our knowledge, no code uses task observers on these threads. This
CL will help us confirm that in the wild.
BUG=653916
Review-Url: https://codereview.chromium.org/2419803002
Cr-Commit-Position: refs/heads/master@{#425418}
This CL disallows nesting on BrowserThreads that will be migrated
to TaskScheduler. Running a nested loop or calling
Add/RemoveNestingObserver on these threads will result in a crash.
To our knowledge, no code runs a nested loop on BrowserThreads that
will be migrated to TaskScheduler. This CL will help us confirm that
in the wild.
BUG=653916
Review-Url: https://codereview.chromium.org/2407313002
Cr-Commit-Position: refs/heads/master@{#424749}
Changes BrowserThread::CurrentlyOn (and thus
BrowserThreadTaskRunner::RunsTasksOnCurrentThread()) to correctly report
a BrowserThread's association during MessageLoop destruction notification.
Also adds an explicit Start() to BrowserThreadImpl as there are tests which
call it and which incorrectly assumed base::Thread's implementation would
call BrowserThreadImpl::StartWithOptions (which it wouldn't since the latter
is not a virtual function.)
This change provokes many tests to delete ExtensionFunction instances that
were otherwise being leaked because UIThreadExtensionFunction::Destroy was
incorrectly deferring destruction when run on the UI thread during shutdown.
This in turn revealed a few small bugs which have also been fixed here.
BUG=631093
R=jam@chromium.org
Committed: https://crrev.com/b02da29fb9116d1a1fb4fd0476628f333ff6bd1a
Review-Url: https://codereview.chromium.org/2180253003
Cr-Original-Commit-Position: refs/heads/master@{#408295}
Cr-Commit-Position: refs/heads/master@{#408411}
Reason for revert:
Test failures in the changes to the ComponentCloudPolicyTest. Doh.
Original issue's description:
> Ensure BrowserThread::CurrentlyOn is correct through MessageLoop teardown
>
> Changes BrowserThread::CurrentlyOn (and thus
> BrowserThreadTaskRunner::RunsTasksOnCurrentThread()) to correctly report
> a BrowserThread's association during MessageLoop destruction notification.
>
> Also adds an explicit Start() to BrowserThreadImpl as there are tests which
> call it and which incorrectly assumed base::Thread's implementation would
> call BrowserThreadImpl::StartWithOptions (which it wouldn't since the latter
> is not a virtual function.)
>
> This change provokes many tests to delete ExtensionFunction instances that
> were otherwise being leaked because UIThreadExtensionFunction::Destroy was
> incorrectly deferring destruction when run on the UI thread during shutdown.
> This in turn revealed a few small bugs which have also been fixed here.
>
> BUG=631093
> R=jam@chromium.org
>
> Committed: https://crrev.com/b02da29fb9116d1a1fb4fd0476628f333ff6bd1a
> Cr-Commit-Position: refs/heads/master@{#408295}
TBR=jam@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=631093
Review-Url: https://codereview.chromium.org/2186213002
Cr-Commit-Position: refs/heads/master@{#408311}
Changes BrowserThread::CurrentlyOn (and thus
BrowserThreadTaskRunner::RunsTasksOnCurrentThread()) to correctly report
a BrowserThread's association during MessageLoop destruction notification.
Also adds an explicit Start() to BrowserThreadImpl as there are tests which
call it and which incorrectly assumed base::Thread's implementation would
call BrowserThreadImpl::StartWithOptions (which it wouldn't since the latter
is not a virtual function.)
This change provokes many tests to delete ExtensionFunction instances that
were otherwise being leaked because UIThreadExtensionFunction::Destroy was
incorrectly deferring destruction when run on the UI thread during shutdown.
This in turn revealed a few small bugs which have also been fixed here.
BUG=631093
R=jam@chromium.org
Review-Url: https://codereview.chromium.org/2180253003
Cr-Commit-Position: refs/heads/master@{#408295}
In current code, some of the DCHECKs are having multiple conditions.
It's difficult to identify the failures if DCHECK fails due to one
of those conditions. Now moved the conditions to separate DCHECKs, so
that failure can identify at correct point.
BUG=
Review-Url: https://codereview.chromium.org/2168093002
Cr-Commit-Position: refs/heads/master@{#407201}
(ref. http://crbug.com/622400 for experiment details)
The mapping should be as follows:
- TaskPriority::USER_BLOCKING : the pool runs tasks that are on the blocking path to responding to
a user action.
- TaskPriority::USER_VISIBLE : the pool runs tasks that must not block (i.e. have visible outcomes)
but can be re-ordered behind USER_BLOCKING work.
- TaskPriority::BACKGROUND : the pool runs non-critical background tasks that should not interfere
with foreground work.
Since each pool can only be assigned a single priority (until we do the full migration and each
TaskRunner extracted from it can have its own priority), the highest priority among all tasks
running in that pool should be picked if the pool runs multiple types of tasks.
Note: for usage in unittests I picked USER_VISIBLE since tests shouldn't run at background OS priority
but it's okay if USER_BLOCKING work gets re-ordered ahead of the test's tasks. A TestTaskScheduler
will likely soon replace those use cases.
BUG=553459, 622400
Review-Url: https://codereview.chromium.org/2077413009
Cr-Commit-Position: refs/heads/master@{#406481}
Why?
The fact that there's a MessageLoop on the thread is an
unnecessary implementation detail. When browser threads
are migrated to base/task_scheduler, tasks will no longer
have access to a MessageLoop.
These changes were generated manually.
Note: Some files outside of base/ had to be modified
because they didn't compile anymore after
#include "base/message_loop/message_loop.h" was removed
from base/ headers.
BUG=616447
TBR=rogerta@chromium.org
Review-Url: https://codereview.chromium.org/2136563002
Cr-Commit-Position: refs/heads/master@{#406378}
Implements base::MessageLoop::GetThreadName by fetching the platform
thread name. This new function replaces MessageLoop::thread_name_ and
avoids having to store the thread name in multiple places.
Review-Url: https://codereview.chromium.org/1942053002
Cr-Commit-Position: refs/heads/master@{#399694}
