Extended error reporting for SVG path parsing

Extended error reporting for SVG path parsing

Adds reporting of errors for the errors detected:

 * Missing starting moveto
 * Missing command verb
 * Unexpected input type (number, arc flag)

The parsing helper parseArcFlag() is adjusted to not consume any
character on error.

BUG=231612
Committed: https://crrev.com/0965f08da8b60d22c1c91668113b40800909e5ec
Cr-Commit-Position: refs/heads/master@{#371849}

[fs, 2016-01-27 12:50:34]

Description was changed from

==========
Extended error reporting for SVG path parsing

Adds reporting of errors for the errors detected:

 * Missing starting moveto
 * Missing command verb
 * Unexpected input types (number, arc flag)

The parsing helper parseArcFlag() is adjusted to not consume any
character on error.

BUG=231612
==========

to

==========
Extended error reporting for SVG path parsing

Adds reporting of errors for the errors detected:

 * Missing starting moveto
 * Missing command verb
 * Unexpected input type (number, arc flag)

The parsing helper parseArcFlag() is adjusted to not consume any
character on error.

BUG=231612
==========

[fs, 2016-01-27 14:23:17]

fs@opera.com changed reviewers:
+ fmalita@chromium.org, pdr@chromium.org

[f(malita), 2016-01-27 14:51:13]

lgtm

https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGParsingError.cpp (right):

https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGParsingError.cpp:32: return
std::make_pair("Expected path flag ('0' or '1'), ", ".");
Nit: "Expected arc flag"? Or maybe you're planning to use this for other flags
too - then maybe change the enum to ExpectedPathFlag?

https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGPathStringSource.cpp (right):

https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGPathStringSource.cpp:171: // only user is
the method mentioned above.
Yeah, it would be nice to get rid of this assumption.  You could even move the
check into initialCommandIsMoveTo() - but that requires a public setErrorMark
IIUC.  I'll leave it up to you (for a follow-up).

[fs, 2016-01-27 14:57:56]

https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGParsingError.cpp (right):

https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGParsingError.cpp:32: return
std::make_pair("Expected path flag ('0' or '1'), ", ".");
On 2016/01/27 at 14:51:13, f(malita) wrote:
> Nit: "Expected arc flag"? Or maybe you're planning to use this for other flags
too - then maybe change the enum to ExpectedPathFlag?

Yes, I think I had intended to say "arc flag" here... Will fix.

https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGPathStringSource.cpp (right):

https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGPathStringSource.cpp:171: // only user is
the method mentioned above.
On 2016/01/27 at 14:51:13, f(malita) wrote:
> Yeah, it would be nice to get rid of this assumption.  You could even move the
check into initialCommandIsMoveTo() - but that requires a public setErrorMark
IIUC.  I'll leave it up to you (for a follow-up).

I have some sort of (outline of a) plan for this [1] - didn't want to mix any
greater logic changes into this CL though.

[1] Which your recent CL(s) may have simplified a bit =)

[pdr., 2016-01-27 19:43:44]

On 2016/01/27 at 14:57:56, fs wrote:
>
https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/svg/SVGParsingError.cpp (right):
> 
>
https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/svg/SVGParsingError.cpp:32: return
std::make_pair("Expected path flag ('0' or '1'), ", ".");
> On 2016/01/27 at 14:51:13, f(malita) wrote:
> > Nit: "Expected arc flag"? Or maybe you're planning to use this for other
flags too - then maybe change the enum to ExpectedPathFlag?
> 
> Yes, I think I had intended to say "arc flag" here... Will fix.
> 
>
https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/svg/SVGPathStringSource.cpp (right):
> 
>
https://codereview.chromium.org/1642463004/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/svg/SVGPathStringSource.cpp:171: // only user
is the method mentioned above.
> On 2016/01/27 at 14:51:13, f(malita) wrote:
> > Yeah, it would be nice to get rid of this assumption.  You could even move
the check into initialCommandIsMoveTo() - but that requires a public
setErrorMark IIUC.  I'll leave it up to you (for a follow-up).
> 
> I have some sort of (outline of a) plan for this [1] - didn't want to mix any
greater logic changes into this CL though.
> 
> [1] Which your recent CL(s) may have simplified a bit =)

LGTM

[fs, 2016-01-27 19:48:00]

The CQ bit was checked by fs@opera.com

[fs, 2016-01-27 19:48:00]

The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org
Link to the patchset: https://codereview.chromium.org/1642463004/#ps20001
(title: ""path flag" -> "arc flag"")

[commit-bot: I haz the power, 2016-01-27 19:48:48]

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

[commit-bot: I haz the power, 2016-01-27 19:56:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-27 19:58:26]

Description was changed from

==========
Extended error reporting for SVG path parsing

Adds reporting of errors for the errors detected:

 * Missing starting moveto
 * Missing command verb
 * Unexpected input type (number, arc flag)

The parsing helper parseArcFlag() is adjusted to not consume any
character on error.

BUG=231612
==========

to

==========
Extended error reporting for SVG path parsing

Adds reporting of errors for the errors detected:

 * Missing starting moveto
 * Missing command verb
 * Unexpected input type (number, arc flag)

The parsing helper parseArcFlag() is adjusted to not consume any
character on error.

BUG=231612

Committed: https://crrev.com/0965f08da8b60d22c1c91668113b40800909e5ec
Cr-Commit-Position: refs/heads/master@{#371849}
==========

[commit-bot: I haz the power, 2016-01-27 19:58:27]

Patchset 2 (id:??) landed as
https://crrev.com/0965f08da8b60d22c1c91668113b40800909e5ec
Cr-Commit-Position: refs/heads/master@{#371849}