* The current translation screenshot presubmit check flags messages
which got modified but for which no new translation screenshot
got uploaded. It thereby also flags messages that got modified, but of
which the message content itself did not change. This is the case, for
example, if only the message description for translators got changed.
* This CL restricts the translation screenshot presubmit flagging
modified messages. It will trigger 1) if the message did not have a
screenshot at all so far, or 2) if the message content itself changed.
Only changing the non-visible part of a message that already has a
translation screenshot does not trigger it anymore.
Bug: 1103844
Change-Id: I254ce2a7267a7d2308b606b16d080c74188d582b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2328669
Commit-Queue: Rainhard Findling <rainhard@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797648}
Re-design of the CL (crrev.com/c/2341362) to enable presubmit_support
to time/ResultDB wrap each individual lower-level check in PRESUBMIT.py.
The approach taken here is to have the PRESUBMIT_VERSION = 2.0.0 in here
so that presubmit_support is aware of the new version of PRESUBMIT.py,
then check input_api.version in CheckChangeOnCommit/Upload to ensure
that presubmit_support itself is the new version.
The associated changes to depot_tools (crrev.com/c/2349979) must be
landed in order for this to work.
Change-Id: I91d09337dd77a74540ac4a01017275bcaaa67cde
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2350234
Commit-Queue: Saagar Sanghavi <saagarsanghavi@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Cr-Commit-Position: refs/heads/master@{#797291}
This reverts commit 065782712e.
Reason for revert: Getting reports of the version check not working from various users; see crbug.com/1115140
Original change's description:
> Rewrote PRESUBMIT.py
>
> Rewrote PRESUBMIT.py (and associated tests) to new format. Runs
> REQUIRE_PRESUBMIT_VERSION at the top so that depot_tools can be
> made aware of the change and can time/rdb wrap each of the Check...
> functions separately, while also ensuring that they have the new
> version of depot_tools.
>
> Requires the newer version of presubmit_support.py in depot_tools,
> which is described in crrev.com/c/2341828
>
> Change-Id: I102e8c75834aff1eccdf4a27ffe6f0953b76ee57
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2341362
> Reviewed-by: Dirk Pranke <dpranke@google.com>
> Commit-Queue: Saagar Sanghavi <saagarsanghavi@google.com>
> Cr-Commit-Position: refs/heads/master@{#796595}
TBR=dpranke@google.com,estaab@chromium.org,saagarsanghavi@google.com
Change-Id: Ib3295d141e997fc6792c381fea10e8892204bcdb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2349428
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Cr-Commit-Position: refs/heads/master@{#796867}
Rewrote PRESUBMIT.py (and associated tests) to new format. Runs
REQUIRE_PRESUBMIT_VERSION at the top so that depot_tools can be
made aware of the change and can time/rdb wrap each of the Check...
functions separately, while also ensuring that they have the new
version of depot_tools.
Requires the newer version of presubmit_support.py in depot_tools,
which is described in crrev.com/c/2341828
Change-Id: I102e8c75834aff1eccdf4a27ffe6f0953b76ee57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2341362
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Saagar Sanghavi <saagarsanghavi@google.com>
Cr-Commit-Position: refs/heads/master@{#796595}
In the spirit of macOS Big Sur, which is labeled as macOS 11.0,
introduce OS_MAC and OS_APPLE. OS_MACOSX implicitly included OS_IOS,
which was confusing, so OS_APPLE is the new replacement for "macOS +
iOS" and OS_MAC is the new replacement for "just macOS, not iOS".
Bug: 1105907
Change-Id: I0f24ff5a74f07eaf2fe7b7fa17bdef7e82a514fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2299189
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791353}
This is a follow up to https://crrev.com/c/2274667
This attribute was recently introduced so that developers wouldn't
get presubmit errors for accessibility labels that are not shown in the
UI. However, screen readers do actually display most accessibility
labels, so having screenshots for these strings are still useful. Still,
there is a small subset of accessibility labels that are never
displayed. This CL renames the existing attribute to make the intention
of the carveout clear.
Bug: 1094077
Change-Id: I4c171d29fea0aac611cb07759422bd5c25688017
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2285076
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#786025}
This CL converts remaining StrongBinding's uses to
MakeSelfOwnedReceiver. Besides that this CL removes
all unnecessary strong_binding.h includes.
Additionally, PRESUMIT scripts start warning the use of
StrongBinding.
Bug: 955171
Change-Id: I3336fc31b7c7f390c00d4dfdd87dca404cce5e1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2281790
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#785614}
Presubmit currently warns when a UI string is missing translation
screenshots. We'll now require all strings to have associated
screenshots. This CL changes the warnings to errors.
Bug: 1087641
Change-Id: I0968dee994e304bd9c7c59bab56d5f1b0325d5d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2283261
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785613}
We require translation screenshots for strings in grd and grdp files.
It's been noted that some of these strings are accessibility labels, and
they aren't displayed in Chrome UI.
This CL introduces a new tag to mark strings that are used as
accessibility labels (is_accessibility=true). If a UI message has this
tag, it'll be excluded from translation screenshot checks in the
presubmit.
Bug: 1094077
Change-Id: I204d068264ff0d0305d1d8afd48de516f9a26ac1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2274667
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784078}
We cannot check if android specific pydeps files are stale in a
non-android checkout because some imported libraries will be missing.
However, we can output a warning letting the developer know so as to
reduce the chances of CLs landing that make android pydeps stale.
Change-Id: Icd1047d4350f6d3cb439d09ea2ba958b51afe61f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2240260
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#778857}
All services now receive custom sandboxes from a service_sandbox_type.h
specialization of GetServiceSandboxType, or use kUtility, so this
API can be removed.
The presubmit check for WithSandboxType has been modified to spot
new or changed instances of GetServiceSandboxType.
Bug: 1065087
Change-Id: I7199fe160292afa5d1b5e35dd7dd4bee5fb82e1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2219392
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#778366}
The presubmit step for checking mojom backward-compatibility does not
work properly when a mojom file is deleted. This is due to incorrect use
of the presubmit input API, effectively not providing the checking tool
with any information about deleted files.
This fixes that.
Fixed: 1091407
Change-Id: I4cd3d7bb5ce07a7d75e8bac99319c75ca6d28118
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2231266
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#775248}
Mojom types marked [Stable] must preserve backward-compatibility over
time. This presubmit tool checks for unsafe changes to any such types.
Fixed: 1070663
Change-Id: Ie1d3ca30d608dd95e256c782593c9df910512e96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2219020
Commit-Queue: Ken Rockot <rockot@google.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773276}
* This CL introduces a Chromium top-level string ICU syntax presubmit
check. It validates the ICU syntax of any added or modified strings
in .grd or .grdp files, and points out any found syntax errors. This
is done to prevent subsequent, time consuming translation extraction
blockages caused by ICU syntax errors (see crbug.com/1061546 for an
example).
* This CL integrates both the string ICU syntax an the string screenshot
presubmit check into a single check for execution time purposes. Both
check need to loop through all added and modified strings, hence
combining both in one check minimizes the execution time overhead.
* More details: go/chromium-string-icu-syntax-check
Bug: 1081730
Change-Id: I9bde972e3e382362cab07f64af8e5d93dccd26f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2199256
Commit-Queue: Rainhard Findling <rainhard@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769216}
Historically SECURITY_OWNERS enforcement has been done by filename
matching. But increasingly security configuration is done in ad-hoc
spots that do not conform to per-file organization. This adds a new
presubmit check that matches CL contents against patterns to determine
if it has security-critical changes, and requires OWNERS enforcement
if so.
The first and only pattern matched so far is the WithSandboxType()
function used to configure the sandbox policy for utility processes
launched via ServiceProcessHost.
Bug: 1077000
Change-Id: Ib4a6b7aa9e83306a106086141a99197f641ea539
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2180446
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766202}
Require that //fuchsia/SECURITY_OWNERS own all:
- Fuchsia IDL (aka FIDL) files defining IPC protocols with other Fuchsia
components.
- Component manifest definitions (CMX & CML), which specify the services
and component framework features that each component relies upon.
PRESUBMIT.py's _CheckSecurityOwners() function is modified to process the
results of both cross-platform and Fuchsia-specific IPC ownership checks.
Bug: 1053551
Change-Id: I3755297c1be395040e0553f22a87cf95526658ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135614
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Auto-Submit: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#763825}
Relax a regex to catch Log.e("log message", ..) as a bad/missing TAG
instead of silently accepting it. Also add a rough TAG-change detection
regex to try and make changes that only touch a TAG also run the checks.
Adjust tests to match.
Bug: 1063573
Change-Id: I6a5c302d581aa76ebfc86a286e6048412641f4de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2113572
Auto-Submit: Tomasz Śniatowski <tsniatowski@vewd.com>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752441}
Background (taken from crbug/583195):
translation_expectations.pyl contains the list of translatable grd
files and the languages they're expected to be translated into. It's
used internally during the translation process to determine which grd
files should be copied to google3 for translation, etc. Each
repository with grd files (desktop, Android, iOS) has a
translation_expectations.pyl file, which must list every grd file that
contains strings.
translation_expectations.pyl isn't used at all when building (neither
locally nor on the bots), so there's no indication to developers if
translation_expectations.pyl needs to be updated (e.g. because a grd
file was added) or is malformed. Instead, an error will occur much
later when the weekly translation run happens, causing unnecessary
back-and-forth between TPMs and developers and introducing delays.
This happens every few weeks.
This CL adds a presubmit that checks if the contents of the
translation_expectations.pyl matches the list of .grd files in the
repository. The presubmit only runs if a .grd or .grdp file is
modified, so in most cases it's a no-op. It lists the names of missing
and unlisted files in the warning.
Bug: 583195
Change-Id: I5013311dac5db1f0578f59022832dc8e33c1ee37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1464078
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748339}
This change adds a warning for new uses of GetInterfaceProvider() and
suggests using GetBrowserInterfaceBroker() instead.
Bug: 718652
Change-Id: I2e99ba36a8cdb198200967d27a64931a32adcdc7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1967759
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726462}
The three migrations proposed in this CL are the only instances left
inside //content after the massive migration to the new types, so let's
migrate these ones too and then update the PRESUBMIT script so that any
attempt to re-introduce old types in this part of the code will raise
an error, instead of a warning.
TBR=kenrb@chromium.org
Bug: 955171
Change-Id: Ib006c869576d122289781d065629db957cd9f347
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972847
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#726303}
Following the work started in CL1796371, this change changes the logic
of the PRESUBMIT.py script in order to raise ERRORS from now on when
deprecated mojo types are introduced in Blink, and raise WARNINGS when
those old types are introduced everywhere else but //components/arc,
which hasn't been migrated yet (see crrev.com/c/1868870).
We didn't do this before to prevent too many "false positives" when
moving code around, but since all of Blink has been migrated now and
nearly everything outside of Blink (but //components/arc) is nearly
finished, this seems like a good moment to flip the switch.
See the "Mojo Bindings Conversion CheatSheet" for more information:
https://docs.google.com/document/d/1Jwfbzbe8ozaoilhqj5mAPYbYGpgZCen_XAAAdwmyP1E
Bug: 955171
Change-Id: I1f7b62ff6841935a43c462dc266d3f3aa632aa80
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1961908
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Cr-Commit-Position: refs/heads/master@{#724971}
This is a reland of 2308d0744f
Original change's description:
> Support grdp files in translation screenshot presubmit
>
> Translation screenshots project requires Chrome devs to upload UI
> screenshots along with string changes in .grd and .grdp files. These
> are XML files containing all user visible strings in Chrome.
>
> Translation screenshots has a presubmit checks that extracts the
> list of modified strings, looks for image files associated with these
> strings and warns the developer to add them if they are missing.
>
> This presubmit parses .grds to extract the list of modified strings but
> it skips <part> tags which are used to reference .grdp files. As a
> result, the presubmit currently ignores string changes in .grdp files.
> .grdp files contain the majority of UI strings in Chrome, so the lack of
> presubmit checks for them results in low translation screenshot coverage.
>
> This CL changes how .grdp files are loaded in the presubmit: Previously,
> they were written in a temporary directory alongside a fake .grd file
> that referenced the loaded .grdp file. The code loaded the fake .grd file
> which resulted in loading the strings in the actual .grdp file. However,
> test mocks override all os.path methods which in turn breaks temporary
> directory creation. As a result, it was not possible to properly test
> the .grdp loading code, so we skipped <part> tags as a workaround.
>
> The new loading code writes a temporary copy of the .grdp file and wraps
> its contents with proper tags so that it can be loaded as a .grd file
> instead. This avoids the need to create a temporary directory and is
> fully testable.
>
> The end result is that Translation Screenshots presubmit will now ask
> Chrome devs to upload screenshots for changes in .grdp files. This should
> significantly increase the coverage of strings with translation screenshots,
> leading to better quality in string localizations.
>
> Bug: 924652
> Change-Id: Iae7933e8147cefdabc15ef21d2b6daa43d5002bc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875554
> Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#714550}
Bug: 924652
Change-Id: Ide7559496fb006724cbed9daa67840c1883a55d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1912835
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#716224}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1953701
Cr-Commit-Position: refs/heads/master@{#723468}
This reverts commit 12e7fee2b9.
Reason for revert: crbug/1025982
Original change's description:
> Reland "Support grdp files in translation screenshot presubmit"
>
> This is a reland of 2308d0744f
>
> Original change's description:
> > Support grdp files in translation screenshot presubmit
> >
> > Translation screenshots project requires Chrome devs to upload UI
> > screenshots along with string changes in .grd and .grdp files. These
> > are XML files containing all user visible strings in Chrome.
> >
> > Translation screenshots has a presubmit checks that extracts the
> > list of modified strings, looks for image files associated with these
> > strings and warns the developer to add them if they are missing.
> >
> > This presubmit parses .grds to extract the list of modified strings but
> > it skips <part> tags which are used to reference .grdp files. As a
> > result, the presubmit currently ignores string changes in .grdp files.
> > .grdp files contain the majority of UI strings in Chrome, so the lack of
> > presubmit checks for them results in low translation screenshot coverage.
> >
> > This CL changes how .grdp files are loaded in the presubmit: Previously,
> > they were written in a temporary directory alongside a fake .grd file
> > that referenced the loaded .grdp file. The code loaded the fake .grd file
> > which resulted in loading the strings in the actual .grdp file. However,
> > test mocks override all os.path methods which in turn breaks temporary
> > directory creation. As a result, it was not possible to properly test
> > the .grdp loading code, so we skipped <part> tags as a workaround.
> >
> > The new loading code writes a temporary copy of the .grdp file and wraps
> > its contents with proper tags so that it can be loaded as a .grd file
> > instead. This avoids the need to create a temporary directory and is
> > fully testable.
> >
> > The end result is that Translation Screenshots presubmit will now ask
> > Chrome devs to upload screenshots for changes in .grdp files. This should
> > significantly increase the coverage of strings with translation screenshots,
> > leading to better quality in string localizations.
> >
> > Bug: 924652
> > Change-Id: Iae7933e8147cefdabc15ef21d2b6daa43d5002bc
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875554
> > Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> > Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> > Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#714550}
>
> Bug: 924652
> Change-Id: Iaf0f68f8abdccd3bca66b4b611bf53d8337d347b
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1912835
> Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#716224}
TBR=meacer@chromium.org,anthonyvd@chromium.org,jochen@chromium.org
Change-Id: Id5265d9bffe86e3179784325eff357f9ac758967
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 924652
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1923155
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716498}
This is a reland of 2308d0744f
Original change's description:
> Support grdp files in translation screenshot presubmit
>
> Translation screenshots project requires Chrome devs to upload UI
> screenshots along with string changes in .grd and .grdp files. These
> are XML files containing all user visible strings in Chrome.
>
> Translation screenshots has a presubmit checks that extracts the
> list of modified strings, looks for image files associated with these
> strings and warns the developer to add them if they are missing.
>
> This presubmit parses .grds to extract the list of modified strings but
> it skips <part> tags which are used to reference .grdp files. As a
> result, the presubmit currently ignores string changes in .grdp files.
> .grdp files contain the majority of UI strings in Chrome, so the lack of
> presubmit checks for them results in low translation screenshot coverage.
>
> This CL changes how .grdp files are loaded in the presubmit: Previously,
> they were written in a temporary directory alongside a fake .grd file
> that referenced the loaded .grdp file. The code loaded the fake .grd file
> which resulted in loading the strings in the actual .grdp file. However,
> test mocks override all os.path methods which in turn breaks temporary
> directory creation. As a result, it was not possible to properly test
> the .grdp loading code, so we skipped <part> tags as a workaround.
>
> The new loading code writes a temporary copy of the .grdp file and wraps
> its contents with proper tags so that it can be loaded as a .grd file
> instead. This avoids the need to create a temporary directory and is
> fully testable.
>
> The end result is that Translation Screenshots presubmit will now ask
> Chrome devs to upload screenshots for changes in .grdp files. This should
> significantly increase the coverage of strings with translation screenshots,
> leading to better quality in string localizations.
>
> Bug: 924652
> Change-Id: Iae7933e8147cefdabc15ef21d2b6daa43d5002bc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875554
> Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#714550}
Bug: 924652
Change-Id: Iaf0f68f8abdccd3bca66b4b611bf53d8337d347b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1912835
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716224}
This reverts commit 2308d0744f.
Reason for revert: This broke `git cl upload` with string changes.
Original change's description:
> Support grdp files in translation screenshot presubmit
>
> Translation screenshots project requires Chrome devs to upload UI
> screenshots along with string changes in .grd and .grdp files. These
> are XML files containing all user visible strings in Chrome.
>
> Translation screenshots has a presubmit checks that extracts the
> list of modified strings, looks for image files associated with these
> strings and warns the developer to add them if they are missing.
>
> This presubmit parses .grds to extract the list of modified strings but
> it skips <part> tags which are used to reference .grdp files. As a
> result, the presubmit currently ignores string changes in .grdp files.
> .grdp files contain the majority of UI strings in Chrome, so the lack of
> presubmit checks for them results in low translation screenshot coverage.
>
> This CL changes how .grdp files are loaded in the presubmit: Previously,
> they were written in a temporary directory alongside a fake .grd file
> that referenced the loaded .grdp file. The code loaded the fake .grd file
> which resulted in loading the strings in the actual .grdp file. However,
> test mocks override all os.path methods which in turn breaks temporary
> directory creation. As a result, it was not possible to properly test
> the .grdp loading code, so we skipped <part> tags as a workaround.
>
> The new loading code writes a temporary copy of the .grdp file and wraps
> its contents with proper tags so that it can be loaded as a .grd file
> instead. This avoids the need to create a temporary directory and is
> fully testable.
>
> The end result is that Translation Screenshots presubmit will now ask
> Chrome devs to upload screenshots for changes in .grdp files. This should
> significantly increase the coverage of strings with translation screenshots,
> leading to better quality in string localizations.
>
> Bug: 924652
> Change-Id: Iae7933e8147cefdabc15ef21d2b6daa43d5002bc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875554
> Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#714550}
TBR=meacer@chromium.org,anthonyvd@chromium.org,jochen@chromium.org
Change-Id: I1a6ee198b1dd76e84bcf962cd2396f4d00c5c126
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 924652
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1913564
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714712}
Translation screenshots project requires Chrome devs to upload UI
screenshots along with string changes in .grd and .grdp files. These
are XML files containing all user visible strings in Chrome.
Translation screenshots has a presubmit checks that extracts the
list of modified strings, looks for image files associated with these
strings and warns the developer to add them if they are missing.
This presubmit parses .grds to extract the list of modified strings but
it skips <part> tags which are used to reference .grdp files. As a
result, the presubmit currently ignores string changes in .grdp files.
.grdp files contain the majority of UI strings in Chrome, so the lack of
presubmit checks for them results in low translation screenshot coverage.
This CL changes how .grdp files are loaded in the presubmit: Previously,
they were written in a temporary directory alongside a fake .grd file
that referenced the loaded .grdp file. The code loaded the fake .grd file
which resulted in loading the strings in the actual .grdp file. However,
test mocks override all os.path methods which in turn breaks temporary
directory creation. As a result, it was not possible to properly test
the .grdp loading code, so we skipped <part> tags as a workaround.
The new loading code writes a temporary copy of the .grdp file and wraps
its contents with proper tags so that it can be loaded as a .grd file
instead. This avoids the need to create a temporary directory and is
fully testable.
The end result is that Translation Screenshots presubmit will now ask
Chrome devs to upload screenshots for changes in .grdp files. This should
significantly increase the coverage of strings with translation screenshots,
leading to better quality in string localizations.
Bug: 924652
Change-Id: Iae7933e8147cefdabc15ef21d2b6daa43d5002bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1875554
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714550}
This cl adds a presubmit warning on upload if the CL contains images.
The warning points the author to
//docs/speed/binary_size/optimization_advice.md#optimizing-images so as
to ensure they have read the optimization tips and tried applying them
to their images. There is no easy way to check if the image is optimized
thus the presubmit always fires if there are images but does not block
CQ.
Bug: 921227
Change-Id: Ib8dd144bd2f728b166ca5b1c9d3882df24e0216b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899645
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714507}
As part of the Split Settings Sync project Chrome OS settings have a
new sync controls UI. This requires that new prefs be registered with
the type SYNCABLE_OS_PREF instead of SYNCABLE_PREF.
Add a presubmit warning if new code in directories known to be
Chrome OS specific register a pref with the browser registration type.
This is a warning and not an error because it's theoretically possible
to add a browser setting that only affects Chrome OS and hence appears
in a Chrome OS-only directory. There are no such prefs today.
Bug: 1019988
Test: added to PRESUBMIT_test.py
Change-Id: I59b46ac831855ae71d3eb5811b54cabf5ec35995
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1901646
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713186}
Type casting related functions like IsHTMLXXXXElement,
ToHTMLXXXXElement, and ToHTMLXXXXElementOrNull are being phased out in
favor of new downcast helpers in t_p/blink/renderer/platform/casting.h.
There is still a long run to fully remove old type casting mechanism,
but this CL aims at preventing new uses to take place.
Bug: 891908
Change-Id: I6a079b9524d2e94805528bdcf17baaa66fe091da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879628
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Abhijeet Kandalkar <abhijeet@igalia.com>
Cr-Commit-Position: refs/heads/master@{#710345}
Remove reference to a former engineer (the linked doc is sufficient) and
broaden the regex for recognising TAG declarations to also find cases
where the field is not private, as a number of these exist in the code.
Change-Id: If5b8c4e542b06881fc073ffcb1849d9ee70acd24
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1873362
Auto-Submit: Richard Coles <torne@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708359}
The presubmit for translation screenshots asks the user to remove screenshots
associated with any strings they removed. This CL checks if the screenshot
exists before asking the user to remove it.
Bug: 924795
Change-Id: I811fe4f3d14ae5448b3dbf87aa0fc0055ec09ce8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834277
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702089}
This change adds a presubmit check preventing new usages of the
following old Mojo types, which are now considered deprecated in
favor of the ones outlined in crbug.com/955171:
- mojo::AssociatedBinding<>
- mojo::AssociatedBindingSet<>
- mojo::AssociatedInterfacePtr<>
- mojo::AssociatedInterfacePtrInfo<>
- mojo::AssociatedInterfaceRequest<>
- mojo::Binding<>
- mojo::BindingSet<>
- mojo::InterfacePtr<>
- mojo::InterfacePtrInfo<>
- mojo::InterfaceRequest<>
- mojo::MakeStrongAssociatedBinding()
- mojo::MakeStrongBinding()
- mojo::MakeRequest()
- mojo::MakeRequestAssociatedWithDedicatedPipe()
- mojo::StrongAssociatedBindingSet<>
- mojo::StrongBindingSet<>
For now, we will only raise warnings during the presubmit stage
whenever any of these types get introduced in //third_party/blink,
but the plan is to progressively cover more directories and convert
such warnings into errors as soon as more areas get fully migrated
to the new types.
See the "Mojo Bindings Conversion CheatSheet" for more information:
https://docs.google.com/document/d/1Jwfbzbe8ozaoilhqj5mAPYbYGpgZCen_XAAAdwmyP1E
Bug: 955171
Change-Id: I0d40b1507d85cbc11397f9c2fb02fb96a7490911
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796371
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697555}
The header (testing/libfuzzer/libfuzzer_exports.h) should be included in fuzz
targets they define optional libFuzzer symbols, as otherwise those symbols may
be stripped by the linker on Mac and fuzz target may work incorrectly, see
https://crbug.com/687076 for more information.
Bug: 991345
Change-Id: I459774f2bc8de708d67c0af455031146592c4799
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1738756
Commit-Queue: Max Moroz <mmoroz@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Abhishek Arya <inferno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685342}
This macro is meant for GTest unit tests. Using this in EarlGrey tests
will cause legitimate OCMock failed expectations to not reported at the
end of a test run.
Bug: 983652
Change-Id: If6bda23eb9fd85a4c271b82d0ef0dd1153ca2655
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1700527
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#677319}
This change adds a check preventing new usages of mojo::ConvertTo since
TypeConverter is deprecated. The check excludes /third_party/blink and
content/renderer paths for now.
Bug: 621383
Change-Id: I63b36171447863cdd9ee0c42f973c2a8480ee4a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1549453
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#660438}
An #include "foo.cc" will often compile, until it causes linking
problems later, for instance in jumbo builds. The error messages are
then not good at pointing out what is wrong.
Change-Id: Ic155dc5542e22b54d13c213230d6df4977263642
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566344
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#653105}
Since userdebug and eng builds are debuggable build of Android.
BuildInfo.isDebugAndroid() is preferred over `Build.TYPE.equals()` or
''.equals(Build.TYPE). The CL adds a presubmit check for that.
Bug: 939924
Change-Id: Id2794374f2185c1f0f50abf9424a87b07467567e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1564363
Commit-Queue: Jinsong Fan <fanjinsong@sogou-inc.com>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#651269}
