This also removes a few direct calls from ash into
crash_reporter::DumpWithoutCrashing() which bypasses setting
crash-location keys. These may (but probably unlikely) be why we're
seeing a few bad-stack DWCs without location keys set.
Bug: 386552744
Change-Id: Id7f3e3a52a12decf2a8a7a75f3de047c3c1f3f1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6246294
Commit-Queue: Peter Boström <pbos@chromium.org>
Owners-Override: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Boström <pbos@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1417969}
On this CQ job, Proguard took 2:45, and the subsequent tracerefernces
step took 15s. So, locally, this should save 10-15s for devs needing
to enable proguard.
Bug: 371461396
Change-Id: I7de30efd388a294d020cff4460055251369dde23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6221598
Reviewed-by: Sam Maier <smaier@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1414931}
Siso uses script name to partition when commands should be run remotely
vs locally, so this makes it possible.
There is also a lot of errorprone-specific logic, so it's more organized
to have it in a separate script.
Bug: 391114802
Change-Id: Ib1eecfd6dd473e4f721d7c16c9c17270ddb72cbf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6191108
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Junji Watanabe <jwata@google.com>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1410095}
Git isn't available in Cog, and so when searching for grd files, we can't do a `git ls-files -- *.grd`. Previously, we tried walking the repo to find all instances of _.grd_ files. However, that meant that we were listing _.grd_ files in submodules, which we don't want.
Instead, we'll just skip the checks which involve _.grd_ file paths which are added to the repo but not added to the expectations file. Since presubmits run in CQ, this shouldn't result in invalid repo states. It's just a bit inconvenient in that edge case.
Alternatives considered:
- Use the grds present in the expectations files.
- This is possible, but is equivalent to the current implementation. (Effectively, we'd be saying that the expectation file is equivalent to the expectation file.)
- Make an RPC call to simulate the git ls-files result.
- This would require getting the correct credentials, and would make the implementation significantly more complex.
For unit tests, I've added a new test data file for each test case involving Cog. While we could have each of the missing files in one test data file, doing so would enforce the order errors are added to the list of errors. So having a separate check for each is less prone to toil if the implementation is changed.
Change-Id: I5552185b817eb4bf962b23d2d6fad15ad9ddc020
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6177203
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Terrence Reilly <treilly@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Cr-Commit-Position: refs/heads/main@{#1409647}
The Lacros platform is deprecated, and its associated code is being removed. Previously, the PRESUBMIT script was updated to recommend transitioning to the IS_CHROMEOS build flag
However, this change introduced ambiguity, leading to the misinterpretation that IS_CHROMEOS_LACROS should be renamed to IS_CHROMEOS alongside the IS_CHROMEOS_ASH build flag.
This CL refines the PRESUBMIT message to clearly state:
1) Code under IS_CHROMEOS_LACROS must be removed.
2) The IS_CHROMEOS_ASH build flag should be renamed to IS_CHROMEOS.
This update aims to eliminate confusion and provide precise guidance during the
deprecation process.
Bug: 388039763
Change-Id: I0cdd7a788da220ecca17f021a1597d2c41b2c05f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6169131
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Rick Byers <rbyers@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1405919}
This is a refactor of EtwSystemDataSource with no intended behavior
change (aside from a small bug fix or two). The motivation is to make
the ETW consumer easier to test and easier to expand with more
functionality in subsequent CLs.
In particular:
- EtwSystemDataSource::Consumer is now EtwConsumer.
- The ProcessEventRecord implementation now dispatches to per-provider
event handling functions based on the provider guid.
- LostEvent records now emit DLOG messages for diagnostic purposes.
- Perfetto events are only generated when there is valid data to give
them. Previously, an EtwTraceEvent was created for every ETW event
originating from the Thread provider, but only populated for valid
CSwitch events.
- EtwConsumerTest (was EtwSystemDataSource) exercises more of the ETW
event processing code.
Bug: 41497361
Change-Id: I544533aed9c7974f3ee8bc77bf5d3d384a93f9de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6084478
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1394798}
This has been true in practice forever, but for a while it wasn't clear
if we would someday migrate to it (bug 40256238). At this point that's
not plausibly feasible due to the API additions in base::span (split_at,
copy_from, to_fixed_extent, etc.), so clarify the status.
Bug: 40284755
Change-Id: Icc3669dae5e309611bb916088f8229f9ffca5442
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6050662
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1388397}
This adds a generic error message to the Credential Provider Extension
for a few edge cases which are currently unhandled:
* User has disabled "Offer to save passwords"
* User has disabled password sync at the account level
This is a bit of an emergency fix to ensure users don't get into a bad
state in M132. Because we are past branch and will need to attempt to
merge this back, we cannot add new strings (hence the generic text) and
don't have time to prepare a custom icon. We did the best we could with
what was already on hand :)
In M133+, we will remove this error message and replace it with better
ones.
This CL also extends the PRESUBMIT exemption for the use of
systemImageNamed: to other app extensions besides just
search_widget_extension. The same include issues apply to extensions
generally (ref.
https://chromium-review.googlesource.com/c/chromium/src/+/4334347).
Screenshot: go/generic-error-screenshot
Bug: 379247744
Change-Id: I5fc32565ad821c800a2c73667573a62901b23255
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6025258
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Rafał Godlewski <rgod@google.com>
Cr-Commit-Position: refs/heads/main@{#1385818}
Lacros is deprecated. We're in process of removal all code under
_LACROS and and renaming IS_CHROMEOS_ASH to IS_CHROMEOS. Once that's
done, the macros will be removed.
This CL adds a presubmit rule so that no new usages of these macros
are introduced.
Bug: 373971535, 373972275
Change-Id: I6e37aee4a86f15e6d67816cf15e61965d984f38e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6023125
Reviewed-by: Rick Byers <rbyers@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1384027}
I don't believe this has a practical effect right now, but it is safer,
and it will have a practical effect in the future as I'm planning to
modify how the span range constructors work; this will ensure any
constraints that check whether base::span is a borrowed range will give
the correct answer.
This also fixes the oversight that we omitted these from the PRESUBMIT
allowlist; understandably, since they're not listed in any of the
categories on the various cppreference.com ranges pages. I had to do
this to allow this change to pass PRESUBMIT :)
Bug: 364987728
Change-Id: I8ff30b686350797a868bf15b250172aa0c24342f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5962876
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1373610}
The check prompted to add an assert(is_chromeos_ash) if not present.
Since we are going to remove is_chromeos_ash in favor of is_chromeos
(due to Lacros removal), change the check to allow both flags and
change the error message so that it mentions is_chromeos.
Also fix an issue in the tests. They were using mock files of the same
name, so only the first one was actually used.
Bug: 373971535
Change-Id: I0510a23df5fc61ac8d4327df1835f9c84bf64392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5951185
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1371890}
This presubmit check was informing developers when an ESLint config file
changed about manually running a command to ensure no ESLint violations
were introduced, and it was broken after the replacement of per-folder
.eslintrc.js files with a global tools/web_dev_style/eslint.config.mjs
file during the update to ESLint v9.
Porting the check to the new single config world, to print the following
when needed:
tools/web_dev_style/eslint.config.mjs modified. Consider running 'git cl presubmit --files "*.js;*.ts"' in order to check and fix the affected files before landing this change.
Bug: 365027672
Change-Id: I5377140b82973b709854676031d7b58565596a07
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5922516
Reviewed-by: Dirk Pranke <dpranke@google.com>
Auto-Submit: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1366526}
This should resolve the module error on the bot:
```
ModuleNotFoundError: No module named 'six'
```
Also turn off logcat streaming in case that was causing the module
error.
Add .pydeps to ensure deps are included (they should already be
transitive deps of the existing data_deps but it's better to be explicit
and know when generate_profile.py's deps are changed).
Bug: 40272686
Change-Id: If35965e2b2657ab9bfbcfa2513eaefe2d033d9b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5917969
Auto-Submit: Peter Wen <wnwen@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1366368}
This reverts commit 74e5e087cb.
Reason for reland: Fixed blink & webapk presubmit tests
Original change's description:
> Revert "Improve mocking of os_path.exists() in PRESUBMIT_test.py"
>
> This reverts commit b35a9a9465.
>
> Reason for revert: Manual bisect shows that this is responsible for failures starting in https://ci.chromium.org/ui/p/chromium/builders/ci/linux-presubmit/19822/blamelist. Local repro on Linux: `vpython3 chrome/android/webapk/PRESUBMIT_test.py --verbose`
>
> Original change's description:
> > Improve mocking of os_path.exists() in PRESUBMIT_test.py
> >
> > I need this for an upcoming change, but it turns out it found 2 real
> > bugs:
> > * A .pydeps check that would never be hit due to test mocks returning
> > that deleted files exist
> > * A translations check that would never be hit due to test mocks using
> > local paths when absolute paths were requested.
> >
> > Bug: None
> > Change-Id: I920e4f67b2ff1193b89c7abfd9664d14a94c44bf
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5910106
> > Reviewed-by: Ted Choc <tedchoc@chromium.org>
> > Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1364918}
>
> Bug: None
> Change-Id: Ibcc1931e5709d1a4209e56211595df8b94a35c9a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5913697
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
> Owners-Override: Łukasz Anforowicz <lukasza@chromium.org>
> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1364999}
Bug: None
Change-Id: I303d4d1abc685e3bad854e9a5ebe8ae2efe48869
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5912982
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Owners-Override: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1365664}
I need this for an upcomping change, but it turns out it found 2 real
bugs:
* A .pydeps check that would never be hit due to test mocks returning
that deleted files exist
* A translations check that would never be hit due to test mocks using
local paths when absolute paths were requested.
Bug: None
Change-Id: I920e4f67b2ff1193b89c7abfd9664d14a94c44bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5910106
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1364918}
On Fuchsia and ARM32, this regresses binary size by a small amount
(~30k), while on ARM64 it's a large win (~175k). Per agrieve@, this
is a win overall and this is OK to land.
That said, I did try to both investigate and reduce binary size
impact. On earlier patchsets, I tried changing inlining, especially
for callsites with more than a few arguments. This made things worse
rather than better. In examining a few assembly differences via
SuperSize, I believe the cause of the regression is the switch from
varargs to pack-of-const-refs causing variables to be forced to the
stack at a few callsites. This might be ameliorated with perfect
forwarding absl::StrFormat() and friends, but such a change is
invasive and nontrivial, and I'm not certain it will turn out to
help, so it doesn't seem worth pursuing for now.
Bug: 1371963
Binary-Size: See commit description.
Fuchsia-Binary-Size: See commit description.
Change-Id: I7d543b716387afc4461d2c2fa07ff3670a2ffa58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4907781
Reviewed-by: Mirko Bonadei <mbonadei@chromium.org>
Owners-Override: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Danil Chapovalov <danilchap@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1356497}
