Implement parseShorthand API for shorthand properties, "overflow", "font" and "font-variant"

third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIFont.cpp

+// Copyright 2017 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/css/properties/CSSShorthandPropertyAPIFont.h"
+
+#include "core/css/CSSFontFamilyValue.h"
+#include "core/css/CSSIdentifierValue.h"
+#include "core/css/CSSPrimitiveValueMappings.h"
+#include "core/css/parser/CSSParserContext.h"
+#include "core/css/parser/CSSParserFastPaths.h"
+#include "core/css/parser/CSSPropertyParserHelpers.h"
+#include "core/css/properties/CSSPropertyFontUtils.h"
+#include "platform/fonts/FontTraits.h"
+
+namespace blink {
+
+class CSSParserLocalContext;

[Bugs Nash, 2017/06/16 03:51:51]

not necessary as guaranteed to be in header file

[Jia, 2017/06/19 03:54:41]

Done.

+
+namespace {
+
+bool ConsumeSystemFont(bool important,
+                       CSSParserTokenRange& range,
+                       HeapVector<CSSProperty, 256>& properties) {
+  CSSValueID system_font_id = range.ConsumeIncludingWhitespace().Id();
+  DCHECK_GE(system_font_id, CSSValueCaption);
+  DCHECK_LE(system_font_id, CSSValueStatusBar);
+  if (!range.AtEnd())
+    return false;
+
+  FontStyle font_style = kFontStyleNormal;
+  FontWeight font_weight = kFontWeightNormal;
+  float font_size = 0;
+  AtomicString font_family;
+  LayoutTheme::GetTheme().SystemFont(system_font_id, font_style, font_weight,
+                                     font_size, font_family);
+
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontStyle, CSSPropertyFont,
+      *CSSIdentifierValue::Create(
+          font_style == kFontStyleItalic ? CSSValueItalic : CSSValueNormal),
+      important, false /* implicit */, properties);

[Bugs Nash, 2017/06/16 03:19:01]

avoid passing literal bools. I would either change this arg to an enum class or
make the implicit arg default to false like in AddParsedProperty (it would then
need to be the last arg, but I think this is a good enough reason to ignore the
outputs last guideline). I have a slight preference for the enum class solution

[Jia, 2017/06/19 03:54:41]

I can change the signature of AddProperty and replace two bool params (important
and implicit) by enums. I'll make them in another cl to avoid bundling up
unrelated changes.

[Bugs Nash, 2017/06/19 04:29:18]

I wouldn't worry about changing the important bool. You're not passing in
literal bools for it here so you're not violating the style guide.

I disagree that changing the implicit argument to an enum is an unrelated
change, as this patch introduces passing literal bools that didn't exist before
(a violation of the style guide) so I believe this patch is the right place to
change it to an enum (this patch is the reason it's required). But it's up to
you, however if you do want to do it in a separate patch I think that patch
should land first to avoid the style guide violation.

+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontWeight, CSSPropertyFont,
+      *CSSIdentifierValue::Create(font_weight), important, false /* implicit */,
+      properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontSize, CSSPropertyFont,
+      *CSSPrimitiveValue::Create(font_size,
+                                 CSSPrimitiveValue::UnitType::kPixels),
+      important, false /* implicit */, properties);
+
+  CSSValueList* font_family_list = CSSValueList::CreateCommaSeparated();
+  font_family_list->Append(*CSSFontFamilyValue::Create(font_family));
+  CSSPropertyParserHelpers::AddProperty(CSSPropertyFontFamily, CSSPropertyFont,
+                                        *font_family_list, important,
+                                        false /* implicit */, properties);
+
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontStretch, CSSPropertyFont,
+      *CSSIdentifierValue::Create(CSSValueNormal), important,
+      false /* implicit */, properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontVariantCaps, CSSPropertyFont,
+      *CSSIdentifierValue::Create(CSSValueNormal), important,
+      false /* implicit */, properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontVariantLigatures, CSSPropertyFont,
+      *CSSIdentifierValue::Create(CSSValueNormal), important,
+      false /* implicit */, properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontVariantNumeric, CSSPropertyFont,
+      *CSSIdentifierValue::Create(CSSValueNormal), important,
+      false /* implicit */, properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyLineHeight, CSSPropertyFont,
+      *CSSIdentifierValue::Create(CSSValueNormal), important,
+      false /* implicit */, properties);
+  return true;
+}
+
+bool ConsumeFont(bool important,
+                 CSSParserTokenRange& range,
+                 const CSSParserContext* context,
+                 HeapVector<CSSProperty, 256>& properties) {
+  DCHECK(context);
+
+  // Let's check if there is an inherit or initial somewhere in the shorthand.
+  CSSParserTokenRange range_copy = range;
+  while (!range_copy.AtEnd()) {
+    CSSValueID id = range_copy.ConsumeIncludingWhitespace().Id();
+    if (id == CSSValueInherit || id == CSSValueInitial)
+      return false;
+  }
+  // Optional font-style, font-variant, font-stretch and font-weight.
+  CSSIdentifierValue* font_style = nullptr;
+  CSSIdentifierValue* font_variant_caps = nullptr;
+  CSSIdentifierValue* font_weight = nullptr;
+  CSSIdentifierValue* font_stretch = nullptr;
+  while (!range.AtEnd()) {
+    CSSValueID id = range.Peek().Id();
+    if (!font_style && CSSParserFastPaths::IsValidKeywordPropertyAndValue(
+                           CSSPropertyFontStyle, id, context->Mode())) {
+      font_style = CSSPropertyParserHelpers::ConsumeIdent(range);
+      continue;
+    }
+    if (!font_variant_caps &&
+        (id == CSSValueNormal || id == CSSValueSmallCaps)) {
+      // Font variant in the shorthand is particular, it only accepts normal or
+      // small-caps.
+      // See https://drafts.csswg.org/css-fonts/#propdef-font
+      font_variant_caps = CSSPropertyFontUtils::ConsumeFontVariantCSS21(range);
+      if (font_variant_caps)
+        continue;
+    }
+    if (!font_weight) {
+      font_weight = CSSPropertyFontUtils::ConsumeFontWeight(range);
+      if (font_weight)
+        continue;
+    }
+    if (!font_stretch && CSSParserFastPaths::IsValidKeywordPropertyAndValue(
+                             CSSPropertyFontStretch, id, context->Mode()))
+      font_stretch = CSSPropertyParserHelpers::ConsumeIdent(range);
+    else
+      break;
+  }
+
+  if (range.AtEnd())
+    return false;
+
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontStyle, CSSPropertyFont,
+      font_style ? *font_style : *CSSIdentifierValue::Create(CSSValueNormal),
+      important, false /* implicit */, properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontVariantCaps, CSSPropertyFont,
+      font_variant_caps ? *font_variant_caps
+                        : *CSSIdentifierValue::Create(CSSValueNormal),
+      important, false /* implicit */, properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontVariantLigatures, CSSPropertyFont,
+      *CSSIdentifierValue::Create(CSSValueNormal), important,
+      false /* implicit */, properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontVariantNumeric, CSSPropertyFont,
+      *CSSIdentifierValue::Create(CSSValueNormal), important,
+      false /* implicit */, properties);
+
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontWeight, CSSPropertyFont,
+      font_weight ? *font_weight : *CSSIdentifierValue::Create(CSSValueNormal),
+      important, false /* implicit */, properties);
+  CSSPropertyParserHelpers::AddProperty(
+      CSSPropertyFontStretch, CSSPropertyFont,
+      font_stretch ? *font_stretch
+                   : *CSSIdentifierValue::Create(CSSValueNormal),
+      important, false /* implicit */, properties);
+
+  // Now a font size _must_ come.
+  CSSValue* font_size =
+      CSSPropertyFontUtils::ConsumeFontSize(range, context->Mode());
+  if (!font_size || range.AtEnd())
+    return false;
+
+  CSSPropertyParserHelpers::AddProperty(CSSPropertyFontSize, CSSPropertyFont,
+                                        *font_size, important,
+                                        false /* implicit */, properties);
+
+  if (CSSPropertyParserHelpers::ConsumeSlashIncludingWhitespace(range)) {
+    CSSValue* line_height =
+        CSSPropertyFontUtils::ConsumeLineHeight(range, context->Mode());
+    if (!line_height)
+      return false;
+    CSSPropertyParserHelpers::AddProperty(
+        CSSPropertyLineHeight, CSSPropertyFont, *line_height, important,
+        false /* implicit */, properties);
+  } else {
+    CSSPropertyParserHelpers::AddProperty(
+        CSSPropertyLineHeight, CSSPropertyFont,
+        *CSSIdentifierValue::Create(CSSValueNormal), important,
+        false /* implicit */, properties);
+  }
+
+  // Font family must come now.
+  CSSValue* parsed_family_value =
+      CSSPropertyFontUtils::ConsumeFontFamily(range);
+  if (!parsed_family_value)
+    return false;
+
+  CSSPropertyParserHelpers::AddProperty(CSSPropertyFontFamily, CSSPropertyFont,
+                                        *parsed_family_value, important,
+                                        false /* implicit */, properties);
+
+  // FIXME: http://www.w3.org/TR/2011/WD-css3-fonts-20110324/#font-prop requires
+  // that "font-stretch", "font-size-adjust", and "font-kerning" be reset to
+  // their initial values but we don't seem to support them at the moment. They
+  // should also be added here once implemented.
+  return range.AtEnd();
+}
+
+}  // namespace
+
+bool CSSShorthandPropertyAPIFont::parseShorthand(
+    bool important,
+    CSSParserTokenRange& range,
+    const CSSParserContext* context,
+    const CSSParserLocalContext&,
+    HeapVector<CSSProperty, 256>& properties) {
+  const CSSParserToken& token = range.Peek();
+  if (token.Id() >= CSSValueCaption && token.Id() <= CSSValueStatusBar)
+    return ConsumeSystemFont(important, range, properties);
+  return ConsumeFont(important, range, context, properties);
+}
+}  // namespace blink