With a hot Bazel cache, https://task-scheduler.skia.org/job/4XhwJfG1wR38Hp3dF1pI
took 59 seconds and did not have to wait 1-2 minutes
for TaskScheduler to schedule a prerequisite task
and Swarming to deduplicate said task, which is
the best case scenario for "all hot caches".
Change-Id: Ic86599e0b886ecded836641cceb19b623065a91e
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546136
Reviewed-by: Eric Boren <borenet@google.com>
Needed to fix the Dawn and Harfbuzz Bazel rules.
Change-Id: I21f63c970bdc972b97e42ef85d82d7f478bd45e2
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545721
Reviewed-by: Eric Boren <borenet@google.com>
Organization v3.5, if we are keeping track :)
This splits the "srcs" filegroup into "srcs" and "private_hdrs",
and renames "hdrs" to "public_hdrs".
To assist with the split, I created the macro split_srcs_and_hdrs.
Rather than keep two separate lists of header and source files,
I figured it would be easiest, at least for the common case,
to keep one list of files and then have a for loop split them
apart. I've tried to be consistent with having the list
of files be named with a _FILES suffix - maybe we can use this
as a marker to generate .gni files in the future?
Suggested review order:
- //bazel/macros.bzl. Note this needs a corresponding
G3 change (http://cl/452279799) as well. The exports_files_legacy
change is the better approach to something I manually
handled yesterday when fixing the G3 roll.
- //BUILD.bazel to see the new target skia_internal and
the previous skia_core renamed to skia_public.
- //src/core/BUILD.bazel to see a typical usage of
split_srcs_and_hdrs.
- //include/... to see the change to public_hdrs and
private_hdrs
- //src/... to see many more usages of split_srcs_and_hdrs
- //tools/... to see changes to skia_internal where
appropriate.
- Everything else. Note that //modules/... might also need
to be built with skia_internal instead of skia_public,
but we can fix that up later, if necessary.
Change-Id: Ie1cc969455d97b029b2d77faa222c4a9bad70671
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545716
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
gazelle ended up being more liability than asset for our C++ rules.
It required devs to manually run the command frequently (and was
easy to forget until the CQ failed). The fact that we still had to
edit the source files (e.g. the "srcs" cc_libraries) meant that
the mixture between generated and hand-written caused some
tension (see include/third_party/vulkan for a good example).
The combination of gazelle and our IWYU enforcement added several
bits of churn without any real benefit. The generated rules
also didn't help identify cases where we were not keeping tight
boundaries (e.g. non-gpu code and gpu code).
Identifying third_party deps automatically ended up being trickier
than anticipated (see the deleted //third_party/file_map_for_bazel.json)
Using the "maximum set of dependencies" worked ok, but ended up
increasing build time unnecessarily. For example, compiling
CanvasKit for WebGL always needed to compile Dawn because
SkSLCompiler.cpp sometimes needs to include tint/tint.h.
Follow-up CLs will rebuild the BUILD.bazel rules without gazelle.
Note to Reviewers:
- The only file worth manually reviewing here is bazel/Makefile.
Change-Id: I36d6fc3747487fabaf699690780c95f1f6765770
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543976
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
This change introduces the use of "Android12" as an OS, so
many checks had to be changed from b.os("Android") to
b.matchOs("Android") which treats "Android" and "Android12"
the same.
Bug: skia:12912
Change-Id: Ib0a5f9d8f1a06b62e529a1567efcde52c7bb44d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527496
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Appeasing IWYU and the Gazelle-generated BUILD.bazel files is currently
annoying for devs working on non-linux environments.
I have been experimenting with removing the gazelle rules (but still
keeping our BUILD.bazel rules organized), which would alleviate
some of the burden here. Until that is finished, we can stop one
of the two jobs.
Change-Id: Id6c43bd98c6ce41e1ae9a6bfc1102ff47a6e5b1f
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543496
Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I6db702c2d231bfe527b3fd4bf4f99beb6e58bf8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542641
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Fix for this landed in ANGLE.
Bug: skia:13290
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac10.15.1-Clang-MacBookAir7.2-GPU-IntelHD6000-x86_64-Debug-All-ANGLE
Change-Id: Id14366c5f9fe33bf1347a9bdaad12ec38b93fa6b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542640
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
subprocess.check_output needs encoding or it defaults
to bytes.
Python 3 has / be float division and // be integer
division.
The skpbench fix is speculative.
Change-Id: I13d0e976c03bba30761c8ab533e6fdd041b795c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541696
Reviewed-by: Ravi Mistry <rmistry@google.com>
This is similar to https://skia-review.googlesource.com/c/buildbot/+/538218
BIG CHANGE: If we fail to download uninteresting hashes
(because Gold is down or the script otherwise fails), we
now crash/fail our Test-* tasks. In an early version of
this removal, that lack of failure masked an issue with
the script (urllib2 is not in Python3) and nearly would
have landed. Failing loudly is better, IMO.
This removes the symbolize_stack_trace script, which
stopped working with Python3 for reasons unknown.
If we need the behavior, we can rewrite it.
Relatedly, we removed 4 jobs, the Docker ones, because
we will not get much value out of them as we migrate
towards Bazel and removing the symbolization script
was tricky to get right.
There are a few cleanups around copypasta that I noticed
when combing through the recipes.
Change-Id: I8dfab416e964fd494267800b4ebe216061895f19
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538636
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
The version in CIPD is 4.46, which has the following
issue when run in Python 3:
https://github.com/GoogleCloudPlatform/gsutil/issues/961
This asset uses the latest version, 5.10, which *only*
supports Python 3.
Our gsutil recipe module had some "Windows-specific"
behavior that was actually running on Linux and needed
to be changed because it was using Python2 to run
gsutil.
Change-Id: I6884b9b6fcc600351a8e608dc9e3c0114907a586
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541220
Reviewed-by: Ravi Mistry <rmistry@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This reverts commit 5c6bf4f692.
Reason for revert: git diff does not work well with deleted files
Original change's description:
> [bazel] Run gazelle only on the files that changed
>
> With this change, make generate takes 1.8 seconds instead of
> 7.9 seconds.
>
> We still have generate_force to run on everything.
>
> Change-Id: I6d57031adbe38a7f25a59570baea89970eea024f
> Bug: skia:12541
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540740
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Bug: skia:12541
Change-Id: I47c23adf09bbc6324817e166f7ab33eb16f4bf61
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540743
Reviewed-by: John Stiles <johnstiles@google.com>
With this change, make generate takes 1.8 seconds instead of
7.9 seconds.
We still have generate_force to run on everything.
Change-Id: I6d57031adbe38a7f25a59570baea89970eea024f
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540740
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Looks like really old drivers struggled with weird no-op blocks.
Change-Id: Ie32754a9c221eb7c20924ee27e5facca7e9701f0
Bug: skia:13309
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539561
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This simplifies the bot script slightly. More importantly, it means that
local developers don't need to remember to do this when building ANGLE
on Mac.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac12-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-ANGLE
Change-Id: Ia60cd07f15e3b447b58cfa1198ea26f68f72384b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539036
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: 1320964
Change-Id: I5f05ea469b824b1382f64c1eb3fb9be0498cf329
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537517
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
For additional context, see "Codifying Certain Build Options"
and "Building on the CI" in the design doc go/skia-bazel
Suggested review order:
- builder_name_schema.json to see the three required and
one optional part of BazelBuild jobs.
- jobs.json to see one new BazelBuild job added. In an
ideal world, this job would have been named
BazelBuild-//modules/canvaskit:canvaskit_wasm-debug-linux_x64
but Buildbucket (?) requires jobs match the regex
^[a-zA-Z0-9\\-_.\\(\\) ]{1,128}$
so we use spaces instead of slashes or colons.
- gen_tasks_logic.go; noting the makeBazelLabel function
expands most of the spaces to / and the last one to a
colon to make a single-target label. If there are three
dots, then it is a multi-target label, and we do not
need to add a colon.
- bazel_build.go; This is a very simple task driver, and
I do not anticipate getting too much more complex.
The place where we decide which args to augment
a build with depend on the host platform and thus
should be set in gen_tasks_logic.go.
- bazel/buildrc to see some initial configurations set,
one of which, "debug", is used by the new job.
The "release" version of CanvasKit probably works on
3.1.10 which had a bugfix, but we are still on
3.1.9
- .bazelrc to see a rename of the linux-rbe config to
linux_rbe (our configs should have no dashes if
we want to specify them verbatim in our Job names).
It also imports the Skia-specified build configs
from //bazel/buildrc and supports the user-specified
//bazel/user/buildrc file if it exists.
- All other files in any order.
Change-Id: Ib954dd6045100eadcbbf4ffee0888f6fbce65fa7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537797
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
This is a reland of commit 094bcdb9e5
Original change's description:
> [infra] Use Python3 for our Presubmits
>
> https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_support.py;l=319;drc=443d9135cc33f3156d5fe25ebec33f9adffbab65
>
> This also makes any errors from `make train -C infra/bots`
> look well formatted because check_output returns a bytestring
> in Python3 and when that is printed, the newlines et al are not
> rendered correctly. Thus we want the output of check_output
> to be encoded to UTF-8.
>
> Without setting the USE_PYTHON3 = True in PRESUBMIT.py,
> it appears that `git cl upload` would try to run our
> infra_tests.py in Python2 mode, which does not have
> the encoding argument for check_output.
>
> Apparently cipd_bin_packages/vpython3 does not have the
> "six" package installed (but cipd_bin_packages/vpython does)
> so I replaced the six.StringIO with io.StringIO, which
> is where that is located in Python3.
>
> Change-Id: Ic8f61bf943531583ba3d110a85260d69bdbf5eb2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537677
> Reviewed-by: Ravi Mistry <rmistry@google.com>
> Reviewed-by: Eric Boren <borenet@google.com>
Change-Id: Ia7fb2f3b6d8b70ca95cf10763782d4a0122053e0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537978
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This reverts commit 094bcdb9e5.
Reason for revert: Breaking Housekeeper-PerCommit-InfraTests_Linux. The bot will have to be updated as part of this change.
Original change's description:
> [infra] Use Python3 for our Presubmits
>
> https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_support.py;l=319;drc=443d9135cc33f3156d5fe25ebec33f9adffbab65
>
> This also makes any errors from `make train -C infra/bots`
> look well formatted because check_output returns a bytestring
> in Python3 and when that is printed, the newlines et al are not
> rendered correctly. Thus we want the output of check_output
> to be encoded to UTF-8.
>
> Without setting the USE_PYTHON3 = True in PRESUBMIT.py,
> it appears that `git cl upload` would try to run our
> infra_tests.py in Python2 mode, which does not have
> the encoding argument for check_output.
>
> Apparently cipd_bin_packages/vpython3 does not have the
> "six" package installed (but cipd_bin_packages/vpython does)
> so I replaced the six.StringIO with io.StringIO, which
> is where that is located in Python3.
>
> Change-Id: Ic8f61bf943531583ba3d110a85260d69bdbf5eb2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537677
> Reviewed-by: Ravi Mistry <rmistry@google.com>
> Reviewed-by: Eric Boren <borenet@google.com>
Change-Id: I0becb25d155ce0b0281c25d83662f1b01aee8033
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537777
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Ravi Mistry <rmistry@google.com>
https://source.chromium.org/chromium/chromium/tools/depot_tools/+/main:presubmit_support.py;l=319;drc=443d9135cc33f3156d5fe25ebec33f9adffbab65
This also makes any errors from `make train -C infra/bots`
look well formatted because check_output returns a bytestring
in Python3 and when that is printed, the newlines et al are not
rendered correctly. Thus we want the output of check_output
to be encoded to UTF-8.
Without setting the USE_PYTHON3 = True in PRESUBMIT.py,
it appears that `git cl upload` would try to run our
infra_tests.py in Python2 mode, which does not have
the encoding argument for check_output.
Apparently cipd_bin_packages/vpython3 does not have the
"six" package installed (but cipd_bin_packages/vpython does)
so I replaced the six.StringIO with io.StringIO, which
is where that is located in Python3.
Change-Id: Ic8f61bf943531583ba3d110a85260d69bdbf5eb2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537677
Reviewed-by: Ravi Mistry <rmistry@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
This CL could, arguably, confuse the issue wrt why these images are
being generated but making the sizes match will make the images easier
to triage.
Bug: skia:13278
Change-Id: Ifb0b82eaaa4351fb589118d3b28673da9dd32e20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537576
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: I1ca07f97757f70debc0589648a57ba10b1729636
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537081
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Bug: skia:13040
Bug: skia:9235
Bug: skia:10412
Bug: skia:12437
Change-Id: Ia96f8332b5372ecf65cb20ffb9549bc0dc8f931b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537080
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
There are two SKP versions we deal with here:
* The min and current supported versions in src/core/SkPicturePriv.h.
* The SKP asset version that is incremented weekly in infra/bots/assets/skp/VERSION.
We will need to update the asset version used by the bot when the min version in src/core/SkPicturePriv.h is updated.
Instructions on how to do that have been documented in src/core/SkPicturePriv.h.
As noted in https://bugs.chromium.org/p/skia/issues/detail?id=13278#c2 DM currently does not fail when it fails to parse an SKP. Till DM is updated to fail, developers will have to look for blank images in Gold via this new bot to determine when we are failing to support the oldest SKP version.
Bug: skia:13278
Change-Id: I8fff62cc289c3bd6abf5179bcee349baf0a8188a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/536106
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Added Test-Ubuntu18-Clang-Golo-GPU-QuadroP400-x86_64-Debug-All-Dawn
to exercise the Dawn Vulkan backend.
Change-Id: I3679822a1460eda36654f99c70ba9510e394b40c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/536736
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:13272
Change-Id: I6ba8d43a723fddf1ecbc06c6f7d1d8295428d342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/536436
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Add a Mac Mini to test using GrSlug drawing instead of using
GrTextBlob for drawing.
Change-Id: I28ee402b7f82ae235518ee585d5803857490f8e1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535964
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Herb Derby <herb@google.com>
G3 prefers license() first.
This was done mechanically with a big find/replace
Change-Id: I8c33c7bc10a6bec42e966cad81c259954e841811
Bug: skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535898
Reviewed-by: Ben Wagner <bungeman@google.com>
This is a reland of commit e6f23f98de
Original change's description:
> Reland "Add anisotropic option to SkSamplingOptions."
>
> This is a reland of commit 9be2d572d4
>
> Original change's description:
> > Add anisotropic option to SkSamplingOptions.
> >
> > Implement on GPU.
> >
> > Bug: skia:13036
> > Change-Id: I35d760596c4f8faaec27fccf284b70802fcf3f9b
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524757
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Reviewed-by: Michael Ludwig <michaelludwig@google.com>
>
> Bug: skia:13036
> Change-Id: I3e411aae389dc880ce32bba78852705059fb88b0
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535197
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:13036
Change-Id: Icb2513cea6c4ec4cec934f78c66071081c232960
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535437
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Ran the following commands:
find -name "BUILD.bazel" -exec sed -i -e '1iload("//bazel:macros.bzl", "cc_library", "exports_files_legacy")\nexports_files_legacy()' {} +
buildifier --lint=fix --mode=fix -r .
This had the effect of making sure we can export all of our
files in G3 (until we no longer have legacy targets) and
making all of our cc_libraries shim-able.
bazel/macros.bzl has the human-contributed changes, the rest
were mechanical.
Change-Id: I8e24e30e74b038cfd072cdbe4078bfd1d213dd46
Bug: skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535359
Reviewed-by: Ben Wagner <bungeman@google.com>
Change-Id: I23b8c5a37a7ccaddccc042f8a4f70a2e29a41211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535399
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is a reland of commit 9be2d572d4
Original change's description:
> Add anisotropic option to SkSamplingOptions.
>
> Implement on GPU.
>
> Bug: skia:13036
> Change-Id: I35d760596c4f8faaec27fccf284b70802fcf3f9b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524757
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Bug: skia:13036
Change-Id: I3e411aae389dc880ce32bba78852705059fb88b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535197
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This was needed to pick up reviews.skia.org/533814
$ go get go.skia.org/infra@9d8975cd94
$ go get cloud.google.com/go/pubsub@v1.8.3 # needed to fix go.mod error
$ go mod download
$ make -C infra/bots train
$ make -C bazel gazelle_update_repo
$ make -C bazel generate
The last step was just to make sure no current BUILD.bazel
files changed as a result of updating Gazelle (they did not).
I then re-introduced the motivating bug from reviews.skia.org/533804
and noted that make -C bazel generate fixed the erroneous
filename (as expected).
Change-Id: Ie65b88f8285bb8a3edf305dbaa68058335469883
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/534856
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This should make it so the HW tessellation path renderers are never
used; it will always select the atlas or direct fixed-count renderers
instead. This CL will give us a good indication of what visual diffs
to expect, layout tests to rebase, and any performance regressions.
If those are acceptable, then we can proceed with the rest of the code
removal.
Bug: skia:13263
Change-Id: I273bb231461932047768c1c7233ae4291483bc95
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/533810
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
