change signature for virtual related to saveLayer, passing SaveLayerRec

change signature for virtual related to saveLayer, passing SaveLayerRec

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1533953002

NOTREECHECKS=True

Committed: https://skia.googlesource.com/skia/+/4960eeec4a1f2a772654883d7f3615d47bcd5dc3

[reed1, 2015-12-17 18:56:56]

Description was changed from

==========
change signature for virtual related to saveLayer, passing SaveLayerRec

BUG=skia:
==========

to

==========
change signature for virtual related to saveLayer, passing SaveLayerRec

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[reed1, 2015-12-17 18:57:12]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 18:57:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/1

[reed1, 2015-12-17 18:57:54]

reed@google.com changed reviewers:
+ fmalita@chromium.org, mtklein@google.com, robertphillips@google.com

[reed1, 2015-12-17 18:57:55]

prepares us to add more state to SaveLayerRec in the future (e.g. backdrop)

[commit-bot: I haz the power, 2015-12-17 18:58:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-17 18:58:14]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on client.skia.compile
(JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)
  Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...)

[reed1, 2015-12-17 19:06:34]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 19:06:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/20001

[commit-bot: I haz the power, 2015-12-17 19:07:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-17 19:07:28]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)

[reed1, 2015-12-17 19:13:41]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 19:13:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/40001

[commit-bot: I haz the power, 2015-12-17 19:15:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-17 19:15:46]

Dry run: Try jobs failed on following builders:
  Build-Ubuntu-GCC-x86_64-Release-Trybot on client.skia.compile (JOB_FAILED,
http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)

[reed1, 2015-12-17 19:18:08]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 19:18:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/60001

[commit-bot: I haz the power, 2015-12-17 19:29:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-17 19:29:41]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-Trybot on client.skia (JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[robertphillips, 2015-12-17 19:30:37]

The CQ bit was checked by robertphillips@google.com

[robertphillips, 2015-12-17 19:30:41]

The CQ bit was unchecked by robertphillips@google.com

[commit-bot: I haz the power, 2015-12-17 19:30:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/60001

[commit-bot: I haz the power, 2015-12-17 19:30:45]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-12-18 01:30 UTC

[robertphillips, 2015-12-17 19:30:50]

The CQ bit was checked by robertphillips@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 19:30:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/60001

[reed1, 2015-12-17 19:33:13]

The CQ bit was unchecked by reed@google.com

[f(malita), 2015-12-17 19:40:49]

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h...
include/core/SkCanvas.h:397: };
Since we don't really refer to the SaveLayerFlags type anywhere, I feel we're
wasting a good name here :)

Could we name the enum something else (SaveLayerPublicFlags?
SaveLayerFlagsFlags?), and then typedef uint32_t SaveLayerFlags?  I think that
would make the API look better.

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h...
include/core/SkCanvas.h:1341: kDontSaveClip_PrivateSaveLayerFlag      = 1 << 30,
I don't think these are needed: we don't ever get partial SaveFlags into Skia.
IIRC the only reason they're still around is Android still refers to them
internally, but we emulate/discard them in the JNI glue.

[reed1, 2015-12-17 19:45:01]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 19:45:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/80001

[reed1, 2015-12-17 19:49:19]

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h...
include/core/SkCanvas.h:1341: kDontSaveClip_PrivateSaveLayerFlag      = 1 << 30,
On 2015/12/17 19:40:49, f(malita) wrote:
> I don't think these are needed: we don't ever get partial SaveFlags into Skia.
> IIRC the only reason they're still around is Android still refers to them
> internally, but we emulate/discard them in the JNI glue.

Meaning I don't need any of these, or I don't need the matrix/clip ones?

[reed1, 2015-12-17 19:53:27]

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h...
include/core/SkCanvas.h:397: };
On 2015/12/17 19:40:49, f(malita) wrote:
> Since we don't really refer to the SaveLayerFlags type anywhere, I feel we're
> wasting a good name here :)
> 
> Could we name the enum something else (SaveLayerPublicFlags?
> SaveLayerFlagsFlags?), and then typedef uint32_t SaveLayerFlags?  I think that
> would make the API look better.

