[webkit-reviews] review granted: [Bug 192671] CSS Typed OM should expose attributeStyleMap : [Attachment 357347] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Justin Michaud
<justin_michaud at apple.com>'s request for review:
Bug 192671: CSS Typed OM should expose attributeStyleMap
https://bugs.webkit.org/show_bug.cgi?id=192671

Attachment 357347: Patch

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




--- Comment #32 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 357347
  --> https://bugs.webkit.org/attachment.cgi?id=357347
Patch

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

> Source/WebCore/ChangeLog:18
> +	   * css/ElementCSSInlineStyle.idl:

You're missing an entry for CodeGeneratorJS.pm. Please add it with a
description as to what you're changing.

> Source/WebCore/css/typedom/StylePropertyMap.h:38
> +public:

I don't think we need to have this access specifier.

> Source/WebCore/dom/StyledElement.cpp:89
> +		   auto* registered =
element.document().getCSSRegisteredCustomPropertySet().get(name);

Nit: we should probably address this in a separate patch but our coding
guideline mandates
that this function, which doesn't have an out argument, to not have "get"
prefix:
https://webkit.org/code-style-guidelines/#names-out-argument

> Source/WebCore/platform/graphics/CustomPaintImage.cpp:62
> +static RefPtr<TypedOMCSSStyleValue> extractComputedProperty(const String&
name, Element& element)

Can we share this code with the one in StyledElement.cpp e.g. using a template?
It's not great to duplicate this much code in two places.