0
Commit Graph

276 Commits

Author SHA1 Message Date
Thiago Perrotta
099034f664 Chrome for Testing: Update in-app strings from Chromium to CfT
Add <if> conditionals to chromium_strings.grd to replace the most
prominent strings in the UI:

- Global menu bar
- 3-dot menu
- View site information
- Settings > Default browser
- Settings > About

As a trade-off, we choose this approach over creating
a new GRD file for CfT as it would be costly to maintain otherwise,
in spite of the considerations in b/1110882.

CfT strings should be identical to Chromium strings, with the exception
of the product name, which is replaced from "Chromium" with "Chromium
for Testing" as per
https://goo.gle/chrome-for-testing#bookmark=id.n1rat320av91

Bug: 1428061
Change-Id: I7cb5afe0d619551daea3204b9611854d36f5af98
Tested: `autoninja -C out/Default chrome/app:chromium_strings components/strings:components_chromium_strings chrome`
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4582551
Reviewed-by: Jens Mueller <muellerj@google.com>
Commit-Queue: Thiago Perrotta <tperrotta@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1153347}
2023-06-05 18:10:20 +00:00
Jens Mueller
054652c1cb Add presubmit check for correct sha1 format in .png.sha1 files.
Benmason@ explained that a recent submission of a png file as png.sha1
file resulted in all images on that day not to be uploaded to TC.

2nd attempt after https://crrev.com/c/4374003 was reverted due to
"Evaluation of CheckStrings failed: Access outside the repository root is denied."

This is now using input_api.AffectedFiles().NewContents instead of
opening the file directly.

Bug: 1428712
Change-Id: Ia1bcb5b132f5d92ce7378a986be74b9958175680
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4432832
Reviewed-by: Ben Mason <benmason@chromium.org>
Reviewed-by: Dominic Battre <battre@chromium.org>
Commit-Queue: Ben Mason <benmason@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1142068}
2023-05-10 15:12:30 +00:00
Vincent Boisselle
861f11db51 Revert "Add presubmit check for correct sha1 format in .png.sha1 files."
This reverts commit 069d82c9d8.

Reason for revert: cause failure in strings presubmit tests when updating grd strings, example: http://crrev.com/c/4377359

Original change's description:
> Add presubmit check for correct sha1 format in .png.sha1 files.
>
> Benmason@ explained that a recent submission of a png file as png.sha1
> file resulted in all images on that day not to be uploaded to TC.
>
> Change-Id: Iea7632de1ae4fd644bb4bce7f1fc907c3254459b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4374003
> Reviewed-by: Dominic Battré <battre@chromium.org>
> Reviewed-by: Ben Mason <benmason@chromium.org>
> Commit-Queue: Jens Mueller <muellerj@google.com>
> Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1123014}

Change-Id: Icc0c43bbd82ed16d6370b139242f2f632449a8ee
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1428712
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4378561
Reviewed-by: Ernesto Izquierdo Clua <eic@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Vincent Boisselle <vincb@google.com>
Commit-Queue: Vincent Boisselle <vincb@google.com>
Cr-Commit-Position: refs/heads/main@{#1123295}
2023-03-28 21:46:38 +00:00
Mohamed Heikal
3d7a94ce2b Migrate android.support.test to androidx.test (the rest)
The java changes should be a noop since we automatically do this rewrite
in compile_java.py. Also replace
//third_party/android_support_test_runner targets with
//third_party/androidx ones.
Additionally fix non-java files eg: docs, presubmits, scripts

This affects files outside //android_webview, //chrome, //components and
//weblayer which are covered by other cls.

Bug: 1428304
Change-Id: I2906b7530127983e6110e9ab5773843df0dd3ba8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4375857
Reviewed-by: Peter Wen <wnwen@chromium.org>
Owners-Override: Peter Wen <wnwen@chromium.org>
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1123104}
2023-03-28 16:55:24 +00:00
Jens Mueller
069d82c9d8 Add presubmit check for correct sha1 format in .png.sha1 files.
Benmason@ explained that a recent submission of a png file as png.sha1
file resulted in all images on that day not to be uploaded to TC.

