- Revision
- 136966
- Author
- [email protected]
- Date
- 2012-12-07 11:23:52 -0800 (Fri, 07 Dec 2012)
Log Message
Improve r136754 by hardening checks of expected values for background-position.
https://bugs.webkit.org/show_bug.cgi?id=104380
Reviewed by Antti Koivisto.
r136754 was landed to fix the problem of checking successively two calc
values with validUnit. It was asserting as validUnit expect you to use
the parsed value of the calc after the call. In this case we pre-check the
background-position longhand to count how many values it has to then
call the right parsing functions accordingly. While r136754 is not
wrong it is better to harden isPotentialPositionValue with the real
expected units and keywords. For this matter we can reuse the
ReleaseParsedCalcValueCondition enum which was created with the same
idea as this patch. If you are not interested of the calc parsed
value when calling validUnit() you can now specify it, I believe it is
good to have it explicit to avoid mistake in the future.
No new tests : this is covered by css3/*, fast/backgrounds/*.
* css/CSSParser.cpp:
(WebCore::CSSParser::validCalculationUnit):
(WebCore::CSSParser::validUnit):
(WebCore::CSSParser::isPotentialPositionValue):
* css/CSSParser.h:
(WebCore::CSSParser::validUnit):
(CSSParser):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (136965 => 136966)
--- trunk/Source/WebCore/ChangeLog 2012-12-07 19:16:49 UTC (rev 136965)
+++ trunk/Source/WebCore/ChangeLog 2012-12-07 19:23:52 UTC (rev 136966)
@@ -1,3 +1,32 @@
+2012-12-07 Alexis Menard <[email protected]>
+
+ Improve r136754 by hardening checks of expected values for background-position.
+ https://bugs.webkit.org/show_bug.cgi?id=104380
+
+ Reviewed by Antti Koivisto.
+
+ r136754 was landed to fix the problem of checking successively two calc
+ values with validUnit. It was asserting as validUnit expect you to use
+ the parsed value of the calc after the call. In this case we pre-check the
+ background-position longhand to count how many values it has to then
+ call the right parsing functions accordingly. While r136754 is not
+ wrong it is better to harden isPotentialPositionValue with the real
+ expected units and keywords. For this matter we can reuse the
+ ReleaseParsedCalcValueCondition enum which was created with the same
+ idea as this patch. If you are not interested of the calc parsed
+ value when calling validUnit() you can now specify it, I believe it is
+ good to have it explicit to avoid mistake in the future.
+
+ No new tests : this is covered by css3/*, fast/backgrounds/*.
+
+ * css/CSSParser.cpp:
+ (WebCore::CSSParser::validCalculationUnit):
+ (WebCore::CSSParser::validUnit):
+ (WebCore::CSSParser::isPotentialPositionValue):
+ * css/CSSParser.h:
+ (WebCore::CSSParser::validUnit):
+ (CSSParser):
+
2012-12-07 Brent Fulgham <[email protected]>
Remove unnecessary casts in transformations.
Modified: trunk/Source/WebCore/css/CSSParser.cpp (136965 => 136966)
--- trunk/Source/WebCore/css/CSSParser.cpp 2012-12-07 19:16:49 UTC (rev 136965)
+++ trunk/Source/WebCore/css/CSSParser.cpp 2012-12-07 19:23:52 UTC (rev 136966)
@@ -1487,7 +1487,7 @@
return completeURL(m_context, url);
}
-bool CSSParser::validCalculationUnit(CSSParserValue* value, Units unitflags)
+bool CSSParser::validCalculationUnit(CSSParserValue* value, Units unitflags, ReleaseParsedCalcValueCondition releaseCalc)
{
bool mustBeNonNegative = unitflags & FNonNeg;
@@ -1525,7 +1525,7 @@
case CalcOther:
break;
}
- if (!b)
+ if (!b || releaseCalc == ReleaseParsedCalcValue)
m_parsedCalculation.release();
return b;
}
@@ -1536,10 +1536,10 @@
return (unitflags & (FLength | FAngle | FTime)) && (!value->fValue || cssParserMode == CSSQuirksMode || cssParserMode == SVGAttributeMode);
}
-bool CSSParser::validUnit(CSSParserValue* value, Units unitflags, CSSParserMode cssParserMode)
+bool CSSParser::validUnit(CSSParserValue* value, Units unitflags, CSSParserMode cssParserMode, ReleaseParsedCalcValueCondition releaseCalc)
{
if (isCalculation(value))
- return validCalculationUnit(value, unitflags);
+ return validCalculationUnit(value, unitflags, releaseCalc);
bool b = false;
switch (value->unit) {
@@ -3875,7 +3875,7 @@
inline bool CSSParser::isPotentialPositionValue(CSSParserValue* value)
{
- return !value->id || isBackgroundPositionKeyword(value->id);
+ return isBackgroundPositionKeyword(value->id) || validUnit(value, FPercent | FLength, ReleaseParsedCalcValue);
}
void CSSParser::parseFillBackgroundPosition(CSSParserValueList* valueList, RefPtr<CSSValue>& value1, RefPtr<CSSValue>& value2)
Modified: trunk/Source/WebCore/css/CSSParser.h (136965 => 136966)
--- trunk/Source/WebCore/css/CSSParser.h 2012-12-07 19:16:49 UTC (rev 136965)
+++ trunk/Source/WebCore/css/CSSParser.h 2012-12-07 19:23:52 UTC (rev 136966)
@@ -589,20 +589,20 @@
return static_cast<Units>(static_cast<unsigned>(a) | static_cast<unsigned>(b));
}
- bool validCalculationUnit(CSSParserValue*, Units);
+ enum ReleaseParsedCalcValueCondition {
+ ReleaseParsedCalcValue,
+ DoNotReleaseParsedCalcValue
+ };
+ bool validCalculationUnit(CSSParserValue*, Units, ReleaseParsedCalcValueCondition releaseCalc = DoNotReleaseParsedCalcValue);
+
bool shouldAcceptUnitLessValues(CSSParserValue*, Units, CSSParserMode);
- inline bool validUnit(CSSParserValue* value, Units unitflags) { return validUnit(value, unitflags, m_context.mode); }
- bool validUnit(CSSParserValue*, Units, CSSParserMode);
+ inline bool validUnit(CSSParserValue* value, Units unitflags, ReleaseParsedCalcValueCondition releaseCalc = DoNotReleaseParsedCalcValue) { return validUnit(value, unitflags, m_context.mode, releaseCalc); }
+ bool validUnit(CSSParserValue*, Units, CSSParserMode, ReleaseParsedCalcValueCondition releaseCalc = DoNotReleaseParsedCalcValue);
bool parseBorderImageQuad(Units, RefPtr<CSSPrimitiveValue>&);
int colorIntFromValue(CSSParserValue*);
-
- enum ReleaseParsedCalcValueCondition {
- ReleaseParsedCalcValue,
- DoNotReleaseParsedCalcValue
- };
double parsedDouble(CSSParserValue*, ReleaseParsedCalcValueCondition releaseCalc = DoNotReleaseParsedCalcValue);
bool isCalculation(CSSParserValue*);
