Add parseFontFaceDescriptor for @font-face handling

Source/core/css/parser/CSSPropertyParser.cpp

 /*
  * Copyright (C) 2003 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Nicholas Shanks <webkit@nickshanks.com>
  * Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2012 Adobe Systems Incorporated. All rights reserved.
  * Copyright (C) 2012 Intel Corporation. All rights reserved.
  *
@@ skipping 401 matching lines @@
     CSSPropertyID propId = resolveCSSPropertyID(unresolvedProperty);
 
     if (!m_valueList)
         return false;
 
     CSSParserValue* value = m_valueList->current();
 
     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.
     // FIXME: This is to avoid having to pass parsedCalc to all validUnit callers.
     ASSERT(!m_parsedCalculation);
 
     CSSValueID id = value->id;
 
     if (id == CSSValueInherit) {
         if (m_valueList->size() != 1)
@@ skipping 486 matching lines @@
             validPrimitive = true;
         else
             validPrimitive = (!id && validUnit(value, FNumber | FPercent | FNonNeg));
         if (validPrimitive && m_context.useCounter()
             && !(id == CSSValueNormal
                 || (value->unit == CSSPrimitiveValue::CSS_NUMBER && value->fValue == 1)
                 || (value->unit == CSSPrimitiveValue::CSS_PERCENTAGE && value->fValue == 100)))
             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 */
 
     case CSSPropertyBorderImage:
     case CSSPropertyWebkitMaskBoxImage:
         return parseBorderImageShorthand(propId, important);
     case CSSPropertyWebkitBorderImage: {
         if (RefPtrWillBeRawPtr<CSSValue> result = parseBorderImage(propId)) {
             addProperty(propId, result, important);
@@ skipping 6700 matching lines @@
 
     ASSERT(!m_parsedCalculation);
     m_parsedCalculation = CSSCalcValue::create(args, range);
 
     if (!m_parsedCalculation)
         return false;
 
     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()));
 
     CSSParserValue* value = m_valueList->current();
     if (!value)
         return false;
 
     CSSValueID id = value->id;
     bool validPrimitive = false;
@@ skipping 754 matching lines @@
         }
     }
 
     if (!list->length())
         return nullptr;
 
     return list.release();
 }
 
 } // namespace blink