Change-Id: Iea7632de1ae4fd644bb4bce7f1fc907c3254459b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4374003
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Ben Mason <benmason@chromium.org>
Commit-Queue: Jens Mueller <muellerj@google.com>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1123014}
2023-03-28 14:42:02 +00:00
Mike Dougherty
4d1050b713 Update iOS JavaScript presubmit to support moved files
The presubmit currently prevents moving a JavaScript file to a new
location. This CL allows such CLs by using a warning instead of an error
if a script of the same name was deleted and added in the examined diff.

Change-Id: I0c5761b5f774bea8a6733e78c2ef0a31d47eab9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4335695
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1117004}
2023-03-14 15:59:53 +00:00
Min Qin
bc44383cb2 Add PRESUBMIT rule for ban VectorDrawableCompat import in Java
VectorDrawableCompat Could take a long time to complete, so adding
a ban to let users use the alternative class with trace events.

BUG=b/242038130

Change-Id: I174c918e7dd5fef15a551eace33d1bdfb3d203e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4266131
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108408}
2023-02-22 17:25:26 +00:00
Kevin McNee
1746306427 Remove MPArch triage rotation presubmit
We are winding down the triage rotation, so the associated presubmit
script can be removed.

Bug: 1190019
Change-Id: If6226bae54a7bfc71df95211d6c00f12db10dd6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4228276
Auto-Submit: Kevin McNee <mcnee@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1103922}
2023-02-10 17:49:18 +00:00
Sven Zheng
76a79ea364 PRESUBMIT: Add crosapi need browser test check
When a new crosapi mojom file being added, we expect there should
also be a browser test being added.

This is a non-blocker check and only happens during git cl upload.
See my local test:

:~/chromium/src$ git diff HEAD~1
diff --git a/chromeos/crosapi/mojom/newfile.mojom b/chromeos/crosapi/mojom/newfile.mojom
new file mode 100644
index 0000000000000..fa1237e46e92b
--- /dev/null
+++ b/chromeos/crosapi/mojom/newfile.mojom
@@ -0,0 +1,4 @@
+// Copyright 2022 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
:~/chromium/src$ git cl upload
Running Python 3 presubmit upload checks ...
** Presubmit Warnings: 1 **
You are adding a new crosapi, but there is no file ends with browsertest.cc file being added. It is important to add crosapi browser test coverage to avoid version skew issues.
Check //docs/lacros/test_instructions.md for more information.

Presubmit checks took 2.5s to calculate.
There were Python 3 presubmit warnings. Are you sure you wish to continue? (y/N):

Bug: 1402823
Change-Id: Ibcac5f90fb7332cf7199e04a8eab48fda3ac5eb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4118248
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Sven Zheng <svenzheng@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1086084}
2022-12-21 21:25:24 +00:00
Bruce Dawson
55776c49a7 Improve messages when .grd entries are modified
It can be helpful to distinguish between modified and added .grd
entries. This came up recently when trying to fix the indentation in a
.grd file and these more specific messages _might_ have helped.

Typical messages will look like this:
** Presubmit ERRORS: 1 **
You are modifying UI strings or their meanings.
To ensure the best translations, take screenshots of the relevant UI (https://g.co/chrome/translation) and add these files to your changelist:
  chromeos\chromeos_strings_grd\IDS_SCANNING_APP_SCAN_BUTTON_TEXT.png.sha1

Actually fixing the indentation will be tried in a follow-up.

Bug: 1394970
Change-Id: I60fee3086a9826ef2f233c681164e5b309a6eec6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4081196
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1081514}
2022-12-09 17:23:47 +00:00
Yuanqing Zhu
9eef0283e2 Fix CheckNoAbbreviationInPngFileName checks path components.
Modify CheckNoAbbreviationInPngFileName presubmit rule and add unittest for it to ensure it only checks the file name, not every path component ahead.

Change-Id: I8743dcec9966c1b8ecdb6dd8d8fe6836dbed1793
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4054771
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Yuanqing Zhu <yuanqingzhu@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1079007}
2022-12-04 14:42:17 +00:00
Clement Yan
9b330cb2fa [WebUI] Add presubmit banrule to prevent chrome.send
Prevent usage of chrome.send in ChromeOS WebUIs

