With the advent of the network service, a lot fewer files use
net::URLRequest than before, but there are still a lot of left over
includes. This CL removes many such unused includes in content/.
This CL also removes a pair of "using" directives for classes that
are no longer in use (ProtocolHandlerMap,
URLRequestInterceptorScopedVector)
TBR=caseq@chromium.org
Bug: 1103784, 1104380
Change-Id: Id3e4c56f820e0b905c933fb1ca7f82b393180117
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2292817
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788636}
Originally the sandbox policies lived in //content, but with
servicification this would create unwanted dependencies between
//services and //content. Instead, create a new //sandbox/policy
library to hold the sandbox integration code. This library can depend
on the low-level //sandbox routines, but not nice versa.
Tbr: ajgo@chromium.org (mechanical change rule)
Bug: 1097376
Change-Id: I1ca9ac0015a625197f2d3aae104e8f7aa78dcfd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2272609
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786385}
This reverts commit 212853047a.
Reason for revert: <INSERT REASONING HERE>
Original change's description:
> Revert "Make the coverage config reusable for generic profiling."
>
> This reverts commit c7695f6312.
>
> Reason for revert: Possible reason for win10_chromium_x64_rel_ng failing builds with error:
>
> lld-link: error: undefined symbol: __llvm_profile_dump
> >>> referenced by .\..\..\base\test\clang_profiling.cc:23
> >>> obj/base\base/clang_profiling.obj:(void __cdecl base::WriteClangProfilingProfile(void))
>
> Original change's description:
> > Make the coverage config reusable for generic profiling.
> >
> > Most of the coverage configuration can be re-used for general
> > Clang-based profiling (e.g. the profiling step of a PGO build).
> > This changes most of the |use_clang_coverage| checks to
> > |use_clang_profiling|.
> >
> > The toolchain opt-out option is still limited to the coverage
> > builds (grep for "toolchain_use_clang_coverage" in CodeSearch).
> > I.e. targets built simply with |use_clang_profiling=true| won't
> > be opted out by the clang_code_coverage_wrapper.py script.
> >
> > This CL is a pure refactoring with no functional change.
> > Getting profiling information from the sandboxed renderers work
> > this way:
> > - After creating a new child process the browser will open a
> > profiling file for this process and pass the handle to the
> > child via the SetProfilingProfile Mojo call (see change in
> > content/browser/renderer_host/render_process_host_impl.cc).
> > - This process will used by the LLVM instrumentation to record
> > profiling information.
> > - When terminating, child processes will call
> > WriteClangProfilingProfile to write the data to the profiling
> > data file.
> > - Once all the raw profile data files have been collected,
> > they'll be merged (via the "llvm-profdata" tool) and used.
> >
> > Note that this isn't meant to be enabled into any build that
> > will be shipped to users (a profiling build is usually 10x
> > bigger than a normal build anyway, not really shippable).
> >
> > The goal of this refactoring is to make it possible to reuse
> > this mechanism for the instrumentation step of PGO builds.
> >
> > Bug: 1058979
> > Change-Id: If8321234f6b6f86befd81f6e9e068a342728d605
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090493
> > Reviewed-by: Will Harris <wfh@chromium.org>
> > Reviewed-by: Nico Weber <thakis@chromium.org>
> > Reviewed-by: Scott Violet <sky@chromium.org>
> > Reviewed-by: Yuke Liao <liaoyuke@chromium.org>
> > Reviewed-by: Sajjad Mirza <sajjadm@chromium.org>
> > Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#749015}
>
> TBR=sky@chromium.org,thakis@chromium.org,wfh@chromium.org,sebmarchand@chromium.org,liaoyuke@chromium.org,sajjadm@chromium.org
>
> Change-Id: I6aaf638f6385afe35c488575aac84e3e952b76a3
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1058979
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2096332
> Reviewed-by: Jeevan Shikaram <jshikaram@chromium.org>
> Commit-Queue: Jeevan Shikaram <jshikaram@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#749040}
TBR=sky@chromium.org,thakis@chromium.org,wfh@chromium.org,sebmarchand@chromium.org,liaoyuke@chromium.org,sajjadm@chromium.org,jshikaram@chromium.org
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1058979
Change-Id: I5492c81b707e5741bd7c494893e3bc7953221263
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097305
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Yuke Liao <liaoyuke@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749185}
This reverts commit c7695f6312.
Reason for revert: Possible reason for win10_chromium_x64_rel_ng failing builds with error:
lld-link: error: undefined symbol: __llvm_profile_dump
>>> referenced by .\..\..\base\test\clang_profiling.cc:23
>>> obj/base\base/clang_profiling.obj:(void __cdecl base::WriteClangProfilingProfile(void))
Original change's description:
> Make the coverage config reusable for generic profiling.
>
> Most of the coverage configuration can be re-used for general
> Clang-based profiling (e.g. the profiling step of a PGO build).
> This changes most of the |use_clang_coverage| checks to
> |use_clang_profiling|.
>
> The toolchain opt-out option is still limited to the coverage
> builds (grep for "toolchain_use_clang_coverage" in CodeSearch).
> I.e. targets built simply with |use_clang_profiling=true| won't
> be opted out by the clang_code_coverage_wrapper.py script.
>
> This CL is a pure refactoring with no functional change.
> Getting profiling information from the sandboxed renderers work
> this way:
> - After creating a new child process the browser will open a
> profiling file for this process and pass the handle to the
> child via the SetProfilingProfile Mojo call (see change in
> content/browser/renderer_host/render_process_host_impl.cc).
> - This process will used by the LLVM instrumentation to record
> profiling information.
> - When terminating, child processes will call
> WriteClangProfilingProfile to write the data to the profiling
> data file.
> - Once all the raw profile data files have been collected,
> they'll be merged (via the "llvm-profdata" tool) and used.
>
> Note that this isn't meant to be enabled into any build that
> will be shipped to users (a profiling build is usually 10x
> bigger than a normal build anyway, not really shippable).
>
> The goal of this refactoring is to make it possible to reuse
> this mechanism for the instrumentation step of PGO builds.
>
> Bug: 1058979
> Change-Id: If8321234f6b6f86befd81f6e9e068a342728d605
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090493
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: Nico Weber <thakis@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Yuke Liao <liaoyuke@chromium.org>
> Reviewed-by: Sajjad Mirza <sajjadm@chromium.org>
> Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#749015}
TBR=sky@chromium.org,thakis@chromium.org,wfh@chromium.org,sebmarchand@chromium.org,liaoyuke@chromium.org,sajjadm@chromium.org
Change-Id: I6aaf638f6385afe35c488575aac84e3e952b76a3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1058979
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2096332
Reviewed-by: Jeevan Shikaram <jshikaram@chromium.org>
Commit-Queue: Jeevan Shikaram <jshikaram@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749040}
Most of the coverage configuration can be re-used for general
Clang-based profiling (e.g. the profiling step of a PGO build).
This changes most of the |use_clang_coverage| checks to
|use_clang_profiling|.
The toolchain opt-out option is still limited to the coverage
builds (grep for "toolchain_use_clang_coverage" in CodeSearch).
I.e. targets built simply with |use_clang_profiling=true| won't
be opted out by the clang_code_coverage_wrapper.py script.
This CL is a pure refactoring with no functional change.
Getting profiling information from the sandboxed renderers work
this way:
- After creating a new child process the browser will open a
profiling file for this process and pass the handle to the
child via the SetProfilingProfile Mojo call (see change in
content/browser/renderer_host/render_process_host_impl.cc).
- This process will used by the LLVM instrumentation to record
profiling information.
- When terminating, child processes will call
WriteClangProfilingProfile to write the data to the profiling
data file.
- Once all the raw profile data files have been collected,
they'll be merged (via the "llvm-profdata" tool) and used.
Note that this isn't meant to be enabled into any build that
will be shipped to users (a profiling build is usually 10x
bigger than a normal build anyway, not really shippable).
The goal of this refactoring is to make it possible to reuse
this mechanism for the instrumentation step of PGO builds.
Bug: 1058979
Change-Id: If8321234f6b6f86befd81f6e9e068a342728d605
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090493
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Yuke Liao <liaoyuke@chromium.org>
Reviewed-by: Sajjad Mirza <sajjadm@chromium.org>
Commit-Queue: Sébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749015}
When the feature is activated HangWatcher will monitor the IOThread
instead of ThreadWatcher. The individual MessageLoop implementations are
instrumented to cover all platforms for the handling of events and
ThreadControllerWithMessagePumpImpl is instrumented to cover the task
execution.
Shutdown hang watching is still covered by ShutdownWatcherHelper.
Bug: 1034046
Change-Id: Ie797a1d1c4ce3b438878ad18e91347ccd02daccf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2046404
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: Oliver Li <olivierli@google.com>
Cr-Commit-Position: refs/heads/master@{#745143}
Removes unused kNaClLoader (was PROCESS_TYPE_NACL_LOADER)
from Windows as these are in fact Ppapi (PPAPI) sandbox types.
Removes unused values from SandboxType enum and replaces default cases.
SandboxType is not used for iteration so these boundary values are removed
from the SandboxType enum.
The kInvalid SandboxType is retained as it is used as an error case in
a couple of places. It might be possible to remove this in the future.
This removes default cases from switch statements where it might make
sense to have a notification in future when new sandbox types are added.
In these cases the default case is replaced with all otherwise
unchecked cases, so retaining the existing behavior.
Change-Id: I76ffc8ae617f3f8fa9aa68236551ebcfa4cce32f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1938076
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723425}
This should reduce the number of shutdown hangs we're seeing with the
network service enabled. According to UMA, ~99.9% of all shutdowns
complete within 1s on ChromeOS.
Bug: 963512
Change-Id: Iec10ca59349be9d6fbbe2d1294f4aaf585711d3c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613121
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#659980}
Some ozone builds (e.g. Chromecast) are not prioritizing latency
sensitive graphics work properly due to narrowly scoped #ifdefs.
Expand them to apply to all ozone builds.
This changes the portion of wall time the UI thread spends waiting
to run from ~19% to ~7% in a simple fling benchmark on Google Home Hub.
Bug: 937462
Change-Id: I59555bcad610e6b95d916f129dbd6db497d55970
Reviewed-on: https://chromium-review.googlesource.com/c/1497297
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Alex Sakhartchouk <alexst@chromium.org>
Commit-Queue: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637131}
Use case: An expensive function wants to make sure it's not running on the UI
thread so the expensive operations don't block that thread.
This assertion fails on threads other than scheduler workers.
Bug: 879181
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I2a7ec2442c2020da4d68605c2476a01d25a45a33
Reviewed-on: https://chromium-review.googlesource.com/1221787
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593191}
With VC++ marking a function as non-optimized with MSVC_DISABLE_OPTIMIZE would
prevent it from being inlined, however with clang-cl that does not work,
leading to confusing crash stacks.
The fix is to remove MSVC_DISABLE_OPTIMIZE; these functions were already marked
as NOINLINE.
Bug: 873359
Change-Id: I00995a17463572545933956cdec0e7e13825abe7
Reviewed-on: https://chromium-review.googlesource.com/1182302
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584607}
This change replaces usage of content::BrowserGpuMemoryBufferManager
with viz::HostGpuMemoryBufferManager.
With this change BrowserGpuMemoryBufferManager is not needed anymore and
code to handle buffer allocation in GpuProcessHost is also removed.
BUG=733482
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I6a9387e2e33a5d98d4ed7559618f4995256bafa4
Reviewed-on: https://chromium-review.googlesource.com/1095815
Commit-Queue: Mohsen Izadi <mohsen@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584455}
Currently many layout test runs are flaking because swarming is not expecting child processes to outlive the browser. This is happening because TerminateProcess fails sometimes because there's pending I/O.
Bug: 820996
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I7595c131faccae03c268cde1139394c0ad48a7b4
Reviewed-on: https://chromium-review.googlesource.com/1071100
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Penny MacNeil <pennymac@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562058}
The previous fix in r560368 didn't work, as the BrowserChildProcessHostImpl::TerminateAll removed all the UtilityProcessHost objects first.
Bug: 820996, 820969
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I88ba647e911c8f7e8d9458b60021aa6d915ccfa0
Reviewed-on: https://chromium-review.googlesource.com/1069668
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560938}
This is a reland of I23ef57eb52bfb1eb363682dadf98c571c12afcd1.
It was reverted in Ia548067acbf640010f4c8fbed29a0012a274af05. Reason for revert:
Findit (https://goo.gl/kROfz5) identified this CL at revision 557680 as the culprit
for introducing flakiness in the tests as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vZmM5NTUwMWZiMjllM2U3NDg3ZmZmOThjMTdlNmFhYmExMDQ2YjMzMAw
The data race has been fixed in:
https://chromium-review.googlesource.com/c/chromium/src/+/1064450.
Original change's description:
> Create BrowserThread::IO thread before browser main loop to start
> ServiceManager.
>
> We need a thread to post/execute tasks when starting the
> ServiceManager. This thread needs to be created before the browser
> main loop is initialized, and will be registered as the
> BrowserThread::IO thread which is currently used by ServiceManager
> connections.
>
> The creation of such a thread is moved to service_manager::main via
> MainDelegate::CreateIOThreadAndGetTaskRunner(). Since it requires no
> thread created before calling fork() on posix, we also move the setup
> of sandbox before creating the IO thread.
>
> Bug: 740677, 729596
> Change-Id: I23ef57eb52bfb1eb363682dadf98c571c12afcd1
> Reviewed-on: https://chromium-review.googlesource.com/969098
> Commit-Queue: Xi Han <hanxi@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#557680}
Bug: 740677, 729596
Change-Id: I9afb0cdc0f11a1d437c2e9bd09c374503c3d5a4b
Reviewed-on: https://chromium-review.googlesource.com/1059949
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560724}
ServiceManager.
We need a thread to post/execute tasks when starting the
ServiceManager. This thread needs to be created before the browser
main loop is initialized, and will be registered as the
BrowserThread::IO thread which is currently used by ServiceManager
connections.
The creation of such a thread is moved to service_manager::main via
MainDelegate::CreateIOThreadAndGetTaskRunner(). Since it requires no
thread created before calling fork() on posix, we also move the setup
of sandbox before creating the IO thread.
Bug: 740677, 729596
Change-Id: I23ef57eb52bfb1eb363682dadf98c571c12afcd1
Reviewed-on: https://chromium-review.googlesource.com/969098
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557680}
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}
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}
A major part of android's ChildProcessLauncher is implemented in Java
so having an easy access to the launcher thread is important.
Refactor JavaHandlerThread in base so that that java part can be created
first, and then connected with native peer after consutrction. This is
needed because the launcher thread can be used before native library is
loaded.
Add LauncherThread in content which is a simple wrapper around
JavaHandlerThread. Then for android, override the launcher thread
message loop. Note this means the launcher thread will no longer be
joined on shutdown, but this is not a problem in practice since android
never does clean shutdowns.
Convert two cases of random background threads in ChildProcessLauncher
to use LauncherThread. There are more in the future.
BUG=701442
Review-Url: https://codereview.chromium.org/2774363003
Cr-Commit-Position: refs/heads/master@{#460509}
This reverts commit d1109b15f8.
The crash rate is too high and polluting crash report data.
I'll reland it when the fix lands.
BUG=636377
TBR=jam
Review-Url: https://codereview.chromium.org/2338113002
Cr-Commit-Position: refs/heads/master@{#418462}
Reason for revert:
Reverting temporary instrumentation following investigation
We want to get this in prior to dev branch.
BUG=636377
Original issue's description:
> Check that all shared workers are terminated.
>
> Adds temporary checks to determine whether shared workers
> prevent RenderProcessHost(s) from cleaning up due to
> RenderProcessHostImpl::worker_ref_count_ > 0.
>
> BUG=608049
>
> Committed: https://crrev.com/529173d1bbeb53b3f9438648fad264b4db042721
> Cr-Commit-Position: refs/heads/master@{#410730}
TBR=atwilson@chromium.org,jam@chromium.org,horo@chromium.org,alokp@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=608049
Review-Url: https://codereview.chromium.org/2254963002
Cr-Commit-Position: refs/heads/master@{#412660}
Adds temporary checks to determine whether shared workers
prevent RenderProcessHost(s) from cleaning up due to
RenderProcessHostImpl::worker_ref_count_ > 0.
BUG=608049
Review-Url: https://codereview.chromium.org/2222063002
Cr-Commit-Position: refs/heads/master@{#410730}
Includes a minor GpuMemoryBufferImpl refactor to provide the
manager with an estimated buffer size.
BUG=481203
Review URL: https://codereview.chromium.org/1055403010
Cr-Commit-Position: refs/heads/master@{#327968}
Adds browser_process_sub_thread.cc to the build on iOS and ifdefs out
BrowserChildProcessHostImpl::TerminateAll() as Chrome on iOS is
single-process.
Review URL: https://chromiumcodereview.appspot.com/11066054
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@161091 0039d316-1c4b-4281-b951-d872f2087c98
While doing so, fix a few issues with the code like ordering of URLRequestContext to ensure correct destruction order. Also fix const correctness in some places.
BUG=58859
TEST=none
TBR=willchan
Review URL: https://chromiumcodereview.appspot.com/10299002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136497 0039d316-1c4b-4281-b951-d872f2087c98
By design, there's no ScopedAllowWait that is reachable by all code. From experience with ScopedAllowIO, it will be abused. So instead the existing callers (which should all be fixed other than two) are friends with ThreadRestrictions.
Review URL: https://chromiumcodereview.appspot.com/10151009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134114 0039d316-1c4b-4281-b951-d872f2087c98
I've also moved the iterator class into its own file in the public directory. Since classes don't derive from BrowserChildProcessHost and so can't static_cast from it, I added a template helper that does this.
BUG=98716
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=118415
Review URL: https://chromiumcodereview.appspot.com/9150017
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@118516 0039d316-1c4b-4281-b951-d872f2087c98