Done.

[reed1, 2015-12-17 19:53:35]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 19:53:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/100001

[robertphillips, 2015-12-17 19:53:57]

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h...
include/core/SkCanvas.h:1349: // These must not overlap the public flags.
??

https://codereview.chromium.org/1533953002/diff/60001/include/utils/SkNoSaveL...
File include/utils/SkNoSaveLayerCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/60001/include/utils/SkNoSaveL...
include/utils/SkNoSaveLayerCanvas.h:23: protected:
rm virtual ?

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:1124: 
We can't just pull some negation trick here ?

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:1156: SaveLayerRec rec(origRec);
Do we still need/use gIgnoreSaveLayerBounds ?

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkPictureFlat....
src/core/SkPictureFlat.h:79: 
This seems problematic

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkRemote.cpp
File src/core/SkRemote.cpp (right):

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkRemote.cpp#n...
src/core/SkRemote.cpp:177: fEncoder->saveLayer(this->id(path),
this->commonIDs(*paint), rec.fSaveLayerFlags);
?

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkRemote.cpp#n...
src/core/SkRemote.cpp:507: SkRect rect;
\n somewhere ?

[reed1, 2015-12-17 19:57:28]

The CQ bit was unchecked by reed@google.com

[f(malita), 2015-12-17 19:57:43]

On 2015/12/17 19:49:19, reed1 wrote:
> https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h
> File include/core/SkCanvas.h (right):
> 
>
https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h...
> include/core/SkCanvas.h:1341: kDontSaveClip_PrivateSaveLayerFlag      = 1 <<
30,
> On 2015/12/17 19:40:49, f(malita) wrote:
> > I don't think these are needed: we don't ever get partial SaveFlags into
Skia.
> > IIRC the only reason they're still around is Android still refers to them
> > internally, but we emulate/discard them in the JNI glue.
> 
> Meaning I don't need any of these, or I don't need the matrix/clip ones?

Matrix/clip is what I believe we got rid off.  Pretty sure they're no longer
being passed for save(), but let me double-check the Android glue to make sure
they're not sneaking in for saveLayer().

[f(malita), 2015-12-17 20:18:26]

On 2015/12/17 19:57:43, f(malita) wrote:
> On 2015/12/17 19:49:19, reed1 wrote:
> >
https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h
> > File include/core/SkCanvas.h (right):
> > 
> >
>
https://codereview.chromium.org/1533953002/diff/60001/include/core/SkCanvas.h...
> > include/core/SkCanvas.h:1341: kDontSaveClip_PrivateSaveLayerFlag      = 1 <<
> 30,
> > On 2015/12/17 19:40:49, f(malita) wrote:
> > > I don't think these are needed: we don't ever get partial SaveFlags into
> Skia.
> > > IIRC the only reason they're still around is Android still refers to them
> > > internally, but we emulate/discard them in the JNI glue.
> > 
> > Meaning I don't need any of these, or I don't need the matrix/clip ones?
> 
> Matrix/clip is what I believe we got rid off.  Pretty sure they're no longer
> being passed for save(), but let me double-check the Android glue to make sure
> they're not sneaking in for saveLayer().

Err, looks like for saveLayer we just pass them through:
https://android.googlesource.com/platform/frameworks/base/+/805f6eb/core/jni/...

We may need to fix that.

Still OK to ignore them in your CL, because we don't do anything with them *in
Skia* (internalSave() no longer takes flags).

[reed1, 2015-12-17 20:22:31]

https://codereview.chromium.org/1533953002/diff/60001/include/utils/SkNoSaveL...
File include/utils/SkNoSaveLayerCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/60001/include/utils/SkNoSaveL...
include/utils/SkNoSaveLayerCanvas.h:23: protected:
On 2015/12/17 19:53:57, robertphillips wrote:
> rm virtual ?

