Bug 241167 – Implement CSSNumericValue.to

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Alex Christensen]

Some strange algorithms, but well specified.

[Alex Christensen]

Created attachment 459920 [details]
Patch

[Chris Dumez]

Comment on attachment 459920 [details]
Patch

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

r=me but please fix the parameter types to use r-value references.

> Source/WebCore/css/typedom/CSSUnitValue.h:53
> +    RefPtr<CSSUnitValue> convertTo(CSSUnitType);

Can this be const?

> Source/WebCore/css/typedom/numeric/CSSMathMax.cpp:42
> +ExceptionOr<Ref<CSSMathMax>> CSSMathMax::create(FixedVector<CSSNumberish> numberishes)

I commented the same thing on an earlier patch but I would really prefer if we used FixedVector<CSSNumberish>&&, especially for copyable types.
While the flexibility to copy or move may sound nice in theory. I fear that people won't be careful and do unnecessary copies.

> Source/WebCore/css/typedom/numeric/CSSMathMax.cpp:47
> +ExceptionOr<Ref<CSSMathMax>> CSSMathMax::create(Vector<Ref<CSSNumericValue>> values)

Vector<Ref<CSSNumericValue>>&&

> Source/WebCore/css/typedom/numeric/CSSMathMax.cpp:59
> +CSSMathMax::CSSMathMax(Vector<Ref<CSSNumericValue>> values, CSSNumericType type)

Vector<Ref<CSSNumericValue>>&&

> Source/WebCore/css/typedom/numeric/CSSMathMax.cpp:91
> +    for (size_t i = 1; i < valuesArray.size(); i++) {

++i

> Source/WebCore/css/typedom/numeric/CSSMathMin.cpp:40
> +ExceptionOr<Ref<CSSMathMin>> CSSMathMin::create(FixedVector<CSSNumberish> numberishes)

FixedVector<CSSNumberish>&&

> Source/WebCore/css/typedom/numeric/CSSMathMin.cpp:45
> +ExceptionOr<Ref<CSSMathMin>> CSSMathMin::create(Vector<Ref<CSSNumericValue>> values)

Vector<Ref<CSSNumericValue>>&&

> Source/WebCore/css/typedom/numeric/CSSMathMin.cpp:57
> +CSSMathMin::CSSMathMin(Vector<Ref<CSSNumericValue>> values, CSSNumericType type)

Vector<Ref<CSSNumericValue>>&&

> Source/WebCore/css/typedom/numeric/CSSMathSum.cpp:88
> +    auto convertToNumericType= [] (const UnitMap& units) -> std::optional<CSSNumericType> {

missing space before =

> Source/WebCore/css/typedom/numeric/CSSMathSum.cpp:123
> +    for (size_t i = 1; i < values.size(); i++) {

++i

[Alex Christensen]

Created attachment 460018 [details]
Patch

[EWS]

Committed r295219 (251276@main): <https://commits.webkit.org/251276@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 460018 [details].

[Radar WebKit Bug Importer]

<rdar://problem/94361549>