Bug 33193 – [V8] CodeGeenratorV8 can't find item() or namedItem() in parent classes

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Kent Tamura]

WebCore/CSS/WebKitCSSTransformValue.idl looks like having IndexGetter extended attributes. However, the attribute is ignored by Bug#33190.
If Bug#33190 is fixed or Bug#32854 is committed, we need to have a WebKitCSSTransformValue index-getter implementation for V8.

[Kent Tamura]

I investigated how to fix this and concluded it's a bug that CodeGeneratorV8.pm didn't understand WebKitCSSTransformValue had item() inherited from it's parent interface.

[Kent Tamura]

Created attachment 45862 [details]
Proposed patch

[WebKit Review Bot]

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

[Adam Barth]

Comment on attachment 45862 [details]
Proposed patch

I'm willing to believe that we need this patch, but does this fix a specific test or do we need to add a new test to show the improvement?

[Kent Tamura]

(In reply to comment #4)
> (From update of attachment 45862 [details])
> I'm willing to believe that we need this patch, but does this fix a specific
> test or do we need to add a new test to show the improvement?

I'm making a test, and probably we don't need to add index access setting to WebKitCSSTransformValue like this patch because its parent interface already has index access setting.

[Dimitri Glazkov (Google)]

Thanks so much for picking finding this! However, I don't think this is the right approach. We should instead work on making our template configuration aware of inheritance.

In the meantime, let's just add an exception to GenerateImplementationIndexer (see HTMLOptionCollection).

[Kent Tamura]

Created attachment 45953 [details]
Proposed patch (rev.2)

(In reply to comment #6)
> Thanks so much for picking finding this! However, I don't think this is the
> right approach. We should instead work on making our template configuration
> aware of inheritance.
> 
> In the meantime, let's just add an exception to GenerateImplementationIndexer
> (see HTMLOptionCollection).

Thank you for the comment.
Ok, the updated patch just adds a workaround for WebKitCSSTransformValue.

[WebKit Review Bot]

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

[Adam Barth]

Comment on attachment 45953 [details]
Proposed patch (rev.2)

Can we add a test for the behavior change?

[Kent Tamura]

(In reply to comment #9)
> (From update of attachment 45953 [details])
> Can we add a test for the behavior change?

Adding "," to "HasIndexGetter" in css/WebKitCSSTransformValue.idl causes a build error of Chromium. This change will fix it. I have no idea how to test this without a build error :-)

[Adam Barth]

Comment on attachment 45953 [details]
Proposed patch (rev.2)

tkent explained the issue to me over IRC and I understand now that this fixes a build error, which is even better than a test.  :)

[WebKit Commit Bot]

Comment on attachment 45953 [details]
Proposed patch (rev.2)

Clearing flags on attachment: 45953

Committed r52913: <http://trac.webkit.org/changeset/52913>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.