http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm#L2860 sub IsActiveDomType needs to die.
Created attachment 110425 [details] work in progress
Created attachment 110432 [details] Patch
Comment on attachment 110432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110432&action=review :-D > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:438 > +ALWAYS_INLINE v8::Handle<v8::Object> ${className}::existingWrapper(${nativeType}* impl) > +{ > +END > + my $getWrapper = IsNodeSubType($dataNode) ? "V8DOMWrapper::getWrapper(impl)" : "${domMapFunction}.get(impl)"; > + push(@headerContent, <<END); > + return ${getWrapper}; > +} I assume this is just for readability?
> I assume this is just for readability? At this call site: - my $domMapFunction = GetDomMapFunction(0, $returnType); It's hard to figure out whether $returnType is an active DOM object, so we let the C++ compiler figure it out for us using the new existingWrapper function.
Comment on attachment 110432 [details] Patch Clearing flags on attachment: 110432 Committed r97109: <http://trac.webkit.org/changeset/97109>
All reviewed patches have been landed. Closing bug.
Looks like this caused a bunch of "binding tests failed" on the bots: http://build.webkit.org/builders/Leopard%20Intel%20Debug%20%28Tests%29/builds/34194
Will fix.
Comment on attachment 110432 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110432&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:432 > +ALWAYS_INLINE v8::Handle<v8::Object> ${className}::existingWrapper(${nativeType}* impl) may this go into common code?
> may this go into common code? I'm not sure I understand your question.
(In reply to comment #10) > > may this go into common code? > > I'm not sure I understand your question. Dreadful sorry, Adam. I meant may we have this existingWrapper as a template function somewhere in Source/WebCore/bindings/v8 instead in generated files. The goal is to have as minimum of logic in generated code as possible. Feel free to ignore.
Ah, I see. The advantage of having it in the generated code is that it depends on the ActiveDOMObject IDL attribute. In some sense, I would have preferred to generate the code inline at the call sites in the code generator, but not all the type information was available, so I made the C++ compiler do that work. I definitely agree that we should prefer to put as much in non-generated code as possible, but I don't see a clean way of doing that in this case.
(In reply to comment #12) > Ah, I see. The advantage of having it in the generated code is that it depends on the ActiveDOMObject IDL attribute. In some sense, I would have preferred to generate the code inline at the call sites in the code generator, but not all the type information was available, so I made the C++ compiler do that work. > > I definitely agree that we should prefer to put as much in non-generated code as possible, but I don't see a clean way of doing that in this case. ok