Tighten error checking from SVGLength parsing

Tighten error checking from SVGLength parsing

When test-parsing width, height before mapping them to style for
SVGSVGElement and SVGForeignObjectElement, testing for unknown length
types is not enough, we must check the exceptionState for errors. This
fixes a case where we apply 'inherit' even though we shouldn't,
according to spec.

Also align SVGForeignObjectElement and SVGSVGElement by always passing
along the original string, not the serialized value. As long as only
valid lengths are passed along, this shouldn't change behavior but
re-generating the string back is unnecessary work.

BUG=370017, 308992
R=pdr

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173371

[pdr., 2014-05-05 16:53:11]

LGTM with a nit

https://codereview.chromium.org/262093007/diff/1/LayoutTests/svg/custom/disal...
File LayoutTests/svg/custom/disallow-non-lengths-in-attrs.html (right):

https://codereview.chromium.org/262093007/diff/1/LayoutTests/svg/custom/disal...
LayoutTests/svg/custom/disallow-non-lengths-in-attrs.html:34: setWidth('#fo',
invalid_width);
This won't match #fO above.

[rwlbuis, 2014-05-05 18:15:50]

I had the below question. Also I think your last sentence has a mistake near "it
re-generating".

https://codereview.chromium.org/262093007/diff/1/Source/core/svg/SVGSVGElemen...
File Source/core/svg/SVGSVGElement.cpp (right):

https://codereview.chromium.org/262093007/diff/1/Source/core/svg/SVGSVGElemen...
Source/core/svg/SVGSVGElement.cpp:275: if (!exceptionState.hadException()) {
Is the exception actually thrown/printed? If not should it be printed as
warning?

[davve, 2014-05-06 07:05:19]

https://codereview.chromium.org/262093007/diff/1/LayoutTests/svg/custom/disal...
File LayoutTests/svg/custom/disallow-non-lengths-in-attrs.html (right):

https://codereview.chromium.org/262093007/diff/1/LayoutTests/svg/custom/disal...
LayoutTests/svg/custom/disallow-non-lengths-in-attrs.html:34: setWidth('#fo',
invalid_width);
On 2014/05/05 16:53:11, pdr wrote:
> This won't match #fO above.

Good eyes!

(I wish I could use querySelector('foreignobject'), but I guess that is
<https://code.google.com/p/chromium/issues/detail?id=237435>)

https://codereview.chromium.org/262093007/diff/1/Source/core/svg/SVGSVGElemen...
File Source/core/svg/SVGSVGElement.cpp (right):

https://codereview.chromium.org/262093007/diff/1/Source/core/svg/SVGSVGElemen...
Source/core/svg/SVGSVGElement.cpp:275: if (!exceptionState.hadException()) {
On 2014/05/05 18:15:51, rwlbuis wrote:
> Is the exception actually thrown/printed? If not should it be printed as
> warning?

It's already printed when the attribute is parsed, in
SVGSVGElement::parseAttribute(). Everything that's not parsable as a length is
reported as error there.

[davve, 2014-05-06 07:05:34]

The CQ bit was checked by davve@opera.com

[commit-bot: I haz the power, 2014-05-06 07:05:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/davve@opera.com/262093007/20001

[commit-bot: I haz the power, 2014-05-06 08:16:53]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_blink_rel on tryserver.blink

[davve, 2014-05-06 08:47:52]

The CQ bit was checked by davve@opera.com

[commit-bot: I haz the power, 2014-05-06 08:48:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/davve@opera.com/262093007/40001

[commit-bot: I haz the power, 2014-05-06 09:56:08]

Change committed as 173371