Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(285)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: Source/core/css/parser/CSSPropertyParser.cpp

Issue 1318093002: Simplify parseShadowValue (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Remove default params Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/core/css/parser/CSSPropertyParser.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/css/parser/CSSPropertyParser.cpp
diff --git a/Source/core/css/parser/CSSPropertyParser.cpp b/Source/core/css/parser/CSSPropertyParser.cpp
index 3ed4a660864f74fb5a0cb1082f61716d827093d0..d08862af53cf57c0a7509af8f5495f06444435a7 100644
--- a/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/Source/core/css/parser/CSSPropertyParser.cpp
@@ -998,15 +998,8 @@ bool CSSPropertyParser::parseValue(CSSPropertyID unresolvedProperty, bool import
case CSSPropertyBoxShadow:
if (id == CSSValueNone)
validPrimitive = true;
- else {
- RefPtrWillBeRawPtr<CSSValueList> shadowValueList = parseShadow(m_valueList, propId);
- if (shadowValueList) {
- addProperty(propId, shadowValueList.release(), important);
- m_valueList->next();
- return true;
- }
- return false;
- }
+ else
+ parsedValue = parseShadow(m_valueList, propId);
break;
case CSSPropertyWebkitBoxReflect:
if (id == CSSValueNone)
@@ -5116,175 +5109,87 @@ bool CSSPropertyParser::parseColorFromValue(const CSSParserValue* value, RGBA32&
return true;
}
-// This class tracks parsing state for shadow values. If it goes out of scope (e.g., due to an early return)
-// without the allowBreak bit being set, then it will clean up all of the objects and destroy them.
-class ShadowParseContext {
- STACK_ALLOCATED();
-public:
- ShadowParseContext(CSSPropertyID prop)
- : property(prop)
- , allowX(true)
- , allowY(false)
- , allowBlur(false)
- , allowSpread(false)
- , allowColor(true)
- , allowStyle(prop == CSSPropertyBoxShadow)
- , allowBreak(true)
- {
- }
-
- bool allowLength() { return allowX || allowY || allowBlur || allowSpread; }
-
- void commitValue()
- {
- // Handle the ,, case gracefully by doing nothing.
- if (x || y || blur || spread || color || style) {
- if (!values)
- values = CSSValueList::createCommaSeparated();
-
- // Construct the current shadow value and add it to the list.
- values->append(CSSShadowValue::create(x.release(), y.release(), blur.release(), spread.release(), style.release(), color.release()));
- }
-
- // Now reset for the next shadow value.
- x = nullptr;
- y = nullptr;
- blur = nullptr;
- spread = nullptr;
- style = nullptr;
- color = nullptr;
-
- allowX = true;
- allowColor = true;
- allowBreak = true;
- allowY = false;
- allowBlur = false;
- allowSpread = false;
- allowStyle = property == CSSPropertyBoxShadow;
- }
-
- void commitLength(PassRefPtrWillBeRawPtr<CSSPrimitiveValue> val)
- {
- if (allowX) {
- x = val;
- allowX = false;
- allowY = true;
- allowColor = false;
- allowStyle = false;
- allowBreak = false;
- } else if (allowY) {
- y = val;
- allowY = false;
- allowBlur = true;
- allowColor = true;
- allowStyle = property == CSSPropertyBoxShadow;
- allowBreak = true;
- } else if (allowBlur) {
- blur = val;
- allowBlur = false;
- allowSpread = property == CSSPropertyBoxShadow;
- } else if (allowSpread) {
- spread = val;
- allowSpread = false;
- }
- }
-
- void commitColor(PassRefPtrWillBeRawPtr<CSSPrimitiveValue> val)
- {
- color = val;
- allowColor = false;
- if (allowX) {
- allowStyle = false;
- allowBreak = false;
- } else {
- allowBlur = false;
- allowSpread = false;
- allowStyle = property == CSSPropertyBoxShadow;
- }
- }
-
- void commitStyle(CSSParserValue* v)
- {
- style = cssValuePool().createIdentifierValue(v->id);
- allowStyle = false;
- if (allowX)
- allowBreak = false;
- else {
- allowBlur = false;
- allowSpread = false;
- allowColor = false;
- }
+PassRefPtrWillBeRawPtr<CSSValueList> CSSPropertyParser::parseShadow(CSSParserValueList* valueList, CSSPropertyID propID)
+{
+ RefPtrWillBeRawPtr<CSSValueList> shadowValueList = CSSValueList::createCommaSeparated();
+ const bool isBoxShadowProperty = propID == CSSPropertyBoxShadow;
+ while (RefPtrWillBeRawPtr<CSSShadowValue> shadowValue = parseSingleShadow(valueList, isBoxShadowProperty, isBoxShadowProperty)) {
+ shadowValueList->append(shadowValue);
+ if (!valueList->current())
+ break;
+ if (!consumeComma(valueList))
+ return nullptr;
}
+ if (shadowValueList->length() == 0)
+ return nullptr;
+ return shadowValueList;
+}
- CSSPropertyID property;
-
- RefPtrWillBeMember<CSSValueList> values;
- RefPtrWillBeMember<CSSPrimitiveValue> x;
- RefPtrWillBeMember<CSSPrimitiveValue> y;
- RefPtrWillBeMember<CSSPrimitiveValue> blur;
- RefPtrWillBeMember<CSSPrimitiveValue> spread;
+PassRefPtrWillBeRawPtr<CSSShadowValue> CSSPropertyParser::parseSingleShadow(CSSParserValueList* valueList, bool allowInset, bool allowSpread)
+{
RefPtrWillBeMember<CSSPrimitiveValue> style;
RefPtrWillBeMember<CSSPrimitiveValue> color;
+ Vector<RefPtrWillBeRawPtr<CSSPrimitiveValue>, 4> lengths;
- bool allowX;
- bool allowY;
- bool allowBlur;
- bool allowSpread;
- bool allowColor;
- bool allowStyle; // inset or not.
- bool allowBreak;
-};
+ CSSParserValue* val = valueList->current();
+ if (!val)
+ return nullptr;
+ if (val->id == CSSValueInset) {
+ if (!allowInset)
+ return nullptr;
+ style = cssValuePool().createIdentifierValue(val->id);
+ val = valueList->next();
+ if (!val)
+ return nullptr;
+ }
+ if ((color = parseColor(val)))
+ val = valueList->next();
-PassRefPtrWillBeRawPtr<CSSValueList> CSSPropertyParser::parseShadow(CSSParserValueList* valueList, CSSPropertyID propId)
-{
- ShadowParseContext context(propId);
- for (CSSParserValue* val = valueList->current(); val; val = valueList->next()) {
- // Check for a comma break first.
- if (val->m_unit == CSSParserValue::Operator) {
- if (val->iValue != ',' || !context.allowBreak) {
- // Other operators aren't legal or we aren't done with the current shadow
- // value. Treat as invalid.
- return nullptr;
- }
- // The value is good. Commit it.
- context.commitValue();
- } else if (validUnit(val, FLength, HTMLStandardMode)) {
- // We required a length and didn't get one. Invalid.
- if (!context.allowLength())
- return nullptr;
+ if (!val || !validUnit(val, FLength, HTMLStandardMode))
+ return nullptr;
+ lengths.append(createPrimitiveNumericValue(val));
+ val = valueList->next();
- // Blur radius must be non-negative.
- if (context.allowBlur && (m_parsedCalculation ? m_parsedCalculation->isNegative() : !validUnit(val, FLength | FNonNeg, HTMLStandardMode)))
- return nullptr;
+ if (!val || !validUnit(val, FLength, HTMLStandardMode))
+ return nullptr;
+ lengths.append(createPrimitiveNumericValue(val));
+ val = valueList->next();
- // A length is allowed here. Construct the value and add it.
- RefPtrWillBeRawPtr<CSSPrimitiveValue> length = createPrimitiveNumericValue(val);
- context.commitLength(length.release());
- } else if (val->id == CSSValueInset) {
- if (!context.allowStyle)
+ if (val && validUnit(val, FLength, HTMLStandardMode)) {
+ // Blur radius must be non-negative.
+ if (m_parsedCalculation ? m_parsedCalculation->isNegative() : !validUnit(val, FLength | FNonNeg, HTMLStandardMode)) {
+ m_parsedCalculation.release();
+ return nullptr;
+ }
+ lengths.append(createPrimitiveNumericValue(val));
+ val = valueList->next();
+ if (val && validUnit(val, FLength, HTMLStandardMode)) {
+ if (!allowSpread)
return nullptr;
+ lengths.append(createPrimitiveNumericValue(val));
+ val = valueList->next();
+ }
+ }
- context.commitStyle(val);
- } else {
- if (!context.allowColor)
+ if (val) {
+ if (RefPtrWillBeMember<CSSPrimitiveValue> colorValue = parseColor(val)) {
+ if (color)
return nullptr;
-
- // The only other type of value that's ok is a color value.
- RefPtrWillBeRawPtr<CSSPrimitiveValue> parsedColor = parseColor(val);
- if (!parsedColor)
+ color = colorValue;
+ val = valueList->next();
+ }
+ if (val && val->id == CSSValueInset) {
+ if (!allowInset || style)
return nullptr;
-
- context.commitColor(parsedColor.release());
+ style = cssValuePool().createIdentifierValue(val->id);
+ val = valueList->next();
}
}
-
- if (context.allowBreak) {
- context.commitValue();
- if (context.values && context.values->length())
- return context.values.release();
- }
-
- return nullptr;
+ unsigned lengthsSeen = lengths.size();
+ return CSSShadowValue::create(lengths.at(0), lengths.at(1),
+ lengthsSeen > 2 ? lengths.at(2) : nullptr,
+ lengthsSeen > 3 ? lengths.at(3) : nullptr,
+ style.release(), color.release());
}
PassRefPtrWillBeRawPtr<CSSValue> CSSPropertyParser::parseReflect()
@@ -6932,11 +6837,10 @@ PassRefPtrWillBeRawPtr<CSSFunctionValue> CSSPropertyParser::parseBuiltinFilterAr
}
case CSSValueDropShadow: {
// drop-shadow() takes a single shadow.
- RefPtrWillBeRawPtr<CSSValueList> shadowValueList = parseShadow(args, CSSPropertyWebkitFilter);
- if (!shadowValueList || shadowValueList->length() != 1)
+ RefPtrWillBeRawPtr<CSSShadowValue> shadowValue = parseSingleShadow(args, false, true);
+ if (!shadowValue)
return nullptr;
-
- filterValue->append((shadowValueList.release())->item(0));
+ filterValue->append(shadowValue.release());
break;
}
default:
« no previous file with comments | « Source/core/css/parser/CSSPropertyParser.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698