Refactor propagation of parsing errors for SVG attributes

Refactor propagation of parsing errors for SVG attributes

Use the return value of SVG{...}::setValueAsString to signal errors
rather than an out parameter and the (mostly faux) ExceptionState
object (for setBaseValueAsString). In the few cases where the latter
is not using an TrackExceptionState - in tear-offs for SVGAngle and
SVGLength - it's easy enough to handle the exception-throwing there
and then.
This makes SVG{...} objects mostly independent of ExceptionState,
saving on footprint from string-construction and argument passing as
a side-effect.
Also remove some unnecessary virtuals on SVGInteger and
SVGPreserveAspectRatio.

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

[fs, 2015-12-22 15:21:11]

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

[fs, 2015-12-22 15:59:08]

Description was changed from

==========
Refactor propagation of parsing errors for SVG attributes

Use the return value of SVG{...}::setValueAsString to signal errors
rather than the (mostly faux) ExceptionState object. In the few cases
where said object is not an TrackExceptionState - in tear-offs for
SVGAngle and SVGLength - it's easy enough to handle the exception-
throwing there and then.
This makes SVG{...} objects mostly independent of ExceptionState,
saving on footprint from string-construction and argument passing as
a side-effect.
Also remove some unnecessary virtuals on SVGInteger and
SVGPreserveAspectRatio.

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

to

==========
Refactor propagation of parsing errors for SVG attributes

Use the return value of SVG{...}::setValueAsString to signal errors
rather than an out parameter and the (mostly faux) ExceptionState
object (for setBaseValueAsString). In the few cases where the latter
is not using an TrackExceptionState - in tear-offs for SVGAngle and
SVGLength - it's easy enough to handle the exception-throwing there
and then.
This makes SVG{...} objects mostly independent of ExceptionState,
saving on footprint from string-construction and argument passing as
a side-effect.
Also remove some unnecessary virtuals on SVGInteger and
SVGPreserveAspectRatio.

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

[pdr., 2015-12-23 04:00:07]

The CQ bit was checked by pdr@chromium.org

[pdr., 2015-12-23 04:00:08]

lgtm

LGTM

https://codereview.chromium.org/1544673003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp (left):

https://codereview.chromium.org/1544673003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp:120:
target()->setValueAsString(oldValue, ASSERT_NO_EXCEPTION); // rollback to old
value
Is this rollback behavior spec'd? Not a big deal either way, just curious
because I don't see it in 1.1.

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

https://codereview.chromium.org/1544673003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGNumber.cpp:88: m_value = 0;
Nit for contemplation: this isn't required, but we're not very consistent with
where the state is set (in parse vs in set).

[commit-bot: I haz the power, 2015-12-23 04:00:17]

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

[commit-bot: I haz the power, 2015-12-23 06:25:42]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-12-23 06:27:08]

Description was changed from

==========
Refactor propagation of parsing errors for SVG attributes

Use the return value of SVG{...}::setValueAsString to signal errors
rather than an out parameter and the (mostly faux) ExceptionState
object (for setBaseValueAsString). In the few cases where the latter
is not using an TrackExceptionState - in tear-offs for SVGAngle and
SVGLength - it's easy enough to handle the exception-throwing there
and then.
This makes SVG{...} objects mostly independent of ExceptionState,
saving on footprint from string-construction and argument passing as
a side-effect.
Also remove some unnecessary virtuals on SVGInteger and
SVGPreserveAspectRatio.

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

to

==========
Refactor propagation of parsing errors for SVG attributes

Use the return value of SVG{...}::setValueAsString to signal errors
rather than an out parameter and the (mostly faux) ExceptionState
object (for setBaseValueAsString). In the few cases where the latter
is not using an TrackExceptionState - in tear-offs for SVGAngle and
SVGLength - it's easy enough to handle the exception-throwing there
and then.
This makes SVG{...} objects mostly independent of ExceptionState,
saving on footprint from string-construction and argument passing as
a side-effect.
Also remove some unnecessary virtuals on SVGInteger and
SVGPreserveAspectRatio.

BUG=231612

Committed: https://crrev.com/bf2f7bc27f6baa44286fea07e39540c3ac8db77d
Cr-Commit-Position: refs/heads/master@{#366715}
==========

[commit-bot: I haz the power, 2015-12-23 06:27:09]

Patchset 1 (id:??) landed as
https://crrev.com/bf2f7bc27f6baa44286fea07e39540c3ac8db77d
Cr-Commit-Position: refs/heads/master@{#366715}

[fs, 2015-12-23 10:04:17]

https://codereview.chromium.org/1544673003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp (left):

https://codereview.chromium.org/1544673003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp:120:
target()->setValueAsString(oldValue, ASSERT_NO_EXCEPTION); // rollback to old
value
On 2015/12/23 at 04:00:08, pdr-slow reviews thru new year wrote:
> Is this rollback behavior spec'd? Not a big deal either way, just curious
because I don't see it in 1.1.

As I recall this was subject of discussion in the WG [1] and only applies to the
cases where "new" units are used (as is the case here). I'll see if I can dig up
something.

[1] citation needed...

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

https://codereview.chromium.org/1544673003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGNumber.cpp:88: m_value = 0;
On 2015/12/23 at 04:00:08, pdr-slow reviews thru new year wrote:
> Nit for contemplation: this isn't required, but we're not very consistent with
where the state is set (in parse vs in set).

Yes, I've been noticing these things here and there as I've gone through all of
these... I have done some cleanup here and there. Will make another sweep with
this in mind.

[fs, 2015-12-23 12:54:47]

https://codereview.chromium.org/1544673003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp (left):

https://codereview.chromium.org/1544673003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp:120:
target()->setValueAsString(oldValue, ASSERT_NO_EXCEPTION); // rollback to old
value
On 2015/12/23 at 10:04:17, fs wrote:
> On 2015/12/23 at 04:00:08, pdr-slow reviews thru new year wrote:
> > Is this rollback behavior spec'd? Not a big deal either way, just curious
because I don't see it in 1.1.
> 
> As I recall this was subject of discussion in the WG [1] and only applies to
the cases where "new" units are used (as is the case here). I'll see if I can
dig up something.
> 
> [1] citation needed...

After some mild digging I found:

http://www.w3.org/2014/06/12-svg-minutes.html#item01
https://svgwg.org/svg2-draft/types.html#InterfaceSVGLength (or SVGAngle; the
note after the list of unit enums)

The former refers to a previous resolution which I haven't dug up. As for the
spec bits I don't see that this behavior ended up as specified. (I only see it
being handled for unitType.)