RESOLVED FIXED 33193
[V8] CodeGeenratorV8 can't find item() or namedItem() in parent classes 
https://bugs.webkit.org/show_bug.cgi?id=33193
Summary [V8] CodeGeenratorV8 can't find item() or namedItem() in parent classes
Kent Tamura
Reported 2010-01-04 19:42:14 PST
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.
Attachments
Proposed patch (3.52 KB, patch)
2010-01-04 21:41 PST, Kent Tamura 		tkent: review-
DetailsFormatted DiffDiff
Proposed patch (rev.2) (2.15 KB, patch)
2010-01-05 20:40 PST, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kent Tamura
Comment 1 2010-01-04 21:07:28 PST
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
Comment 2 2010-01-04 21:41:53 PST
Created attachment 45862 [details] Proposed patch
WebKit Review Bot
Comment 3 2010-01-04 21:59:57 PST
style-queue ran check-webkit-style on attachment 45862 [details] without any errors.
Adam Barth
Comment 4 2010-01-04 22:06:40 PST
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
Comment 5 2010-01-05 02:28:41 PST
(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)
Comment 6 2010-01-05 09:23:46 PST
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
Comment 7 2010-01-05 20:40:15 PST
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
Comment 8 2010-01-05 20:41:10 PST
style-queue ran check-webkit-style on attachment 45953 [details] without any errors.
Adam Barth
Comment 9 2010-01-05 22:04:44 PST
Comment on attachment 45953 [details] Proposed patch (rev.2) Can we add a test for the behavior change?
Kent Tamura
Comment 10 2010-01-05 22:13:00 PST
(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 11 2010-01-07 00:50:10 PST
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 12 2010-01-07 02:52:11 PST
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
Comment 13 2010-01-07 02:52:19 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.