Just a few strays.
Change-Id: Ib209bc8dd228850b837b850dce14967a2112593e
Reviewed-on: https://skia-review.googlesource.com/c/191161
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
a54104803d..52d861bd49
git log a54104803d72..52d861bd49cc --date=short --no-merges --format='%ad %ae %s'
2019-02-02 syoussefi@chromium.org Disable -Wextra-semi-stmt
2019-02-02 ianelliott@google.com Implement EGL_ANDROID_recordable for Vulkan back-end.
2019-02-01 jonahr@google.com Fix unnecessary copy of for loop variables in ANGLE
2019-02-01 syoussefi@chromium.org Use env variable to select default backend
2019-02-01 jmadill@chromium.org Enable -Wextra-semi and -Wextra-semi-stmt.
2019-02-01 syoussefi@chromium.org Initial support for compiler AST validation
2019-02-01 jmadill@chromium.org Roll glslang.
2019-02-01 ckulakowski@opera.com Fix for linking of non-component angle_unittests
Created with:
gclient setdep -r third_party/externals/angle2@52d861bd49cc
The AutoRoll server is located here: https://autoroll.skia.org/r/angle-skia-autoroll
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
CQ_INCLUDE_TRYBOTS=skia.primary:Build-Debian9-Clang-x86_64-Release-ANGLE;skia.primary:Perf-Win10-Clang-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-All-ANGLE;skia.primary:Perf-Win10-Clang-NUC5i7RYH-GPU-IntelIris6100-x86_64-Debug-All-ANGLE;skia.primary:Perf-Win10-Clang-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-All-ANGLE;skia.primary:Perf-Win10-Clang-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-All-ANGLE;skia.primary:Perf-Win10-Clang-ShuttleC-GPU-GTX960-x86_64-Debug-All-ANGLE;skia.primary:Test-Win10-Clang-AlphaR2-GPU-RadeonR9M470X-x86_64-Debug-All-ANGLE;skia.primary:Test-Win10-Clang-NUC6i5SYK-GPU-IntelIris540-x86_64-Debug-All-ANGLE;skia.primary:Test-Win10-Clang-NUCD34010WYKH-GPU-IntelHD4400-x86_64-Debug-All-ANGLE;skia.primary:Test-Win10-Clang-ShuttleC-GPU-GTX960-x86_64-Debug-All-ANGLE
TBR=borenet@google.com
Change-Id: I008df064f6301658404c371cf47a5656d8c11621
Reviewed-on: https://skia-review.googlesource.com/c/188852
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: skia-autoroll <skia-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
This enables four different options in the compiler, described
below. I also added enough masks to satisfy RTCc when running
all GMs in both 8888 and gl configs.
---
/RTCc - Detects when a value is assigned to a smaller data
type and results in data loss. This happens even when casting.
Masking is required to suppress this.
/RTCs - Various stack-related checks, including uninitialized
data (by initializing locals to a non-zero value), array bounds
checking, and stack pointer corruption that can occur with a
calling convention mismatch.
/RTCu - Reports when a variable is used without having been
initialized. Mostly redundant with compile-time checks.
/guard:cf - This is more of a security option, that computes
all possible targets for indirect calls at compile time, and
verifies that those are the only targets reached at compile
time. Also generates similar logic around switch statements
that turn into jump tables.
Bug: skia:
Change-Id: I7b527af8fd67dec0b6556f38bcd0efc3fd505856
Reviewed-on: https://skia-review.googlesource.com/c/188625
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:
Change-Id: I09c2312c3678bdb1c48b427f446cb5865acb92a2
Reviewed-on: https://skia-review.googlesource.com/c/177074
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Might open up some neat options like LTO builds.
If nothing it's nice to use each whole toolchain consistently.
Move around some bonus quotes a bit.
LLD doesn't understand /DEBUG:FASTLINK.
Change-Id: I27e3c97acea6980c5bd006394aebb1e103007edd
Reviewed-on: https://skia-review.googlesource.com/c/175981
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: I9ba1caa4862bdf9ffc9c0e637bd69cce91fd8468
Reviewed-on: https://skia-review.googlesource.com/c/168740
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This is a reland of 166dbd3135
Since last attempt,
- update SkPath::Direction docs
- kIllegal is not an advanced blend mode
Original change's description:
> make enum santizer fatal
>
> This enum sanitizer checks that all the values of the enum we use fall
> within the range of the enumerated values.
>
> The main thing this helps point out is that the size of enum types in
> C++ need only be large enough to hold the largest declared value; larger
> values are undefined. In practice, most enums are implemented as ints
> for compatibility with C, so while this hasn't pointed out anything
> egregiously broken, the sanitizer has found a couple possibly dangerous
> situations in our codebase.
>
> For most types using values outside the enum range, we can just
> explicitly size them to int. This makes their de facto size de jure.
>
> But we need to actually make GrBlendEquation and GrBlendCoeff not store
> values outside their enumerated range. They're packed into bitfields
> that really can't represent those (negative) values. So for these I've
> added new kIllegal values to the enums, forcing us to deal with our
> once-silent illegal values a bit more explicitly.
>
> Change-Id: Ib617694cf1aaa83ae99289e9e760f49cb6393a2f
> Reviewed-on: https://skia-review.googlesource.com/c/168484
> Reviewed-by: Brian Osman <brianosman@google.com>
Cq-Include-Trybots: skia.primary:Housekeeper-PerCommit-Bookmaker,Test-Android-Clang-AndroidOne-GPU-Mali400MP2-arm-Debug-All-Android
Change-Id: Id93b80bbeae11872542c9b76715e3c3cb10609fd
Reviewed-on: https://skia-review.googlesource.com/c/168582
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit 166dbd3135.
Reason for revert: illegal is not advanced, docs
Original change's description:
> make enum santizer fatal
>
> This enum sanitizer checks that all the values of the enum we use fall
> within the range of the enumerated values.
>
> The main thing this helps point out is that the size of enum types in
> C++ need only be large enough to hold the largest declared value; larger
> values are undefined. In practice, most enums are implemented as ints
> for compatibility with C, so while this hasn't pointed out anything
> egregiously broken, the sanitizer has found a couple possibly dangerous
> situations in our codebase.
>
> For most types using values outside the enum range, we can just
> explicitly size them to int. This makes their de facto size de jure.
>
> But we need to actually make GrBlendEquation and GrBlendCoeff not store
> values outside their enumerated range. They're packed into bitfields
> that really can't represent those (negative) values. So for these I've
> added new kIllegal values to the enums, forcing us to deal with our
> once-silent illegal values a bit more explicitly.
>
> Change-Id: Ib617694cf1aaa83ae99289e9e760f49cb6393a2f
> Reviewed-on: https://skia-review.googlesource.com/c/168484
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=mtklein@chromium.org,mtklein@google.com,brianosman@google.com
Change-Id: I691c08092340a6273e442c0f098b844f7d0363ba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/168581
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This enum sanitizer checks that all the values of the enum we use fall
within the range of the enumerated values.
The main thing this helps point out is that the size of enum types in
C++ need only be large enough to hold the largest declared value; larger
values are undefined. In practice, most enums are implemented as ints
for compatibility with C, so while this hasn't pointed out anything
egregiously broken, the sanitizer has found a couple possibly dangerous
situations in our codebase.
For most types using values outside the enum range, we can just
explicitly size them to int. This makes their de facto size de jure.
But we need to actually make GrBlendEquation and GrBlendCoeff not store
values outside their enumerated range. They're packed into bitfields
that really can't represent those (negative) values. So for these I've
added new kIllegal values to the enums, forcing us to deal with our
once-silent illegal values a bit more explicitly.
Change-Id: Ib617694cf1aaa83ae99289e9e760f49cb6393a2f
Reviewed-on: https://skia-review.googlesource.com/c/168484
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of c766370d86
Original change's description:
> always optimize third_party code
>
> Change-Id: I5b2244460a4760e9336640f597d0f74c374a0d04
> Reviewed-on: https://skia-review.googlesource.com/155641
> Commit-Queue: Mike Klein <mtklein@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Auto-Submit: Mike Klein <mtklein@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I63e7f9ca852fc99728d7a01d9987b3506115d266
Reviewed-on: https://skia-review.googlesource.com/155760
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit c766370d86.
Reason for revert: speculative -- trying to fix Debian breaks (pdf?)
Original change's description:
> always optimize third_party code
>
> Change-Id: I5b2244460a4760e9336640f597d0f74c374a0d04
> Reviewed-on: https://skia-review.googlesource.com/155641
> Commit-Queue: Mike Klein <mtklein@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Auto-Submit: Mike Klein <mtklein@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=mtklein@google.com,brianosman@google.com
Change-Id: I5467c95f9487c31e6f538f13579e490cdaeeee2e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/155607
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Change-Id: I5b2244460a4760e9336640f597d0f74c374a0d04
Reviewed-on: https://skia-review.googlesource.com/155641
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Without this, debug clang builds on windows have very little debug info.
Fix is based on Chromium's handling of this (they have several tactics,
depending on linker, etc...)
https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?l=2194
Change-Id: Ib1b3aa4cdf4e2daa762d994bea94dc3c691cf359
Reviewed-on: https://skia-review.googlesource.com/154540
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
People using is_official_build don't really want to see warnings.
They're for devs, not users.
The somewhat odd update to gn/BUILDCONFIG.gn keeps command
line flag precedence (later == more important) unchanged.
Change-Id: I1a04a35f066b7408021d474535f0dbf4928e21d3
Reviewed-on: https://skia-review.googlesource.com/151380
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
We're close enough that it's easier to use "undefined"
to turn on all supported UBSAN sanitizers and then
keep a couple in fyi_sanitizers as a blacklist.
I'm going to try to fix "enum" next too, so hopefully
that won't be in there too long.
I did a little flag cleanup too. -fno-omit-frame-pointer
was harmlessly in there twice for Android builds.
Change-Id: I8216fb0685423b2ff56db2e2be5bbeb4b48f932f
Reviewed-on: https://skia-review.googlesource.com/146760
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Change-Id: Ibfead1121f22277297e16196b37e6e86dc2d2166
Reviewed-on: https://skia-review.googlesource.com/146649
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This sanitizer checks for overaligned reads and writes,
or put another way, use of underaligned pointers.
This usually happens when you cast, e.g. char* to int*
without checking that the char* is 4-byte aligned. Each
of the changes under src/ fixes something just like that.
The unusual setup for tools/xsan.blacklist is there to
force a rebuild whenever tools/xsan.blacklist changes.
I spent a good few minutes debugging rebuilds not happening
this morning, perhaps from some strange ccache interaction.
Align SkTextBlobs as void* (today they're just 4-byte) so the
SkTextBlob::RunRecords we put after them in SkTextBlobBuilder
buffers are properly aligned (for the SkTypeface* inside).
There's no obvious error in void SkRRect::computeType(),
but one bot seems to have seen some sort of issue with
SK_AT_SCOPE_EXIT(SkASSERT(this->isValid()));
I can't reproduce it locally, so I'm just going to unroll it.
Change-Id: I904d94f65f695e1b626b684c32216a4930b72b0c
Reviewed-on: https://skia-review.googlesource.com/146104
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
build debug on linux for gdb
Building with this define enables additional
checks for standard library functions when
running gdb, and allows printing std::string.
R=halcanary@google.com
Bug: skia:
Change-Id: I7f2dc12336f9239443328f13264be7a6b285c4b5
Reviewed-on: https://skia-review.googlesource.com/140786
Commit-Queue: Cary Clark <caryclark@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-on: https://skia-review.googlesource.com/141083
Commit-Queue: Cary Clark <caryclark@skia.org>
This reverts commit 3c3db72e69.
Reason for revert: I suspect this is breaking the Build-Debian9-Clang-arm-Debug-Chromebook_GLES bot
Original change's description:
> build debug on linux for gdb
>
> Building with this define enables additional
> checks for standard library functions when
> running gdb, and allows printing std::string.
>
> R=halcanary@google.com
>
> Bug: skia:
> Change-Id: Ib71c179d8cbacbcd7f17a1c85f912072918e6be8
> Reviewed-on: https://skia-review.googlesource.com/140786
> Commit-Queue: Cary Clark <caryclark@google.com>
> Reviewed-by: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,halcanary@google.com,caryclark@google.com
Change-Id: I56ae1c6c02032e6445f5585de51c48423d283636
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/140840
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Building with this define enables additional
checks for standard library functions when
running gdb, and allows printing std::string.
R=halcanary@google.com
Bug: skia:
Change-Id: Ib71c179d8cbacbcd7f17a1c85f912072918e6be8
Reviewed-on: https://skia-review.googlesource.com/140786
Commit-Queue: Cary Clark <caryclark@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: I4bbf3e5e6d9d4e11e968a0bc88ce43444047a382
Reviewed-on: https://skia-review.googlesource.com/130903
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
I know SkRasterPipelineBlitter will add back offsets
to the mask pointer to make these fMaskPtr.pixels safe.
SkImageInfo wasn't making out-of-bounds pointers, but was moving
between two in-bounds pointers by subtracting large (negative)
size_t. Switching to explicit negate and add quiets this down.
Bug: chromium:836282
Change-Id: Ia65e380ec41dbfedce0659106830fbacb1a5cf4a
Reviewed-on: https://skia-review.googlesource.com/131147
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Bug: skia:
Change-Id: Ic5efe371b6365913a47d833c698958cc842d16ae
Reviewed-on: https://skia-review.googlesource.com/128685
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
We haven't done this in a while.
The only interesting change I've noticed so far is that we no longer
need to link libandroid_support when targeting API 21+.
Change-Id: I6506fbe7bd02534f1ea136f2164acab7f44c29e0
Reviewed-on: https://skia-review.googlesource.com/128009
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Change-Id: I4a0259f82272099e08df0e7dea91489f6d641ca4
Reviewed-on: https://skia-review.googlesource.com/113670
Commit-Queue: Ben Wagner <benjaminwagner@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
The only tangible effect this CL should have is to use __vectorcall on
all Windows builds, including scalar ones. The code generation is a
little better there with __vectorcall than not, so might as well. This
is a baby step towards vector stages with MSVC, but a very baby step
indeed.
Mostly this refactors and regroups a bunch of logic to reflect my
current thoughts. The BUILD.gn changes are essentially no-ops, but they
simplify things and make our flags more similar to how those targets are
built in Chromium.
(And I cleaned up other /arch: uses so this works.)
Change-Id: I73dd39d15cdc7b3d268231a707952bbbfd91496e
Reviewed-on: https://skia-review.googlesource.com/115644
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
This suppresses the noexcept-type warning, since Skia doesn't have a
stable ABI in any event. GCC now warns on more printf style formats,
so we have to hide our bad test formats a little better. GCC now
also warns on implicit enum to bool conversions, which did catch two
issues.
Change-Id: Ib81769c421757186506873f0fe298ecd0106ae87
Reviewed-on: https://skia-review.googlesource.com/114263
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Enable /arch:SSE2 for Win x86 builds (both MSVC and Clang).
Change-Id: I0618e938b6542b60fae3947b7e0dcde22650ba54
Reviewed-on: https://skia-review.googlesource.com/56721
Commit-Queue: Ben Wagner <benjaminwagner@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:7643
Change-Id: Ieeca5414f51c7c113c54b47578ec840fc7d684e0
Reviewed-on: https://skia-review.googlesource.com/109520
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Bug: skia:
Change-Id: I97bf83ca001acc762a42d26207a394faee6d90fb
Reviewed-on: https://skia-review.googlesource.com/107782
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Android+ASAN was already using libc++, so don't confuse it with the host
library.
Bug: skia:
Change-Id: Ibdeb40007f9d346a84adb39de240299049c0c55e
Reviewed-on: https://skia-review.googlesource.com/107721
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Bug: skia:
Change-Id: I0dda67283b32aaf06a34463847e760158bf47392
Reviewed-on: https://skia-review.googlesource.com/102422
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Bug: skia:7566
Change-Id: I89f662b6493fee4d9f8a10ba6aa791634cb39bba
Reviewed-on: https://skia-review.googlesource.com/102621
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
It should work on 32 and 64 bit android running M or newer.
Bug: skia:
Change-Id: I7e6d4000f4fee8f2704b84b7174174dd0e68d21c
Reviewed-on: https://skia-review.googlesource.com/93700
Reviewed-by: Mike Klein <mtklein@chromium.org>
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
After tinkering with libstdc++ -> libc++, looks like we need this
warning again. We already turn it on in Android's own x86 builds;
see gn_to_bp.py for that.
Change-Id: I7b7a76d1c22dd3f3b7712e9ce89b78d9a229a302
Reviewed-on: https://skia-review.googlesource.com/92360
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>
All other options are going away real soon now.
Change-Id: I57051fdabc2b0fd18fd133c9fb345ffa5a46e5db
Reviewed-on: https://skia-review.googlesource.com/92300
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Bug: skia:7462
Change-Id: Iba0af2f0ff8ef646f93a0e2ced083d99020a38c9
Reviewed-on: https://skia-review.googlesource.com/90842
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Bug: skia:2679
Change-Id: Ia462af01b9832da90206b9e9be2278cb48c6c502
Reviewed-on: https://skia-review.googlesource.com/86401
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Ben Wagner <benjaminwagner@google.com>
'windk' is no longer a thing. There are two separate variables to point
at your compiler (win_vc), and SDK (win_sdk).
'msvc' is no longer a thing, either. By default, we look for 2017 and
then 2015 (in the default locations). If neither is located, use an
assert to let users know they should set win_vc. Then, detect if win_vc
points at a 2017 or 2015 installation, and configure it automatically.
Because the toolchain is now 2017, update the GN files to handle building
x86 in that configuration. In fact, we only support x86 builds (with 2017
or 2015) using the toolchain assets. Keep a 2015 toolchain around as a
new asset, so we can add bot coverage.
Docs-Preview: https://skia.org/?cl=81841
Bug: skia:
Change-Id: I8c68a6f949e54c0e798a219450bbb9406f8dc6ac
Reviewed-on: https://skia-review.googlesource.com/81841
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I9a2a061e7a021587441f3f39427306309e0341e8
Reviewed-on: https://skia-review.googlesource.com/82042
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