Initially only preventing for ChromeOS WebUIs, we will look at expanding this to all of Chrome in the future.

Context:
https://chromium.googlesource.com/chromium/src/+/refs/heads/main/docs/security/handling-messages-from-web-content.md

Bug: 1382986
Change-Id: Ic12d43876613a6f2e514d9b1573cc5f75fe224c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4011838
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Clement Yan <clamclamyan@google.com>
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1072640}
2022-11-17 05:25:29 +00:00
Daniel Cheng
192683f4c4 Ban std random number engines in PRESUBMIT.py.
Also fix an incorrect pattern for absl::any / std::any and update the
guidance for absl::bind_front.

Bug: 1380528
Change-Id: I7641d2032297ff70e04de9578a0ebc292fa807d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3995164
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1066129}
2022-11-01 20:52:44 +00:00
Sean Kau
cb7c9b3ead Add PRESUBMIT warning for rebase_path and target_gen_dir
Provide a warning when rebase_path specifies target_gen_dir and an
absolute path.  This can cause build failures when building Chromium
with Portage because that can keep the out/ directory outside of the
source root.  Advise that root_build_dir is the right way to specify
the path.

Bug: 1376581
Change-Id: I8ef0a9c32358e086abfffa53b61c4c4189cee185
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3964725
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Sean Kau <skau@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1063502}
2022-10-25 21:25:52 +00:00
Mike Dougherty
1b8be711c3 Reland "Add presubmit check to prevent new JavaScript in iOS directories"
This is a reland of commit 6168e0b65d

Original change's description:
> Add presubmit check to prevent new JavaScript in iOS directories
>
> `//ios/*` now fully supports TypeScript which is preferred over
> JavaScript for new feature scripts.
>
> In addition to a PRESUBMIT error, the boilerplate script was updated to
> help developers avoid creating new JS files. Without this check,
> developers may not be aware of the restriction until their feature is
> implemented is complete locally.
>
> Bug: 1325547
> Change-Id: I40a9cc9f701f5396d14c7b87648ebff747bb6f47
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3939161
> Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1060096}

Bug: 1325547
Change-Id: I0aa048972377afa434350803079049169a0dcd82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3964730
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1061334}
2022-10-20 00:15:13 +00:00
Bruce Dawson
2972d99f73 Revert "Add presubmit check to prevent new JavaScript in iOS directories"
This reverts commit 6168e0b65d.

Reason for revert: The presubmit is buggy, and triggers presubmit errors on existing files. See crbug.com/1376103

Original change's description:
> Add presubmit check to prevent new JavaScript in iOS directories
>
> `//ios/*` now fully supports TypeScript which is preferred over
> JavaScript for new feature scripts.
>
> In addition to a PRESUBMIT error, the boilerplate script was updated to
> help developers avoid creating new JS files. Without this check,
> developers may not be aware of the restriction until their feature is
> implemented is complete locally.
>
> Bug: 1325547
> Change-Id: I40a9cc9f701f5396d14c7b87648ebff747bb6f47
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3939161
> Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1060096}

Bug: 1325547, 1376103
Change-Id: I343d5e13c3ab4fb58fc30acc257257541c15ec2c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3964250
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1060521}
2022-10-18 16:54:09 +00:00
Mike Dougherty
6168e0b65d Add presubmit check to prevent new JavaScript in iOS directories
`//ios/*` now fully supports TypeScript which is preferred over
JavaScript for new feature scripts.

In addition to a PRESUBMIT error, the boilerplate script was updated to
help developers avoid creating new JS files. Without this check,
developers may not be aware of the restriction until their feature is
implemented is complete locally.

Bug: 1325547
Change-Id: I40a9cc9f701f5396d14c7b87648ebff747bb6f47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3939161
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1060096}
2022-10-17 19:36:06 +00:00
Sam Maier
4cef92469c Adding presubmit warning for Mockito.mock footgun
Bug: 1336346
Change-Id: I0e5649d3c868649f0d22c3b59c65fc63bd0b97f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3927801
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Sam Maier <smaier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054152}
2022-10-03 14:21:24 +00:00
Allen Bauer
8477868eb7 Add presubmit to warn when Layout() is called within a test.
View::Layout() is getting an overhaul. Direct calls to Layout() from within tests and eventually production code will be forbidden. There are new test-only functions available to manually invoke a layout pass properly.

