Bug 211528 - [Conditional=] not working for operations in WebGLRenderingContextBase.idl
Summary: [Conditional=] not working for operations in WebGLRenderingContextBase.idl
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-06 13:27 PDT by Sergio Villar Senin
Modified: 2020-05-07 11:13 PDT (History)
9 users (show)

See Also:


Attachments
Minimal test case (1.16 KB, patch)
2020-05-07 02:55 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
Patch (41.48 KB, patch)
2020-05-07 06:34 PDT, Sergio Villar Senin
cdumez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Villar Senin 2020-05-06 13:27:47 PDT
In https://bugs.webkit.org/show_bug.cgi?id=211506 I'm adding a new method to the IDL with a Conditional. However the generated JSWebGLRenderingContext.cpp (note that WebGLRenderingContext inherits from WebGLRenderingContexBase) only has the ENABLE(WEBGL) guard in the method which comes from the value of Conditional in the interface definition in WebGLRenderingContextBase.idl.

The new method is not the problem. I've verified that any Conditional specified in any attribute/operation in that IDL file is either ignored or overwritten by the conditional in the interface definition.

I've also verified that this does not happen on other IDL files which do already define per-operation conditionals in IDL interfaces with no implementation.
Comment 1 Darin Adler 2020-05-06 14:00:10 PDT
Seems likely the perl script just doesn’t have the support for the multiple layers of conditional. Won’t be super-hard to fix, I don’t think.
Comment 2 Darin Adler 2020-05-06 14:01:58 PDT
Maybe you can reproduce with a test case in the bindings tests directory.
Comment 3 Sergio Villar Senin 2020-05-06 23:50:39 PDT
(In reply to Darin Adler from comment #1)
> Seems likely the perl script just doesn’t have the support for the multiple
> layers of conditional. Won’t be super-hard to fix, I don’t think.

That's what I thought in the beginning but it just works fine with other IDL files as mentioned.
Comment 4 Sergio Villar Senin 2020-05-07 02:55:44 PDT
Created attachment 398718 [details]
Minimal test case

This pretty reduced test case reproduces exactly what I'm seeing. Using generate-bindings.pl and the supplemental file attached generates JSTestOperationConditional.* files where the conditionalOperation() is guarded by Condition1 instead of Condition2.

The command I used to generate the bindings is:
Source/WebCore/bindings/scripts/generate-bindings.pl --generator JS --outputDir . --idlAttributesFile Source/WebCore/bindings/scripts/IDLAttributes.json --supplementalDependencyFile supplemental  Source/WebCore/bindings/scripts/test/TestOperationConditional.idl Source/WebCore/bindings/scripts/test/TestOperationBase.idl
Comment 5 Sergio Villar Senin 2020-05-07 03:35:13 PDT
OK I think I found a way to "fix" it. I am not not an expert on bindings generators so I am not sure whether what I am doing is an unsupported configuration, a limitation of the generator or just a bug in the IDL.

So if I remove the Conditional in the Base interface then everything works fine. The TestOperationConditional is properly generated. The Condition1 guards all the code (as if it were specified in the base class and Condition2 guards the method that specified its own conditional.

Using the same fix for 211506 (i.e. removing Conditional=WEBGL from WebGLRenderingContextBase.idl) makes the build succeed.
Comment 6 Sergio Villar Senin 2020-05-07 04:07:09 PDT
(In reply to Sergio Villar Senin from comment #5)
> OK I think I found a way to "fix" it. I am not not an expert on bindings
> generators so I am not sure whether what I am doing is an unsupported
> configuration, a limitation of the generator or just a bug in the IDL.
> 
> So if I remove the Conditional in the Base interface then everything works
> fine. The TestOperationConditional is properly generated. The Condition1
> guards all the code (as if it were specified in the base class and
> Condition2 guards the method that specified its own conditional.
> 
> Using the same fix for 211506 (i.e. removing Conditional=WEBGL from
> WebGLRenderingContextBase.idl) makes the build succeed.

After checking the WebGL example again I think that Darin is right. The generator is working fine as it's guarding the base class methods with the base class conditional (while the whole file is guarded by the derived interface conditional). The problem is that the generator should also add the operation specific conditional for the operation.
Comment 7 Sergio Villar Senin 2020-05-07 06:34:10 PDT
Created attachment 398730 [details]
Patch
Comment 8 Sergio Villar Senin 2020-05-07 11:12:53 PDT
Committed r261317: <https://trac.webkit.org/changeset/261317>
Comment 9 Radar WebKit Bug Importer 2020-05-07 11:13:15 PDT
<rdar://problem/62984434>