[webkit-reviews] review granted: [Bug 190039] Registered custom properties should support syntax parameter for <length> and * : [Attachment 352848] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Justin Michaud
<justin_michaud at apple.com>'s request for review:
Bug 190039: Registered custom properties should support syntax parameter for
<length> and *
https://bugs.webkit.org/show_bug.cgi?id=190039

Attachment 352848: Patch

https://bugs.webkit.org/attachment.cgi?id=352848&action=review




--- Comment #36 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 352848
  --> https://bugs.webkit.org/attachment.cgi?id=352848
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=352848&action=review

> Source/WebCore/css/StyleProperties.cpp:794
> +    auto* registered = document?
document->getCSSRegisteredCustomPropertySet().get(propertyName) : nullptr;

Space before '?'

> Source/WebCore/css/StyleResolver.h:545
> +    HashSet<CSSPropertyID> appliedProperties;
> +    HashSet<String> appliedCustomProperties;
> +    HashSet<CSSPropertyID> inProgressProperties;
> +    HashSet<String> inProgressPropertiesCustom;
> +
> +    ApplyCascadedPropertyState(StyleResolver* styleResolver,
StyleResolver::CascadedProperties* cascade, const StyleResolver::MatchResult*
matchResult)
> +	   : styleResolver(styleResolver)
> +	   , cascade(cascade)
> +	   , matchResult(matchResult)
> +    {
> +    }

Wonder if just having

HashSet<CSSPropertyID> appliedProperties = { }; 
...

would work instead of constructor? This stuff has been changing in C++11/14/17

> Source/WebCore/css/parser/CSSParser.cpp:222
> +    const CSSCustomPropertyValue& customPropValue =
downcast<CSSCustomPropertyValue>(value);

could just use auto to avoid repeating CSSCustomPropertyValue

> Source/WebCore/css/parser/CSSParser.cpp:223
> +    const auto& valueWithReferences =
WTF::get<Ref<CSSVariableReferenceValue>>(customPropValue.value()).get();

Should there be an assert before get? Not sure if get asserts itself.