This presubmit will attempt catch direct calls to Layout() from within files ending in unittest, browsertest, or ui_test. It should still allow Layout() overrides. While this presubmit should not trigger for existing calls to Layout(), those calls will have been eliminated in other CLs.

To manually test this presubmit:

> git cl presubmit --force --files path_to/some/test/foo_unittest.cc

This was tested with and without the Layout() call and was confirmed to trigger when the line of code with the Layout() call was added or changed.

Change-Id: Ib784203e7f795cc4a437508d4c37aecf8f90d6fd
Bug: 1350521
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3899633
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1050232}
2022-09-22 16:28:56 +00:00
Bruce Dawson
95eb756a46 Make CheckForIncludeGuards run ~1.5x faster
This function was previously optimized (70x faster?) by not calling
f.OldContents() so often, but the correct call count is actually zero.
The correct way to tell whether a file is new in the presubmit system
is f.Action() == 'A'. The three possible actions are 'A'dded,
'M'odified, and 'D'eleted.

Note that there is no guarantee that the mocked files will be
internally consistent which is why I had to adjust the tests. Also
note that a test that the comment says was supposed to fail has been
passing for years. This change does not address that.

Bug: 1309977
Change-Id: I846b0403c1e196daf620dda8a02b4de82d8bf463
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3895053
Reviewed-by: Mike Pinkerton <pinkerton@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1046909}
2022-09-14 15:27:16 +00:00
Avi Drissman
2497659363 Update copyright headers in the top-level files
This CL was generated manually.

No-Try: true
Bug: 1098010
Change-Id: I23f14f4c85edc1c6498fb40d2f415031398324b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3887619
Auto-Submit: Avi Drissman <avi@chromium.org>
Owners-Override: Avi Drissman <avi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1045836}
2022-09-12 15:24:31 +00:00
Gao Sheng
a79ebd4d91 Fix some issues in PRESUBMIT script
When I read the Chromium source, I first noticed that some python files for presubmit located in the root directory had some problems (like incorrect function names, indentation, typos, etc.), so I tried to fix them.

BUG=nobug

Change-Id: I3646b9fc1761d3cd2af884aba8d815d09665e7be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3812981
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Erik Staab <estaab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032601}
2022-08-08 17:25:59 +00:00
Kevin McNee
29c0e82381 Reduce monitored methods in the MPArch triage rotation
From an analysis of the last six months of the MPArch triage
rotation [1], we now have a better idea of which methods tend to
require intervention. We now reduce the scope of the rotation to
those methods to reduce the review burden of the rotation.

[1] https://docs.google.com/document/d/1pQcjhXqdh_JcWucqFmI5r8TdKUHxh5qgIAguc3GHRvM/edit?usp=sharing&resourcekey=0-dlmVoj6MqryQxG5gJAK51A

Bug: 1190019
Change-Id: I19aa087040f1a98f09eb1e2ab16582bc8ecfa277
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3806539
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031929}
2022-08-05 15:36:09 +00:00
Evan Stade
7cd4a2c15c Add presubmit check to prevent adding trademarked icons to repo
So far it only applies to png, svg and icon files, and only complains
about ones with "google" in the name (although this could fail to
catch some cases like `gmail.icon`).

Bug: 1245662
Change-Id: I5e4fa3520f4532aeab6965100ae7097788e13e85
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3808278
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031665}
2022-08-04 23:37:25 +00:00
ckitagawa
e8fd23b274 [Fixit] Warn for Batch Annotations on not instumentation tests
Batch annotations don't do anything for non-instrumentation tests.
Add a warning to remove them if added unnecessarily.

Example where this was caught manually:
https://chromium-review.googlesource.com/c/chromium/src/+/3704237

It is easy enough to extend the existing CheckBatchAnnotation
PRESUBMIT to handle this.

