diff options
author | fs <fs@opera.com> | 2016-01-20 12:40:25 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-01-20 20:41:40 +0000 |
commit | 1611c9b2863c1ba9cd3067186090e8917a8a23c3 (patch) | |
tree | 5d2a404138ffec7950455b1f4709718e4d6247af | |
parent | 89b8559b921b3a523dea4c7002cb79e2531579c3 (diff) | |
download | chromium_src-1611c9b2863c1ba9cd3067186090e8917a8a23c3.zip chromium_src-1611c9b2863c1ba9cd3067186090e8917a8a23c3.tar.gz chromium_src-1611c9b2863c1ba9cd3067186090e8917a8a23c3.tar.bz2 |
Extended error reporting for SVG attribute parsing
This CL extends the SVG attribute parsing error reporting functionality
to allow more precise (and hopefully helpful) reporting.
The main improvements consist of:
1) More (precise) status codes
Avoids generic error messages.
2) Locus support
Allows reducing the amount of context, which should make it easier
to pinpoint the actual error. (Preferably the offending character
should be highlighted somehow in the error message, but that is
left as future work.)
To achieve this, the SVGParsingError enumeration is turned into a
thin wrapper class around a status code and a locus. The status codes
move to a new enumeration 'SVGStatus'.
Formatting of error messages are moved out of
SVGElement::reportAttributeParsingError and into SVGParsingError.cpp
(new file).
This CL start adding extended reporting to a few of the value classes:
SVGBoolean, SVGEnumeration and SVGPreserverAspectRatio; to illustrate
the mechanism. Further value classes will be annotated in later CLs.
For that reason the "generic" errors are kept in their current form - to
be removed as more value class parsers get converted.
BUG=231612
Review URL: https://codereview.chromium.org/1588993005
Cr-Commit-Position: refs/heads/master@{#370479}
33 files changed, 276 insertions, 117 deletions
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 474930b..3e0caac 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations @@ -45,6 +45,14 @@ crbug.com/527242 virtual/spv2/paint/invalidation/spv2/cached-change-tbody-border crbug.com/536138 virtual/spv2/paint/invalidation/repaint-subsequence-on-ancestor-clip-change.html [ Failure ] +crbug.com/231612 svg/css/parse-length.html [ NeedsRebaseline ] +crbug.com/231612 svg/custom/preserve-aspect-ratio-syntax.svg [ NeedsRebaseline ] +crbug.com/231612 svg/custom/viewbox-syntax.svg [ NeedsRebaseline ] +crbug.com/231612 svg/dom/preserve-aspect-ratio-parser.html [ NeedsRebaseline ] +crbug.com/231612 svg/dom/svgpath-negative-pathLength.html [ NeedsRebaseline ] +crbug.com/231612 svg/hixie/error/007.xml [ NeedsRebaseline ] +crbug.com/231612 svg/hixie/error/013.xml [ NeedsRebaseline ] + crbug.com/537172 [ Mac10.6 XP Win10 ] virtual/spv2/paint/invalidation/spv2/background-image-paint-invalidation.html [ Failure ] crbug.com/564443 [ Mac10.7 Mac10.8 Mac10.9 Mac10.10 Win7 Linux ] virtual/spv2/paint/invalidation/spv2/background-image-paint-invalidation.html [ Failure Pass ] diff --git a/third_party/WebKit/Source/core/core.gypi b/third_party/WebKit/Source/core/core.gypi index dc1dbe4..bccc91c 100644 --- a/third_party/WebKit/Source/core/core.gypi +++ b/third_party/WebKit/Source/core/core.gypi @@ -3477,6 +3477,7 @@ 'svg/SVGNumberTearOff.h', 'svg/SVGParserUtilities.cpp', 'svg/SVGParserUtilities.h', + 'svg/SVGParsingError.cpp', 'svg/SVGParsingError.h', 'svg/SVGPath.cpp', 'svg/SVGPath.h', diff --git a/third_party/WebKit/Source/core/svg/SVGAngle.cpp b/third_party/WebKit/Source/core/svg/SVGAngle.cpp index 282f645..8cfd6e5 100644 --- a/third_party/WebKit/Source/core/svg/SVGAngle.cpp +++ b/third_party/WebKit/Source/core/svg/SVGAngle.cpp @@ -237,18 +237,18 @@ SVGParsingError SVGAngle::setValueAsString(const String& value) { if (value.isEmpty()) { newValueSpecifiedUnits(SVG_ANGLETYPE_UNSPECIFIED, 0); - return NoError; + return SVGParseStatus::NoError; } if (value == "auto") { newValueSpecifiedUnits(SVG_ANGLETYPE_UNSPECIFIED, 0); m_orientType->setEnumValue(SVGMarkerOrientAuto); - return NoError; + return SVGParseStatus::NoError; } if (value == "auto-start-reverse") { newValueSpecifiedUnits(SVG_ANGLETYPE_UNSPECIFIED, 0); m_orientType->setEnumValue(SVGMarkerOrientAutoStartReverse); - return NoError; + return SVGParseStatus::NoError; } float valueInSpecifiedUnits = 0; @@ -257,12 +257,12 @@ SVGParsingError SVGAngle::setValueAsString(const String& value) bool success = value.is8Bit() ? parseValue<LChar>(value, valueInSpecifiedUnits, unitType) : parseValue<UChar>(value, valueInSpecifiedUnits, unitType); if (!success) - return ParsingAttributeFailedError; + return SVGParseStatus::ParsingFailed; m_orientType->setEnumValue(SVGMarkerOrientAngle); m_unitType = unitType; m_valueInSpecifiedUnits = valueInSpecifiedUnits; - return NoError; + return SVGParseStatus::NoError; } void SVGAngle::newValueSpecifiedUnits(SVGAngleType unitType, float valueInSpecifiedUnits) diff --git a/third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp b/third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp index ee043be..0125235 100644 --- a/third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp +++ b/third_party/WebKit/Source/core/svg/SVGAngleTearOff.cpp @@ -116,11 +116,11 @@ void SVGAngleTearOff::setValueAsString(const String& value, ExceptionState& exce SVGParsingError status = target()->setValueAsString(value); - if (status == NoError && !hasExposedAngleUnit()) { + if (status == SVGParseStatus::NoError && !hasExposedAngleUnit()) { target()->setValueAsString(oldValue); // rollback to old value - status = ParsingAttributeFailedError; + status = SVGParseStatus::ParsingFailed; } - if (status != NoError) { + if (status != SVGParseStatus::NoError) { exceptionState.throwDOMException(SyntaxError, "The value provided ('" + value + "') is invalid."); return; } diff --git a/third_party/WebKit/Source/core/svg/SVGAnimatedLength.cpp b/third_party/WebKit/Source/core/svg/SVGAnimatedLength.cpp index 7a2b26a..9004c9d 100644 --- a/third_party/WebKit/Source/core/svg/SVGAnimatedLength.cpp +++ b/third_party/WebKit/Source/core/svg/SVGAnimatedLength.cpp @@ -43,10 +43,10 @@ SVGParsingError SVGAnimatedLength::setBaseValueAsString(const String& value) { SVGParsingError parseStatus = baseValue()->setValueAsString(value); - if (parseStatus != NoError) + if (parseStatus != SVGParseStatus::NoError) baseValue()->newValueSpecifiedUnits(CSSPrimitiveValue::UnitType::UserUnits, 0); else if (SVGLength::negativeValuesForbiddenForAnimatedLengthAttribute(attributeName()) && baseValue()->valueInSpecifiedUnits() < 0) - parseStatus = NegativeValueForbiddenError; + parseStatus = SVGParseStatus::NegativeValue; return parseStatus; } diff --git a/third_party/WebKit/Source/core/svg/SVGAnimationElement.cpp b/third_party/WebKit/Source/core/svg/SVGAnimationElement.cpp index e16b94c..3c60008 100644 --- a/third_party/WebKit/Source/core/svg/SVGAnimationElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGAnimationElement.cpp @@ -163,7 +163,7 @@ void SVGAnimationElement::parseAttribute(const QualifiedName& name, const Atomic { if (name == SVGNames::valuesAttr) { if (!parseValues(value, m_values)) { - reportAttributeParsingError(ParsingAttributeFailedError, name, value); + reportAttributeParsingError(SVGParseStatus::ParsingFailed, name, value); return; } updateAnimationMode(); @@ -172,7 +172,7 @@ void SVGAnimationElement::parseAttribute(const QualifiedName& name, const Atomic if (name == SVGNames::keyTimesAttr) { if (!parseKeyTimes(value, m_keyTimes, true)) - reportAttributeParsingError(ParsingAttributeFailedError, name, value); + reportAttributeParsingError(SVGParseStatus::ParsingFailed, name, value); return; } @@ -181,14 +181,14 @@ void SVGAnimationElement::parseAttribute(const QualifiedName& name, const Atomic // This is specified to be an animateMotion attribute only but it is simpler to put it here // where the other timing calculatations are. if (!parseKeyTimes(value, m_keyPoints, false)) - reportAttributeParsingError(ParsingAttributeFailedError, name, value); + reportAttributeParsingError(SVGParseStatus::ParsingFailed, name, value); } return; } if (name == SVGNames::keySplinesAttr) { if (!parseKeySplines(value, m_keySplines)) - reportAttributeParsingError(ParsingAttributeFailedError, name, value); + reportAttributeParsingError(SVGParseStatus::ParsingFailed, name, value); return; } diff --git a/third_party/WebKit/Source/core/svg/SVGBoolean.cpp b/third_party/WebKit/Source/core/svg/SVGBoolean.cpp index 5af4bd3..4ad5570 100644 --- a/third_party/WebKit/Source/core/svg/SVGBoolean.cpp +++ b/third_party/WebKit/Source/core/svg/SVGBoolean.cpp @@ -43,13 +43,13 @@ SVGParsingError SVGBoolean::setValueAsString(const String& value) { if (value == "true") { m_value = true; - return NoError; + return SVGParseStatus::NoError; } if (value == "false") { m_value = false; - return NoError; + return SVGParseStatus::NoError; } - return ParsingAttributeFailedError; + return SVGParseStatus::ExpectedBoolean; } void SVGBoolean::add(PassRefPtrWillBeRawPtr<SVGPropertyBase>, SVGElement*) diff --git a/third_party/WebKit/Source/core/svg/SVGElement.cpp b/third_party/WebKit/Source/core/svg/SVGElement.cpp index ff9b791..22a3cb8 100644 --- a/third_party/WebKit/Source/core/svg/SVGElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGElement.cpp @@ -52,7 +52,6 @@ #include "core/svg/SVGTitleElement.h" #include "core/svg/SVGUseElement.h" #include "core/svg/properties/SVGProperty.h" -#include "platform/JSONValues.h" #include "wtf/TemporaryChange.h" #include "wtf/Threading.h" @@ -188,23 +187,9 @@ bool SVGElement::isOutermostSVGSVGElement() const void SVGElement::reportAttributeParsingError(SVGParsingError error, const QualifiedName& name, const AtomicString& value) { - if (error == NoError) + if (error == SVGParseStatus::NoError) return; - - String errorString = "<" + tagName() + "> attribute " + name.toString() + "=" + JSONValue::quoteString(value); - SVGDocumentExtensions& extensions = document().accessSVGExtensions(); - - if (error == NegativeValueForbiddenError) { - extensions.reportError("Invalid negative value for " + errorString); - return; - } - - if (error == ParsingAttributeFailedError) { - extensions.reportError("Invalid value for " + errorString); - return; - } - - ASSERT_NOT_REACHED(); + document().accessSVGExtensions().reportError(error.format(tagName(), name, value)); } String SVGElement::title() const diff --git a/third_party/WebKit/Source/core/svg/SVGEnumeration.cpp b/third_party/WebKit/Source/core/svg/SVGEnumeration.cpp index ed579ac..1bea312 100644 --- a/third_party/WebKit/Source/core/svg/SVGEnumeration.cpp +++ b/third_party/WebKit/Source/core/svg/SVGEnumeration.cpp @@ -72,12 +72,12 @@ SVGParsingError SVGEnumerationBase::setValueAsString(const String& string) ASSERT(entry.first); m_value = entry.first; notifyChange(); - return NoError; + return SVGParseStatus::NoError; } } notifyChange(); - return ParsingAttributeFailedError; + return SVGParseStatus::ExpectedEnumeration; } void SVGEnumerationBase::add(PassRefPtrWillBeRawPtr<SVGPropertyBase>, SVGElement*) diff --git a/third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp b/third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp index 4f7ecf2..f683bf2 100644 --- a/third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGFEConvolveMatrixElement.cpp @@ -63,7 +63,7 @@ SVGParsingError SVGAnimatedOrder::setBaseValueAsString(const String& value) SVGParsingError parseStatus = SVGAnimatedIntegerOptionalInteger::setBaseValueAsString(value); ASSERT(contextElement()); - if (parseStatus == NoError && (firstInteger()->baseValue()->value() < 1 || secondInteger()->baseValue()->value() < 1)) { + if (parseStatus == SVGParseStatus::NoError && (firstInteger()->baseValue()->value() < 1 || secondInteger()->baseValue()->value() < 1)) { contextElement()->document().accessSVGExtensions().reportWarning( "feConvolveMatrix: problem parsing order=\"" + value + "\"."); } diff --git a/third_party/WebKit/Source/core/svg/SVGFitToViewBox.cpp b/third_party/WebKit/Source/core/svg/SVGFitToViewBox.cpp index 9346b98..932ab30 100644 --- a/third_party/WebKit/Source/core/svg/SVGFitToViewBox.cpp +++ b/third_party/WebKit/Source/core/svg/SVGFitToViewBox.cpp @@ -51,8 +51,8 @@ SVGParsingError SVGAnimatedViewBoxRect::setBaseValueAsString(const String& value { SVGParsingError parseStatus = baseValue()->setValueAsString(value); - if (parseStatus == NoError && (baseValue()->width() < 0 || baseValue()->height() < 0)) { - parseStatus = NegativeValueForbiddenError; + if (parseStatus == SVGParseStatus::NoError && (baseValue()->width() < 0 || baseValue()->height() < 0)) { + parseStatus = SVGParseStatus::NegativeValue; baseValue()->setInvalid(); } return parseStatus; diff --git a/third_party/WebKit/Source/core/svg/SVGInteger.cpp b/third_party/WebKit/Source/core/svg/SVGInteger.cpp index 8ccddc5..5587629 100644 --- a/third_party/WebKit/Source/core/svg/SVGInteger.cpp +++ b/third_party/WebKit/Source/core/svg/SVGInteger.cpp @@ -54,7 +54,7 @@ SVGParsingError SVGInteger::setValueAsString(const String& string) { if (string.isEmpty()) { m_value = 0; - return NoError; + return SVGParseStatus::NoError; } bool valid = true; @@ -62,9 +62,9 @@ SVGParsingError SVGInteger::setValueAsString(const String& string) if (!valid) { m_value = 0; - return ParsingAttributeFailedError; + return SVGParseStatus::ParsingFailed; } - return NoError; + return SVGParseStatus::NoError; } void SVGInteger::add(PassRefPtrWillBeRawPtr<SVGPropertyBase> other, SVGElement*) diff --git a/third_party/WebKit/Source/core/svg/SVGIntegerOptionalInteger.cpp b/third_party/WebKit/Source/core/svg/SVGIntegerOptionalInteger.cpp index b064168..689c2c0 100644 --- a/third_party/WebKit/Source/core/svg/SVGIntegerOptionalInteger.cpp +++ b/third_party/WebKit/Source/core/svg/SVGIntegerOptionalInteger.cpp @@ -79,9 +79,9 @@ String SVGIntegerOptionalInteger::valueAsString() const SVGParsingError SVGIntegerOptionalInteger::setValueAsString(const String& value) { float x, y; - SVGParsingError parseStatus = NoError; + SVGParsingError parseStatus; if (!parseNumberOptionalNumber(value, x, y)) { - parseStatus = ParsingAttributeFailedError; + parseStatus = SVGParseStatus::ParsingFailed; x = y = 0; } diff --git a/third_party/WebKit/Source/core/svg/SVGLength.cpp b/third_party/WebKit/Source/core/svg/SVGLength.cpp index 28419a4..98bc6db 100644 --- a/third_party/WebKit/Source/core/svg/SVGLength.cpp +++ b/third_party/WebKit/Source/core/svg/SVGLength.cpp @@ -63,8 +63,7 @@ PassRefPtrWillBeRawPtr<SVGPropertyBase> SVGLength::cloneForAnimation(const Strin RefPtrWillBeRawPtr<SVGLength> length = create(); length->m_unitMode = m_unitMode; - SVGParsingError status = length->setValueAsString(value); - if (status != NoError) + if (length->setValueAsString(value) != SVGParseStatus::NoError) length->m_value = cssValuePool().createValue(0, CSSPrimitiveValue::UnitType::UserUnits); return length.release(); @@ -137,21 +136,21 @@ SVGParsingError SVGLength::setValueAsString(const String& string) { if (string.isEmpty()) { m_value = cssValuePool().createValue(0, CSSPrimitiveValue::UnitType::UserUnits); - return NoError; + return SVGParseStatus::NoError; } CSSParserContext svgParserContext(SVGAttributeMode, 0); RefPtrWillBeRawPtr<CSSValue> parsed = CSSParser::parseSingleValue(CSSPropertyX, string, svgParserContext); if (!parsed || !parsed->isPrimitiveValue()) - return ParsingAttributeFailedError; + return SVGParseStatus::ParsingFailed; CSSPrimitiveValue* newValue = toCSSPrimitiveValue(parsed.get()); // TODO(fs): Enable calc for SVG lengths if (newValue->isCalculated() || !isSupportedCSSUnitType(newValue->typeWithCalcResolved())) - return ParsingAttributeFailedError; + return SVGParseStatus::ParsingFailed; m_value = newValue; - return NoError; + return SVGParseStatus::NoError; } String SVGLength::valueAsString() const diff --git a/third_party/WebKit/Source/core/svg/SVGLengthList.cpp b/third_party/WebKit/Source/core/svg/SVGLengthList.cpp index 433d9ae..27134ab 100644 --- a/third_party/WebKit/Source/core/svg/SVGLengthList.cpp +++ b/third_party/WebKit/Source/core/svg/SVGLengthList.cpp @@ -84,19 +84,19 @@ SVGParsingError SVGLengthList::parseInternal(const CharType*& ptr, const CharTyp RefPtrWillBeRawPtr<SVGLength> length = SVGLength::create(m_mode); SVGParsingError lengthParseStatus = length->setValueAsString(valueString); - if (lengthParseStatus != NoError) + if (lengthParseStatus != SVGParseStatus::NoError) return lengthParseStatus; append(length); skipOptionalSVGSpacesOrDelimiter(ptr, end); } - return NoError; + return SVGParseStatus::NoError; } SVGParsingError SVGLengthList::setValueAsString(const String& value) { if (value.isEmpty()) { clear(); - return NoError; + return SVGParseStatus::NoError; } SVGParsingError parseStatus; diff --git a/third_party/WebKit/Source/core/svg/SVGLengthTearOff.cpp b/third_party/WebKit/Source/core/svg/SVGLengthTearOff.cpp index 4b99836..caa2591 100644 --- a/third_party/WebKit/Source/core/svg/SVGLengthTearOff.cpp +++ b/third_party/WebKit/Source/core/svg/SVGLengthTearOff.cpp @@ -158,11 +158,11 @@ void SVGLengthTearOff::setValueAsString(const String& str, ExceptionState& excep SVGParsingError status = target()->setValueAsString(str); - if (status == NoError && !hasExposedLengthUnit()) { + if (status == SVGParseStatus::NoError && !hasExposedLengthUnit()) { target()->setValueAsString(oldValue); // rollback to old value - status = ParsingAttributeFailedError; + status = SVGParseStatus::ParsingFailed; } - if (status != NoError) { + if (status != SVGParseStatus::NoError) { exceptionState.throwDOMException(SyntaxError, "The value provided ('" + str + "') is invalid."); return; } diff --git a/third_party/WebKit/Source/core/svg/SVGNumber.cpp b/third_party/WebKit/Source/core/svg/SVGNumber.cpp index a20b364..7bf3879 100644 --- a/third_party/WebKit/Source/core/svg/SVGNumber.cpp +++ b/third_party/WebKit/Source/core/svg/SVGNumber.cpp @@ -70,7 +70,7 @@ SVGParsingError SVGNumber::setValueAsString(const String& string) { if (string.isEmpty()) { m_value = 0; - return NoError; + return SVGParseStatus::NoError; } bool valid = false; @@ -86,9 +86,9 @@ SVGParsingError SVGNumber::setValueAsString(const String& string) if (!valid) { m_value = 0; - return ParsingAttributeFailedError; + return SVGParseStatus::ParsingFailed; } - return NoError; + return SVGParseStatus::NoError; } void SVGNumber::add(PassRefPtrWillBeRawPtr<SVGPropertyBase> other, SVGElement*) @@ -120,10 +120,10 @@ PassRefPtrWillBeRawPtr<SVGNumber> SVGNumberAcceptPercentage::clone() const SVGParsingError SVGNumberAcceptPercentage::setValueAsString(const String& string) { if (parseNumberOrPercentage(string, m_value)) - return NoError; + return SVGParseStatus::NoError; m_value = 0; - return ParsingAttributeFailedError; + return SVGParseStatus::ParsingFailed; } SVGNumberAcceptPercentage::SVGNumberAcceptPercentage(float value) diff --git a/third_party/WebKit/Source/core/svg/SVGNumberList.cpp b/third_party/WebKit/Source/core/svg/SVGNumberList.cpp index 03f74df..a701cef 100644 --- a/third_party/WebKit/Source/core/svg/SVGNumberList.cpp +++ b/third_party/WebKit/Source/core/svg/SVGNumberList.cpp @@ -73,7 +73,7 @@ SVGParsingError SVGNumberList::setValueAsString(const String& value) { if (value.isEmpty()) { clear(); - return NoError; + return SVGParseStatus::NoError; } bool valid = false; @@ -90,9 +90,9 @@ SVGParsingError SVGNumberList::setValueAsString(const String& value) if (!valid) { // No call to |clear()| here. SVG policy is to use valid items before error. // Spec: http://www.w3.org/TR/SVG/single-page.html#implnote-ErrorProcessing - return ParsingAttributeFailedError; + return SVGParseStatus::ParsingFailed; } - return NoError; + return SVGParseStatus::NoError; } void SVGNumberList::add(PassRefPtrWillBeRawPtr<SVGPropertyBase> other, SVGElement* contextElement) diff --git a/third_party/WebKit/Source/core/svg/SVGNumberOptionalNumber.cpp b/third_party/WebKit/Source/core/svg/SVGNumberOptionalNumber.cpp index 6e90076..b5c95f2 100644 --- a/third_party/WebKit/Source/core/svg/SVGNumberOptionalNumber.cpp +++ b/third_party/WebKit/Source/core/svg/SVGNumberOptionalNumber.cpp @@ -76,9 +76,9 @@ String SVGNumberOptionalNumber::valueAsString() const SVGParsingError SVGNumberOptionalNumber::setValueAsString(const String& value) { float x, y; - SVGParsingError parseStatus = NoError; + SVGParsingError parseStatus; if (!parseNumberOptionalNumber(value, x, y)) { - parseStatus = ParsingAttributeFailedError; + parseStatus = SVGParseStatus::ParsingFailed; x = y = 0; } diff --git a/third_party/WebKit/Source/core/svg/SVGParsingError.cpp b/third_party/WebKit/Source/core/svg/SVGParsingError.cpp new file mode 100644 index 0000000..32472a0 --- /dev/null +++ b/third_party/WebKit/Source/core/svg/SVGParsingError.cpp @@ -0,0 +1,105 @@ +// Copyright 2016 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/svg/SVGParsingError.h" + +#include "core/dom/QualifiedName.h" +#include "platform/JSONValues.h" +#include "wtf/text/CharacterNames.h" +#include "wtf/text/StringBuilder.h" + +#include <utility> + +namespace blink { + +namespace { + +void appendErrorContextInfo(StringBuilder& builder, const String& tagName, const QualifiedName& name) +{ + builder.append('<'); + builder.append(tagName); + builder.appendLiteral("> attribute "); + builder.append(name.toString()); +} + +std::pair<const char*, const char*> messageForStatus(SVGParseStatus status) +{ + switch (status) { + case SVGParseStatus::TrailingGarbage: + return std::make_pair("Trailing garbage, ", "."); + case SVGParseStatus::ExpectedBoolean: + return std::make_pair("Expected 'true' or 'false', ", "."); + case SVGParseStatus::ExpectedEnumeration: + return std::make_pair("Unrecognized enumerated value, ", "."); + case SVGParseStatus::NegativeValue: + return std::make_pair("A negative value is not valid. (", ")"); + case SVGParseStatus::ParsingFailed: + default: + ASSERT_NOT_REACHED(); + break; + } + return std::make_pair("", ""); +} + +bool disableLocus(SVGParseStatus status) +{ + // Disable locus for semantic errors and generic errors (see TODO below). + return status == SVGParseStatus::NegativeValue + || status == SVGParseStatus::ParsingFailed; +} + +void appendValue(StringBuilder& builder, SVGParsingError error, const AtomicString& value) +{ + builder.append('"'); + if (!error.hasLocus() || disableLocus(error.status())) { + escapeStringForJSON(value.string(), &builder); + } else { + // Emit a string on the form: '"[...]<before><after>[...]"' + unsigned locus = error.locus(); + ASSERT(locus <= value.length()); + + // Amount of context to show before/after the error. + const unsigned kContext = 16; + + unsigned contextStart = std::max(locus, kContext) - kContext; + unsigned contextEnd = std::min(locus + kContext, value.length()); + ASSERT(contextStart <= contextEnd); + ASSERT(contextEnd <= value.length()); + if (contextStart != 0) + builder.append(horizontalEllipsisCharacter); + escapeStringForJSON(value.string().substring(contextStart, contextEnd - contextStart), &builder); + if (contextEnd != value.length()) + builder.append(horizontalEllipsisCharacter); + } + builder.append('"'); +} + +} + +String SVGParsingError::format(const String& tagName, const QualifiedName& name, const AtomicString& value) const +{ + StringBuilder builder; + + // TODO(fs): Remove this case once enough specific errors have been added. + if (status() == SVGParseStatus::ParsingFailed) { + builder.appendLiteral("Invalid value for "); + appendErrorContextInfo(builder, tagName, name); + builder.append('='); + appendValue(builder, *this, value); + } else { + appendErrorContextInfo(builder, tagName, name); + builder.appendLiteral(": "); + + if (hasLocus() && locus() == value.length()) + builder.appendLiteral("Unexpected end of attribute. "); + + auto message = messageForStatus(status()); + builder.append(message.first); + appendValue(builder, *this, value); + builder.append(message.second); + } + return builder.toString(); +} + +} // namespace blink diff --git a/third_party/WebKit/Source/core/svg/SVGParsingError.h b/third_party/WebKit/Source/core/svg/SVGParsingError.h index 5a40f9f..a553131 100644 --- a/third_party/WebKit/Source/core/svg/SVGParsingError.h +++ b/third_party/WebKit/Source/core/svg/SVGParsingError.h @@ -27,14 +27,71 @@ #ifndef SVGParsingError_h #define SVGParsingError_h +#include "wtf/MathExtras.h" +#include "wtf/text/WTFString.h" + namespace blink { -enum SVGParsingError { +class QualifiedName; + +enum class SVGParseStatus { NoError, - ParsingAttributeFailedError, - NegativeValueForbiddenError + + // Syntax errors + TrailingGarbage, + ExpectedBoolean, + ExpectedEnumeration, + + // Semantic errors + NegativeValue, + + // Generic error + ParsingFailed, +}; + +class SVGParsingError { + STACK_ALLOCATED(); +public: + SVGParsingError(SVGParseStatus status = SVGParseStatus::NoError, size_t locus = 0) + : m_status(static_cast<unsigned>(status)) + , m_locus(checkLocus(locus)) + { + ASSERT(this->status() == status); + } + + SVGParseStatus status() const { return static_cast<SVGParseStatus>(m_status); } + + bool hasLocus() const { return m_locus != kNoLocus; } + unsigned locus() const { return m_locus; } + + // Generates a string describing this error for |value| in the context of + // an <element, attribute>-name pair. + String format(const String& tagName, const QualifiedName&, const AtomicString& value) const; + +private: + static const int kLocusBits = 24; + static const unsigned kNoLocus = (1u << kLocusBits) - 1; + + static unsigned checkLocus(size_t locus) + { + // Clamp to fit in the number of bits available. If the character index + // encoded by the locus does not fit in the number of bits allocated + // for it, the locus will be disabled (set to kNoLocus). This means + // that very long values will be output in their entirety. That should + // however be rather uncommon. + return clampTo<unsigned>(locus, 0, kNoLocus); + } + + unsigned m_status : 8; + unsigned m_locus : kLocusBits; // The locus (character index) of the error within the parsed string. }; +inline bool operator==(const SVGParsingError& error, SVGParseStatus status) +{ + return error.status() == status; +} +inline bool operator!=(const SVGParsingError& error, SVGParseStatus status) { return !(error == status); } + } // namespace blink #endif // SVGParsingError_h diff --git a/third_party/WebKit/Source/core/svg/SVGPath.cpp b/third_party/WebKit/Source/core/svg/SVGPath.cpp index c6b00ba..b44fdf9 100644 --- a/third_party/WebKit/Source/core/svg/SVGPath.cpp +++ b/third_party/WebKit/Source/core/svg/SVGPath.cpp @@ -95,10 +95,10 @@ PassRefPtrWillBeRawPtr<SVGPath> SVGPath::clone() const SVGParsingError SVGPath::setValueAsString(const String& string) { - SVGParsingError parseStatus = NoError; + SVGParsingError parseStatus; RefPtr<SVGPathByteStream> byteStream = SVGPathByteStream::create(); if (!buildByteStreamFromString(string, *byteStream)) - parseStatus = ParsingAttributeFailedError; + parseStatus = SVGParseStatus::ParsingFailed; m_pathValue = CSSPathValue::create(byteStream.release()); return parseStatus; } diff --git a/third_party/WebKit/Source/core/svg/SVGPathElement.cpp b/third_party/WebKit/Source/core/svg/SVGPathElement.cpp index c0a5e54..5cd714a 100644 --- a/third_party/WebKit/Source/core/svg/SVGPathElement.cpp +++ b/third_party/WebKit/Source/core/svg/SVGPathElement.cpp @@ -38,8 +38,8 @@ public: SVGParsingError setBaseValueAsString(const String& value) override { SVGParsingError parseStatus = SVGAnimatedNumber::setBaseValueAsString(value); - if (parseStatus == NoError && baseValue()->value() < 0) - parseStatus = NegativeValueForbiddenError; + if (parseStatus == SVGParseStatus::NoError && baseValue()->value() < 0) + parseStatus = SVGParseStatus::NegativeValue; return parseStatus; } diff --git a/third_party/WebKit/Source/core/svg/SVGPoint.cpp b/third_party/WebKit/Source/core/svg/SVGPoint.cpp index 8f0c894..b0fc684 100644 --- a/third_party/WebKit/Source/core/svg/SVGPoint.cpp +++ b/third_party/WebKit/Source/core/svg/SVGPoint.cpp @@ -84,7 +84,7 @@ SVGParsingError SVGPoint::setValueAsString(const String& string) { if (string.isEmpty()) { m_value = FloatPoint(0.0f, 0.0f); - return NoError; + return SVGParseStatus::NoError; } bool valid; @@ -97,7 +97,7 @@ SVGParsingError SVGPoint::setValueAsString(const String& string) const UChar* end = ptr + string.length(); valid = parse(ptr, end); } - return valid ? NoError : ParsingAttributeFailedError; + return valid ? SVGParseStatus::NoError : SVGParseStatus::ParsingFailed; } String SVGPoint::valueAsString() const diff --git a/third_party/WebKit/Source/core/svg/SVGPointList.cpp b/third_party/WebKit/Source/core/svg/SVGPointList.cpp index 117969e..e7f67b8 100644 --- a/third_party/WebKit/Source/core/svg/SVGPointList.cpp +++ b/third_party/WebKit/Source/core/svg/SVGPointList.cpp @@ -92,7 +92,7 @@ SVGParsingError SVGPointList::setValueAsString(const String& value) { if (value.isEmpty()) { clear(); - return NoError; + return SVGParseStatus::NoError; } bool valid = false; @@ -105,7 +105,7 @@ SVGParsingError SVGPointList::setValueAsString(const String& value) const UChar* end = ptr + value.length(); valid = parse(ptr, end); } - return valid ? NoError : ParsingAttributeFailedError; + return valid ? SVGParseStatus::NoError : SVGParseStatus::ParsingFailed; } void SVGPointList::add(PassRefPtrWillBeRawPtr<SVGPropertyBase> other, SVGElement* contextElement) diff --git a/third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.cpp b/third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.cpp index 5ec1b4f..f74c9a0 100644 --- a/third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.cpp +++ b/third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.cpp @@ -52,7 +52,7 @@ PassRefPtrWillBeRawPtr<SVGPreserveAspectRatio> SVGPreserveAspectRatio::clone() c } template<typename CharType> -bool SVGPreserveAspectRatio::parseInternal(const CharType*& ptr, const CharType* end, bool validate) +SVGParsingError SVGPreserveAspectRatio::parseInternal(const CharType*& ptr, const CharType* end, bool validate) { SVGPreserveAspectRatioType align = SVG_PRESERVEASPECTRATIO_XMIDYMID; SVGMeetOrSliceType meetOrSlice = SVG_MEETORSLICE_MEET; @@ -60,19 +60,20 @@ bool SVGPreserveAspectRatio::parseInternal(const CharType*& ptr, const CharType* setAlign(align); setMeetOrSlice(meetOrSlice); + const CharType* start = ptr; if (!skipOptionalSVGSpaces(ptr, end)) - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); if (*ptr == 'n') { if (!skipString(ptr, end, "none")) - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); align = SVG_PRESERVEASPECTRATIO_NONE; skipOptionalSVGSpaces(ptr, end); } else if (*ptr == 'x') { if ((end - ptr) < 8) - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); if (ptr[1] != 'M' || ptr[4] != 'Y' || ptr[5] != 'M') - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); if (ptr[2] == 'i') { if (ptr[3] == 'n') { if (ptr[6] == 'i') { @@ -81,11 +82,11 @@ bool SVGPreserveAspectRatio::parseInternal(const CharType*& ptr, const CharType* else if (ptr[7] == 'd') align = SVG_PRESERVEASPECTRATIO_XMINYMID; else - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } else if (ptr[6] == 'a' && ptr[7] == 'x') { align = SVG_PRESERVEASPECTRATIO_XMINYMAX; } else { - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } } else if (ptr[3] == 'd') { if (ptr[6] == 'i') { @@ -94,14 +95,14 @@ bool SVGPreserveAspectRatio::parseInternal(const CharType*& ptr, const CharType* else if (ptr[7] == 'd') align = SVG_PRESERVEASPECTRATIO_XMIDYMID; else - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } else if (ptr[6] == 'a' && ptr[7] == 'x') { align = SVG_PRESERVEASPECTRATIO_XMIDYMAX; } else { - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } } else { - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } } else if (ptr[2] == 'a' && ptr[3] == 'x') { if (ptr[6] == 'i') { @@ -110,29 +111,29 @@ bool SVGPreserveAspectRatio::parseInternal(const CharType*& ptr, const CharType* else if (ptr[7] == 'd') align = SVG_PRESERVEASPECTRATIO_XMAXYMID; else - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } else if (ptr[6] == 'a' && ptr[7] == 'x') { align = SVG_PRESERVEASPECTRATIO_XMAXYMAX; } else { - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } } else { - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } ptr += 8; skipOptionalSVGSpaces(ptr, end); } else { - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); } if (ptr < end) { if (*ptr == 'm') { if (!skipString(ptr, end, "meet")) - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); skipOptionalSVGSpaces(ptr, end); } else if (*ptr == 's') { if (!skipString(ptr, end, "slice")) - return false; + return SVGParsingError(SVGParseStatus::ExpectedEnumeration, ptr - start); skipOptionalSVGSpaces(ptr, end); if (align != SVG_PRESERVEASPECTRATIO_NONE) meetOrSlice = SVG_MEETORSLICE_SLICE; @@ -140,12 +141,12 @@ bool SVGPreserveAspectRatio::parseInternal(const CharType*& ptr, const CharType* } if (end != ptr && validate) - return false; + return SVGParsingError(SVGParseStatus::TrailingGarbage, ptr - start); setAlign(align); setMeetOrSlice(meetOrSlice); - return true; + return SVGParseStatus::NoError; } SVGParsingError SVGPreserveAspectRatio::setValueAsString(const String& string) @@ -153,29 +154,26 @@ SVGParsingError SVGPreserveAspectRatio::setValueAsString(const String& string) setDefault(); if (string.isEmpty()) - return NoError; + return SVGParseStatus::NoError; - bool valid = false; if (string.is8Bit()) { const LChar* ptr = string.characters8(); const LChar* end = ptr + string.length(); - valid = parseInternal(ptr, end, true); - } else { - const UChar* ptr = string.characters16(); - const UChar* end = ptr + string.length(); - valid = parseInternal(ptr, end, true); + return parseInternal(ptr, end, true); } - return valid ? NoError : ParsingAttributeFailedError; + const UChar* ptr = string.characters16(); + const UChar* end = ptr + string.length(); + return parseInternal(ptr, end, true); } bool SVGPreserveAspectRatio::parse(const LChar*& ptr, const LChar* end, bool validate) { - return parseInternal(ptr, end, validate); + return parseInternal(ptr, end, validate) == SVGParseStatus::NoError; } bool SVGPreserveAspectRatio::parse(const UChar*& ptr, const UChar* end, bool validate) { - return parseInternal(ptr, end, validate); + return parseInternal(ptr, end, validate) == SVGParseStatus::NoError; } void SVGPreserveAspectRatio::transformRect(FloatRect& destRect, FloatRect& srcRect) diff --git a/third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.h b/third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.h index 0bd39e0..ab4b61b 100644 --- a/third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.h +++ b/third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.h @@ -92,7 +92,7 @@ private: void setDefault(); template<typename CharType> - bool parseInternal(const CharType*& ptr, const CharType* end, bool validate); + SVGParsingError parseInternal(const CharType*& ptr, const CharType* end, bool validate); SVGPreserveAspectRatioType m_align; SVGMeetOrSliceType m_meetOrSlice; diff --git a/third_party/WebKit/Source/core/svg/SVGRect.cpp b/third_party/WebKit/Source/core/svg/SVGRect.cpp index a8f6970..328b59f 100644 --- a/third_party/WebKit/Source/core/svg/SVGRect.cpp +++ b/third_party/WebKit/Source/core/svg/SVGRect.cpp @@ -73,12 +73,12 @@ SVGParsingError SVGRect::setValueAsString(const String& string) setInvalid(); if (string.isNull()) - return NoError; + return SVGParseStatus::NoError; if (string.isEmpty()) { m_value = FloatRect(0.0f, 0.0f, 0.0f, 0.0f); m_isValid = true; - return NoError; + return SVGParseStatus::NoError; } bool valid; @@ -91,7 +91,7 @@ SVGParsingError SVGRect::setValueAsString(const String& string) const UChar* end = ptr + string.length(); valid = parse(ptr, end); } - return valid ? NoError : ParsingAttributeFailedError; + return valid ? SVGParseStatus::NoError : SVGParseStatus::ParsingFailed; } String SVGRect::valueAsString() const diff --git a/third_party/WebKit/Source/core/svg/SVGString.h b/third_party/WebKit/Source/core/svg/SVGString.h index 7388e99..0cd90e0e 100644 --- a/third_party/WebKit/Source/core/svg/SVGString.h +++ b/third_party/WebKit/Source/core/svg/SVGString.h @@ -62,7 +62,7 @@ public: SVGParsingError setValueAsString(const String& value) { m_value = value; - return NoError; + return SVGParseStatus::NoError; } void add(PassRefPtrWillBeRawPtr<SVGPropertyBase>, SVGElement*) override; diff --git a/third_party/WebKit/Source/core/svg/SVGStringList.cpp b/third_party/WebKit/Source/core/svg/SVGStringList.cpp index 120a4d38..1c35d08 100644 --- a/third_party/WebKit/Source/core/svg/SVGStringList.cpp +++ b/third_party/WebKit/Source/core/svg/SVGStringList.cpp @@ -108,7 +108,7 @@ SVGParsingError SVGStringList::setValueAsString(const String& data) m_values.clear(); if (data.isEmpty()) - return NoError; + return SVGParseStatus::NoError; if (data.is8Bit()) { const LChar* ptr = data.characters8(); @@ -119,7 +119,7 @@ SVGParsingError SVGStringList::setValueAsString(const String& data) const UChar* end = ptr + data.length(); parseInternal(ptr, end); } - return NoError; + return SVGParseStatus::NoError; } String SVGStringList::valueAsString() const diff --git a/third_party/WebKit/Source/core/svg/SVGTransformList.cpp b/third_party/WebKit/Source/core/svg/SVGTransformList.cpp index 28861ba..2785b3b 100644 --- a/third_party/WebKit/Source/core/svg/SVGTransformList.cpp +++ b/third_party/WebKit/Source/core/svg/SVGTransformList.cpp @@ -270,7 +270,7 @@ SVGParsingError SVGTransformList::setValueAsString(const String& value) { if (value.isEmpty()) { clear(); - return NoError; + return SVGParseStatus::NoError; } bool valid = false; @@ -286,10 +286,10 @@ SVGParsingError SVGTransformList::setValueAsString(const String& value) if (!valid) { clear(); - return ParsingAttributeFailedError; + return SVGParseStatus::ParsingFailed; } - return NoError; + return SVGParseStatus::NoError; } PassRefPtrWillBeRawPtr<SVGPropertyBase> SVGTransformList::cloneForAnimation(const String& value) const diff --git a/third_party/WebKit/Source/platform/JSONValues.cpp b/third_party/WebKit/Source/platform/JSONValues.cpp index 76ec444..3001d16 100644 --- a/third_party/WebKit/Source/platform/JSONValues.cpp +++ b/third_party/WebKit/Source/platform/JSONValues.cpp @@ -66,9 +66,8 @@ void writeIndent(int depth, StringBuilder* output) } // anonymous namespace -void doubleQuoteStringForJSON(const String& str, StringBuilder* dst) +void escapeStringForJSON(const String& str, StringBuilder* dst) { - dst->append('"'); for (unsigned i = 0; i < str.length(); ++i) { UChar c = str[i]; if (!escapeChar(c, dst)) { @@ -84,6 +83,12 @@ void doubleQuoteStringForJSON(const String& str, StringBuilder* dst) } } } +} + +void doubleQuoteStringForJSON(const String& str, StringBuilder* dst) +{ + dst->append('"'); + escapeStringForJSON(str, dst); dst->append('"'); } diff --git a/third_party/WebKit/Source/platform/JSONValues.h b/third_party/WebKit/Source/platform/JSONValues.h index bdb9d2d..44e42c1 100644 --- a/third_party/WebKit/Source/platform/JSONValues.h +++ b/third_party/WebKit/Source/platform/JSONValues.h @@ -323,6 +323,7 @@ public: using JSONArrayBase::end; }; +PLATFORM_EXPORT void escapeStringForJSON(const String&, StringBuilder*); void doubleQuoteStringForJSON(const String&, StringBuilder*); } // namespace blink |