Bug 230284 - Addition of CSSTransformValue, CSSTransformComponent & subclasses
Summary: Addition of CSSTransformValue, CSSTransformComponent & subclasses
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Qiaosong Zhou
URL:
Keywords: InRadar
Depends on:
Blocks: 175733
  Show dependency tree / graph
 
Reported: 2021-09-14 16:38 PDT by Qiaosong Zhou
Modified: 2021-09-17 16:03 PDT (History)
16 users (show)

See Also:

Attachments
Patch (101.13 KB, patch)
2021-09-14 16:40 PDT, Qiaosong Zhou 		no flags Details | Formatted Diff | Diff
Patch (179.80 KB, patch)
2021-09-14 18:18 PDT, Qiaosong Zhou 		no flags Details | Formatted Diff | Diff
Patch (179.91 KB, patch)
2021-09-15 01:43 PDT, Qiaosong Zhou 		no flags Details | Formatted Diff | Diff
Patch (179.97 KB, patch)
2021-09-15 12:16 PDT, Qiaosong Zhou 		no flags Details | Formatted Diff | Diff
Patch (200.43 KB, patch)
2021-09-16 17:57 PDT, Qiaosong Zhou 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (203.76 KB, patch)
2021-09-16 18:14 PDT, Qiaosong Zhou 		no flags Details | Formatted Diff | Diff
Patch (203.83 KB, patch)
2021-09-17 14:07 PDT, Qiaosong Zhou 		no flags Details | Formatted Diff | Diff
Patch (206.16 KB, patch)
2021-09-17 15:57 PDT, Alex Christensen 		no flags Details | Formatted Diff | Diff
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Qiaosong Zhou 2021-09-14 16:38:40 PDT 
Addition of CSSTransformValue, CSSTransformComponent & subclasses
Comment 1 Qiaosong Zhou 2021-09-14 16:40:23 PDT 
Created attachment 438189 [details]
Patch
Comment 2 Alex Christensen 2021-09-14 16:46:39 PDT 
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
Comment 3 Qiaosong Zhou 2021-09-14 18:18:20 PDT 
Created attachment 438198 [details]
Patch
Comment 4 Alex Christensen 2021-09-14 19:26:43 PDT 
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>
Comment 5 Qiaosong Zhou 2021-09-15 01:43:52 PDT 
Created attachment 438224 [details]
Patch
Comment 6 Qiaosong Zhou 2021-09-15 12:16:36 PDT 
Created attachment 438272 [details]
Patch
Comment 7 Qiaosong Zhou 2021-09-15 14:47:18 PDT 
WPT test crashes are due to the lack of CustomToJSObject wrapper. Fixing.
Comment 8 Qiaosong Zhou 2021-09-16 17:57:39 PDT 
Created attachment 438422 [details]
Patch
Comment 9 Qiaosong Zhou 2021-09-16 18:14:13 PDT 
Created attachment 438424 [details]
Patch
Comment 10 Alex Christensen 2021-09-17 13:02:45 PDT 
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
Comment 11 Qiaosong Zhou 2021-09-17 14:07:53 PDT 
Created attachment 438515 [details]
Patch
Comment 12 Alex Christensen 2021-09-17 15:50:22 PDT 
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
Comment 13 Alex Christensen 2021-09-17 15:55:22 PDT 
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.
Comment 14 Alex Christensen 2021-09-17 15:57:04 PDT 
Created attachment 438526 [details]
Patch
Comment 15 Alex Christensen 2021-09-17 16:02:34 PDT 
http://trac.webkit.org/r282702
Comment 16 Radar WebKit Bug Importer 2021-09-17 16:03:17 PDT 
<rdar://problem/83259158>