[webkit-changes] [WebKit/WebKit] ef0496: Make CSSPrimitiveValueMappings use the CSSValuePool

[Darin Adler]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: ef049694b61c0c82f351d1a33f736c4b5f0f3532
      https://github.com/WebKit/WebKit/commit/ef049694b61c0c82f351d1a33f736c4b5f0f3532
  Author: Darin Adler <darin at apple.com>
  Date:   2023-01-18 (Wed, 18 Jan 2023)

  Changed paths:
    M Source/WebCore/css/CSSBackgroundRepeatValue.cpp
    M Source/WebCore/css/CSSBackgroundRepeatValue.h
    M Source/WebCore/css/CSSContentDistributionValue.cpp
    M Source/WebCore/css/CSSContentDistributionValue.h
    M Source/WebCore/css/CSSPrimitiveValue.cpp
    M Source/WebCore/css/CSSPrimitiveValue.h
    M Source/WebCore/css/CSSPrimitiveValueMappings.h
    M Source/WebCore/css/CSSToStyleMap.cpp
    M Source/WebCore/css/ComputedStyleExtractor.cpp
    M Source/WebCore/css/DeprecatedCSSOMRGBColor.h
    M Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp
    M Source/WebCore/css/typedom/CSSUnitValue.cpp
    M Source/WebCore/css/typedom/numeric/CSSMathValue.h
    M Source/WebCore/html/HTMLElement.h
    M Source/WebCore/rendering/style/RenderStyleConstants.cpp
    M Source/WebCore/rendering/style/RenderStyleConstants.h
    M Source/WebCore/style/StyleBuilderConverter.h

  Log Message:
  -----------
  Make CSSPrimitiveValueMappings use the CSSValuePool
https://bugs.webkit.org/show_bug.cgi?id=250648
rdar://problem/104278110

Reviewed by Tim Nguyen.

CSSPrimitiveValueMappings.h is a file full of constructor template specializations and conversion
operator template instances for CSSPrimitiveValue, along with an assortment of other things.

The main purpose of this change is to get rid of all those constructor template specializations,
since we would like to allocate the values out of the StaticCSSValuePool, and so we need to use
create functions, not direct construction.

We do this by specializing a create function template rather than a constructor template. But
making the observation that almost all of these involve mapping to and from CSSValueID to
enumerations, in most cases we do not specialize the create function template or the conversion
operator template. Instead we overload a function named toCSSValueID and specialize a function
name fromCSSValueID in all but the most exotic cases. And further, for the simplest mappings, we
use a macro, so both toCSSValueID and fromCSSValueID are generated by the macro.

In addition, we simplified call sites that are unnecessarily using CSSPrimitiveValue in
cases where instead a CSSValueID suffices.

Part of this is taking the knowledge about these mappings and moving it out into functions
that are not specific to CSSPrimitiveValue. In future i's not clear we should have a single
CSSPrimitiveValue. Instead we could have a separate CSSValue class for each of the types in
the union. Having two levels of polymorphism isn't all that helpful. The way that CSSUnitType
mixes structure types and different types of numeric value is pretty messy and also seems like
something we do not need to keep around in its current form.

* Source/WebCore/css/CSSBackgroundRepeatValue.cpp:
(WebCore::CSSBackgroundRepeatValue::CSSBackgroundRepeatValue): Use CSSValueID instead of
CSSPrimitiveValue.
(WebCore::CSSBackgroundRepeatValue::create): Moved this out of the header for better inlining.
(WebCore::CSSBackgroundRepeatValue::customCSSText const): Use CSSValueID.
(WebCore::CSSBackgroundRepeatValue::equals const): Ditto.
* Source/WebCore/css/CSSBackgroundRepeatValue.h: Ditto.

* Source/WebCore/css/CSSContentDistributionValue.cpp:
(WebCore::CSSContentDistributionValue::~CSSContentDistributionValue): Removed this since it can
be generated in the header without explicitly defining it at all and since this class adds no
data members with destructors it will be trivial.
(WebCore::CSSContentDistributionValue::create): Moved this out of the header for better inlining.
(WebCore::CSSContentDistributionValue::customCSSText const): Build this all at once with
makeString rather than using StringBuilder.
* Source/WebCore/css/CSSContentDistributionValue.h: Removed unneeded includes and destructor.

