Bug 212458 – CodeGenerator should use DOMAttributeGetterSetter when it is DOMAttribute

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Philip Jägenstedt]

Created attachment 400435 [details]
crash log

This is a report for the problem discovered in https://github.com/mdittmer/web-apis/issues/94.

Safari version: 13.1.1 (15609.2.9.1.2)

I found and have attached ~/Library/Logs/DiagnosticReports/com.apple.WebKit.WebContent_2020-05-28-104123_foolip-macbookpro.crash.

I will attach the standalone repro case separately.

It doesn't repro in STP and it also doesn't repro when running with Web Inspector open.

[Philip Jägenstedt]

Created attachment 400436 [details]
standalone repro

[Radar WebKit Bug Importer]

<rdar://problem/63754110>

[Yusuke Suzuki]

Reproduced. Looking!

[Yusuke Suzuki]

Created attachment 401252 [details]
Patch

[Yusuke Suzuki]

I need to update bindings

[Yusuke Suzuki]

Created attachment 401253 [details]
Patch

[Mark Lam]

Comment on attachment 401253 [details]
Patch

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

r=me

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4341
> +                    push(@implContent, "        putDirectCustomAccessor(vm, static_cast<JSVMClientData*>(vm.clientData)->builtinNames()." . $attributeName . "PublicName(), CustomGetterSetter::create(vm, $getter, $setter), attributesForStructure($jscAttributes));\n");

Can you also add a bindings test case for this?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4467
> +        if (IsAcceleratedDOMAttribute($interface, $attribute)) {

Can you add bindings test for these 2 cases as well?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4487
> +        if (IsAcceleratedDOMAttribute($interface, $attribute)) {

Is it possible to add a bindings test for these 2 cases of PrivateName DOMAttributeGetterSetter and CustomGetterSetter?

[Yusuke Suzuki]

Comment on attachment 401253 [details]
Patch

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

>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4341
>> +                    push(@implContent, "        putDirectCustomAccessor(vm, static_cast<JSVMClientData*>(vm.clientData)->builtinNames()." . $attributeName . "PublicName(), CustomGetterSetter::create(vm, $getter, $setter), attributesForStructure($jscAttributes));\n");
> 
> Can you also add a bindings test case for this?

Added.

>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4467
>> +        if (IsAcceleratedDOMAttribute($interface, $attribute)) {
> 
> Can you add bindings test for these 2 cases as well?

It turned out that this never happens. I've added assertion.

>> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4487
>> +        if (IsAcceleratedDOMAttribute($interface, $attribute)) {
> 
> Is it possible to add a bindings test for these 2 cases of PrivateName DOMAttributeGetterSetter and CustomGetterSetter?

It turned out that this never happens. I've added assertion.

[Yusuke Suzuki]

Committed r262693: <https://trac.webkit.org/changeset/262693>