Bug: 1337314
Change-Id: I4861f3825329f6313b2f06b9a101ee5c4e26c031
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708730
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1015361}
2022-06-17 15:29:38 +00:00
James Shen
81cc0e2b5e Add PRESUBMIT for android/javatests to encourage @Batch.
If tests are not safe to run in a batch, it should explicitly have
@DoNotBatch annotation with reasons.

Bug: 1330328
Change-Id: If94c5c3de521b5248813bc98b4de0740b43429e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3699290
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: James Shen <zhiyuans@google.com>
Cr-Commit-Position: refs/heads/main@{#1014636}
2022-06-15 21:10:45 +00:00
Aleksey Khoroshilov
9b28c039ae Restore CheckNoStrCatRedefines check and speed up it a little.
CheckNoStrCatRedefines was not used since its inception. This CL enables
the check and adds header/impl filters to make it run faster.

Bug: 1330868
Change-Id: Ie9b57474675a57cb6d6b5fddbf1945c8081c9888
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3679759
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1010614}
2022-06-03 16:35:32 +00:00
Kalvin Lee
4a3b79de47 PRESUBMIT.py: Adjust DCHECK_IS_ON() regex
This CL attempts to limit the `DCHECK_IS_ON()` parentheses presubmit to
cases where exactly that macro is being used. Outside that scope,
`FOO_DCHECK_IS_ON` is permissible, as is `BUILDFLAG(FOO_DCHECK_IS_ON)`.

Change-Id: I15923ea342dc0e76714400ceb2947d5ac0235312
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3669918
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Kalvin Lee <kdlee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1007856}
2022-05-26 16:00:16 +00:00
Daniel Cheng
171dad8de9 Trigger missing security reviewers check only on OWNERS file additions
Since the original bug is primarily about the case of missed reviews
when an OWNERS file is newly modified, only trigger it when new per-file
security review restrictions are added to OWNERS files.

This still adds a bit of annoyance for initial changes in directories
where //chromeos/LACROS_OWNERS or //chromeos/SECURITY_OWNERS should also
be able to security review, but it becomes a one-time signoff instead.
This seems like an acceptable tradeoff between ease of use and not
missing security reviews.

Bug: 1327341
Change-Id: Ia0263c9b5b185322a83f92cd0fc552bfa1196841
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3657265
Reviewed-by: Alex Gough <ajgo@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1006055}
2022-05-21 00:40:25 +00:00
Daniel Cheng
681bc127a9 Relax missing security reviewer warning to a notify at upload.
Originally, this was selected to match the warning level of other
security owners checks. However:

- The other security owners check has been upgraded into an error at all
  times, to prevent less last-minute surprises from missing OWNERS.
- Having a warning that prompts at upload time is annoying when
  repeatedly uploading CLs for something that's still heavily in
  revision.

Bug: 801315
Change-Id: I7fb2ebc3e1f491fb4274ddb2a966fa041c419e9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3655008
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1005079}
2022-05-19 02:23:44 +00:00
Daniel Cheng
d8824447b1 Fix security owners check to be compatible with presubmit --files/--all
The security owners check only issues a blocking error now if there is
a Gerrit issue attached to the change: it is not possible (through the
normal tooling anyway) to directly commit a change without first
uploading it to Gerrit, so no safety is lost there. In addition, the
security owners check now respects Owners-Override as well.

Testing this fix also revealed an issue where the security review
exceptions for Blink metrics enums did not work quite correctly, since
the same directory contained both mojoms that required security review
and mojoms that did not.

To fix this, move all the mojoms that do not require a security review
into their own subdirectory and merge the PRESUBMIT scripts into one,
updating the PRESUBMIT to the latest style.

Moving the enums also required updating numerous paths throughout
Chrome. It also slightly changes the output of the enum validation
script, so regenerate enums.xml to reflect the new paths. This also
updates the shebang line in the enum generation scripts to use
python3 explicitly instead of just python (which is often not present
on gLinux machines, or is mapped to python2).

Bug: 801315
Change-Id: If1809126a0103e295b7c350a8996401c766a2931
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3648594
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Owners-Override: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1003666}
2022-05-16 09:08:47 +00:00
Daniel Cheng
3008dc1af7 Don't require approval during CQ+1 for security owners check.
CQ+1 runs the presubmit commit hooks, but it is not useful for the
missing security reviewers check to require approval, since there may be
reviewers that have yet to respond. Only CQ+2 should generate an error
for a missing approval.

