|
|
DescriptionReland: Experimental parsing expression grammar (PEG) template library
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002
Committed: https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42
Committed: https://skia.googlesource.com/skia/+/6cf896d7ce03b87b3a5595bc66caf0a34c993755
Patch Set 1 #Patch Set 2 : comments #Patch Set 3 : speculative gn fix #Patch Set 4 : SkTLazy initializer fix #
Total comments: 24
Patch Set 5 : review comments #
Total comments: 10
Patch Set 6 : moar review #Patch Set 7 : rebase #Patch Set 8 : drop inadvertent SkTLazy change #Patch Set 9 : Fixes for MSan, g3 roll #
Messages
Total messages: 43 (23 generated)
Description was changed from ========== Experimental parsing expression grammar template lib BUG=skia: ========== to ========== Experimental parsing expression grammar template lib BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 ==========
Description was changed from ========== Experimental parsing expression grammar template lib BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 ========== to ========== Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 ==========
fmalita@chromium.org changed reviewers: + bungeman@google.com, mtklein@google.com, reed@google.com
Fiddling with a different parsing approach. (inspired by https://en.wikipedia.org/wiki/Parsing_expression_grammar, https://github.com/ColinH/PEGTL) The nice parts: * implicit parser state - the state (really just a position) is hauled around implicitly as part of the match call graph => this allows arbitrary backtracking * typed match results - the results are strongly typed and cannot be hold the wrong way Not so nice: * accessing match result data fields is kind of awkward * template explosion? May turn out useful in refactoring the SVG attribute parsers - I'm imagining stuff like using parser = Opt<Seq<LIT<'s','c','a','l','e','('>, Number>, Seq<LIT<'t','r','a','n','s','l','a','t','e','('>, Number>>; if (const auto match = parser::Match(input)) { if (match->v1.isValid()) { SkMatrix m = SkMatrix::MakeScale(match->v1.get()->v2); ... } else { SkASSERT(match->v2.isValid()); SkMatrix m = SkMatrix::MakeTrans(match->v2.get()->v2); ... } } Also, unrelated, Mac/clang was kind enough to point out that SkTLazy(const T*) initialization is unsafe: https://build.chromium.org/p/client.skia/builders/Test-Mac-Clang-MacMini6.2-C... (introduced in https://codereview.chromium.org/2232913003)
My two cents from the peanut gallery. https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... File experimental/svg/model/SkPEG.h (right): https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:73: struct Seq { While I'm not sure off hand how to do it, having Seq (especially) take a variable number of types here would be nice, since otherwise these get nested like crazy, making it difficult to follow the logic. https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp File tests/SkPEGTest.cpp (right): https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp#new... tests/SkPEGTest.cpp:221: // [0-9]+(,[0-9]+)? Doesn't this need '$' at the end? https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp#new... tests/SkPEGTest.cpp:239: REPORTER_ASSERT(r, P0::Match("123,456")); For example P0::Match("1,2 ") should be false. https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp#new... tests/SkPEGTest.cpp:241: // [ ]*[Ff]oo([Bb]ar)+[Bb]az[ ]* Needs a '$' to match the EOS below? https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp#new... tests/SkPEGTest.cpp:260: EOS This seems a strange grouping for EOS. Properly parenthesizing the regex to match this is weird. (The '$' would be inside a paren).
https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... File experimental/svg/model/SkPEG.h (right): https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:24: template <typename T> Not clear here why this is parameterized on T and not V directly. https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:60: return m ? MatchResult<Opt>(m.fNext, V(m.fValue.get())) Depending on how that T/V parameterization question goes, it might be neat to write a little helper to make creating positive MatchResults less repetitive: template <typename V> static MatchResult<V> match_result(const char* next, const V& v) { return { next, v }; } https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:73: struct Seq { On 2016/08/23 20:49:18, bungeman-skia wrote: > While I'm not sure off hand how to do it, having Seq (especially) take a > variable number of types here would be nice, since otherwise these get nested > like crazy, making it difficult to follow the logic. Might make sense to have Seq stay binary, but build another n-ary List operator on top. May be as simple as: template <typename... Es> struct List; template <typename E> struct List<E> : public E {} template <typename First, typename... Rest> struct List<First, Rest...> : public Seq<First, List<Rest...>> {} https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:161: struct Some { Perhaps template <typename E> using Some = Seq<E, Any<E>>; ? https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:185: return *in ? nullptr : MatchResult<EOS>(in, V()); probably nice to write out (*in != '\0') https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:191: * Literal atom. Matches a list of char literals. You may find this more or less annoying, but it is possible to take const char* as a template parameter as long as you do not inline the literal: #include <stdio.h> #include <string.h> template <const char* A> struct Printer { void print() { printf("%zu, %s\n", strlen(A), A); } }; static const char hello[] = "Hello!"; int main(void) { Printer<hello> p; p.print(); return 0; } Printer<"Hello!"> would not work. https://codereview.chromium.org/2271743002/diff/60001/include/core/SkTLazy.h File include/core/SkTLazy.h (right): https://codereview.chromium.org/2271743002/diff/60001/include/core/SkTLazy.h#... include/core/SkTLazy.h:25: // Not in initializer list because it depends on fStorage. Alternatively, swap the order of the member variables?
https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... File experimental/svg/model/SkPEG.h (right): https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:24: template <typename T> On 2016/08/24 01:19:28, mtklein wrote: > Not clear here why this is parameterized on T and not V directly. True, doesn't do much, does it? Changed to straight V. https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:60: return m ? MatchResult<Opt>(m.fNext, V(m.fValue.get())) On 2016/08/24 01:19:28, mtklein wrote: > Depending on how that T/V parameterization question goes, it might be neat to > write a little helper to make creating positive MatchResults less repetitive: > > template <typename V> > static MatchResult<V> match_result(const char* next, const V& v) { > return { next, v }; > } I ended up adding a MatchT alias, as it's also applicable to the return type here. I think that covers your comment, PTAL. https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:73: struct Seq { On 2016/08/24 01:19:28, mtklein wrote: > On 2016/08/23 20:49:18, bungeman-skia wrote: > > While I'm not sure off hand how to do it, having Seq (especially) take a > > variable number of types here would be nice, since otherwise these get nested > > like crazy, making it difficult to follow the logic. > > Might make sense to have Seq stay binary, but build another n-ary List operator > on top. May be as simple as: > > template <typename... Es> > struct List; > > template <typename E> > struct List<E> : public E {} > > template <typename First, typename... Rest> > struct List<First, Rest...> : public Seq<First, List<Rest...>> {} That's an elegant approach. One thing I like about making Seq itself a list is that we have this cool/uniform accessor get<I>(). Maybe something similar could be implemented on top of List? https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:73: struct Seq { On 2016/08/23 20:49:18, bungeman-skia wrote: > While I'm not sure off hand how to do it, having Seq (especially) take a > variable number of types here would be nice, since otherwise these get nested > like crazy, making it difficult to follow the logic. Yeah, that would definitely be nice. At the time I hadn't figured out all the details, but by now I think I've beaten it into submission. Certainly seems to help. (Done) https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:161: struct Some { On 2016/08/24 01:19:28, mtklein wrote: > Perhaps > template <typename E> > using Some = Seq<E, Any<E>>; > ? The only downside is that the value accessor is no longer uniform (seq accessor for the first value, seq + any for subsequent). Probably not all that important. Done. https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:185: return *in ? nullptr : MatchResult<EOS>(in, V()); On 2016/08/24 01:19:28, mtklein wrote: > probably nice to write out (*in != '\0') Done. https://codereview.chromium.org/2271743002/diff/60001/experimental/svg/model/... experimental/svg/model/SkPEG.h:191: * Literal atom. Matches a list of char literals. On 2016/08/24 01:19:28, mtklein wrote: > You may find this more or less annoying, but it is possible to take const char* > as a template parameter as long as you do not inline the literal: > > #include <stdio.h> > #include <string.h> > > template <const char* A> > struct Printer { > void print() { > printf("%zu, %s\n", strlen(A), A); > } > }; > > static const char hello[] = "Hello!"; > > int main(void) { > Printer<hello> p; > p.print(); > > return 0; > } > > Printer<"Hello!"> would not work. Heh, I heard rumors of this unnatural ability :) I guess the no-inline literal part makes it kinda awkward to use - maybe we could add as a separate LITS version? https://codereview.chromium.org/2271743002/diff/60001/include/core/SkTLazy.h File include/core/SkTLazy.h (right): https://codereview.chromium.org/2271743002/diff/60001/include/core/SkTLazy.h#... include/core/SkTLazy.h:25: // Not in initializer list because it depends on fStorage. On 2016/08/24 01:19:28, mtklein wrote: > Alternatively, swap the order of the member variables? Done. https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp File tests/SkPEGTest.cpp (right): https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp#new... tests/SkPEGTest.cpp:221: // [0-9]+(,[0-9]+)? On 2016/08/23 20:49:18, bungeman-skia wrote: > Doesn't this need '$' at the end? Done. https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp#new... tests/SkPEGTest.cpp:239: REPORTER_ASSERT(r, P0::Match("123,456")); On 2016/08/23 20:49:18, bungeman-skia wrote: > For example P0::Match("1,2 ") should be false. Ack. Added as a test. https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp#new... tests/SkPEGTest.cpp:241: // [ ]*[Ff]oo([Bb]ar)+[Bb]az[ ]* On 2016/08/23 20:49:18, bungeman-skia wrote: > Needs a '$' to match the EOS below? Done. https://codereview.chromium.org/2271743002/diff/60001/tests/SkPEGTest.cpp#new... tests/SkPEGTest.cpp:260: EOS On 2016/08/23 20:49:18, bungeman-skia wrote: > This seems a strange grouping for EOS. Properly parenthesizing the regex to > match this is weird. (The '$' would be inside a paren). Mostly because of two-param Seq impl, trying out tail-nesting. Now that we have arbitrary param Seq, it all looks much better.
The CQ bit was checked by fmalita@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Haven't thought the tests through too carefully, but if those are good, this lgtm! Looks Really Neat to Me. LISP with angle brackets... https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... File experimental/svg/model/SkPEG.h (right): https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... experimental/svg/model/SkPEG.h:183: for (;;) { Is this loop different from while (const auto m = E::Match(in)) { in = m.fNext; *values.append() = *m; } ?
Mostly seems like a good idea, but I fret over encodings and such. https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... File experimental/svg/model/SkPEG.h (right): https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... experimental/svg/model/SkPEG.h:234: struct LIT<C, Cs...> { I'm assuming this is for svg and possibly parsing bits of css. Those are defined in terms of unicode code points and not 'char'. Is there a plan for that? pegtl seems to have a fair amount of code devoted to turning various encodings into code points. For example these template parameters could be 'uint32_t' (or 'char32_t') instead of 'char' which would work with the 'U' UCS-4 character literal. Of course, the Match input would need to be decoded somehow as well. https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... experimental/svg/model/SkPEG.h:248: * Matches a letter (['a'-'z','A'-Z']) and returns the matched nit: missing a ' before Z https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... experimental/svg/model/SkPEG.h:269: struct Digit { Most of the structs here are nice and generic, but Alpha and Digit aren't really. Every specification seems to have it's own definitions like which numeral systems (https://en.wikipedia.org/wiki/Numeral_system) are supported and which code points are legal or considered 'Alpha'. These structs do document what they do and are useful, it just makes me feel uneasy that the purely mechanical part is in the same header with these (which seem more part of a specific parser grammar). https://codereview.chromium.org/2271743002/diff/80001/include/core/SkTLazy.h File include/core/SkTLazy.h (right): https://codereview.chromium.org/2271743002/diff/80001/include/core/SkTLazy.h#... include/core/SkTLazy.h:103: T* fPtr; // nullptr or fStorage Should we just land this separately since it's more or less unrelated to the rest of this?
https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... File experimental/svg/model/SkPEG.h (right): https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... experimental/svg/model/SkPEG.h:183: for (;;) { On 2016/08/24 15:01:55, mtklein wrote: > Is this loop different from > > while (const auto m = E::Match(in)) { > in = m.fNext; > *values.append() = *m; > } > > ? Done. https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... experimental/svg/model/SkPEG.h:234: struct LIT<C, Cs...> { On 2016/08/24 15:04:20, bungeman-skia wrote: > I'm assuming this is for svg and possibly parsing bits of css. Those are defined > in terms of unicode code points and not 'char'. Is there a plan for that? pegtl > seems to have a fair amount of code devoted to turning various encodings into > code points. Good question, I haven't really thought much about this. The immediate plan is to use this for parsing SVG *attribute values*. In general, these seem to be well-behaved (as in Latin1) - e.g. stuff like https://www.w3.org/TR/SVG/types.html#DataTypeColor The only way I can think of for non-Latin1 chars to sneak into attribute values is via reference strings (fill="#什么他妈的"). But then again these are treated as opaque strings, and we're not going to attempt parsing them anyway. We also have a couple more layers sitting between the attribute parser and the source document: Expat & SkDOM. We use the former in UTF-8 mode, and the latter treats everything as 8bit/char strings - but it is mostly pass-through, so it prolly doesn't matter. So I guess we're currently getting fed UTF-8 strings, which seems fine as far as SVG attribute parsing is concerned. > For example these template parameters could be 'uint32_t' (or 'char32_t') > instead of 'char' which would work with the 'U' UCS-4 character literal. Of > course, the Match input would need to be decoded somehow as well. Yeah, off hand I think we could either a) parameterize everything on CharType (Match, LIT, ...?) b) keep using char, declare everything UTF-8, leave it up to the implementations to decode if/as needed. I'm inclined to stick with b) for now, as it seems sufficient for the immediate use case. WDYT? https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... experimental/svg/model/SkPEG.h:248: * Matches a letter (['a'-'z','A'-Z']) and returns the matched On 2016/08/24 15:04:20, bungeman-skia wrote: > nit: missing a ' before Z Done. https://codereview.chromium.org/2271743002/diff/80001/experimental/svg/model/... experimental/svg/model/SkPEG.h:269: struct Digit { On 2016/08/24 15:04:20, bungeman-skia wrote: > Most of the structs here are nice and generic, but Alpha and Digit aren't > really. Every specification seems to have it's own definitions like which > numeral systems (https://en.wikipedia.org/wiki/Numeral_system) are supported and > which code points are legal or considered 'Alpha'. These structs do document > what they do and are useful, it just makes me feel uneasy that the purely > mechanical part is in the same header with these (which seem more part of a > specific parser grammar). Good point. I was thinking it'd be helpful to provide some basic atoms, but yeah, if we remove these then we gain a lot in generality. Except for maybe LIT, no one even cares what the encoding is anymore. I like that. Moved to the test file. https://codereview.chromium.org/2271743002/diff/80001/include/core/SkTLazy.h File include/core/SkTLazy.h (right): https://codereview.chromium.org/2271743002/diff/80001/include/core/SkTLazy.h#... include/core/SkTLazy.h:103: T* fPtr; // nullptr or fStorage On 2016/08/24 15:04:20, bungeman-skia wrote: > Should we just land this separately since it's more or less unrelated to the > rest of this? SGTM, I'll upload a separate CL.
lgtm, don't want to stand in the way of coolness.
The CQ bit was checked by fmalita@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mtklein@google.com Link to the patchset: https://codereview.chromium.org/2271743002/#ps100001 (title: "moar review")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Ubuntu-GCC-x86_64-Release-GN-Trybot on master.client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)
The CQ bit was checked by fmalita@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bungeman@google.com, mtklein@google.com Link to the patchset: https://codereview.chromium.org/2271743002/#ps140001 (title: "drop inadvertent SkTLazy change")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Ubuntu-GCC-x86_64-Release-CMake-Trybot on master.client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-x86...)
The CQ bit was checked by fmalita@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 ========== to ========== Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 Committed: https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42
Message was sent while issue was closed.
On 2016/08/25 01:23:29, commit-bot: I haz the power wrote: > Committed patchset #8 (id:140001) as > https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 Looks like the Google3 roll needs some help.
Message was sent while issue was closed.
On 2016/08/25 12:47:24, mtklein wrote: > On 2016/08/25 01:23:29, commit-bot: I haz the power wrote: > > Committed patchset #8 (id:140001) as > > https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 > > Looks like the Google3 roll needs some help. Yup, Msan is not happy either - I'll revert while investigating.
Message was sent while issue was closed.
A revert of this CL (patchset #8 id:140001) has been created in https://codereview.chromium.org/2275943004/ by fmalita@chromium.org. The reason for reverting is: G3 roll & Msan woes..
Message was sent while issue was closed.
Description was changed from ========== Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 Committed: https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 ========== to ========== Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 Committed: https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 ==========
Fixes for reland: * use SkTArray (rather than SkTDArray, doh) for Any<>, to ensure proper struct initialization on append. * guard the unit test on SK_XML (kind of a big hammer, but the only signal we have for SVG availability ATM)
The CQ bit was checked by fmalita@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by fmalita@chromium.org
Description was changed from ========== Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 Committed: https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 ========== to ========== Reland: Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 Committed: https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 ==========
The CQ bit was checked by fmalita@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bungeman@google.com, mtklein@google.com Link to the patchset: https://codereview.chromium.org/2271743002/#ps160001 (title: "Fixes for MSan, g3 roll")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Reland: Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 Committed: https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 ========== to ========== Reland: Experimental parsing expression grammar (PEG) template library BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2271743002 Committed: https://skia.googlesource.com/skia/+/9d08cbc8c6131ff61a1e71cc5c8cf27841d62b42 Committed: https://skia.googlesource.com/skia/+/6cf896d7ce03b87b3a5595bc66caf0a34c993755 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as https://skia.googlesource.com/skia/+/6cf896d7ce03b87b3a5595bc66caf0a34c993755 |