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}
`BanRule.explanation` should be a tuple of `str`s, but some `BanRule`s
passes `str` itself as an explanation. It leads to super long warning
message of one-line one-char.
This CL fixes these errors by adding one extra comma comma inside
parentheses.
Fixed: 362216911
Change-Id: I3f480fe72e7617a939911c81fefc2aba44730ca6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5812005
Commit-Queue: Mikihito Matsuura <mikt@google.com>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1347168}
Create a finer-grained method for exempting code from unsafe buffer
warnings that the per-file pragma. Its use will enable more files
to be opted in to these checks, and prevent re-exempting entire files
as more UNSAFE_BUFFER_USAGE functions are tagged.
-- Insist that UNSAFE_BUFFERS() requires security review.
-- Add a presubmit check to block usage.
Change-Id: Icb599340c38fd5fae5f739d7c25c6e025dbf5707
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5767792
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1339193}
The obsolete js2gtest testing framework heavily depends on the v4
version of Chai, since it is incompatible with the latest v5 version,
which only exposes chai as an ES module. Several CrOS tets still use
js2gtest (for example c/b/r/chromeos/accessibility/ among others).
Checking in the Chai v4 version at [1], so that the rest of the
non-js2gtest WebUI tests can be updated to the latest v5 version, in
a follow-up CL.
[1] chrome/test/data/webui/chromeos/chai_v4.js
Bug: 347991521
Change-Id: I7717fa95327191d75c24efb100d697ea5b08e5aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5759162
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Cr-Commit-Position: refs/heads/main@{#1338629}
A Perl+R variant of this has long been used out-of-tree; implement
it all in C++ and in the tree, so that it is more generally useful.
A future tool using the same class will interpret output logs from
blink_perf_tests, which has been using a similar tool.
Change-Id: I849869028f8d55a921cada3cf31bdd6fd7ee0688
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5688930
Reviewed-by: Rick Byers <rbyers@chromium.org>
Commit-Queue: Steinar H Gunderson <sesse@chromium.org>
Reviewed-by: John Chen <johnchen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1338603}
This is meant to run as a standalone binary and intentionally has
minimal dependencies on the rest of Chromium, so we'd prefer not to
pull in all the logging infrastructure. Since it doesn't impact end
users, just disable the presubmit.
Change-Id: Ie17eafe34225a0f1d8774469393afe6833dc2ef2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5751022
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1335794}
Currently the tests are running inside of media lab rather
than chrome lab. An ongoing effort is to migrate away from
media lab,
so the plan is to re-implement the tests in regular luci
swarming.
The detailed implementation is still ongoing, indeed the
hardware setup is also ongoing. But I need to have the
builder up and running for further hardware setup.
The test case itself will be used in a chrome internal
builder [1].
[1]: https://crrev.com/i/7508414
Bug: 40935291
Change-Id: I6da72d25655c08cc4a246e5c573720631a433688
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5727141
Commit-Queue: Zijie He <zijiehe@google.com>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1331198}
* Add ios/chrome/common where code is prevented from including the
symbol_helper.h since ios/chrome/common/DEPS does not allow depending
on ios/chrome/browser/... where symbol_helpers.h is located.
* Update the path to symbol_helpers.h in the presubmit message.
Bug: 339887700, b/338625793
Change-Id: I84a79bd830944ae07a6c22ad406b7e5571292656
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5683109
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Slobodan Pejic <slobodan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1326902}
Add a range-based constructor, tagged with `base::from_range_t` as a
backport of the C++23 `std::from_range_t`.
Add a range-based assign, named assign_range(), matching std vector and
string in C++23.
Then mark the iterator-pair-based constructor and assign() as
UNSAFE_BUFFER_USAGE.
Change VectorBuffer (the internal class) to stop allowing dereference of
the pointer one past the allocation as this is UB, and CHECK for this
instead. Then stop using derefences in circular_deque that are used to
construct pointers (as we were doing this with the end pointer too and
thus CHECKing). Use pointer arithmetic instead which is valid to
construct the end pointer without invoking UB.
Write SAFETY comments and UNSAFE_BUFFERS() on the pointer arithmetic
inside circular_deque. Move to using spans of the VectorBuffer instead
in most cases.
Make the iterators check for moving out of bounds and dereferencing
end(). Stop going through a pointer to the deque to do iterator
operations to make things easier on the optimizer. However this means
we must reconstruct iterators internally when we change the begin_ or
end_ of the circular_deque, so some minor reordering was needed
internally. This doesn't affect external users as their iterators
would be considered invalidated when begin_ or end_ move anyways.
The binary size regressions are entirely due to the PGO profile being
incorrect after this change: https://crbug.com/344608183#comment7
Bug: 40284755
Fuchsia-Binary-Size: Regression is a temporary PGO artifact
Binary-Size: Regression is a temporary PGO artifact
Change-Id: I20e74d02901387d75aa611ea1e7e98a699a6265a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5655271
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1325442}
Cronet generates its own javadoc internally in google3 through a
different set of scripts, generating through google3 provides two better
functionality over doing it in Chromium:
1. APIs in google3 are ensured to have been well-tested after a series
of rollouts.
2. The current setup in Chromium is brittle as it requires to have
source files for all of the API, if some auto-generated file is
introduced through srcjar, it blows up.
Change-Id: I54e317e9f0672b88d22ad29526634ee17be9f7e7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5667544
Reviewed-by: Chidera Olibie <colibie@google.com>
Commit-Queue: Chidera Olibie <colibie@google.com>
Auto-Submit: Mohannad Farrag <aymanm@google.com>
Reviewed-by: Peter Wen <wnwen@chromium.org>
Reviewed-by: Etienne Dechamps <edechamps@google.com>
Reviewed-by: Stefano Duo <stefanoduo@google.com>
Cr-Commit-Position: refs/heads/main@{#1322664}
Skipping the presubmit, because ironically enough, PRESUBMIT.py and
PRESUBMIT_test.py trigger a number of pre-existing presubmit violations.
No-Presubmit: true
Change-Id: I9aabeb8496a0582d6cb92af90db4ef3b32f70b04
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5668998
Reviewed-by: Steinar H Gunderson <sesse@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1321358}
When a new DEP is added, the presubmit check enforces reviews from
the owner of the additional DEP. However, when the DEP is from another
repo, the enforcement becomes trickier especially it is from a different
host.
This patch disables the check if a newly added DEP is from a submodule.
Change-Id: Id7c2feac005c1c769313dedb254d5a0b641ff68b
Bug: 324496840
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5656359
Reviewed-by: Josip Sokcevic <sokcevic@chromium.org>
Commit-Queue: Scott Lee <ddoman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1320079}
The config provider is responsible for fetching tokens and proxy list on
demand for the network service. An instance will be initialized in AwBrowserContext::ConfigureNetworkParams and bound to the network service using Mojo interfaces. Majority of the logic and unittests come from the preexisting chrome/browser/ip_protection_config_provider*
Bug: b:335420700
Change-Id: Ic704fcdc1d966a1006a8787f1e117353e447fdff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5549298
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Dustin Mitchell <djmitche@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ciara McMullin <ciaramcmullin@google.com>
Reviewed-by: Matthew Wang <matthewmwang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1315907}
We intend to exclude only a handful specific sub-directories (for perf
reasons). This CL removes the broad exclusions from the rewriter and
the enforcing clang plugin.
There is a risk that by the time the plugin rolls into Chromium,
someone will add a raw pointer, which will cause the plugin
to error out and block the roll-out. Therefore, add those broad
exclusions into BUILD.gn temporarily -- they'll be removed after
the plugin rolls, once we confirm that all pointers in those
directories are properly rewritten.
In other words, this CL doesn't change the clang plugin enforcement
behavior.
Bug: 40064499, 331846123
Change-Id: Ia95e8df846d167521a5ac43cc63e556002702ee9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5600329
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1312344}
The explanation parameter is an iterable. The previous explanations were
written as ("explanation") which becomes a string iterable, one
character per line. This should instead be ("explanation",), which is a
one-tuple.
Change-Id: Ided5a74938300dd3eab73bdeec9eb5cc1e756459
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5605485
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1312186}
