Bug 167274 – [WebIDL] Add support for inherit serializer attribute

• Component: Bindings
• Status: RESOLVED FIXED

[Joseph Pecoraro]

Add support for inherit serializer attribute

Also extend support for detecting if attributes are serializable.

WebIDL Spec:
https://heycam.github.io/webidl/#idl-serializers

---

Resource Timing wants serializer { inherit }:
https://w3c.github.io/resource-timing/

> interface PerformanceResourceTiming : PerformanceEntry {
>     ...
>     serializer = {inherit, attribute};
> };

The Performance API also has non-simple attributes that can be serialized:

> interface Performance : EventTarget {
>    readonly attribute PerformanceNavigation navigation;
>    readonly attribute PerformanceTiming timing;
>    ...
>    serializer = {attribute};
> };

[Joseph Pecoraro]

Created attachment 299416 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 299416 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/bindings/scripts/CodeGenerator.pm:941
> +    my $interface = GetInterfaceForAttribute($object, $currentInterface, $attribute);
> +    return 1 if $interface && $interface->serializable;

Seems EventHandler makes it down to here and we try to look up EventHandler type and fail. I can easily bail above, but maybe there is a better list of non-Interface built-in types.

> Source/WebCore/bindings/scripts/test/JS/JSTestSerialization.cpp:424
> +    auto sixthNodeAttributeValue = jsTestSerializationSixthNodeAttributeGetter(*state, *thisObject, throwScope);
> +    ASSERT(!throwScope.exception());
> +    result->putDirect(vm, Identifier::fromString(&vm, "sixthNodeAttribute"), sixthNodeAttributeValue);

r- For interface types that are serializable, I should be serializing them. So I should be doing a toJSON on thing, not putting the value directly.

[Joseph Pecoraro]

Created attachment 300039 [details]
[PATCH] Proposed Fix

[Darin Adler]

Comment on attachment 300039 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/ChangeLog:16
> +        serializing and attribute that is itself a serializable interface.

an attribute

> Source/WebCore/ChangeLog:29
> +        special strings. The spec does not accurately define where the
> +        special "attribute" keyword is allowed.

What does "accurately" mean here? Do you mean "precisely"?

> Source/WebCore/bindings/scripts/CodeGenerator.pm:945
> +    my $interface = GetInterfaceForAttribute($object, $currentInterface, $attribute);
> +    if ($interface && $interface->serializable) {
> +        die "Serializer for non-primitive types is not currently supported";
> +    }

Should this have a newline at the end of the die message? How useful is this error when it happens? Is it easy to understand what type we are talking about and what we did wrong?

[Joseph Pecoraro]

Comment on attachment 300039 [details]
[PATCH] Proposed Fix

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

>> Source/WebCore/ChangeLog:29
>> +        special "attribute" keyword is allowed.
> 
> What does "accurately" mean here? Do you mean "precisely"?

"precisely" is much better.

>> Source/WebCore/bindings/scripts/CodeGenerator.pm:945
>> +    }
> 
> Should this have a newline at the end of the die message? How useful is this error when it happens? Is it easy to understand what type we are talking about and what we did wrong?

It should have a newline.

Here we are bailing when you reach unimplemented functionality in our generator. Previously `inherit` was unsupported and just did the same thing `die "Serializer inherit keyword is not currently supported.";`. This patch gets inherit working for primitive types, but not Interfaces that are serializable.

When the time comes that someone needs serialization functionality for Interfaces:

    interface SerializableObject {
        attribute long timestamp;
        serializer = { attribute };
    };

    interface SomeInterface {
        attribute SerializableObject object;
        serializer = { attribute };
    };

Then they will need to extend our generator to support it, and that is what this die case if fore. No such case exists yet.

--

Thinking deeper, we probably need to group sequence/dictionary in with Interface and bail. Since none of these cases exist I didn't have a real JavaScript case to test with, but it looks like the generation would just put the object there instead of a serialized version of the object. I'll change this to be more conservative.

[Joseph Pecoraro]

<https://trac.webkit.org/changeset/211409>