Refactor CSSPropertyDescriptor structure, rename it to CSSPropertyAPI.

Desktop / Chromium - Eddy Mead [chromium.org] - 7 August 2017 00:44 EDT

Design doc (with benchmarks!): https://docs.google.com/document/d/1uwS_TT8VBWSQNYMnOggwqcnv6YU85FIFvD_egxPSn6o/edit

This CL changes the current "struct that contains function-pointer-or-nulls" CSSPropertyDescriptor design to instead use virtual function dispatch andstatic constexpr for returning a pointer to a CSSPropertyAPI or subclass.


This makes the call sites cleaner, as since there cannot be null function pointers any more, this allows us to write:

CSSPropertyAPI::Get(property_id).ParseSingleValue(...);

and avoid the if statement that we would require with the current implementation:

CSSPropertyDescriptor& d = CSSPropertyDescriptor::Get(property_id); if (d.ParseSingleValue) d.ParseSingleValue(...);

My benchmarks (details in the linked doc) show no speed penalty for doing this for getting an API, or for calling functions on it (in fact, it's probably faster). Inspecting the assembly generated from the benchmarks also shows that the use of constexpr correctly causes the compiler to avoid allocating any memory for instances of CSSPropertyAPI or subclasses, so function calls are still just dereferencing straight into the vtable of the CSSPropertyAPI subclass.


Notes:- CSSPropertyAPI.h/.cpp and CSSPropertyAPICustom.cpp replace CSSPropertyDescriptor.- To avoid CSSPropertyAPICustom.cpp from getting too big, I created a temporary "CSSPropertyAPICustomHelper" containing the not yet ribbonized helper functions it requires.

file moves undetected by gerrit:- The generator css_property_apis.py was split into two files: make_css_property_api.py, and make_css_property_api_headers.py.- CSSPropertyAPIFiles.h.tmpl was renamed to CSSPropertyAPIHeaders.h.tmpl


Gist showing curated diff of generated files: CSSPropertyAPI.cpp CSSPropertyAPIAlignItems.h // A longhand API header CSSPropertyDescriptor.cpp CSSPropertyDescriptor.h CSSShorthandPropertyAPIAnimation.h // A shorthand API header https://gist.github.com/wilddamon/2261ef381b648f9fe3cf9ed8ac037cd3/revisions

Gist showing diff between CSSPropertyAPI.cpp and the old CSSPropertyDescriptor.cpp: https://gist.github.com/wilddamon/ee1afa93637b85b4d1291b8a1d985b16/revisions

Gist showing diff between CSSPropertyParser.cpp and CSSPropertyAPICustomHelper.cpp: https://gist.github.com/wilddamon/9e81c157f98ff1dbb68d43a79f89fe9c/revisions

Gist showing diff between make_css_property_apis.py and make_css_property_api.py: https://gist.github.com/wilddamon/4d4ee2c2223c9b5ab7c857ab31aa68ec/revisions

Gist showing diff between make_css_property_apis.py and make_css_property_api_headers.py https://gist.github.com/wilddamon/08443f2980054866ebeef972b00d1746/revisions

Bug: 545324 Change-Id: Ie5465c02faa010094f78821c024f87c231bd2264 Reviewed-on: https://chromium-review.googlesource.com/589469 Commit-Queue: meade_UTC10

cfd7e50 Refactor CSSPropertyDescriptor structure, rename it to CSSPropertyAPI.
.../css/properties/make_css_property_api_base.py | 85 ++++
.../properties/make_css_property_api_headers.py | 67 ++++
.../core/css/properties/make_css_property_apis.py | 139 -------
.../properties/templates/CSSPropertyAPI.cpp.tmpl | 42 ++
.../templates/CSSPropertyAPIFiles.h.tmpl | 40 --
.../templates/CSSPropertyAPISubclass.h.tmpl | 34 ++
.../templates/CSSPropertyDescriptor.cpp.tmpl | 61 ---
.../templates/CSSPropertyDescriptor.h.tmpl | 36 --
third_party/WebKit/Source/core/BUILD.gn | 20 +-
third_party/WebKit/Source/core/css/BUILD.gn | 4 +
.../WebKit/Source/core/css/CSSProperties.json5 | 16 +-
.../Source/core/css/parser/CSSPropertyParser.cpp | 434 +--------------------
.../Source/core/css/parser/CSSPropertyParser.h | 11 -
.../core/css/parser/CSSPropertyParserHelpers.cpp | 60 +--
.../core/css/parser/CSSPropertyParserHelpers.h | 3 +-
.../Source/core/css/properties/CSSPropertyAPI.h | 38 ++
.../css/properties/CSSPropertyAPIAlignItems.cpp | 3 +-
.../CSSPropertyAPIAlignOrJustifyContent.cpp | 3 +-
.../CSSPropertyAPIAlignOrJustifySelf.cpp | 3 +-
.../CSSPropertyAPIAnimationDirection.cpp | 3 +-
.../properties/CSSPropertyAPIAnimationFillMode.cpp | 3 +-
.../CSSPropertyAPIAnimationIterationCount.cpp | 3 +-
.../css/properties/CSSPropertyAPIAnimationName.cpp | 3 +-
.../CSSPropertyAPIAnimationPlayState.cpp | 3 +-
.../css/properties/CSSPropertyAPIAutoOrString.cpp | 3 +-
.../CSSPropertyAPIBackgroundAttachment.cpp | 3 +-
.../CSSPropertyAPIBackgroundBlendMode.cpp | 3 +-
.../css/properties/CSSPropertyAPIBackgroundBox.cpp | 3 +-
.../properties/CSSPropertyAPIBackgroundColor.cpp | 3 +-
.../CSSPropertyAPIBackgroundOrMaskImage.cpp | 3 +-
.../css/properties/CSSPropertyAPIBaseCustom.cpp | 164 ++++++++
.../css/properties/CSSPropertyAPIBaseHelper.cpp | 359 +++++++++++++++++
.../core/css/properties/CSSPropertyAPIBaseHelper.h | 48 +++
.../css/properties/CSSPropertyAPIBaselineShift.cpp | 3 +-
.../css/properties/CSSPropertyAPIBorderColor.cpp | 3 +-
.../properties/CSSPropertyAPIBorderImageOutset.cpp | 3 +-
.../properties/CSSPropertyAPIBorderImageRepeat.cpp | 3 +-
.../properties/CSSPropertyAPIBorderImageSlice.cpp | 3 +-
.../properties/CSSPropertyAPIBorderImageWidth.cpp | 3 +-
.../css/properties/CSSPropertyAPIBorderRadius.cpp | 3 +-
.../css/properties/CSSPropertyAPIBorderWidth.cpp | 3 +-
.../css/properties/CSSPropertyAPIBoxShadow.cpp | 3 +-
.../css/properties/CSSPropertyAPICaretColor.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIClip.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIClipPath.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIColor.cpp | 3 +-
.../css/properties/CSSPropertyAPIColorNoQuirks.cpp | 3 +-
.../css/properties/CSSPropertyAPIColumnCount.cpp | 3 +-
.../css/properties/CSSPropertyAPIColumnGap.cpp | 3 +-
.../properties/CSSPropertyAPIColumnRuleWidth.cpp | 3 +-
.../css/properties/CSSPropertyAPIColumnSpan.cpp | 3 +-
.../css/properties/CSSPropertyAPIColumnWidth.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIContain.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIContent.cpp | 3 +-
.../properties/CSSPropertyAPICounterIncrement.cpp | 3 +-
.../css/properties/CSSPropertyAPICounterReset.cpp | 3 +-
.../core/css/properties/CSSPropertyAPICursor.cpp | 3 +-
.../Source/core/css/properties/CSSPropertyAPID.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIDelay.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIDuration.cpp | 3 +-
.../CSSPropertyAPIFillOrStrokeOpacity.cpp | 6 +-
.../core/css/properties/CSSPropertyAPIFilter.cpp | 3 +-
.../css/properties/CSSPropertyAPIFlexBasis.cpp | 3 +-
.../properties/CSSPropertyAPIFlexGrowOrShrink.cpp | 3 +-
.../css/properties/CSSPropertyAPIFontFamily.cpp | 3 +-
.../CSSPropertyAPIFontFeatureSettings.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIFontSize.cpp | 3 +-
.../properties/CSSPropertyAPIFontSizeAdjust.cpp | 3 +-
.../css/properties/CSSPropertyAPIFontStretch.cpp | 3 +-
.../css/properties/CSSPropertyAPIFontStyle.cpp | 3 +-
.../properties/CSSPropertyAPIFontVariantCaps.cpp | 3 +-
.../CSSPropertyAPIFontVariantLigatures.cpp | 3 +-
.../CSSPropertyAPIFontVariantNumeric.cpp | 3 +-
.../CSSPropertyAPIFontVariationSettings.cpp | 3 +-
.../css/properties/CSSPropertyAPIFontWeight.cpp | 3 +-
.../css/properties/CSSPropertyAPIGridAutoFlow.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIGridGap.cpp | 3 +-
.../properties/CSSPropertyAPIGridTemplateAreas.cpp | 3 +-
.../properties/CSSPropertyAPIImageOrientation.cpp | 3 +-
.../css/properties/CSSPropertyAPIImageSource.cpp | 6 +-
.../css/properties/CSSPropertyAPIJustifyItems.cpp | 3 +-
.../core/css/properties/CSSPropertyAPILength.cpp | 4 +-
.../CSSPropertyAPILetterAndWordSpacing.cpp | 3 +-
.../css/properties/CSSPropertyAPILineHeight.cpp | 3 +-
.../properties/CSSPropertyAPILineHeightStep.cpp | 3 +-
.../properties/CSSPropertyAPIListStyleImage.cpp | 6 +-
.../CSSPropertyAPILogicalWidthOrHeight.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIMargin.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIMarker.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIMask.cpp | 3 +-
.../properties/CSSPropertyAPIMaskSourceType.cpp | 3 +-
.../css/properties/CSSPropertyAPIMethods.json5 | 15 +-
.../properties/CSSPropertyAPIObjectPosition.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIOffset.cpp | 6 +-
.../css/properties/CSSPropertyAPIOffsetAnchor.cpp | 3 +-
.../properties/CSSPropertyAPIOffsetDistance.cpp | 3 +-
.../css/properties/CSSPropertyAPIOffsetPath.cpp | 3 +-
.../properties/CSSPropertyAPIOffsetPosition.cpp | 3 +-
.../css/properties/CSSPropertyAPIOffsetRotate.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIOpacity.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIOrder.cpp | 3 +-
.../properties/CSSPropertyAPIOrphansOrWidows.cpp | 3 +-
.../css/properties/CSSPropertyAPIOutlineColor.cpp | 3 +-
.../css/properties/CSSPropertyAPIOutlineOffset.cpp | 3 +-
.../css/properties/CSSPropertyAPIOutlineWidth.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIPadding.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIPage.cpp | 3 +-
.../css/properties/CSSPropertyAPIPaintOrder.cpp | 3 +-
.../css/properties/CSSPropertyAPIPaintStroke.cpp | 3 +-
.../css/properties/CSSPropertyAPIPerspective.cpp | 3 +-
.../properties/CSSPropertyAPIPerspectiveOrigin.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIQuotes.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIRadius.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIRotate.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIScale.cpp | 3 +-
.../css/properties/CSSPropertyAPIScrollPadding.cpp | 3 +-
.../properties/CSSPropertyAPIScrollSnapAlign.cpp | 3 +-
.../properties/CSSPropertyAPIScrollSnapMargin.cpp | 3 +-
.../properties/CSSPropertyAPIScrollSnapType.cpp | 3 +-
.../CSSPropertyAPIShapeImageThreshold.cpp | 3 +-
.../css/properties/CSSPropertyAPIShapeMargin.cpp | 3 +-
.../css/properties/CSSPropertyAPIShapeOutside.cpp | 3 +-
.../core/css/properties/CSSPropertyAPISize.cpp | 3 +-
.../properties/CSSPropertyAPIStrokeDasharray.cpp | 3 +-
...CSSPropertyAPIStrokeDashoffsetOrStrokeWidth.cpp | 4 +-
.../properties/CSSPropertyAPIStrokeMiterlimit.cpp | 3 +-
.../core/css/properties/CSSPropertyAPITabSize.cpp | 3 +-
.../CSSPropertyAPITextDecorationColor.cpp | 3 +-
.../CSSPropertyAPITextDecorationLine.cpp | 3 +-
.../CSSPropertyAPITextDecorationSkip.cpp | 3 +-
.../css/properties/CSSPropertyAPITextIndent.cpp | 3 +-
.../css/properties/CSSPropertyAPITextShadow.cpp | 3 +-
.../properties/CSSPropertyAPITextSizeAdjust.cpp | 3 +-
.../CSSPropertyAPITextUnderlinePosition.cpp | 3 +-
.../properties/CSSPropertyAPITimingFunction.cpp | 3 +-
.../css/properties/CSSPropertyAPITouchAction.cpp | 3 +-
.../css/properties/CSSPropertyAPITransform.cpp | 3 +-
.../properties/CSSPropertyAPITransformOrigin.cpp | 3 +-
.../CSSPropertyAPITransitionProperty.cpp | 3 +-
.../css/properties/CSSPropertyAPITranslate.cpp | 3 +-
.../css/properties/CSSPropertyAPIVerticalAlign.cpp | 3 +-
.../properties/CSSPropertyAPIWebkitBorderColor.cpp | 6 +-
.../properties/CSSPropertyAPIWebkitBorderImage.cpp | 3 +-
.../CSSPropertyAPIWebkitBorderSpacing.cpp | 3 +-
.../properties/CSSPropertyAPIWebkitBorderWidth.cpp | 3 +-
.../css/properties/CSSPropertyAPIWebkitBoxFlex.cpp | 3 +-
.../CSSPropertyAPIWebkitBoxFlexGroup.cpp | 3 +-
.../CSSPropertyAPIWebkitBoxOrdinalGroup.cpp | 9 +-
.../properties/CSSPropertyAPIWebkitBoxReflect.cpp | 7 +-
.../CSSPropertyAPIWebkitColorNoQuirks.cpp | 6 +-
.../CSSPropertyAPIWebkitFontSizeDelta.cpp | 3 +-
.../properties/CSSPropertyAPIWebkitHighlight.cpp | 3 +-
.../properties/CSSPropertyAPIWebkitLineClamp.cpp | 3 +-
.../css/properties/CSSPropertyAPIWebkitMargin.cpp | 3 +-
.../CSSPropertyAPIWebkitMaskComposite.cpp | 3 +-
...CSSPropertyAPIWebkitMaxLogicalWidthOrHeight.cpp | 3 +-
.../css/properties/CSSPropertyAPIWebkitOriginX.cpp | 3 +-
.../css/properties/CSSPropertyAPIWebkitOriginY.cpp | 3 +-
.../css/properties/CSSPropertyAPIWebkitPadding.cpp | 3 +-
...CSSPropertyAPIWebkitTextDecorationsInEffect.cpp | 3 +-
.../CSSPropertyAPIWebkitTextEmphasisStyle.cpp | 3 +-
.../CSSPropertyAPIWebkitTextStrokeColor.cpp | 6 +-
.../CSSPropertyAPIWebkitTextStrokeWidth.cpp | 3 +-
.../CSSPropertyAPIWebkitTransformOriginZ.cpp | 3 +-
.../css/properties/CSSPropertyAPIWillChange.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIZIndex.cpp | 3 +-
.../core/css/properties/CSSPropertyAPIZoom.cpp | 3 +-
.../CSSShorthandPropertyAPIAnimation.cpp | 3 +-
.../CSSShorthandPropertyAPIBackgroundPosition.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderBottom.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderColor.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderImage.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderLeft.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderRadius.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderRight.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderSpacing.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderStyle.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderTop.cpp | 3 +-
.../CSSShorthandPropertyAPIBorderWidth.cpp | 3 +-
.../CSSShorthandPropertyAPIColumnRule.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIColumns.cpp | 3 +-
.../css/properties/CSSShorthandPropertyAPIFlex.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIFlexFlow.cpp | 3 +-
.../css/properties/CSSShorthandPropertyAPIFont.cpp | 3 +-
.../CSSShorthandPropertyAPIFontVariant.cpp | 5 +-
.../css/properties/CSSShorthandPropertyAPIGrid.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIGridArea.cpp | 3 +-
.../CSSShorthandPropertyAPIGridColumn.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIGridGap.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIGridRow.cpp | 3 +-
.../CSSShorthandPropertyAPIGridTemplate.cpp | 3 +-
.../CSSShorthandPropertyAPIListStyle.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIMargin.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIMarker.cpp | 9 +-
.../properties/CSSShorthandPropertyAPIOffset.cpp | 19 +-
.../properties/CSSShorthandPropertyAPIOutline.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIOverflow.cpp | 3 +-
.../properties/CSSShorthandPropertyAPIPadding.cpp | 3 +-
.../CSSShorthandPropertyAPIPageBreakAfter.cpp | 3 +-
.../CSSShorthandPropertyAPIPageBreakBefore.cpp | 3 +-
.../CSSShorthandPropertyAPIPageBreakInside.cpp | 3 +-
.../CSSShorthandPropertyAPIPlaceContent.cpp | 3 +-
.../CSSShorthandPropertyAPIPlaceItems.cpp | 3 +-
.../CSSShorthandPropertyAPIPlaceSelf.cpp | 3 +-
...SShorthandPropertyAPIScrollBoundaryBehavior.cpp | 3 +-
.../CSSShorthandPropertyAPIScrollPadding.cpp | 3 +-
.../CSSShorthandPropertyAPIScrollPaddingBlock.cpp | 3 +-
.../CSSShorthandPropertyAPIScrollPaddingInline.cpp | 3 +-
.../CSSShorthandPropertyAPIScrollSnapMargin.cpp | 3 +-
...SSShorthandPropertyAPIScrollSnapMarginBlock.cpp | 3 +-
...SShorthandPropertyAPIScrollSnapMarginInline.cpp | 3 +-
.../CSSShorthandPropertyAPITextDecoration.cpp | 3 +-
.../CSSShorthandPropertyAPITransition.cpp | 3 +-
.../CSSShorthandPropertyAPIWebkitBorderAfter.cpp | 3 +-
.../CSSShorthandPropertyAPIWebkitBorderBefore.cpp | 3 +-
.../CSSShorthandPropertyAPIWebkitBorderEnd.cpp | 3 +-
.../CSSShorthandPropertyAPIWebkitBorderStart.cpp | 3 +-
...SShorthandPropertyAPIWebkitColumnBreakAfter.cpp | 3 +-
...ShorthandPropertyAPIWebkitColumnBreakBefore.cpp | 3 +-
...ShorthandPropertyAPIWebkitColumnBreakInside.cpp | 3 +-
...CSSShorthandPropertyAPIWebkitMarginCollapse.cpp | 3 +-
.../CSSShorthandPropertyAPIWebkitMaskBoxImage.cpp | 3 +-
.../CSSShorthandPropertyAPIWebkitMaskPosition.cpp | 3 +-
.../CSSShorthandPropertyAPIWebkitTextEmphasis.cpp | 3 +-
.../CSSShorthandPropertyAPIWebkitTextStroke.cpp | 3 +-
.../WebKit/Source/core/css/properties/README.md | 4 +-
226 files changed, 1337 insertions(+), 1015 deletions(-)

Upstream: git.chromium.org


  • Share