[webkit-reviews] review granted: [Bug 238125] [css-cascade] revert-layer ignores properties sharing a computed style : [Attachment 458264] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Oriol Brufau
<obrufau at igalia.com>'s request for review:
Bug 238125: [css-cascade] revert-layer ignores properties sharing a computed
style
https://bugs.webkit.org/show_bug.cgi?id=238125

Attachment 458264: Patch

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




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

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

> Source/WebCore/style/PropertyCascade.cpp:157
> +const PropertyCascade::Property*
PropertyCascade::lastDeferredPropertyForIdOrRelated(CSSPropertyID propertyID)
const

The name is hard to parse

Maybe lastDeferredPropertyResolvingRelated() or something?

> Source/WebCore/style/PropertyCascade.cpp:161
> +    auto relatedID = getRelatedPropertyId(propertyID);
> +    auto indexForPropertyID = deferredPropertyIndex(propertyID);
> +    auto indexForRelatedID = deferredPropertyIndex(relatedID);

This is bit hard to understand. getRelatedPropertyId() returns
CSSProperyInvalid if there is no related property which would cause assert in
deferredPropertyIndex(). This seems to rely on every deferred property having a
related property. Is that just a coincidence? I thought those are separate
concepts.

Should CSSProperyInvalid case be handled?