Bug 230284 – Addition of CSSTransformValue, CSSTransformComponent & subclasses

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Qiaosong Zhou]

Addition of CSSTransformValue, CSSTransformComponent & subclasses

[Qiaosong Zhou]

Created attachment 438189 [details]
Patch

[Alex Christensen]

Comment on attachment 438189 [details]
Patch

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

probably needs some test expectations to be updated.

> Source/WebCore/css/typedom/transform/CSSMatrixComponent.cpp:58
> +    return ""_s;

FIXME: Implement.

> Source/WebCore/css/typedom/transform/CSSMatrixComponent.cpp:63
> +    return DOMMatrix::fromMatrix(DOMMatrixInit { });

ditto

> Source/WebCore/css/typedom/transform/CSSSkew.h:54
> +    Ref<CSSNumericValue> m_ax, m_ay;

We usually put variable declarations on their own line with the type repeated in WebKit.

> Source/WebCore/css/typedom/transform/CSSTranslate.h:56
> +    Ref<CSSNumericValue> m_x, m_y;

ditto

[Qiaosong Zhou]

Created attachment 438198 [details]
Patch

[Alex Christensen]

Comment on attachment 438198 [details]
Patch

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

> Source/WebCore/css/typedom/transform/CSSTransformComponent.h:38
> +    WTF_MAKE_ISO_ALLOCATED(CSSTransformComponent);

Looks like you need to #include <wtf/IsoMalloc.h>

[Qiaosong Zhou]

Created attachment 438224 [details]
Patch

[Qiaosong Zhou]

Created attachment 438272 [details]
Patch

[Qiaosong Zhou]

WPT test crashes are due to the lack of CustomToJSObject wrapper. Fixing.

[Qiaosong Zhou]

Created attachment 438422 [details]
Patch

[Qiaosong Zhou]

Created attachment 438424 [details]
Patch

[Alex Christensen]

Comment on attachment 438424 [details]
Patch

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

> Source/WebCore/bindings/js/JSCSSTransformComponentCustom.cpp:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

2021

> Source/WebCore/bindings/js/JSCSSTransformComponentCustom.cpp:46
> +    if (is<CSSMatrixComponent>(value.get()))

Can this be a switch statement instead?

> Source/WebCore/css/typedom/transform/CSSTransformComponent.h:61
> +

nit: extra space

[Qiaosong Zhou]

Created attachment 438515 [details]
Patch

[Alex Christensen]

Comment on attachment 438515 [details]
Patch

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

> Source/WebCore/css/typedom/transform/CSSMatrixComponent.cpp:33
> +

nit: extra space

> Source/WebCore/css/typedom/transform/CSSTransformComponent.h:61
> +

nit: extra space.

> Source/WebCore/css/typedom/transform/CSSTransformComponent.h:63
> +    bool m_is2D;

This is uninitialized memory.  I'll add { false } while landing.

> Source/WebCore/css/typedom/transform/CSSTransformValue.h:56
> +    bool m_is2D;

ditto

[Alex Christensen]

Comment on attachment 438515 [details]
Patch

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

> Source/WebCore/css/typedom/transform/CSSMatrixComponentOptions.h:33
> +    bool is2D;

Here too.

[Alex Christensen]

Created attachment 438526 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/r282702

[Radar WebKit Bug Importer]

<rdar://problem/83259158>