Summary: | [V8] CodeGeneratorV8.pm does not correctly work with inherited HasIndexGetter | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Erik Arvidsson <arv> | ||||||
Component: | DOM | Assignee: | Erik Arvidsson <arv> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, japhet, mihnea, senorblanco, tony, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Erik Arvidsson
2011-12-07 14:21:40 PST
Created attachment 118844 [details]
Patch
Looks good, but I'll leave for someone more wise in the ways of V8 bindings. You should also remove bindings/v8/custom/V8WebKitCSSFilterValueCustom.cpp, otherwise I think the linker will complain. Thanks Steven, updating the patch. (The linker does not complain because the .h file still declares this function even though it is never used.) View in context: https://bugs.webkit.org/attachment.cgi?id=118844&action=review > Source/WebCore/bindings/scripts/CodeGenerator.pm:267 > + foreach my $function (@{$interface->functions}) { > + if ($function->signature->name eq $functionName) { > + $indexer = $function->signature; > + return 'prune'; What about overloads? Also, what does "prune" mean? > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-2027 > - # FIXME: The item() getter is not inherited from CSSValueList, seemingly due to the way > - # the CodeGenerator->AddMethodsConstantsAndAttributesFromParentClasses() method works, Why isn't the right solution to change AddMethodsConstantsAndAttributesFromParentClasses to understand indexers? (In reply to comment #3) > Thanks Steven, updating the patch. (The linker does not complain because the .h file still declares this function even though it is never used.) Actually, I think it doesn't complain because CSS_FILTERS is not enabled by default (yet). If it was, I think you'd get a duplicate symbol. (In reply to comment #4) > View in context: https://bugs.webkit.org/attachment.cgi?id=118844&action=review > > > Source/WebCore/bindings/scripts/CodeGenerator.pm:267 > > + foreach my $function (@{$interface->functions}) { > > + if ($function->signature->name eq $functionName) { > > + $indexer = $function->signature; > > + return 'prune'; > > What about overloads? I'll check. I don't think we support overloads in indexed getters. > Also, what does "prune" mean? "prune" is the sentinel used to stop the iteration in ForAllParents(). > > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-2027 > > - # FIXME: The item() getter is not inherited from CSSValueList, seemingly due to the way > > - # the CodeGenerator->AddMethodsConstantsAndAttributesFromParentClasses() method works, > > Why isn't the right solution to change AddMethodsConstantsAndAttributesFromParentClasses to understand indexers? I'll check if that would be cleaner. It is important that we do not add an "item" method to the DOMSettableToken.prototype. Created attachment 118877 [details]
Patch
(In reply to comment #6) > (In reply to comment #4) > > View in context: https://bugs.webkit.org/attachment.cgi?id=118844&action=review > > > > > Source/WebCore/bindings/scripts/CodeGenerator.pm:267 > > > + foreach my $function (@{$interface->functions}) { > > > + if ($function->signature->name eq $functionName) { > > > + $indexer = $function->signature; > > > + return 'prune'; > > > > What about overloads? We don't support overloaded indexers or named property getters. There is currently no use of this in any of the idl files. > > > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-2027 > > > - # FIXME: The item() getter is not inherited from CSSValueList, seemingly due to the way > > > - # the CodeGenerator->AddMethodsConstantsAndAttributesFromParentClasses() method works, > > > > Why isn't the right solution to change AddMethodsConstantsAndAttributesFromParentClasses to understand indexers? > > I'll check if that would be cleaner. It is important that we do not add an "item" method to the DOMSettableToken.prototype. This could have been done by adding an indexer and namedPropertyGetter to the interface object but to me that seems less clean than this solution. Comment on attachment 118877 [details] Patch Clearing flags on attachment: 118877 Committed r102646: <http://trac.webkit.org/changeset/102646> All reviewed patches have been landed. Closing bug. |