case CSSPropertyBaselineShift: { switch (svgStyle->baselineShift()) { case BS_BASELINE: return CSSPrimitiveValue::createIdentifier(CSSValueBaseline); case BS_SUPER: return CSSPrimitiveValue::createIdentifier(CSSValueSuper); case BS_SUB: return CSSPrimitiveValue::createIdentifier(CSSValueSub); case BS_LENGTH: return svgStyle->baselineShiftValue(); } } case CSSPropertyGlyphOrientationHorizontal: return glyphOrientationToCSSPrimitiveValue(svgStyle->glyphOrientationHorizontal()); If svgStyle->baselineShift() does not match any of the above four values, which could only happen if parsing was added for the value and not added here, then we'd return a value from glyphOrientationToCSSPrimitiveValue(). I basically copied the comment and assertion from a couple lines below, which checks for the same case on different level. Attaching patch shortly.
Created attachment 33909 [details] Assert on unhandled values for baselineShift
Comment on attachment 33909 [details] Assert on unhandled values for baselineShift By adding the default case, you are turning off the gcc feature that complains if an enum value is not handled in a switch statement. So you are turning off compile-time detection (under gcc) of the very thing you are trying to catch at runtime. And compile-time detection is even better, so we don't want it turned off! Please put the assertion outside the switch statement instead of adding a default case. Or just drop this while thing, since we'll catch missing enum values immediately due to gcc's warning as soon as this file is compiled with gcc.
Ok, thanks for letting me know about that. Would you be opposed to me moving the assert outside the switch statement for the sake of older and/or non-gcc compilers?
Yes, we can certainly add ASSERT_NOT_REACHED after the switch. That would be beneficial, because even with a compile time check, there can be e.g. memory corruption that results in a value that's not covered by case statements.
Closing since the assertion suggested in the previous commented was added in bug 83536. The motivation was the same of this bug. *** This bug has been marked as a duplicate of bug 83536 ***