Remove calc types from CSSPrimitiveValue::UnitType

The calc unit types no longer make too much sense after we have split
CSSMathFunctionValue from CSSPrimitiveValue, as:
- They complicate UnitType by introducing some entries that are not
  really unit types
- If a client really relies on these calc types, they can refer to
  CSSMathFunctionValue::Category() instead

Hence, this patch removes them for code health.

Bug: 979895
Change-Id: I9c8720bd68d64f756a6c4a251f59666350649b29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1691554Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675690}

third_party/blink/renderer/core/css/css_math_expression_node.cc

@@ -116,17 +116,9 @@ static bool HasDoubleValue(CSSPrimitiveValue::UnitType type) {
     case CSSPrimitiveValue::UnitType::kFraction:
     case CSSPrimitiveValue::UnitType::kInteger:
       return true;
-    case CSSPrimitiveValue::UnitType::kUnknown:
-    case CSSPrimitiveValue::UnitType::kCalc:
-    case CSSPrimitiveValue::UnitType::kCalcPercentageWithNumber:
-    case CSSPrimitiveValue::UnitType::kCalcPercentageWithLength:
-    case CSSPrimitiveValue::UnitType::kCalcLengthWithNumber:
-    case CSSPrimitiveValue::UnitType::kCalcPercentageWithLengthAndNumber:
-    case CSSPrimitiveValue::UnitType::kQuirkyEms:
+    default:
       return false;
-  };
-  NOTREACHED();
-  return false;
+  }
 }
 
 // ------ Start of CSSMathExpressionNumericLiteral member functions ------

third_party/blink/renderer/core/css/css_math_function_value.cc