Done.

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkCanvas.cpp
File src/core/SkCanvas.cpp (right):

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:1124: 
On 2015/12/17 19:53:57, robertphillips wrote:
> We can't just pull some negation trick here ?

The bits are in different places, so I'm not sure it would be any clearer.

The gooder news is, 3/4 of these flags should be nuked (android)

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkCanvas.cpp#n...
src/core/SkCanvas.cpp:1156: SaveLayerRec rec(origRec);
On 2015/12/17 19:53:57, robertphillips wrote:
> Do we still need/use gIgnoreSaveLayerBounds ?

Dunno. Separate CL I propose.

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkRemote.cpp
File src/core/SkRemote.cpp (right):

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkRemote.cpp#n...
src/core/SkRemote.cpp:507: SkRect rect;
On 2015/12/17 19:53:57, robertphillips wrote:
> \n somewhere ?

Done.

[reed1, 2015-12-17 20:23:22]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 20:23:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/120001

[reed1, 2015-12-17 20:23:46]

ptal

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkPictureFlat.h
File src/core/SkPictureFlat.h (right):

https://codereview.chromium.org/1533953002/diff/60001/src/core/SkPictureFlat....
src/core/SkPictureFlat.h:79: 
On 2015/12/17 19:53:57, robertphillips wrote:
> This seems problematic

Done.

[f(malita), 2015-12-17 20:48:59]

https://codereview.chromium.org/1533953002/diff/120001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/120001/include/core/SkCanvas....
include/core/SkCanvas.h:1245: kCallLegacyMethod_SaveLayerStrategy,   //
TEMPORARY until we can remove legacy willSaveLayer
no longer used?

https://codereview.chromium.org/1533953002/diff/120001/include/core/SkCanvas....
include/core/SkCanvas.h:1344: static bool BoundsAffectsClip(SaveLayerFlags);
If this is only used in SkCanvas.cpp, can we keep it out of the header?

https://codereview.chromium.org/1533953002/diff/120001/include/private/SkReco...
File include/private/SkRecords.h (right):

https://codereview.chromium.org/1533953002/diff/120001/include/private/SkReco...
include/private/SkRecords.h:200: uint32_t saveLayerFlags);
SkCanvas::SaveLayerFlags flags?

[reed1, 2015-12-17 20:53:00]

https://codereview.chromium.org/1533953002/diff/120001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/120001/include/core/SkCanvas....
include/core/SkCanvas.h:1245: kCallLegacyMethod_SaveLayerStrategy,   //
TEMPORARY until we can remove legacy willSaveLayer
On 2015/12/17 20:48:59, f(malita) wrote:
> no longer used?

good catch, done.

https://codereview.chromium.org/1533953002/diff/120001/include/core/SkCanvas....
include/core/SkCanvas.h:1344: static bool BoundsAffectsClip(SaveLayerFlags);
On 2015/12/17 20:48:59, f(malita) wrote:
> If this is only used in SkCanvas.cpp, can we keep it out of the header?

Needs access to private flag :(

https://codereview.chromium.org/1533953002/diff/120001/include/private/SkReco...
File include/private/SkRecords.h (right):

https://codereview.chromium.org/1533953002/diff/120001/include/private/SkReco...
include/private/SkRecords.h:200: uint32_t saveLayerFlags);
On 2015/12/17 20:48:59, f(malita) wrote:
> SkCanvas::SaveLayerFlags flags?

Done.

[reed1, 2015-12-17 20:53:07]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 20:53:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/140001

[robertphillips, 2015-12-17 20:59:52]

lgtm % question

https://codereview.chromium.org/1533953002/diff/140001/src/core/SkRemote.cpp
File src/core/SkRemote.cpp (right):

https://codereview.chromium.org/1533953002/diff/140001/src/core/SkRemote.cpp#...
src/core/SkRemote.cpp:177: fEncoder->saveLayer(this->id(path),
this->commonIDs(*paint), rec.fSaveLayerFlags);
So we're just going to let this die in debug?

[reed1, 2015-12-17 21:06:28]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-17 21:06:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/160001