Bug: 801315
Change-Id: I76ed35d515987a1efb9a8a2344210bc601a22849
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3646825
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002990}
2022-05-13 04:02:11 +00:00
Daniel Cheng
a37c03dbe3 Ensure newly-added security-sensitive files are security reviewed.
The current security owners check ensures that the per-file rules are
present in the various OWNERS files; however, for security reasons [1],
the actual owners checks evaluates owners requirements against the
*currently* checked-in version of OWNERS files and not the updated
version of OWNERS files being reviewed. The net result is that it was
a fairly common occurrence to add a new .mojom file, add new per-file
restrictions to OWNERS, but not actually send it to a security reviewer.

It turns out that there is already logic to check for security reviewers
for changes that include calls to security-sensitive functions. This
logic has been factored out so it can be reused by the per-file security
owners checker. In addition:

- The required per-file rules checker has been refactored so that IPC
  and Fuchsia can re-use the same core logic.
- ipc-security-reviews@chromium.org is no longer CCed on changes that
  are security-sensitive for Fuchsia but not IPC.
- Test logic consolidation and cleanup.

[1] If the owners requirements were evaluated against the updated
version of OWNERS files being reviewed, it would be possible for someone
to add a new OWNER and for that newly-added OWNER to approve the CL
adding themselves.

Bug: 801315
Change-Id: If824e4698368a089474ba992601edbcf4c7d009c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3641150
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002697}
2022-05-12 17:20:34 +00:00
Bruce Dawson
32114b6efd Simplify missing-include-guard messages
The messages for missing include guards are quite verbose, listing the
file name and recommended include guard twice each. This is because they
set message, items, and long_text. When there is just a single warning
this is okay, but when there is a long list of warnings it makes it
harder to read them. The old output looks like this:

  Missing include guard CONTENT_SHELL_APP_RESOURCE_H_
    content\shell\app\resource.h

  ***************
  Missing include guard in content\shell\app\resource.h
  Recommended name: CONTENT_SHELL_APP_RESOURCE_H_
  This check can be disabled by having the string
  no-include-guard-because-multiply-included in the header.
  ***************

The new warning - just setting message - looks like this:

  Missing include guard in content\shell\app\resource.h
  Recommended name: CONTENT_SHELL_APP_RESOURCE_H_
  This check can be disabled by having the string
  no-include-guard-because-multiply-included in the header.

All of the information is included, in less than half the space. The
presubmit tests were also updated as needed.