@@ -23,8 +23,7 @@ void CSSMathFunctionValue::TraceAfterDispatch(blink::Visitor* visitor) {
 
 CSSMathFunctionValue::CSSMathFunctionValue(CSSMathExpressionNode* expression,
                                            ValueRange range)
-    : CSSPrimitiveValue(UnitType::kCalc, kMathFunctionClass),
-      expression_(expression) {
+    : CSSPrimitiveValue(kMathFunctionClass), expression_(expression) {
   is_non_negative_math_function_ = range == kValueRangeNonNegative;
 }
 
@@ -59,20 +58,11 @@ CSSPrimitiveValue::UnitType CSSMathFunctionValue::TypeWithMathFunctionResolved()
       return UnitType::kPercentage;
     case kCalcLength:
       return UnitType::kPixels;
-    case kCalcPercentNumber:
-      return UnitType::kCalcPercentageWithNumber;
-    case kCalcPercentLength:
-      return UnitType::kCalcPercentageWithLength;
-    case kCalcLengthNumber:
-      return UnitType::kCalcLengthWithNumber;
-    case kCalcPercentLengthNumber:
-      return UnitType::kCalcPercentageWithLengthAndNumber;
     case kCalcTime:
       return UnitType::kMilliseconds;
-    case kCalcOther:
+    default:
       return UnitType::kUnknown;
   }
-  return UnitType::kUnknown;
 }
 
 bool CSSMathFunctionValue::MayHaveRelativeUnit() const {

third_party/blink/renderer/core/css/css_numeric_literal_value.cc

@@ -22,8 +22,9 @@ void CSSNumericLiteralValue::TraceAfterDispatch(blink::Visitor* visitor) {
 }
 
 CSSNumericLiteralValue::CSSNumericLiteralValue(double num, UnitType type)
-    : CSSPrimitiveValue(type, kNumericLiteralClass), num_(num) {
+    : CSSPrimitiveValue(kNumericLiteralClass), num_(num) {
   DCHECK(std::isfinite(num));
+  numeric_literal_unit_type_ = static_cast<unsigned>(type);
 }
 
 // static

third_party/blink/renderer/core/css/css_primitive_value.cc

@@ -94,16 +94,19 @@ CSSPrimitiveValue::UnitCategory CSSPrimitiveValue::UnitTypeToUnitCategory(
 }
 
 CSSPrimitiveValue::UnitType CSSPrimitiveValue::TypeWithCalcResolved() const {
-  if (GetType() != UnitType::kCalc)
+  if (IsNumericLiteralValue())
     return GetType();
   return To<CSSMathFunctionValue>(this)->TypeWithMathFunctionResolved();
 }
 
-CSSPrimitiveValue::CSSPrimitiveValue(UnitType unit_type, ClassType class_type)
-    : CSSValue(class_type) {
-  primitive_unit_type_ = static_cast<unsigned>(unit_type);
+bool CSSPrimitiveValue::IsCalculatedPercentageWithLength() const {
+  return IsCalculated() &&
+         To<CSSMathFunctionValue>(this)->Category() == kCalcPercentLength;
 }
 
+CSSPrimitiveValue::CSSPrimitiveValue(ClassType class_type)
+    : CSSValue(class_type) {}
+
 // static
 CSSPrimitiveValue* CSSPrimitiveValue::CreateFromLength(const Length& length,
                                                        float zoom) {
@@ -462,12 +465,7 @@ const char* CSSPrimitiveValue::UnitTypeToString(UnitType type) {
       return "vmin";
     case UnitType::kViewportMax:
       return "vmax";
-    case UnitType::kUnknown:
-    case UnitType::kCalc:
-    case UnitType::kCalcPercentageWithNumber:
-    case UnitType::kCalcPercentageWithLength:
-    case UnitType::kCalcLengthWithNumber:
-    case UnitType::kCalcPercentageWithLengthAndNumber:
+    default:
       break;
   }
   NOTREACHED();

third_party/blink/renderer/core/css/css_primitive_value.h

@@ -106,14 +106,6 @@ class CORE_EXPORT CSSPrimitiveValue : public CSSValue {
     kFraction,
     kInteger,
 
-    // TODO(crbug.com/979895): Remove |kCalc|, and move/remove the remaining
-    // calc flags to CSSMathFunctionValue.
-    kCalc,
-    kCalcPercentageWithNumber,
-    kCalcPercentageWithLength,
-    kCalcLengthWithNumber,
-    kCalcPercentageWithLengthAndNumber,
-
     // This value is used to handle quirky margins in reflow roots (body, td,
     // and th) like WinIE. The basic idea is that a stylesheet can use the value
     // __qem (for quirky em) instead of em. When the quirky value is used, if
@@ -205,13 +197,8 @@ class CORE_EXPORT CSSPrimitiveValue : public CSSValue {
   static bool IsFrequency(UnitType unit) {
     return unit == UnitType::kHertz || unit == UnitType::kKilohertz;
   }
-  bool IsCalculated() const { return GetType() == UnitType::kCalc; }
-  bool IsCalculatedPercentageWithNumber() const {
-    return TypeWithCalcResolved() == UnitType::kCalcPercentageWithNumber;
-  }
-  bool IsCalculatedPercentageWithLength() const {
-    return TypeWithCalcResolved() == UnitType::kCalcPercentageWithLength;
-  }
+  bool IsCalculated() const { return IsMathFunctionValue(); }
+  bool IsCalculatedPercentageWithLength() const;
   static bool IsResolution(UnitType type) {
     return type >= UnitType::kDotsPerPixel &&
            type <= UnitType::kDotsPerCentimeter;
@@ -275,7 +262,7 @@ class CORE_EXPORT CSSPrimitiveValue : public CSSValue {
   static UnitType LengthUnitTypeToUnitType(LengthUnitType);
 
  protected:
-  CSSPrimitiveValue(UnitType unit_type, ClassType class_type);
+  explicit CSSPrimitiveValue(ClassType class_type);
 
   // Code generated by css_primitive_value_unit_trie.cc.tmpl
   static UnitType StringToUnitType(const LChar*, unsigned length);
@@ -283,8 +270,9 @@ class CORE_EXPORT CSSPrimitiveValue : public CSSValue {
 
   double ComputeLengthDouble(const CSSToLengthConversionData&) const;
 
+  // TODO(crbug.com/979895): This should be moved to CSSNumericLiteralValue
   inline UnitType GetType() const {
-    return static_cast<UnitType>(primitive_unit_type_);
+    return static_cast<UnitType>(numeric_literal_unit_type_);
   }
 };
 

third_party/blink/renderer/core/css/css_value.h

@@ -268,7 +268,7 @@ class CORE_EXPORT CSSValue : public GarbageCollectedFinalized<CSSValue> {
   ClassType GetClassType() const { return static_cast<ClassType>(class_type_); }
 
   explicit CSSValue(ClassType class_type)
-      : primitive_unit_type_(0),
+      : numeric_literal_unit_type_(0),
         value_list_separator_(kSpaceSeparator),
         is_non_negative_math_function_(false),
         class_type_(class_type) {}
@@ -280,8 +280,8 @@ class CORE_EXPORT CSSValue : public GarbageCollectedFinalized<CSSValue> {
   // The bits in this section are only used by specific subclasses but kept here
   // to maximize struct packing.
 
-  // CSSPrimitiveValue bits:
-  unsigned primitive_unit_type_ : 7;  // CSSPrimitiveValue::UnitType
+  // CSSNumericLiteralValue bits:
+  unsigned numeric_literal_unit_type_ : 7;  // CSSPrimitiveValue::UnitType
 
   unsigned value_list_separator_ : kValueListSeparatorBits;
 

third_party/blink/renderer/core/svg/svg_length.cc

@@ -21,6 +21,7 @@
 
 #include "third_party/blink/renderer/core/svg/svg_length.h"
 
+#include "third_party/blink/renderer/core/css/css_math_function_value.h"
 #include "third_party/blink/renderer/core/css/css_numeric_literal_value.h"
 #include "third_party/blink/renderer/core/css/css_primitive_value.h"
 #include "third_party/blink/renderer/core/css/css_value.h"
@@ -132,20 +133,28 @@ void SVGLength::SetValue(float value, const SVGLengthContext& context) {
       value_->TypeWithCalcResolved());
 }
 
-static bool IsCalcCSSUnitType(CSSPrimitiveValue::UnitType type) {
-  return type >= CSSPrimitiveValue::UnitType::kCalc &&
-         type <=
-             CSSPrimitiveValue::UnitType::kCalcPercentageWithLengthAndNumber;
-}
-
 static bool IsSupportedCSSUnitType(CSSPrimitiveValue::UnitType type) {
   return (CSSPrimitiveValue::IsLength(type) ||
           type == CSSPrimitiveValue::UnitType::kNumber ||
-          type == CSSPrimitiveValue::UnitType::kPercentage ||
-          IsCalcCSSUnitType(type)) &&
+          type == CSSPrimitiveValue::UnitType::kPercentage) &&
          type != CSSPrimitiveValue::UnitType::kQuirkyEms;
 }
 
+static bool IsSupportedCalculationCategory(CalculationCategory category) {
+  switch (category) {
+    case kCalcLength:
+    case kCalcNumber:
+    case kCalcPercent:
+    case kCalcPercentNumber:
+    case kCalcPercentLength:
+    case kCalcLengthNumber:
+    case kCalcPercentLengthNumber:
+      return true;
+    default:
+      return false;
+  }
+}
+
 void SVGLength::SetUnitType(CSSPrimitiveValue::UnitType type) {
   DCHECK(IsSupportedCSSUnitType(type));
   value_ = CSSNumericLiteralValue::Create(value_->GetFloatValue(), type);
@@ -212,8 +221,13 @@ SVGParsingError SVGLength::SetValueAsString(const String& string) {
   if (!new_value)
     return SVGParseStatus::kExpectedLength;
 
-  if (!IsSupportedCSSUnitType(new_value->TypeWithCalcResolved()))
-    return SVGParseStatus::kExpectedLength;
+  if (const auto* math_value = DynamicTo<CSSMathFunctionValue>(new_value)) {
+    if (!IsSupportedCalculationCategory(math_value->Category()))
+      return SVGParseStatus::kExpectedLength;
+  } else {
+    if (!IsSupportedCSSUnitType(new_value->TypeWithCalcResolved()))
+      return SVGParseStatus::kExpectedLength;
+  }
 
   value_ = new_value;
   return SVGParseStatus::kNoError;