* Source/WebCore/css/CSSPrimitiveValue.cpp:
(WebCore::CSSPrimitiveValue::CSSPrimitiveValue): Got rid of most constructors. The only ones
we kept are the ones for the actual primitive types that this class stores. Changed the
CSSCalcValue one to take a Ref like the others instead of a RefPtr. Changed the Color
constructor to store the color as an integer rather than on the heap. Our Color object
basically works as a union of integer and smart pointer and for our use here we just need
to know how to copy and destroy it. We copy it using a placement new operator and destroy
it using an explicit call to the destructor.
(WebCore::CSSPrimitiveValue::init): Deleted. We don't need these functions any more because
they can be constructors now. The reason they needed to exist before was that we wanted to
share code iwth the code in CSSPrimitiveValueMappings.h and also this was long ago before
one constructor could call through to another.
(WebCore::CSSPrimitiveValue::cleanup): Updated the CSS_CALC case since it can't be nullptr
any more. Updated the CSS_RGBCOLOR case since we are now storing the Color directly rather
than allocating each Color on the heap.
(WebCore::CSSPrimitiveValue::create): Put most create functions here, where they can inline
the constructors. Also, some of the create functions simply call the other create functions.
This works well with the CSS value pool.

* Source/WebCore/css/CSSPrimitiveValue.h: Removed unneded include of Color.h since we can
just use a forward declaration. Added explicit overloads of create for all the basic types.
Also, create functions that call constructors that are not in the header are now moved to
the CSSPrimitiveValue.cpp file to give us better inlining of the constructors. Continue to
use a create function template for the automatic mapping, but now we specialize this instead
of having a default version that calls through to a constructor template. In the future, we
could consider giving the create function template a different name since it's not clear we
need call sites that mix the overloaded functions and function template, but for now this is
more like how the class worked before. Moved the conversion operators for integer types here,
and have them be separate functions, not template specializations. Add more constructors, one
for each of the value types the union knows how to store. Get rid of all the template
constructors. Replaced the color pointer with the colorAsInteger, which makes all the values
containing a Color more efficient. Added a function template named fromCSSValueID and a
conversion function template definition that calls it so we can specialize that instead of
specializing the conversion operator in almost all cases.

* Source/WebCore/css/CSSPrimitiveValueMappings.h: Removed all the specializations for
numeric types. Added the macros that we can use to define the toCSSValueID and fromCSSValueID
function for trivial cases. Used specializations of create and the conversion operator
in a few special cases, but in most cases, just overload toCSSValueID and specialize
fromCSSValueID.

* Source/WebCore/css/CSSToStyleMap.cpp:
(WebCore::CSSToStyleMap::mapFillRepeat): Use fromCSSValueID since CSSBackgroundRepeatValue
now returns CSSValueID rather than a CSSPrimitiveValue.

* Source/WebCore/css/ComputedStyleExtractor.cpp:
(WebCore::fontSizeAdjustFromStyle): Handle the std::optional<float> here explicitly
rather than using a CSSPrimitiveValue feature specific to std::optional<float>.
(WebCore::ComputedStyleExtractor::valueForPropertyInStyle): Call toCSSValueID explicitly
in the case where we have to pass the property ID in, getting rid of the need to overload
CSSPrimitiveValue::create for this cases. Update a couple places that were relying on the
conversion operator to make a CSS_NUMBER to instead do that directly.

* Source/WebCore/css/DeprecatedCSSOMRGBColor.h: Added Color.h include since
CSSPrimitiveValue.h does not pull it in any more.

* Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::CalcParser::consumeValue): Use releaseNonNull since
we have already checked for null and create now takes a Ref rather than RefPtr.
(WebCore::CSSPropertyParserHelpers::CalcParser::consumeValueIfCategory): Ditto.
(WebCore::CSSPropertyParserHelpers::consumeRepeatStyle): Use consumeIdentRaw because
CSSBackgroundRepeatValue::create now takes CSSValueID, not CSSPrimitiveValue.

* Source/WebCore/css/typedom/CSSUnitValue.cpp:
(WebCore::CSSUnitValue::toCSSValueWithProperty const): Add an explicit check for
nodes we can't parse that cause CSSCalcValue::create to return null, and handle them
by returning nullptr rather than a CSSPrimitiveValue with a nullptr for its CSSCalcValue.
* Source/WebCore/css/typedom/numeric/CSSMathValue.h: Ditto.

* Source/WebCore/html/HTMLElement.h: Added ColorTypes.h include since CSSPrimitiveValue.h
does not pull in Color.h any more.

* Source/WebCore/rendering/style/RenderStyleConstants.cpp:
(WebCore::operator<<): Removed code to handle SpeakAs::Normal.

* Source/WebCore/rendering/style/RenderStyleConstants.h: Removed the Normal value from
SpeakAs, since we use OptionSet<SpeakAs> so we don't need a named empty value.

* Source/WebCore/style/StyleBuilderConverter.h:
(WebCore::Style::BuilderConverter::convertContentAlignmentData): Updated since
CSSContentDistributionValue returns CSSValueID, not CSSPrimitiveValue. We can use the
fromCSSValueID functions to do the conversion. The only thing less elegant is that we
now need to specify the type name. We could do this with implicit type conversion, we
would need to make a class to put the CSSValueID in that could do the conversion. That's
because enumerations themselves can't implement custom conversion functions.

Canonical link: https://commits.webkit.org/259025@main