aboutsummaryrefslogtreecommitdiffhomepage
Commit message (Collapse)AuthorAge
* make getContentKey() available in GrGpuResource public interfaceGravatar bsalomon2015-02-02
| | | | | | | | TBR=robertphillips@google.com Committed: https://skia.googlesource.com/skia/+/85ed2db092e75db41b711b9116a8d5b465fc2b0c Review URL: https://codereview.chromium.org/886313005
* Hairline batchGravatar joshualitt2015-02-02
| | | | | | BUG=skia: Review URL: https://codereview.chromium.org/876673002
* Whitespace change to test new GCE compile botGravatar Ravi Mistry2015-02-02
|
* Whitespace change to test new GCE compile botGravatar Ravi Mistry2015-02-02
|
* Whitespace change to test new GCE compile botGravatar Ravi Mistry2015-02-02
|
* Revert of make getContentKey() available in GrGpuResource public interface ↵Gravatar stephana2015-02-02
| | | | | | | | | | | | | | | | | | | | | | | (patchset #2 id:20001 of https://codereview.chromium.org/886313005/) Reason for revert: This causes in one case the linker and in the other the compiler to segfault. On Test-Ubuntu12-ShuttleA-GTX660-x86_64-Debug/Release Original issue's description: > make getContentKey() available in GrGpuResource public interface > > TBR=robertphillips@google.com > > Committed: https://skia.googlesource.com/skia/+/85ed2db092e75db41b711b9116a8d5b465fc2b0c TBR=robertphillips@google.com,bsalomon@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/864343005
* make getContentKey() available in GrGpuResource public interfaceGravatar bsalomon2015-02-02
| | | | | | TBR=robertphillips@google.com Review URL: https://codereview.chromium.org/886313005
* DM: wire up --leaks / -l againGravatar mtklein2015-02-02
| | | | | | | | | No public API changes. TBR=reed@google.com BUG=skia: Review URL: https://codereview.chromium.org/893043002
* Move npot resizing out of GrContext and simplify GrContext texture functions.Gravatar bsalomon2015-02-02
| | | | | | | | Committed: https://skia.googlesource.com/skia/+/8a8100349105c8c6de39fcb34e47679da7a67f54 Committed: https://skia.googlesource.com/skia/+/6c96672491b04cb782bce8fee778124df66524a0 Review URL: https://codereview.chromium.org/882223003
* move HelloWorld to be a peer of SampleAppGravatar caryclark2015-02-02
| | | | | | | | This is working towards making a simple example part of the buildbot compile step and removing SkExamples from the experimental directory. This works on Mac, Windows, and Linux but isn't complete for Android, ChromeOS and iOS. Review URL: https://codereview.chromium.org/886413004
* add SkAnimTimer, SPACE = pause/resume, ESP = stopGravatar reed2015-02-02
| | | | | | BUG=skia: Review URL: https://codereview.chromium.org/894083003
* Atomics overhaul.Gravatar mtklein2015-02-02
| | | | | | | | | | | | | | | | | | | | | | | | | This merges and refactors SkAtomics.h and SkBarriers.h into SkAtomics.h and some ports/ implementations. The major new feature is that we can express memory orders explicitly rather than only through comments. The porting layer is reduced to four template functions: - sk_atomic_load - sk_atomic_store - sk_atomic_fetch_add - sk_atomic_compare_exchange From those four we can reconstruct all our previous sk_atomic_foo. There are three ports: - SkAtomics_std: uses C++11 <atomic>, used with MSVC - SkAtomics_atomic: uses newer GCC/Clang intrinsics, used on not-MSVC where possible - SkAtomics_sync: uses older GCC/Clang intrinsics, used where SkAtomics_atomic not supported No public API changes. TBR=reed@google.com BUG=skia: Review URL: https://codereview.chromium.org/896553002
* Add missing SK_OVERRIDEGravatar rmistry2015-02-02
| | | | | | | | | | Tested by running on clang head + ubuntu 14.04: GYP_DEFINES=”skia_gpu=0 skia_warnings_as_errors=1" tools/xsan_build thread dm BUILDTYPE=Release out/Release/dm -v BUG=skia:3386 Review URL: https://codereview.chromium.org/894833002
* add missing SK_OVERRIDEGravatar mtklein2015-02-02
| | | | | | BUG=skia: Review URL: https://codereview.chromium.org/894153002
* Don't read unecessary font files on Android.Gravatar bungeman2015-02-02
| | | | | | | | | | | | With a new style fonts.xml (version >= 21) fallback fonts are specified as font families without a name. The current code reads these, but then also reads in the old fallback and vendor xml files. With this change, when a new style xml file is encoutered, no further files will be read. This both lowers memory use and speeds font lookup. Locally, this change reduces the number of font families loaded on my 'L' device from 148 to 80. All of the families removed are duplicates. Review URL: https://codereview.chromium.org/888923003
* Revert of Revert of SSE4 opaque blend using intrinsics instead of assembly. ↵Gravatar stephana2015-02-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | (patchset #1 id:1 of https://codereview.chromium.org/873553003/) Reason for revert: Reverted the wrong CL. Original issue's description: > Revert of SSE4 opaque blend using intrinsics instead of assembly. (patchset #16 id:300001 of https://codereview.chromium.org/874863002/) > > Reason for revert: > This causes a bug on the 'hittestpath' GM on MacMini 4,1 > > See: > > https://gold.skia.org/#/triage/hittestpath?head=0 > > for details. > > Original issue's description: > > SSE4 opaque blend using intrinsics instead of assembly. > > > > Since we had such a hard time with the assembly versions of this blit (to the > > point that we have them completely disabled everywhere), I thought I'd take > > a shot at writing a version of the blit using intrinsics. > > > > The key feature of SSE4 we're exploiting is that we can use ptest (_mm_test*) > > to skip the blend when the 16 src pixels we consider each loop are all opaque > > or all transparent. _mm_shuffle_epi8 from SSSE3 also lends a hand to extract > > all those alphas. > > > > It's worth looking to see if we can backport this type of logic to SSE2 using > > _mm_movemask_epi8, or up to 32 pixels at a time using AVX. > > > > My local performance testing doesn't show this to be an unambiguous win > > (there are probably microbenchmarks and SKPs where we'd be better off just > > powering through the blend rather than looking at alphas), but the potential > > does seem tantalizing enough to let skiaperf vet it on the bots. (< 1.0x is a win.) > > > > DM says it draws pixel perfect compare to the old code. > > > > Microbenchmarks: > > bitmap_RGBA_8888_A_source_stripes_two 14us -> 14.4us 1.03x > > bitmap_RGBA_8888_A_source_stripes_three 14.3us -> 14.5us 1.01x > > bitmap_RGBA_8888_scale_bilerp 61.9us -> 62.2us 1.01x > > bitmap_RGBA_8888_update_volatile_scale_rotate_bilerp 102us -> 101us 0.99x > > bitmap_RGBA_8888_scale_rotate_bilerp 103us -> 101us 0.99x > > bitmap_RGBA_8888_scale 18.4us -> 18.2us 0.99x > > bitmap_RGBA_8888_A_scale_rotate_bicubic 71us -> 70us 0.99x > > bitmap_RGBA_8888_update_scale_rotate_bilerp 103us -> 101us 0.99x > > bitmap_RGBA_8888_A_scale_rotate_bilerp 112us -> 109us 0.98x > > bitmap_RGBA_8888_update_volatile 5.72us -> 5.58us 0.98x > > bitmap_RGBA_8888 5.73us -> 5.58us 0.97x > > bitmap_RGBA_8888_update 5.78us -> 5.6us 0.97x > > bitmap_RGBA_8888_A_scale_bilerp 70.7us -> 68us 0.96x > > bitmap_RGBA_8888_A_scale_bicubic 23.7us -> 21.8us 0.92x > > bitmap_RGBA_8888_A 13.9us -> 10.9us 0.78x > > bitmap_RGBA_8888_A_source_opaque 14us -> 6.29us 0.45x > > bitmap_RGBA_8888_A_source_transparent 14us -> 3.65us 0.26x > > > > Running over our ~70 SKP web page captures, this looks like we spend 0.7x > > the time in S32A_Opaque_BlitRow compared to the SSE2 version, which should > > be a decent predictor of real-world impact. > > > > BUG=chromium:399842 > > > > Committed: https://skia.googlesource.com/skia/+/04bc91b972417038fecfa87c484771eac2b9b785 > > > > CQ_EXTRA_TRYBOTS=client.skia:Test-Mac10.6-MacMini4.1-GeForce320M-x86_64-Release-Trybot > > > > Committed: https://skia.googlesource.com/skia/+/6dbfb21a6c88af6d94e8c823c3ad559f1a41b493 > > TBR=henrik.smiding@intel.com,mtklein@google.com,herb@google.com,reed@google.com,thakis@chromium.org,mtklein@chromium.org > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=chromium:399842 > > Committed: https://skia.googlesource.com/skia/+/4988891a1173cd405bf1c1dd3a3668c451f45e4c TBR=henrik.smiding@intel.com,mtklein@google.com,herb@google.com,reed@google.com,thakis@chromium.org,mtklein@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=chromium:399842 Review URL: https://codereview.chromium.org/894083002
* Revert of SSE4 opaque blend using intrinsics instead of assembly. (patchset ↵Gravatar stephana2015-02-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | #16 id:300001 of https://codereview.chromium.org/874863002/) Reason for revert: This causes a bug on the 'hittestpath' GM on MacMini 4,1 See: https://gold.skia.org/#/triage/hittestpath?head=0 for details. Original issue's description: > SSE4 opaque blend using intrinsics instead of assembly. > > Since we had such a hard time with the assembly versions of this blit (to the > point that we have them completely disabled everywhere), I thought I'd take > a shot at writing a version of the blit using intrinsics. > > The key feature of SSE4 we're exploiting is that we can use ptest (_mm_test*) > to skip the blend when the 16 src pixels we consider each loop are all opaque > or all transparent. _mm_shuffle_epi8 from SSSE3 also lends a hand to extract > all those alphas. > > It's worth looking to see if we can backport this type of logic to SSE2 using > _mm_movemask_epi8, or up to 32 pixels at a time using AVX. > > My local performance testing doesn't show this to be an unambiguous win > (there are probably microbenchmarks and SKPs where we'd be better off just > powering through the blend rather than looking at alphas), but the potential > does seem tantalizing enough to let skiaperf vet it on the bots. (< 1.0x is a win.) > > DM says it draws pixel perfect compare to the old code. > > Microbenchmarks: > bitmap_RGBA_8888_A_source_stripes_two 14us -> 14.4us 1.03x > bitmap_RGBA_8888_A_source_stripes_three 14.3us -> 14.5us 1.01x > bitmap_RGBA_8888_scale_bilerp 61.9us -> 62.2us 1.01x > bitmap_RGBA_8888_update_volatile_scale_rotate_bilerp 102us -> 101us 0.99x > bitmap_RGBA_8888_scale_rotate_bilerp 103us -> 101us 0.99x > bitmap_RGBA_8888_scale 18.4us -> 18.2us 0.99x > bitmap_RGBA_8888_A_scale_rotate_bicubic 71us -> 70us 0.99x > bitmap_RGBA_8888_update_scale_rotate_bilerp 103us -> 101us 0.99x > bitmap_RGBA_8888_A_scale_rotate_bilerp 112us -> 109us 0.98x > bitmap_RGBA_8888_update_volatile 5.72us -> 5.58us 0.98x > bitmap_RGBA_8888 5.73us -> 5.58us 0.97x > bitmap_RGBA_8888_update 5.78us -> 5.6us 0.97x > bitmap_RGBA_8888_A_scale_bilerp 70.7us -> 68us 0.96x > bitmap_RGBA_8888_A_scale_bicubic 23.7us -> 21.8us 0.92x > bitmap_RGBA_8888_A 13.9us -> 10.9us 0.78x > bitmap_RGBA_8888_A_source_opaque 14us -> 6.29us 0.45x > bitmap_RGBA_8888_A_source_transparent 14us -> 3.65us 0.26x > > Running over our ~70 SKP web page captures, this looks like we spend 0.7x > the time in S32A_Opaque_BlitRow compared to the SSE2 version, which should > be a decent predictor of real-world impact. > > BUG=chromium:399842 > > Committed: https://skia.googlesource.com/skia/+/04bc91b972417038fecfa87c484771eac2b9b785 > > CQ_EXTRA_TRYBOTS=client.skia:Test-Mac10.6-MacMini4.1-GeForce320M-x86_64-Release-Trybot > > Committed: https://skia.googlesource.com/skia/+/6dbfb21a6c88af6d94e8c823c3ad559f1a41b493 TBR=henrik.smiding@intel.com,mtklein@google.com,herb@google.com,reed@google.com,thakis@chromium.org,mtklein@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=chromium:399842 Review URL: https://codereview.chromium.org/873553003
* Add resourcePath to running dm on Android doc.Gravatar bungeman2015-02-02
| | | | | | | | Without specifying the resource path a number of tests and gms do not work properly. The example given in the docs should specify the resource path so that the example run is exemplary. Review URL: https://codereview.chromium.org/898453003
* Optimize SSE2 opaque blendGravatar qiankun.miao2015-02-02
| | | | | | | | | | | | | | | | | | | | | | | | | Backport optimization from https://codereview.chromium.org/874863002/. Microbenchmarks data compared to previous SSE2 implementation: bitmap_BGRA_8888_A_source_stripes_three 7.52us -> 8.67us 1.15x bitmap_BGRA_8888_A_source_stripes_two 7.48us -> 8.56us 1.15x bitmap_BGRA_8888_update_scale_rotate_bilerp 63.4us -> 64us 1.01x bitmap_BGRA_8888_update_volatile 3.31us -> 3.33us 1.01x bitmap_BGRA_8888_scale 11.1us -> 11.2us 1x bitmap_BGRA_8888_scale_bilerp 35.8us -> 35.9us 1x bitmap_BGRA_8888 3.33us -> 3.33us 1x bitmap_BGRA_8888_A_scale_rotate_bicubic 66.7us -> 66.5us 1x bitmap_BGRA_8888_update_volatile_scale_rotate_bilerp 65.1us -> 64us 0.98x bitmap_BGRA_8888_scale_rotate_bilerp 65.1us -> 64us 0.98x bitmap_BGRA_8888_A_scale_bicubic 30.6us -> 29.9us 0.98x bitmap_BGRA_8888_A_scale_bilerp 42.7us -> 41.4us 0.97x bitmap_BGRA_8888_A_scale_rotate_bilerp 71us -> 67.7us 0.95x bitmap_BGRA_8888_A 7.44us -> 5.7us 0.77x bitmap_BGRA_8888_A_source_opaque 7.46us -> 3.72us 0.5x bitmap_BGRA_8888_A_source_transparent 7.46us -> 1.96us 0.26x BUG=skia: Review URL: https://codereview.chromium.org/886403002
* Disable LCD text when rasterizing SkPictureShader tiles.Gravatar fmalita2015-02-02
| | | | | | | BUG=chromium:453299 R=reed@google.com Review URL: https://codereview.chromium.org/884163003
* Update SKP versionGravatar skia.buildbots2015-02-01
| | | | | | | | Automatic commit by the RecreateSKPs bot. TBR= Review URL: https://codereview.chromium.org/890293003
* allow GMs to animateGravatar reed2015-02-01
| | | | | | BUG=skia: Review URL: https://codereview.chromium.org/888283002
* Update SKP versionGravatar skia.buildbots2015-01-31
| | | | | | | | Automatic commit by the RecreateSKPs bot. TBR= Review URL: https://codereview.chromium.org/888343002
* Revert of Move npot resizing out of GrContext and simplify GrContext texture ↵Gravatar bsalomon2015-01-31
| | | | | | | | | | | | | | | | | | | | | functions. (patchset #10 id:200001 of https://codereview.chromium.org/882223003/) Reason for revert: perf fix didn't fix the cr webgl conformance tests Original issue's description: > Move npot resizing out of GrContext and simplify GrContext texture functions. > > Committed: https://skia.googlesource.com/skia/+/8a8100349105c8c6de39fcb34e47679da7a67f54 > > Committed: https://skia.googlesource.com/skia/+/6c96672491b04cb782bce8fee778124df66524a0 TBR=robertphillips@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/887303002
* SVG backend in DMGravatar mtklein2015-01-31
| | | | | | | | | | Not enabled by default, but this should get you SKPs, GMs etc for free to play with. $ out/Debug/dm -w svgs --src gm skp --config svg BUG=skia: Review URL: https://codereview.chromium.org/892693002
* Move npot resizing out of GrContext and simplify GrContext texture functions.Gravatar bsalomon2015-01-31
| | | | | | Committed: https://skia.googlesource.com/skia/+/8a8100349105c8c6de39fcb34e47679da7a67f54 Review URL: https://codereview.chromium.org/882223003
* Remove extraneous newline.Gravatar Brian Salomon2015-01-31
| | | | | | TBR=mtklein@google.com Review URL: https://codereview.chromium.org/878333004
* Add standard way to indicate GM is GPU-only.Gravatar bsalomon2015-01-31
| | | | | | TBR=egdaniel@google.com,mtklein@google.com Review URL: https://codereview.chromium.org/869393007
* Revert of Move npot resizing out of GrContext and simplify GrContext texture ↵Gravatar fmalita2015-01-31
| | | | | | | | | | | | | | | | | | | | | | | | | functions. (patchset #9 id:160001 of https://codereview.chromium.org/882223003/) Reason for revert: webGL conformance failures: WebglConformance.conformance_textures_tex_image_and_sub_image_2d_with_video WebglConformance.conformance_textures_texture_npot_video https://codereview.chromium.org/892773003/ http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/29272 Original issue's description: > Move npot resizing out of GrContext and simplify GrContext texture functions. > > Committed: https://skia.googlesource.com/skia/+/8a8100349105c8c6de39fcb34e47679da7a67f54 TBR=robertphillips@google.com,bsalomon@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/868233005
* Update SKP versionGravatar skia.buildbots2015-01-30
| | | | | | | | Automatic commit by the RecreateSKPs bot. TBR= Review URL: https://codereview.chromium.org/875243006
* Update dump code in FontConfigParser test.Gravatar bungeman2015-01-30
| | | | | | | This code is still quite useful for debugging, but has bit rotted. Update to match current structure of code. Review URL: https://codereview.chromium.org/890073002
* Convert FamilyData to Skia style field names.Gravatar bungeman2015-01-30
| | | | | | | The Skia convention is to prefix field names with 'f'. This change is mostly mechanical. Review URL: https://codereview.chromium.org/892733002
* Do not leak FamilyData in Android SkFontMgr.Gravatar bungeman2015-01-30
| | | | | | | Currently, it appears one FamilyData is leaking per scanned file. This also cleans up the hanging currentFamily memory management. Review URL: https://codereview.chromium.org/889943005
* export SkImageInfoGravatar reed2015-01-30
| | | | | | BUG=skia: Review URL: https://codereview.chromium.org/893603005
* Do not add indirection to XML_Parser.Gravatar bungeman2015-01-30
| | | | | | | The expat type XML_Parser is already a pointer, so pass it by value instead of by indirection. Review URL: https://codereview.chromium.org/880783003
* Revert of Add device space "nudge" to gpu draws (patchset #6 id:90001 of ↵Gravatar robertphillips2015-01-30
| | | | | | | | | | | | | | | | | | | | | | | | | | https://codereview.chromium.org/877473005/) Reason for revert: Blink layout tests this time :( Original issue's description: > Add device space "nudge" to gpu draws > > This CL nudges all the GPU draws and clips slightly to match raster's round behavior for BW draws. We assume the effect will be negligible and do it for AA draws too. > > BUG=423834 > > Committed: https://skia.googlesource.com/skia/+/2d55d07501c56310f97d2092d789a2bc9fa01b78 > > Committed: https://skia.googlesource.com/skia/+/b9329991426d0b77ea194a380d72d73fb855308a TBR=bsalomon@google.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=423834 Review URL: https://codereview.chromium.org/886153002
* Suggestions and merge in the other CL.Gravatar mtklein2015-01-30
| | | | | | | | | I had some suggestions on the subset CL, and took the opportunity to rebase it against head and merge in the other color type CL. BUG=skia: Review URL: https://codereview.chromium.org/893703002
* Reland "remove unused SkAvoidXfermode"Gravatar scroggo2015-01-30
| | | | | | | | (patchset #2 id:20001 of https://codereview.chromium.org/860583002/) SkAvoidXfermode has been moved into Android, so it is safe to remove. Review URL: https://codereview.chromium.org/890893003
* Disable the noisiest /analyze warning in Chrome. ~3,700/12,000Gravatar brucedawson2015-01-30
| | | | | | | | | | | | | | | | | | Decades ago Intel decided that the bsr (Bit Scan Reverse) instruction should have undefined results if its argument is zero. This probably makes the instruction harder to implement and it definitely makes it more difficult to use. In SkCLZ_portable it requires a check for a zero argument, but despite that check /analyze still warns that _BitScanReverse might fail (because it doesn't know what can cause failures). Because this warning occurs in a frequently included header file it ends up being very noisy, accounting for ~30% of all warnings (before deduplication). Suppressing this useless warning will make the raw results easier to look through. Review URL: https://codereview.chromium.org/872673007
* Move npot resizing out of GrContext and simplify GrContext texture functions.Gravatar bsalomon2015-01-30
| | | | Review URL: https://codereview.chromium.org/882223003
* First cut at cleaning up Sergio's example code and moving some common code ↵Gravatar caryclark2015-01-30
| | | | | | | | | | to SkWindow. Eventually, this will be moved to be a peer of SampleApp so it is compiled by the bots to avoid future bit rot. Also ignore XCode auto-generated flag in CommandLineFlags, and remove the unused multiple-example part. Review URL: https://codereview.chromium.org/890873003
* DM: paths as implict strings too.Gravatar mtklein2015-01-30
| | | | | | BUG=skia: Review URL: https://codereview.chromium.org/891823002
* Add device space "nudge" to gpu drawsGravatar robertphillips2015-01-30
| | | | | | | | | | This CL nudges all the GPU draws and clips slightly to match raster's round behavior for BW draws. We assume the effect will be negligible and do it for AA draws too. BUG=423834 Committed: https://skia.googlesource.com/skia/+/2d55d07501c56310f97d2092d789a2bc9fa01b78 Review URL: https://codereview.chromium.org/877473005
* DM::NullSinkGravatar halcanary2015-01-30
| | | | | | Motivation: The null sink can act as a control for experiments. Review URL: https://codereview.chromium.org/873723007
* dm: allow multiple --images flags, allow single filesGravatar halcanary2015-01-30
| | | | Review URL: https://codereview.chromium.org/872993005
* Initial SVG backend stubbingGravatar fmalita2015-01-30
| | | | | | | | | This adds SkSVGDevice and a small utility for converting SKP files to SVG (skp2svg). R=reed@google.com,jcgregorio@google.com BUG=skia:3368 Review URL: https://codereview.chromium.org/892533002
* Remove unused globalRef/globalUnref.Gravatar scroggo2015-01-30
| | | | | | BUG=skia:1482 Review URL: https://codereview.chromium.org/887993003
* dm: allow multiple --skp flags, allow single filesGravatar halcanary2015-01-30
| | | | Review URL: https://codereview.chromium.org/885353002
* Remove unused hasLocalMatrix.Gravatar scroggo2015-01-30
| | | | | | | | | | | This method is not called anywhere on Android or Chrome, and it has a FIXME that it may not be correct. A client can still getLocalMatrix().isIdentity() if they need this information. (It has the same FIXME, and perhaps we should revisit it. In the meantime, this convenience method is not needed.) Review URL: https://codereview.chromium.org/882443007
* documentation: Writing Unit and Rendering TestsGravatar halcanary2015-01-30
| | | | Review URL: https://codereview.chromium.org/885133002