Bug 239462

Summary: Implement CSSTransformValue.is2D
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, kondapallykalyan, macpherson, menard, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Alex Christensen 2022-04-18 12:44:40 PDT 
Implement CSSTransformValue.is2D
Comment 1 Alex Christensen 2022-04-18 13:19:02 PDT 
Created attachment 457819 [details]
Patch
Comment 2 Alex Christensen 2022-04-18 15:30:12 PDT 
Created attachment 457831 [details]
Patch
Comment 3 EWS 2022-04-18 19:47:01 PDT 
Committed r293005 (249744@main): <https://commits.webkit.org/249744@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457831 [details].
Comment 4 Radar WebKit Bug Importer 2022-04-18 19:48:15 PDT 
<rdar://problem/91934123>
Comment 5 Darin Adler 2022-04-19 13:23:42 PDT 
Comment on attachment 457831 [details]
Patch

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

> Source/WebCore/css/typedom/numeric/CSSNumericType.h:80
> +    const std::optional<long>& valueForType(CSSNumericBaseType type) const

This seems like the wrong return type. Should just be std::optional<long>.

> Source/WebCore/css/typedom/transform/CSSPerspective.h:42
> +    static ExceptionOr<Ref<CSSPerspective>> create(CSSPerspectiveValue);

Why pass by value instead of by rvalue reference? I see this change throughout this patch and I don’t understand the rationale. For bindings, I guess both are the same, we will WTFMove from the local to the argument and then from the argument inside the function. But how did you decide this is the preferred style?

> Source/WebCore/css/typedom/transform/CSSTransformComponent.h:53
> +    CSSTransformComponent(Is2D is2D)

explicit

> Source/WebCore/css/typedom/transform/CSSTransformComponent.h:64
> -    bool m_is2D { false };
> +    Is2D m_is2D;

Why change the type of the data member?