Bug 232557

Summary: WebIDL: const in namespace is not supported
Product: WebKit Reporter: Myles C. Maxfield <mmaxfield>
Component: BindingsAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, djg, esprehn+autocc, ews-watchlist, joepeck, kondapallykalyan, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 232558    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Myles C. Maxfield 2021-11-01 00:53:50 PDT
namespace GPUMapMode {
    const GPUFlagsConstant READ  = 0x0001;
    const GPUFlagsConstant WRITE = 0x0002;
};
Comment 1 Radar WebKit Bug Importer 2021-11-07 23:54:19 PST
<rdar://problem/85142162>
Comment 2 Dan Glastonbury 2022-03-02 15:43:18 PST
Created attachment 453669 [details]
Patch
Comment 3 Sam Weinig 2022-03-02 17:11:16 PST
Comment on attachment 453669 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        * bindings/scripts/CodeGeneratorJS.pm:

Please fill in the ChangeLog with details about the change.

> Source/WebCore/ChangeLog:23
> +        * bindings/scripts/test/JS/JSTestNamespaceConst.cpp: Added.

We often replace all the bindings/scripts/tests/JS/ lines with a single:

* bindings/scripts/test/JS/*: Updated

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7883
> +    if ($interface->isNamespaceObject && scalar(@runtimeEnabledProperties) == 0) {
> +        push(@$outputArray, "    UNUSED_PARAM(globalObject);\n");
> +    }

Is this related?

> Source/WebCore/bindings/scripts/IDLAttributes.json:464
> -            "contextsAllowed": ["interface", "dictionary", "enum", "attribute", "operation", "constant"],
> +            "contextsAllowed": ["interface", "namespace", "dictionary", "enum", "attribute", "operation", "constant"],

This seems unrelated.
Comment 4 Sam Weinig 2022-03-02 17:24:59 PST
Comment on attachment 453669 [details]
Patch

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

> Source/WebCore/bindings/scripts/IDLParser.pm:1111
> +    if ($next->value() eq "const") {

Would be good to update $nextNamespaceMembers_1 as well to include "|const" if it doesn't already have it. (Though I am unclear if it is ever used in a context where that would matter, I try to keep those correct in case they do).
Comment 5 Dan Glastonbury 2022-03-02 17:31:48 PST
(In reply to Sam Weinig from comment #3)
> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7883
> > +    if ($interface->isNamespaceObject && scalar(@runtimeEnabledProperties) == 0) {
> > +        push(@$outputArray, "    UNUSED_PARAM(globalObject);\n");
> > +    }
> 
> Is this related?

Yes. This is fix a compilation error when a namespace contains just constants. For example, this:

namespace GPUMapMode {
    const GPUFlagsConstant READ  = 0x0001;
    const GPUFlagsConstant WRITE = 0x0002;
};

Generates the following code which has a compile error because globalObject is unused.

template<> void JSGPUMapModeDOMConstructor::initializeProperties(VM& vm, JSDOMGlobalObject& globalObject)
{
    JSC_TO_STRING_TAG_WITHOUT_TRANSITION();
    reifyStaticProperties(vm, JSGPUMapMode::info(), JSGPUMapModeConstructorTableValues, *this);
}

If there are no attributes or operations in the namespace, globalObject is unused, so I added the UNUNSED_PARAM to quiet the error.


> > Source/WebCore/bindings/scripts/IDLAttributes.json:464
> > -            "contextsAllowed": ["interface", "dictionary", "enum", "attribute", "operation", "constant"],
> > +            "contextsAllowed": ["interface", "namespace", "dictionary", "enum", "attribute", "operation", "constant"],
> 
> This seems unrelated.

This allows [SecureContext] to be applied to namespace, which is allowed by the spec. Yes, it's not related to constants in namespace, so I'll open another bug for this change.
Comment 6 Dan Glastonbury 2022-03-02 19:03:42 PST
Created attachment 453690 [details]
Patch
Comment 7 Dan Glastonbury 2022-03-03 17:35:36 PST
Created attachment 453804 [details]
Patch
Comment 8 EWS 2022-03-04 14:09:32 PST
Committed r290845 (248078@main): <https://commits.webkit.org/248078@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453804 [details].