[reed1, 2015-12-17 21:07:32]

https://codereview.chromium.org/1533953002/diff/140001/src/core/SkRemote.cpp
File src/core/SkRemote.cpp (right):

https://codereview.chromium.org/1533953002/diff/140001/src/core/SkRemote.cpp#...
src/core/SkRemote.cpp:177: fEncoder->saveLayer(this->id(path),
this->commonIDs(*paint), rec.fSaveLayerFlags);
On 2015/12/17 20:59:52, robertphillips wrote:
> So we're just going to let this die in debug?

Done.

[f(malita), 2015-12-17 21:22:00]

lgtm

[reed1, 2015-12-17 21:23:55]

The CQ bit was unchecked by reed@google.com

[reed1, 2015-12-17 21:24:00]

The CQ bit was checked by reed@google.com

[reed1, 2015-12-17 21:24:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1533953002/#ps160001
(title: "remove assert")

[commit-bot: I haz the power, 2015-12-17 21:24:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/160001

[reed1, 2015-12-17 21:49:28]

Description was changed from

==========
change signature for virtual related to saveLayer, passing SaveLayerRec

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
change signature for virtual related to saveLayer, passing SaveLayerRec

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

NOTREECHECKS=True
==========

[reed1, 2015-12-17 21:50:13]

The CQ bit was unchecked by reed@google.com

[f(malita), 2015-12-17 22:23:28]

https://codereview.chromium.org/1533953002/diff/180001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/180001/include/core/SkCanvas....
include/core/SkCanvas.h:1254: virtual SaveLayerStrategy
getSaveLayerStrategy(const SaveLayerRec&) {
While reviewing the Chromium side of this CL, it occurred to me that
"getSaveLayerStrategy" is somewhat misleading: it sounds like a plain getter,
which may suggest one doesn't necessarily have to call the inherited version. 
But it acts more like a notifier, with some ancestor classes performing related
work and relying on receiving this notification.

I can't think of a better name, but maybe we should document that overriders are
required to call up the inheritance chain.

[reed1, 2015-12-17 22:29:40]

https://codereview.chromium.org/1533953002/diff/180001/include/core/SkCanvas.h
File include/core/SkCanvas.h (right):

https://codereview.chromium.org/1533953002/diff/180001/include/core/SkCanvas....
include/core/SkCanvas.h:1254: virtual SaveLayerStrategy
getSaveLayerStrategy(const SaveLayerRec&) {
On 2015/12/17 22:23:28, f(malita) wrote:
> While reviewing the Chromium side of this CL, it occurred to me that
> "getSaveLayerStrategy" is somewhat misleading: it sounds like a plain getter,
> which may suggest one doesn't necessarily have to call the inherited version. 
> But it acts more like a notifier, with some ancestor classes performing
related
> work and relying on receiving this notification.
> 
> I can't think of a better name, but maybe we should document that overriders
are
> required to call up the inheritance chain.

That is documented on the strategy enum, but I will push it down to this method.

[reed1, 2015-12-18 14:55:11]

The CQ bit was checked by reed@google.com

[reed1, 2015-12-18 14:55:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
fmalita@chromium.org, robertphillips@google.com
Link to the patchset: https://codereview.chromium.org/1533953002/#ps200001
(title: "update comment")

[commit-bot: I haz the power, 2015-12-18 14:55:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533953002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533953002/200001

[commit-bot: I haz the power, 2015-12-18 15:09:21]

Description was changed from

==========
change signature for virtual related to saveLayer, passing SaveLayerRec

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

NOTREECHECKS=True
==========

to

==========
change signature for virtual related to saveLayer, passing SaveLayerRec

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

NOTREECHECKS=True

Committed:
https://skia.googlesource.com/skia/+/4960eeec4a1f2a772654883d7f3615d47bcd5dc3
==========

[commit-bot: I haz the power, 2015-12-18 15:09:23]

Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/4960eeec4a1f2a772654883d7f3615d47bcd5dc3