Bug 33190 – Missing commas in IDL extended attributes

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kent Tamura]

Some IDL files lack "," delimiters for multiple extended attributes.
For example,
WebCore/css/WebKitCSSTransformValue.idl

    interface [
            HasIndexGetter
            InterfaceUUID=303fe632-5dcf-4472-b977-33a5481e1d12,
            ImplementationUUID=eb49e5c6-6075-45b8-b5c4-7e775c01e7c4
    ] WebKitCSSTransformValue : CSSValueList {

There is no "," which should follow HasIndexGetter. In this case, both of HasIndexGetter and InterfaceUUID are not processed because they are recognized as a single attribute with spaces like "HasIndexGetter  InterfaceUUID".

[Kent Tamura]

Created attachment 45946 [details]
Proposed patch

- Fix commas
- Check such attributes by IDLParser.pm

Note: This patch will break Chromium build because of Bug#33193.

[Darin Adler]

Comment on attachment 45946 [details]
Proposed patch

Thanks for finding this. I would like to see test cases to demonstrate the bad thing that was happening due to HasIndexGetter and OmitConstructor being ignored in these two classes.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45946 [details] without any errors.

[Kent Tamura]

(In reply to comment #2)
> (From update of attachment 45946 [details])
> Thanks for finding this. I would like to see test cases to demonstrate the bad
> thing that was happening due to HasIndexGetter and OmitConstructor being
> ignored in these two classes.

This issue is not causing any real problems at this moment.

WebKitCSSTransformValue is missing HasIndexGetter and generated code for it had no code for index getter. Fortunately the parent interface of WebKitCSSTransformValue has HasIndexGetter code. So WebKitCSSTransformValue objects supprot index getter.

OmitConstructor is used only by CodeGeneratorJS. But JS binding excludes EventTarget.idl. (See JS_DOM_HEADERS in WebCore/DerivedSources.make)

[WebKit Review Bot]

Attachment 45946 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/163557

[Eric Seidel (no email)]

Comment on attachment 45946 [details]
Proposed patch

OK.  Please don't land until the chromium build fix is in.

Won't this patch fail to apply now anyway?  I thoguth we killed all of the COM stuff?

[Eric Seidel (no email)]

Attachment 45946 [details] was posted by a committer and has review+, assigning to Kent Tamura for commit.

[Kent Tamura]

(In reply to comment #6)
> (From update of attachment 45946 [details])
> OK.  Please don't land until the chromium build fix is in.
> 
> Won't this patch fail to apply now anyway?  I thoguth we killed all of the COM
> stuff?

We can apply this patch for now because the patch of Bug#32854 (Kill COM stuff) is not landed yet. Both of this and Bug#32854 trigger a Chromium build failure.
I'll commit Bug#33193 first, then this, and Bug#32854.

[WebKit Commit Bot]

Comment on attachment 45946 [details]
Proposed patch

Clearing flags on attachment: 45946

Committed r52915: <http://trac.webkit.org/changeset/52915>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.