Add parseFontFaceDescriptor for @font-face handling

Source/core/css/parser/CSSPropertyParser.cpp

Index: Source/core/css/parser/CSSPropertyParser.cpp
diff --git a/Source/core/css/parser/CSSPropertyParser.cpp b/Source/core/css/parser/CSSPropertyParser.cpp
index d13d2db2576c42833a3d18b6803b5cbffd6e57c2..ced39fca9bb33d7df6ad59266d98eed3e9d5a7f9 100644
--- a/Source/core/css/parser/CSSPropertyParser.cpp
+++ b/Source/core/css/parser/CSSPropertyParser.cpp
@@ -419,12 +419,15 @@ bool CSSPropertyParser::parseValue(CSSPropertyID unresolvedProperty, bool import
     if (!value)
         return false;
 
-    if (inViewport()) {
-        // Allow @viewport rules from UA stylesheets even if the feature is disabled.
-        if (!RuntimeEnabledFeatures::cssViewportEnabled() && !isUASheetBehavior(m_context.mode()))
-            return false;
+    if (m_ruleType >= StyleRule::FontFace) {
+        if (m_ruleType == StyleRule::Viewport) {

[Timothy Loh, 2015/05/01 04:32:16]

This reads weird, and also probably breaks @viewport with the Bison parser. Yes
there have probably been changes that affect it, but we should avoid breaking it
if we can. Can't we just add an else after this if?

+            // Allow @viewport rules from UA stylesheets even if the feature is disabled.
+            if (!RuntimeEnabledFeatures::cssViewportEnabled() && !isUASheetBehavior(m_context.mode()))
+                return false;
 
-        return parseViewportProperty(propId, important);
+            return parseViewportProperty(propId, important);
+        }
+        return parseFontFaceProperty(propId, important);
     }
 
     // Note: m_parsedCalculation is used to pass the calc value to validUnit and then cleared at the end of this function.
@@ -931,12 +934,9 @@ bool CSSPropertyParser::parseValue(CSSPropertyID unresolvedProperty, bool import
             m_context.useCounter()->count(UseCounter::CSSZoomNotEqualToOne);
         break;
 
-    case CSSPropertySrc: // Only used within @font-face so cannot use inherit | initial or be !important. This is a list of urls or local references.
-        parsedValue = parseFontFaceSrc();
-        break;
-
+    case CSSPropertySrc:
     case CSSPropertyUnicodeRange:
-        parsedValue = parseFontFaceUnicodeRange();
+        /* @font-face only descriptors */
         break;
 
     /* CSS3 properties */
@@ -7657,6 +7657,62 @@ bool CSSPropertyParser::parseCalculation(CSSParserValue* value, ValueRange range
     return true;
 }
 
+bool CSSPropertyParser::parseFontFaceProperty(CSSPropertyID propId, bool important)

[Timothy Loh, 2015/05/01 04:32:16]

I think this would be cleaner as

PassRefPtrWillBeRawPtr<CSSValue>
CSSPropertyParser::parseFontFaceDescriptorInternal(CSSPropertyID) {
   ..
}

PassRefPtrWillBeRawPtr<CSSValue>
CSSPropertyParser::parseFontFaceDescriptor(CSSPropertyID property) {
    RefPtrWillBeRawPtr<CSSValue> result =
parseFontFaceDescriptorInternal(property);
    if (!result || m_valueList->next())
        return nullptr;
    return result;
}

Also, might as well fix crbug.com/408484 in the same patch.

+{
+    CSSParserValue* value = m_valueList->current();
+    ASSERT(value);
+    CSSValueID id = value->id;
+    bool validPrimitive = false;
+    RefPtrWillBeRawPtr<CSSValue> parsedValue = nullptr;
+
+    switch (propId) {
+    case CSSPropertyFontFamily:
+        // <family-name>
+        // TODO: check there is only one family-name

[Timothy Loh, 2015/05/01 04:32:16]

TODO(rwlbuis)?

+        parsedValue = parseFontFamily();
+        break;
+    case CSSPropertySrc: // This is a list of urls or local references.
+        parsedValue = parseFontFaceSrc();
+        break;
+    case CSSPropertyUnicodeRange:
+        parsedValue = parseFontFaceUnicodeRange();
+        break;
+    case CSSPropertyFontWeight: // normal | bold | 100 | 200 | 300 | 400 | 500 | 600 | 700 | 800 | 900
+        if (m_valueList->size() != 1)
+            return false;
+        return parseFontWeight(important);
+    case CSSPropertyFontStyle:
+        if (!CSSParserFastPaths::isValidKeywordPropertyAndValue(CSSPropertyFontStyle, id))
+            return false;
+        addProperty(propId, cssValuePool().createIdentifierValue(id), important);
+        return true;
+    case CSSPropertyFontVariant: // normal | small-caps | inherit
+        return parseFontVariant(important);
+    case CSSPropertyWebkitFontFeatureSettings:
+        if (id == CSSValueNormal)
+            validPrimitive = true;
+        else
+            parsedValue = parseFontFeatureSettings();
+        break;
+    default:

[Timothy Loh, 2015/05/01 04:32:16]

FontStretch?

+        break;
+    }
+
+    if (validPrimitive) {
+        parsedValue = parseValidPrimitive(id, value);
+        m_valueList->next();
+    }
+
+    if (parsedValue) {
+        if (!m_valueList->current() || inShorthand()) {

[Timothy Loh, 2015/05/01 04:32:16]

No shorthands in here, right?

+            addProperty(propId, parsedValue.release(), important);
+            return true;
+        }
+    }
+
+    return false;
+}
+
 bool CSSPropertyParser::parseViewportProperty(CSSPropertyID propId, bool important)
 {
     ASSERT(RuntimeEnabledFeatures::cssViewportEnabled() || isUASheetBehavior(m_context.mode()));