Bug: 1309977
Change-Id: Ia8c2de3ff3f1e71df6afc260c5edc1b430f80a17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3579758
Reviewed-by: Mike Pinkerton <pinkerton@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/main@{#991060}
2022-04-11 16:45:49 +00:00
Daniel Cheng
8bf998982a Trim down functionality in mojo::AssociatedInterfacePtrInfo.
At this point, this is just a helper class for conveying info when
binding and unbinding an associated remote endpoint, so remove all other
functionality.

Bug: 875030, 955171
Change-Id: I7711b97deb306536fe4951f5a575ef1231c2ebec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3527063
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#982544}
2022-03-18 01:26:24 +00:00
Daniel Cheng
92c15e3c88 Block usage of handle<shared_buffer> in Chrome mojom files.
Chrome Mojo interfaces should prefer one of the higher-level primitives
built on top of the native Mojo handle<shared_buffer>:

- mojo_base.mojom.ReadOnlySharedMemoryRegion
- mojo_base.mojom.WritableSharedMemoryRegion
- mojo_base.mojom.UnsafeSharedMemoryRegion

Fixed: 1132241
Change-Id: I082d512594db6cedb9146928e40f7387a492bbe2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3526187
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#981728}
2022-03-16 17:48:22 +00:00
Mark Schillaci
6f568a5281 Fix small typo bug in PRESUBMIT, and update accessibility rebase script
This CL fixes a small typo in the PRESUBMIT script that was creating a
false positive warning during certain presubmit checks. We also make
a small change to the accessibility rebase script to make it more
python3 friendly.

AX-Relnotes: N/A
Bug: N/A
Change-Id: I51af1814a5d781dcbcf42609ca1e2f9965545ba4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3469668
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Cr-Commit-Position: refs/heads/main@{#972551}
2022-02-17 18:41:44 +00:00
Kevin McNee
4eeec7981f Include explanatory text when adding the MPArch watchlist during PRESUBMIT
We now have a good idea of the average volume of CLs affected by this
PRESUBMIT (0 to 9 per day, typically around 4). It seems like we could
add some guidance here without being too spammy.

Bug: 1190019
Change-Id: Iaea9f969c6af7b651fef88290ca1012dabbcd2d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3457088
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/main@{#970772}
2022-02-14 20:02:04 +00:00
Andrew Grieve
4deedb119c Reland "PRESUBMIT.py: Make CheckPydepsNeedsUpdating faster using concurrency"
This reverts commit cadd78c55c.

Reason for reland: Removed py3 syntax

Original change's description:
> Revert "PRESUBMIT.py: Make CheckPydepsNeedsUpdating faster using concurrency"
>
> This reverts commit 09650bc010.
>
> Reason for revert: Running as py2 for some.
>
> Original change's description:
> > PRESUBMIT.py: Make CheckPydepsNeedsUpdating faster using concurrency
> >
> > For DEPS rolls, all .pydeps files are checked, which can take ~30
> > seconds when done sequentially. Check them all concurrently, which on my
> > machine runs in < 2 seconds.
> >
> > Bug: None
> > Change-Id: I621a04456d8c18b0522b586c8eea4befe0e9674f
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3433606
> > Reviewed-by: Erik Staab <estaab@chromium.org>
> > Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#966727}
>
> Bug: None
> Change-Id: Ic5d902ba6d42312ff9fde8430eec23e47e0977ed
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3437351
> Auto-Submit: Andrew Grieve <agrieve@chromium.org>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#966840}

Bug: None
Change-Id: I1cab290df83d411d6d06a37c3b4563b608df38ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3437577
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Erik Staab <estaab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#966930}
2022-02-03 21:34:50 +00:00
Andrew Grieve
cadd78c55c Revert "PRESUBMIT.py: Make CheckPydepsNeedsUpdating faster using concurrency"
This reverts commit 09650bc010.

Reason for revert: Running as py2 for some.

Original change's description:
> PRESUBMIT.py: Make CheckPydepsNeedsUpdating faster using concurrency
>
> For DEPS rolls, all .pydeps files are checked, which can take ~30
> seconds when done sequentially. Check them all concurrently, which on my
> machine runs in < 2 seconds.
>
> Bug: None
> Change-Id: I621a04456d8c18b0522b586c8eea4befe0e9674f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3433606
> Reviewed-by: Erik Staab <estaab@chromium.org>
> Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#966727}

Bug: None
Change-Id: Ic5d902ba6d42312ff9fde8430eec23e47e0977ed
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3437351
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#966840}
2022-02-03 19:11:31 +00:00
Andrew Grieve
09650bc010 PRESUBMIT.py: Make CheckPydepsNeedsUpdating faster using concurrency
For DEPS rolls, all .pydeps files are checked, which can take ~30
seconds when done sequentially. Check them all concurrently, which on my
machine runs in < 2 seconds.

Bug: None
Change-Id: I621a04456d8c18b0522b586c8eea4befe0e9674f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3433606
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#966727}
2022-02-03 14:58:27 +00:00
Andrew Grieve
3f9b9667ae PRESUBMIT.py: Re-enable checks for stale pydeps files
This slipped in during a perious where PRESUBMIT_tests.py were not being
run due to py3 migration :/.

Bug: 1289871
Change-Id: I834d4b37db8e969a3aae571c0ec8004f00029a9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3433046
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Erik Staab <estaab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#966310}
2022-02-02 19:07:55 +00:00
Shimi Zhang
975c0c2607 Disable 4 PRESUBMIT_test cases
- testGnPathsAndMissingOutputFlag
- testRelevantAndroidPyInNonAndroidCheckout
- testRelevantPyOneChange
- testRelevantPyTwoChanges

Found they are failing when working on a PRESUMBIT.py change.

Verified that they are also failing on main branch. So disabling them.

Bug: 1289871
Change-Id: I37ff038b42cf5e86080a01f8e37276f8d1fb18c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3408275
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#962147}
2022-01-21 23:45:56 +00:00
Xiaohan Wang
cef8b0015a presubmit: Fix linux-only test check
For features only implemented on linux, the corresponding test should
only be run on Linux. But the current check does exactly the opposite.
This CL adds "not" to fix the issue.

Previously there are 6 tests failing on Windows, now they are passing.

Bug: 1234043
Change-Id: I62f1362e2f6d2315637451396fe5194a83fb1f1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3405026
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#961595}
2022-01-20 21:34:30 +00:00
Xiaohan Wang
42d96c2901 build: Add presubmit for deprecated OS defines
This CL updates presubmit defines for build config macros to make it
clear OS defines macros are deprecated.

Here's an example output:
```
OS macros have been deprecated. Please use BUILDFLAGs instead:
    media\base\media.cc:39: defined(OS_WIN) -> BUILDFLAG(IS_WIN) \
    media\base\media.cc:51: defined(OS_WIN) -> BUILDFLAG(IS_WIN)
```

R=thakis@chromium.org

Bug: 1234043
Test: Manually tested
Change-Id: Iae6201e98bfe518e451a19ddf7ff12588066ad7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3402062
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#961504}
2022-01-20 17:23:11 +00:00
Mark Schillaci
e5a0be2079 Port old tests to new "missing layer" Android accessibility suite
This continues the work on the "missing layer" test fixes for the
Android accessibility code.

With this CL we remove some previously existing tests from the
WebContentsAccessibilityTest file and instead include them in the
corresponding Event* and Tree* test suites. These tests are easier to
understand and maintain, and this makes the original
WebContentsAccessibilityTest file contain only tests that cannot be
done with the static dump tests.

We also include new PRESUBMIT checks to give helpful hints to other
developers so we can more easily maintain consistency across platforms.

AX-Relnotes: N/A
Bug: 1258230
Change-Id: I5dd19ef407afba10a8b711cf55458c84860dc4f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3390858
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Cr-Commit-Position: refs/heads/main@{#960717}
2022-01-19 00:38:39 +00:00
Giovanni Ortuño Urquidi
ab84da6d01 Fix CheckAssertAshOnlyCode presubmit
Follow up for https://crrev.com/c/3325066 that adds a test to check
deleted files are ignored and sets include_deletes to true in
the mock AffectedFiles function to match the real function.

Bug: 1278154
Change-Id: I713f462b4741f8c9d5d13e4ed576788e0de57b95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3325628
Commit-Queue: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Auto-Submit: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#950362}
2021-12-10 00:53:21 +00:00
Takuto Ikuta
3697651942 PRESUBMIT: fix CheckPythonShebang for deletion diff
This is fix for
https://ci.chromium.org/ui/p/chromium/builders/try/chromium_presubmit/1473703/overview

Bug: 1191100
Change-Id: I6ccf95b1a293a5f9889e4acc956b31c1ede9df27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3307145
Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#946738}
2021-11-30 23:15:27 +00:00
Henrique Ferreiro
f9819f2e35 Use #!/usr/bin/env python/2/3 in remaining .py files
Replace every shebang like /usr/bin/python[23] with /usr/bin/env
python[23] as appropriate in Chromium and Blink, except for WPT. Also,
add a presubmit check to disallow the former going forward.

build/print_python_deps.py is changed from using python2.7 to python2.

remoting/ python scripts are left out as they are meant to be run in
users' machines.

Bug: 1191100
Change-Id: If7ebf7bd8ad9b2695a471e0a403a592815a0d959
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537832
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/main@{#946427}
2021-11-30 13:31:56 +00:00
Lukasz Anforowicz
7016d05e80 Presubmit checks that reject raw_ptr<T> in Renderer-only code.
Bug: 1073933
Change-Id: I2ab8ce4c0f73d763a784e4252ed2704a2a9f087a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3296265
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/main@{#946313}
2021-11-30 03:56:27 +00:00