With nested batch files it is very common for /? arguments to end up
being interpreted as a request for help on the batch file "call"
command. That is, if a user invokes "goma_ctl /?" then this turns into
something like this:
@call ...\python3.bat %~dp0\.cipd_bin\goma_ctl.py %*
The name of the script to be invoked is ignored and this is treated
like:
@call /?
This is particularly problematic for commands like goma_ctl and
autoninja which don't print help if no arguments are passed, thus
making finding the correct help incantation slightly challenging.
This special-cases /? in these two batch files. Some other common batch
files were tested and found to be not affected, so while this is not a
complete fix it does hit some of the most important places.
Change-Id: Ic9b0455c2f8b085666862bc4495d9bad445dfeaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4080991
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This improves output from ninja a bit on Windows when
NINJA_SUMMARIZE_BUILD is set.
The trailing space was removed in https://crrev.com/c/3400399.
Change-Id: Ifb14129a436a2673969f529d0c9143118dd87d6b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3833272
Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
This reverts commit ba352e00ba.
Reason for revert: Unfortunately this breaks this command when run
from a normal cmd.exe prompt:
autoninja ..\..\base\win\access_token.cc^^
The escaping of ^^ as it passes through batch files is very finicky, but important.
Original change's description:
> Update autoninja.bat to work with other shells.
>
> Other shells, such as TCC, have subtle differences in how batch files are processed. This will not break cmd.exe compatibility.
>
> Without the "call" command, running a nested batch file should, according to CMD.exe "rules", cancel the current batch file and run the new one. However, the 'FOR /f "usebackq"' command will call the batch file and return. One could argue for both approaches, however, adding the "call" command doesn't seem to affect cmd.exe and fixes the other shell.
>
> There is a similar issue with the "do" (in the FOR command) block and the use of the '&' separator. The '%a' variable isn't expanded, but adding the '()' block works in both instances.
>
> Bug: 1351817
> Change-Id: Idd685a64404ea950b71e1f3cc0f5d1e3bf13b8b6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3824199
> Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Bug: 1351817
Change-Id: I9d204a3369c468664c0cff247ed1a53065dfebfd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3829729
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Other shells, such as TCC, have subtle differences in how batch files are processed. This will not break cmd.exe compatibility.
Without the "call" command, running a nested batch file should, according to CMD.exe "rules", cancel the current batch file and run the new one. However, the 'FOR /f "usebackq"' command will call the batch file and return. One could argue for both approaches, however, adding the "call" command doesn't seem to affect cmd.exe and fixes the other shell.
There is a similar issue with the "do" (in the FOR command) block and the use of the '&' separator. The '%a' variable isn't expanded, but adding the '()' block works in both instances.
Bug: 1351817
Change-Id: Idd685a64404ea950b71e1f3cc0f5d1e3bf13b8b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3824199
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Fixed goma_auth/ctl return code on Windows which was always zero.
Fixed other places where exit /b was used without %errorlevel%.
Change-Id: Ib0a647818c595f843a95762db023b111a447a66a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3532153
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Auto-Submit: Aleksey Khoroshilov <akhoroshilov@brave.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
Though the Microsoft docs recommend using rem to write comments, it has
a shortcoming - redirection characters (< or >) or pipes (|) aren't
allowed. However, :: though undocumented, is a better alternative as it
allows such characters.
Signed-off-by: Darshan Sen <raisinten@gmail.com>
Change-Id: I95a0445168527ab5087246238f0216d5f6177046
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400399
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Commit-Queue: Marc-Antoine Ruel <maruel@chromium.org>
This changes autoninja.bat so that it refers explicitly to the
depot_tools version of Python rather than depending on what is in the
path. This helps to make autoninja behave more consistently and makes it
easier for us to remove python.bat from the path.
Bug: 777069
Change-Id: I6d878f58e2f8be70d06a182d059caa9fbedaedbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2753458
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This reverts commit 428143ee24.
Reason for revert: Fixing the issues revealed by the original change by
avoiding python3 and by checking for the existence of gomacc[.exe]
before running it.
This also relands 2241db8a1f - "Avoid
capture_output to support Python 3.6", to simplify relanding and any
possible reverts.
Original change's description:
> Revert "Check whether goma is running when it is enabled"
>
> This reverts commit b7ddc5a009.
>
> Reason for revert:
> This broke the builder where depot_tools is not in PATH.
> https://logs.chromium.org/logs/infra-internal/buildbucket/cr-buildbucket.appspot.com/8858077852309878080/+/u/build/stdout
>
> Original change's description:
> > Check whether goma is running when it is enabled
> >
> > One of the mistakes one can make when running ninja is having goma
> > enabled (use_goma=true in args.gn) but not having goma running. This can
> > lead to ~1,000 failed compile steps, which is messy.
> >
> > This change teaches autoninja.py to check whether goma is running. If
> > not then it tells autoninja to just print a warning message. The
> > check costs roughly 30 ms which seems reasonable.
> >
> > In fact, because this change also switches away from vpython (necessary
> > to use python3 to use subprocess.run) it actually runs about 600 ms
> > _faster_ than before this change.
> >
> > If build acceleration is requested through use_rbe then no checking for
> > whether the service is running is done. That could be added in the
> > future.
> >
> > autoninja.py could auto-start goma but that is error prone and has
> > limited additional value.
> >
> > This was tested on Linux, OSX, and Windows.
> >
> > Bug: 868590, b/174673874
> > Change-Id: Ie773e574878471e5136b9b82d52f86af3d848318
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2627014
> > Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> > Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
>
> TBR=yyanagisawa@google.com,dpranke@google.com,brucedawson@chromium.org,sanfin@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com
>
> Change-Id: I57a6c73ea853259f3d1ec7ad0ce51e495acc96db
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 868590
> Bug: b/174673874
> Bug: 1167064
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2632018
> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
> Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@google.com>
TBR=yyanagisawa@google.com,dpranke@google.com,brucedawson@chromium.org,sanfin@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com
# Not skipping CQ checks because this is a reland.
Bug: 868590
Bug: b/174673874
Bug: 1167064
Change-Id: I8aa6830259bc18f8e7926cd0bf5c62e671c74a2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2634201
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Reviewed-by: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This reverts commit b7ddc5a009.
Reason for revert:
This broke the builder where depot_tools is not in PATH.
https://logs.chromium.org/logs/infra-internal/buildbucket/cr-buildbucket.appspot.com/8858077852309878080/+/u/build/stdout
Original change's description:
> Check whether goma is running when it is enabled
>
> One of the mistakes one can make when running ninja is having goma
> enabled (use_goma=true in args.gn) but not having goma running. This can
> lead to ~1,000 failed compile steps, which is messy.
>
> This change teaches autoninja.py to check whether goma is running. If
> not then it tells autoninja to just print a warning message. The
> check costs roughly 30 ms which seems reasonable.
>
> In fact, because this change also switches away from vpython (necessary
> to use python3 to use subprocess.run) it actually runs about 600 ms
> _faster_ than before this change.
>
> If build acceleration is requested through use_rbe then no checking for
> whether the service is running is done. That could be added in the
> future.
>
> autoninja.py could auto-start goma but that is error prone and has
> limited additional value.
>
> This was tested on Linux, OSX, and Windows.
>
> Bug: 868590, b/174673874
> Change-Id: Ie773e574878471e5136b9b82d52f86af3d848318
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2627014
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
TBR=yyanagisawa@google.com,dpranke@google.com,brucedawson@chromium.org,sanfin@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: I57a6c73ea853259f3d1ec7ad0ce51e495acc96db
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 868590
Bug: b/174673874
Bug: 1167064
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2632018
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@google.com>
One of the mistakes one can make when running ninja is having goma
enabled (use_goma=true in args.gn) but not having goma running. This can
lead to ~1,000 failed compile steps, which is messy.
This change teaches autoninja.py to check whether goma is running. If
not then it tells autoninja to just print a warning message. The
check costs roughly 30 ms which seems reasonable.
In fact, because this change also switches away from vpython (necessary
to use python3 to use subprocess.run) it actually runs about 600 ms
_faster_ than before this change.
If build acceleration is requested through use_rbe then no checking for
whether the service is running is done. That could be added in the
future.
autoninja.py could auto-start goma but that is error prone and has
limited additional value.
This was tested on Linux, OSX, and Windows.
Bug: 868590, b/174673874
Change-Id: Ie773e574878471e5136b9b82d52f86af3d848318
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2627014
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@google.com>
In addition to setting GOMA_DISABLED, this CL also sets the
RBE_remote_disabled environment variable to autoninja.
Bug: 1149386
Change-Id: I9aa8be005a9cd473b7371eadcab83d6068e0e070
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2565550
Reviewed-by: Dirk Pranke <dpranke@google.com>
Commit-Queue: Michael Savigny <msavigny@google.com>
It is sometimes helpful to do a full build of Chrome using goma while
online and then do incremental builds without goma while offline or with
poor network bandwidth. Turning off goma in gn args triggers a full
rebuild but the GOMA_DISABLED environment variable can be used to
disable goma without triggering a rebuild.
In order to make this feature easier to discover and use this change
adds support for -o/--offline to autoninja. autoninja -h will mention
this flag and autoninja.bat/autoninja.py handle it appropriately. This
means setting the environment variable in autoninja.bat, and using it
to adjust the -j value and stripping it out in autoninja.py.
The bash script that wraps autoninja.py on Linux and OSX did not need
updating because the Python script can emit 'GOMA_DISABLED=1 ninja ...'
and this is executed by bash.
This was produced during pair programming with jessemckenna@
Bug: b/172039612
Change-Id: Ifcfbc598ac20f23e5fe013e02979b5b8c2851b01
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2510818
Auto-Submit: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@google.com>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Also make autoninja.py a vpython script to use reliable version of
psutil.
Note: this change also makes autoninja always make a decision about
-j; there's no longer a default where it lets ninja pick. The code is
simpler this way and I think it's better because it lets developers
always see which -j is in effect when using autoninja (and that's its
exact purpose, if you wanted default you shouldn't have used autoninja).
R=dpranke@chromium.org
Bug: 976265
Change-Id: Ic9d12469729e4bf58da1ec1bd70437329519fc46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1663904
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Auto-Submit: Gabriel Charette <gab@chromium.org>
Ran "2to3 -w -n -f print ./" and manually added imports.
Ran "^\s*print " and "\s+print " to find batch/shell scripts, comments and the like with embedded code, and updated them manually.
Also manually added imports to files, which used print as a function, but were missing the import.
The scripts still work with Python 2.
There are no intended behaviour changes.
Bug: 942522
Change-Id: Id777e4d4df4adcdfdab1b18bde89f235ef491b9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1595684
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Auto-Submit: Raul Tambre <raul@tambre.ee>
When NINJA_SUMMARIZE_BUILD=1 then autoninja prints a summary of build
performance at the end. This change makes it so that ninja prints more
detailed build statistics during the build. In particular, slow process
creation is a common but difficult to see bottleneck. This change makes
it so that if NINJA_SUMMARIZE_BUILD=1 then the number of running build
processes will be displayed.
Bug: 787983
Change-Id: Ic0907e23e7f762e23e4795059b37085e0cb8c4ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1582802
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
I noticed current autoninja does not upload build log in failed build.
Change-Id: Ie58646b483e130769ad22113953c0b95b3145548
Reviewed-on: https://chromium-review.googlesource.com/c/1441892
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
For ease of distinguishing a series of build from others in Goma,
let me make autoninja set unique build ID.
The ID can be usable for statistics purpose to see how much time is
usually spent for each build.
Since the unique ID is not specific to Goma, i.e. anybody invoked by
ninja can use it to distinguish builds, I chose AUTONINJA_BUILD_ID.
Since we do not need to distinguish each node, I chose uuid4.
Bug: b/77176746
Change-Id: I0b6a67811b420c51d1fd3fd113bf7f039a41e2ab
Reviewed-on: https://chromium-review.googlesource.com/c/1275685
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Fumitoshi Ukai <ukai@chromium.org>
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
A handy usage pattern for autoninja.bat that I was not aware of is to go
autoninja -C out\Default chrome && chrome
This will build chrome and then run it, but only run it if the build
succeeds. The addition of post_build_ninja_summary.py broke this by
losing the error code. This change fixes it by using black magic to
set an error code in the failure case.
Bug: chromium:787983
Change-Id: Ib87fd1799816e19d56de76e08e0f9688be903d80
Reviewed-on: https://chromium-review.googlesource.com/916705
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
post_build_ninja_summary.py analyzes the .ninja_log file in an output
directory to summarize build performance. It lists the most expensive
build steps, and build-step types, printing both their elapsed time and,
more importantly, their "weighted" time which is a measure of their
contribution to the total build time.
For each time segment within the build the running build steps
accumulate weighted time that is proportional to the elapsed time
divided by the number of tasks running. If a thousand compilation steps
are running in parallel then they will each be "charged" for 1/1,000th
of the elapsed time. If a single link step is running alone then it is
charged with all of the elapsed time.
Bug: chromium:787983
Change-Id: Id5aea715f798a16415dd0365a27f0051202668e5
Reviewed-on: https://chromium-review.googlesource.com/871988
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Erik Staab <estaab@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This is the second attempt at fixing autoninja to allow passing '^^' to
it to specify that ninja should build the outputs of the specified file
instead of building that file itself. The problem is that '^' is a
special character and when extra layers of indirection are added the
number of '^' characters needed grows exponentially in some poorly
understood way. The first fix attempt just quoted the arguments that
autoninja.bat passed to autoninja.py, but that meant they came in as
one argument. This fix expands on that by modifying autoninja.py to
understand how to deal with the monolithic argument. With this change
this once again works:
autoninja -C out\debug_component ..\..\base\win\enum_variant.cc^^
It can be convenient to have a ninja.bat file which starts goma and lets
users keep typing the same build commands. However even with this fix
the previously recommended ninja.bat file must be invoked with four
'^' characters. If that is too much then the new recommended ninja.bat
is to copy autoninja.bat and modify as needed, perhaps like this:
@echo off
call python c:\goma\goma-win64\goma_ctl.py ensure_start >nul
FOR /f "usebackq tokens=*" %%a in (`python c:\src\depot_tools\autoninja.py "%*"`) do echo %%a & %%a
BUG: 758725
Change-Id: Ieee9cf343ee5f22e9988a1969cb7a7a90687666b
Reviewed-on: https://chromium-review.googlesource.com/656478
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
This reverts commit ee3946be4e.
Reason for revert: This broke autoninja. The arguments all come in as one and aren't recognized. I think I can fix but I'm reverting for now.
Original change's description:
> Fix autoninja.bat to not swallow ^^ sequences
>
> Ninja uses the '^' character to indicate that ninja should build the
> targets that are generated from the specified file, rather than
> building the specified file. On Windows '^^' is needed because '^' is
> the line continuation character. However autoninja.bat complicates
> things because the multiple levels of batch files successfully swallow
> pairs of '^' characters.
>
> By adding quotes around %* in autoninja.bat it becomes possible to
> invoke autoninja.bat normally. That is, this works:
>
> autoninja -C out\debug_component ..\..\base\win\enum_variant.cc^^
>
> It can be convenient to have a ninja.bat file which starts goma and lets
> users keep typing the same build commands. However even with this fix
> the previously recommended ninja.bat file requires four '^' characters.
> If that is too much then the new recommended ninja.bat is to copy
> autoninja.bat and modify as needed, perhaps like this:
>
> @echo off
> call python c:\goma\goma-win64\goma_ctl.py ensure_start >nul
> FOR /f "usebackq tokens=*" %%a in (`python c:\src\depot_tools\autoninja.py "%*"`) do echo %%a & %%a
>
> BUG=758725
>
> Change-Id: I648cf42675af2f946be7aa4033956b015d953829
> Reviewed-on: https://chromium-review.googlesource.com/651826
> Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
TBR=dpranke@chromium.org,brucedawson@chromium.org,sebmarchand@chromium.org
Change-Id: I131b9ba00882acb5a2d009a2a444f186740d7394
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 758725
Reviewed-on: https://chromium-review.googlesource.com/654117
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Ninja uses the '^' character to indicate that ninja should build the
targets that are generated from the specified file, rather than
building the specified file. On Windows '^^' is needed because '^' is
the line continuation character. However autoninja.bat complicates
things because the multiple levels of batch files successfully swallow
pairs of '^' characters.
By adding quotes around %* in autoninja.bat it becomes possible to
invoke autoninja.bat normally. That is, this works:
autoninja -C out\debug_component ..\..\base\win\enum_variant.cc^^
It can be convenient to have a ninja.bat file which starts goma and lets
users keep typing the same build commands. However even with this fix
the previously recommended ninja.bat file requires four '^' characters.
If that is too much then the new recommended ninja.bat is to copy
autoninja.bat and modify as needed, perhaps like this:
@echo off
call python c:\goma\goma-win64\goma_ctl.py ensure_start >nul
FOR /f "usebackq tokens=*" %%a in (`python c:\src\depot_tools\autoninja.py "%*"`) do echo %%a & %%a
BUG=758725
Change-Id: I648cf42675af2f946be7aa4033956b015d953829
Reviewed-on: https://chromium-review.googlesource.com/651826
Reviewed-by: Sébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Using goma requires the developer to remember which build directories
use goma and which don't so that they can pass an appropriate -j number.
Getting this wrong makes builds slower, either by under utilizing
resources or by causing a self-inflicted DOS attack. Usage:
autoninja -C out/debug
autoninja looks at the settings for the specified build directory and
then selects either -j num_cores*20 or no -j flag based on the
use_goma setting.
You can set the NINJA_CORE_MULTIPLIER variable to change from the
default 20* multiplier. You can also use NINJA_CORE_ADDITION if you
want non-goma builds to specify -j with an offset to the number of
cores, such as this Linux command:
NINJA_CORE_ADDITION=-2 autoninja -C out/release base
This will tell autoninja to pass -j to ninja with num_cores-2 as the
parameter.
On Windows you can have a ninja.bat file (ahead of ninja on the path)
such that autoninja will automatically be used. It should contain this:
@call autoninja.bat %*
Change-Id: I4003e3fc323d1cbab612999c945b5a8dc5bc6655
Reviewed-on: https://chromium-review.googlesource.com/517662
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Fumitoshi Ukai <ukai@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>