This is a reland of 893230151e
The previous version of this CL mistakenly removed a default
case statement for an 'int' field, so an expected compile error
was not generated but instead an early return was skipped.
Original change's description:
> Add launch failure notifications to BrowserChildProcessObserver
>
> Also, fix a bug where the Windows sandbox would return SBOX_ALL_OK
> even if base::LaunchProcess failed, and launch_result was not
> being set for elevated processes.
>
> Add reporting of GetLastError on Windows in the
> ChildProcessTerminationInfo for failed launches.
>
> Also, clean up some switch statements to remove default cases.
>
> BUG=1280005
>
> Change-Id: I1001fc950b8456b78ef1a9a985ca07cf288e8a04
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3340072
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Will Harris <wfh@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#952182}
Bug: 1280005, 1280541
Change-Id: I81f3354e9870a455644e77144a40c4acbdf14ebe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3345720
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#952667}
This reverts commit 893230151e.
Reason for revert: "browser_tests" failing on builder "Linux ChromiumOS MSan Tests"
Original change's description:
> Add launch failure notifications to BrowserChildProcessObserver
>
> Also, fix a bug where the Windows sandbox would return SBOX_ALL_OK
> even if base::LaunchProcess failed, and launch_result was not
> being set for elevated processes.
>
> Add reporting of GetLastError on Windows in the
> ChildProcessTerminationInfo for failed launches.
>
> Also, clean up some switch statements to remove default cases.
>
> BUG=1280005
>
> Change-Id: I1001fc950b8456b78ef1a9a985ca07cf288e8a04
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3340072
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Will Harris <wfh@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#952182}
Bug: 1280005, 1280541
Change-Id: I368d0c89ca6911cb4145bcec13edf2ad8bd4d829
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3344787
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Aya Elsayed <ayaelattar@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Owners-Override: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/main@{#952303}
Also, fix a bug where the Windows sandbox would return SBOX_ALL_OK
even if base::LaunchProcess failed, and launch_result was not
being set for elevated processes.
Add reporting of GetLastError on Windows in the
ChildProcessTerminationInfo for failed launches.
Also, clean up some switch statements to remove default cases.
BUG=1280005
Change-Id: I1001fc950b8456b78ef1a9a985ca07cf288e8a04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3340072
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#952182}
This CL moves the code that sets the profiling file from ChildProcess
into BrowserChildProcess.
It then no longer attempts to set the profiling file for elevated
browser child processes since it is not possible to pass a handle
to such a process as it is not permitted to duplicate a handle into
a process that cannot be opened, such as one running at a higher
integrity level.
This CL also has the consequence that the profiling file is no
longer passed to non-browser child processes which is just
chrome's ServiceUtilityProcessHost. Since this process is also
often running elevated (e.g. cloud print service) it is likely
this operation was never working correctly anyway.
BUG=1276198
Change-Id: I9684363ee4630583e90e31a4005cd1f26e60e050
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3313495
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#948049}
The enum is defined but only handled in some files for Lacros. This
appears to have been an oversight. This CL fixes all callsites to have
the same behavior for ash and lacros.
Bug: 1235874
Change-Id: Ibe3f3b9c19ac292e162230bc1ddb628082996028
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3269048
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#946647}
This refactors FieldTrial to simplify its public API and unify it
across all platforms so every public method has the same signature on
every platform.
This simplifies the call sites in the content layer by removing
per-platform specific call sites in favor of unified calls and moving
some of the logic done in //content to //base. In particular, command
line switches used only by FieldTrial have been moved to //base, from
//content. They are also no longer passed as parameters to the public
API of FieldTrial.
In addition, this adds supports for shared-memory distribution of
FieldTrial configuration to child processes on Fuchsia. This is
achieved by duplicating the handle for the VMO backing the FieldTrial
configuration in the parent process and passing it to the child
process startup handles.
This also changes the Windows child process launcher logic to make use
of the provided LaunchOptions argument, rather than initializing a new
object.
Finally, this cleans up header usage in //base/metrics/field_trial.h
and fixes missing includes in various parts of the code base.
Bug: 752368, 1262370
Change-Id: I9b0836b467790f1da96d85fe16a6e2c6d334f709
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3230582
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#938902}
This CL makes several changes:
(1) Causes the browser to reset non-browser
mojo::PendingReceiver<Coordinator>. This means that non-browser
processes will never be able to use the Coordinator interface.
(2) Add CHECKs to existing code to prevent non-browser processes from
attempting to use the Coordinator interface.
A code audit shows that all Coordinator usages should already only be
from the browser process.
Note that (2) is important since attempting to use an unbound interface
will trigger a nullptr dereference, which is undefined behavior.
Bug: 1251787
Change-Id: Ifbe9610cc0e373edaaa60fad46b447e8bdb3ec04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3174305
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Auto-Submit: Erik Chen <erikchen@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#923693}
This modifies Browser/ChildProcessHost and ChildThreadImpl to support
child processes which never establish a legacy IPC Channel, and
modifies UtilityProcessHost and UtilityThreadImpl to take advantage of
this.
This makes utility processes the first type of process to be completely
free of legacy IPC. GPU processes can follow shortly hereafter.
To facilitate this change, a few other details are modified here:
- BrowserChildProcessObserver no longer exposes a separate
BrowserProcessHostConnected event. The only two consumers are
migrated to BrowserChildProcessLaunchedAndConnected. This avoids
potential re-entrancy issues when launching a process, since any
hosts not using legacy IPC are considered to be connected
synchronously as soon as launch is started.
- Browser/ChildProcessHost no longer acknowledges the PID received
from the child process in IPC::Listener::OnChannelConnected,
instead using the PID exposed directly from the
ChildProcessLauncher. This is consistent whether or not the
host creates a legacy IPC channel.
Bug: 616980, 993189
Change-Id: I881db61db335b70b658fe0dbf0c149703e7219e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2959467
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#893209}
base::size() has been moved to base/cxx17_backports.h, so .cc files that
use base::size(), but no other function from base/stl_util.h, can
directly include base/cxx17_backports.h and not base/stl_util.h.
Bug: 1210983
Change-Id: I42a598a9c2b8fcbfd1e225329109ae3308bd9518
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2915348
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#886358}
List of files to delete from generated using the following command:
git grep -l base/strings/stringprintf.h | \
xargs grep -L 'StringPrint[fV]' | xargs grep -L StringAppend | \
grep -E '(cc|mm|h)$'
Change-Id: Ibc72245f08730b4d25283e2d966235b61513c7ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2849392
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Owners-Override: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#876365}
They were on the IO thread to avoid deadlocks with NPAPI plugins, but we don't have that anymore.
Currently this is off by default behind a feature flag. I have tried to minimize the churn in the code to enable this. Some places where it would involve too much duplication or refactoring I've kept with PostTasks from UI to UI thread. Once this is enabled by default then I'll simplify all the callsites.
Bug: 904556
Change-Id: Ib664d644e0af016d5736348218bcc38cb79341f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2806425
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Owners-Override: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#870614}
This relands commit 305cf78bae with fixing
the flakiness of BackgroundTracingManagerBrowserTest.* in
content_browsertests. Fix by opening the socket to the system tracing
service on the right task runner.
Original change's description:
> The security sandbox forbids making socket connections directly from
> within most child processes (except the network utility process). Trace
> data for renderers are missing from a tracing session with Chrome data
> sources on non-Android Posix platforms.
>
> This change introduces a new Mojo interface, SystemTracingService, that
> is shared between the browser and child processes for making a socket
> connection to the system tracing service daemon. A child process
> receives a Mojo remote interface and then invokes the OpenProducerSocket
> method to open the socket connection in the browser process. The browser
> process then passes the socket connection to the child in the response.
> The child process then adopts the connected socket in initializing the
> connection in PosixSystemProducer.
>
> Full design doc: go/crosetto-chrome-producer-dd
>
> Bug: b/147789115
> Test: services_unittests: SystemTracingServiceTest.*,
> SystemPerfettoTest.SandboxedOpenProducerSocket
> Change-Id: Id698dc656b7b9ede723e3a887532c33544914edc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435129
> Commit-Queue: Chinglin Yu <chinglinyu@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Stephen Nusko <nuskos@chromium.org>
> Reviewed-by: Eric Seckler <eseckler@chromium.org>
> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#844743}
Bug: 1169185
Test:content_browsertests --test-launcher-bot-mode \
--gtest_filter="BackgroundTracingManagerBrowserTest.*" \
--gtest_repeat=100
Change-Id: I02b4bc206013b9dbc91766b3d643d07d695ff87b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2643425
Reviewed-by: Ricky Liang <jcliang@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Chinglin Yu <chinglinyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#847517}
This reverts commit 305cf78bae.
Reason for revert: Crashing many browser tests, in particular BackgroundTracingManagerBrowserTest.*
Original change's description:
> tracing: make PosixSystemProducer work in child processes on Posix.
>
> The security sandbox forbids making socket connections directly from
> within most child processes (except the network utility process). Trace
> data for renderers are missing from a tracing session with Chrome data
> sources on non-Android Posix platforms.
>
> This change introduces a new Mojo interface, SystemTracingService, that
> is shared between the browser and child processes for making a socket
> connection to the system tracing service daemon. A child process
> receives a Mojo remote interface and then invokes the OpenProducerSocket
> method to open the socket connection in the browser process. The browser
> process then passes the socket connection to the child in the response.
> The child process then adopts the connected socket in initializing the
> connection in PosixSystemProducer.
>
> Full design doc: go/crosetto-chrome-producer-dd
>
> SystemPerfettoTest.SandboxedOpenProducerSocket
>
> Bug: b/147789115
> Test: services_unittests: SystemTracingServiceTest.*,
> Change-Id: Id698dc656b7b9ede723e3a887532c33544914edc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435129
> Commit-Queue: Chinglin Yu <chinglinyu@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Stephen Nusko <nuskos@chromium.org>
> Reviewed-by: Eric Seckler <eseckler@chromium.org>
> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#844743}
TBR=kinuko@chromium.org,primiano@chromium.org,skyostil@chromium.org,eseckler@chromium.org,chinglinyu@chromium.org,nuskos@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: b/147789115, 1169185
Change-Id: Ibe932297b317fdee6cbd9ef5b09ad1aeffd2f1ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2643258
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845763}
The security sandbox forbids making socket connections directly from
within most child processes (except the network utility process). Trace
data for renderers are missing from a tracing session with Chrome data
sources on non-Android Posix platforms.
This change introduces a new Mojo interface, SystemTracingService, that
is shared between the browser and child processes for making a socket
connection to the system tracing service daemon. A child process
receives a Mojo remote interface and then invokes the OpenProducerSocket
method to open the socket connection in the browser process. The browser
process then passes the socket connection to the child in the response.
The child process then adopts the connected socket in initializing the
connection in PosixSystemProducer.
Full design doc: go/crosetto-chrome-producer-dd
SystemPerfettoTest.SandboxedOpenProducerSocket
Bug: b/147789115
Test: services_unittests: SystemTracingServiceTest.*,
Change-Id: Id698dc656b7b9ede723e3a887532c33544914edc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435129
Commit-Queue: Chinglin Yu <chinglinyu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Stephen Nusko <nuskos@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844743}
This class added an unnecessary layer of complexity to access
memory instrumentation functionality.
Follow-up: Rename //services/resource_coordinator/ to
//services/memory_instrumentation and remove "memory_instrumentation"
subdirectories.
Change-Id: I556423d2004ab0d2b2462ae5b0cac529712d50de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593829
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Auto-Submit: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840316}
The change is mostly mechanical replacing defined(OS_CHROMEOS) with
BUILDFLAG(IS_CHROMEOS_ASH) and GN variable is_chromeos with
is_chromeos_ash with some special cases (For those cases please
refer to http://go/lacros-macros).
The patch is made in preparation to switching lacros build from
target_os=linux to target_os=chromeos. This will prevent lacros from
changing behaviour after the switch.
Bug: 1052397
Change-Id: Ieb265e116ff6ada5e2f99d609ff12fb9f92727e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2534271
Commit-Queue: Yuta Hijikata <ythjkt@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829687}
Remove command-line flags --websocket-read-buffer-size and
--websocket-renderer-receive-quota-max which are no longer needed as the
performance investigation has finished.
Also replace the kReceiveQuotaThreshold constant in websocket_channel.cc
with a kReceiveDataPipeCapacity constant in websocket.cc, which better
reflects its current usage.
Change the types of kReadBufferSize and kReceiveDataPipeCapacity to
reflect the types at the points where they are used.
Also mark WebSocketBasicStream as final, as its destructor calls the
Close() virtual method and it would not behave correctly in a subclass.
Also change some constants from "const" to "constexpr".
BUG=865001
Change-Id: I761bd08de534ef27a77a9523c86ea6f1a61b9266
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2383396
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803308}
It turns out that the registration code path runs on NaCl
modules too, however it doesn't work well with tracing
component (in fact, no tracing data are collected for NaCl
modules). Since NaCl is not working yet registering,
services/tracing/perfetto/consumer_host.cc waits for
the activation of NaCl processes but it always have to wait
until the timeout (10 seconds).
Although NaCl ended already, ChromeOS is an exception. It still
has a few NaCl processes by default in the production, and so
this 10-second delay happens always.
Bug: 1101468
Test: with Tast test
Change-Id: I55f8664e0b6089c6eb59fcde87c6dfd099530f8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2278349
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785463}
This is a reland of 002e044483
No changes other than disabling the test under MSan where
it is broken and probably not worth fixing.
Original change's description:
> Support dynamic Mojo Core on Linux
>
> This introduces a new --mojo-core-library-path Content switch which
> instructs Content to initialize each process with an implementation
> of Mojo Core found in the referenced shared library rather than using
> the version linked into the main binary.
>
> This allows for IPC interoperability between a Content embedder and
> another application which provides its own copy of Mojo Core.
>
> Fixed: 1082473
> Change-Id: I1e50c505e91a53e60056a4b8c691d91728f7a5ea
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2229664
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Ken Rockot <rockot@google.com>
> Cr-Commit-Position: refs/heads/master@{#778002}
Tbr: avi@chromium.org
Change-Id: I6a2270c87c294455907fb369640b3be0819b2431
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2247166
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#780151}
This introduces a new --mojo-core-library-path Content switch which
instructs Content to initialize each process with an implementation
of Mojo Core found in the referenced shared library rather than using
the version linked into the main binary.
This allows for IPC interoperability between a Content embedder and
another application which provides its own copy of Mojo Core.
Fixed: 1082473
Change-Id: I1e50c505e91a53e60056a4b8c691d91728f7a5ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2229664
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#778002}
Android has a browser-side timeout for establish channel which is
usually the #1 crash on the stable channel. This CL will try to perform
a DumpWithoutCrashing in the gpu process when this timeout happens to
gather more data.
Bug: 680777
Change-Id: I21e1701c359afc57e31a50403d59ba9a94a2baae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2222919
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773783}
Note to QA: This CL is purely mechanical and shouldn't be blamed
for future regressions on touched files.
This is a follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/2211138
which already removed all usage using content::BrowserThread.
Hence this script now matches unqualified BrowserThread:: without
risking having "content::" be selected as "traits_before" by the regex
(ran on same revision as step #1).
content:: is now always added if outside namespace content {}
(deleting unused using content::BrowserThread; decls)
Script @ https://crbug.com/1026641#c92
(will TBR fdoray@ post-review for mechanical change)
TBR=fdoray@hchromium.org
AX-Relnotes: n/a.
Bug: 1026641
Change-Id: I51ae2f83eb17d19b54563fd9b4fc040d2aa0c948
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212469
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772458}
The service performs expensive computations on UI thread in both browser
and renderer processes. The computation of VM regions, creating OS
memory dump, graph computation of chrome dumps are all expensive. So,
this moves the coordinator and clients to memory-infra thread which is
created for this purpose.
Change-Id: I6da7e5a0a8d6f8e4b91b8071bf464fb9eaced2ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2109457
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: ssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753697}
NetworkService was incorrectly using "bad_message_reason" crash key name
when reporting a bad mojo message. This CL switches to using the
correct "mojo-message-error" name (one that is recognized by the crash
analysis services). This CL also moves the crash key definition to the
//mojo layer, so that it can be reused both from //content and from
//services/network.
Bug: 987986
Change-Id: I772ae563df2f62ea430235524397b8492455de23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2118690
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753484}
This CL makes sure that reports of bad mojo messages are preserving the
callstack and the crash keys. This is done by making sure that
DumpWithoutCrashing is called synchronously from
BrowserChildProcessHostImpl::OnMojoError.
The CL also opportunitically reduces code duplication related to
actually terminating a child process. The duplicated code is extracted
into a new TerminateProcessForBadMessage private method and reused both
from OnMojoError and from TerminateOnBadMessageReceived.
Bug: 1062418
Change-Id: I8f5bf802e9aabec4d92ee9fd37c1c55012f1250f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106806
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751478}
Android specifically generally do not use exit_code or TerminationStatus
to communicate how a "child" process exited. Android does not have real
child processes; they are separate service processes forked from
android's zygote.
One scenario that is correctly detected is if the child process exists
cleanly by quitting its main loop. So avoid treating case this in
BrowserChildProcessHostImpl as a crash. This is in preparation fo
cleanly shutting down the gpu process when not in need.
Not Android will still call NotifyProcessKilled. NotifyProcessKilled is
used more like a general process exited notification due to the reasons
above, so keep calling it.
Bug: 1058509
Change-Id: I07102e7ddbec7d68c8ce3d3ffaf5a3d56a4309fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2100729
Reviewed-by: ssid <ssid@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749951}
Child processes send a message to BrowserChildProcessHostImpl to
have the host register Resource Coordinator client pipes on their
behalf.
The implementation of this message accesses the corresponding
ChildProcessLauncher's PID indiscriminately. Meanwhile the process
may have died by the time the message is actually dispatched, and
it's invalid to access that PID after process termination.
While this race doesn't seem to affect security or stability of
production code in practice, it does cause browser tests to hit
a DCHECK on PID access, causing fairly common flake.
This fixes the race by ensuring the Process is still valid before
attempting to grab its PID.
Fixed: 1029627
Change-Id: I3f8eb6e9f6cd5c94c4011b76446e7fce63c0d12c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1944618
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#721726}
This reverts commit bfca6c8096.
Reason for revert: Reverting the revert which was only done make reverting an earlier CL possible. Earlier CL has been fixed and relanded.
Original change's description:
> Revert "Move Utility processes off Service Manager"
>
> This reverts commit 0a311ff4af.
>
> Reason for revert: breaks graphics on some Chrome OS devices.
>
> Original change's description:
> > Move Utility processes off Service Manager
> >
> > This migrates Utility processes to direct ChildProcess API usage instead
> > of bootstrapping IPC through the Service Manager.
> >
> > As this is the last remaining use of Service Manager IPC to bootstrap
> > Content child processes, this also rips out a bunch of infrastructure
> > that existed only to support that.
> >
> > Bug: 977637
> > Change-Id: I08e542f1d9f294bc1c387ea5845e8ba0d5a7d2b8
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919719
> > Commit-Queue: Ken Rockot <rockot@google.com>
> > Reviewed-by: Robert Sesek <rsesek@chromium.org>
> > Reviewed-by: Avi Drissman <avi@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#718352}
>
> NOPRESUBMIT=true
>
> Bug: 1028852
> Bug: 977637
> Tbr: rockot@google.com
> Tbr: avi@chromium.org
> Tbr: rsesek@chromium.org
> Change-Id: I9b2e8ca44f5b6accc2d3718cab3cb547d979c230
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948402
> Commit-Queue: Shuhei Takahashi <nya@chromium.org>
> Reviewed-by: Shuhei Takahashi <nya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#720938}
TBR=avi@chromium.org,rockot@google.com,nya@chromium.org,rsesek@chromium.org
NOPRESUBMIT=true
Change-Id: I79ea423c97880366125abfb659dae223d0185023
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1028852, 977637
Tbr: rsesek@chromium.org
Tbr: avi@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1949304
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#721383}
This reverts commit 881a65d746.
Reason for revert: breaks graphics on some Chrome OS devices.
Original change's description:
> Move Renderer processes off Service Manager
>
> Migrates Renderer processes to use the simpler ChildProcess IPC
> interface in place of deprecated Service Manager IPC.
>
> Support for preloaded files in service manifest definitions is
> effectively removed in favor of explicit parameters on
> BrowserChildProcessHost and ChildProcessLauncher. The only use case (V8
> snapshot files) has been migrated from manifest data to a simple map
> definition, and it's used during renderer and utility process
> launching.
>
> After this CL, only utility processes remain to be moved off of
> Service
> Manager IPC.
>
> NOPRESUBMIT=true
>
> Bug: 977637
> Change-Id: I1d8205cb73ead904aa21b85d6cbee11cb3fc84f5
> Tbr: boliu@chromium.org
> Tbr: yucliu@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918375
> Reviewed-by: Martin Barbella <mbarbella@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Ken Rockot <rockot@google.com>
> Cr-Commit-Position: refs/heads/master@{#717920}
NOPRESUBMIT=true
Bug: 1028852
Bug: 977637
Tbr: rockot@google.com
Tbr: boliu@chromium.org
Tbr: yucliu@chromium.org
Tbr: mbarbella@chromium.org
Tbr: rsesek@chromium.org
Tbr: avi@chromium.org
Change-Id: I30a52d825cc156b066d7c2ec455c8fb588a408af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948663
Commit-Queue: Shuhei Takahashi <nya@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720939}
This reverts commit 0a311ff4af.
Reason for revert: breaks graphics on some Chrome OS devices.
Original change's description:
> Move Utility processes off Service Manager
>
> This migrates Utility processes to direct ChildProcess API usage instead
> of bootstrapping IPC through the Service Manager.
>
> As this is the last remaining use of Service Manager IPC to bootstrap
> Content child processes, this also rips out a bunch of infrastructure
> that existed only to support that.
>
> Bug: 977637
> Change-Id: I08e542f1d9f294bc1c387ea5845e8ba0d5a7d2b8
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919719
> Commit-Queue: Ken Rockot <rockot@google.com>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#718352}
NOPRESUBMIT=true
Bug: 1028852
Bug: 977637
Tbr: rockot@google.com
Tbr: avi@chromium.org
Tbr: rsesek@chromium.org
Change-Id: I9b2e8ca44f5b6accc2d3718cab3cb547d979c230
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948402
Commit-Queue: Shuhei Takahashi <nya@chromium.org>
Reviewed-by: Shuhei Takahashi <nya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720938}
This migrates Utility processes to direct ChildProcess API usage instead
of bootstrapping IPC through the Service Manager.
As this is the last remaining use of Service Manager IPC to bootstrap
Content child processes, this also rips out a bunch of infrastructure
that existed only to support that.
Bug: 977637
Change-Id: I08e542f1d9f294bc1c387ea5845e8ba0d5a7d2b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1919719
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718352}
This is a reland of 45cef59a78
Original change's description:
> Move Renderer processes off Service Manager
>
> Migrates Renderer processes to use the simpler ChildProcess IPC
> interface in place of deprecated Service Manager IPC.
>
> Support for preloaded files in service manifest definitions is
> effectively removed in favor of explicit parameters on
> BrowserChildProcessHost and ChildProcessLauncher. The only use case (V8
> snapshot files) has been migrated from manifest data to a simple map
> definition, and it's used during renderer and utility process launching.
>
> After this CL, only utility processes remain to be moved off of Service
> Manager IPC.
>
> NOPRESUBMIT=true
>
> Bug: 977637
> Change-Id: I1d8205cb73ead904aa21b85d6cbee11cb3fc84f5
> Tbr: boliu@chromium.org
> Tbr: yucliu@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918375
> Reviewed-by: Martin Barbella <mbarbella@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Commit-Queue: Ken Rockot <rockot@google.com>
> Cr-Commit-Position: refs/heads/master@{#717920}
NOPRESUBMIT=true
Bug: 977637
Change-Id: I9dd89104626ef73f81abd93c8a35310a2d6997a2
Tbr: boliu@chromium.org
Tbr: yucliu@chromium.org
Tbr: mbarbella@chromium.org
Tbr: avi@chromium.org
Tbr: rsesek@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930104
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#718334}
