1023698 - Missing JSContext* parameter in Generated Event Constructor

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: test case

===Example WebIDL===
[Constructor(DOMString type, optional TestBaseEventInit eventInitDict)]
interface TestBaseEvent: Event {
  readonly attribute any data;
};

dictionary TestBaseEventInit : EventInit {
  any data;
};

[Constructor(DOMString type, optional TestEventInit eventInitDict)]
interface TestEvent: TestBaseEvent {
  readonly attribute DOMString someString;
};

dictionary TestEventInit : TestBaseEventInit {
  DOMString someString = "";
};


===Compile Error===
3:45.54 ./TestEventBinding.cpp:303:12: error: no matching function for call to 'Constructor'
 3:45.54   result = mozilla::dom::TestEvent::Constructor(global, cx, NonNullHelper(Constify(arg0)), Constify(arg1), rv);
 3:45.55            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 3:45.55 ./TestEvent.cpp:61:12: note: candidate function not viable: requires 3 arguments, but 5 were provided
 3:45.55 TestEvent::Constructor(mozilla::dom::EventTarget* aOwner, const nsAString& aType, const TestEventInit& aEventInitDict)
 3:45.55            ^
 3:45.55 ./TestEvent.cpp:74:12: note: candidate function not viable: requires 4 arguments, but 5 were provided
 3:45.55 TestEvent::Constructor(const GlobalObject& aGlobal, const nsAString& aType, const TestEventInit& aEventInitDict, ErrorResult& aRv)
 3:45.55            ^

Comment 1

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

CGEventMethod should also check inherited interfaces for appending JSContext* in argument list.
http://dxr.mozilla.org/mozilla-central/source/dom/bindings/Codegen.py?from=codegen.py&case=true#13567

Comment 2

[Boris Zbarsky [:bzbarsky]]

Or maybe just examine the types in the signature of self.member instead?  That's what really matters here; the fact that those happen to match the interface member types in the case of events is somewhat accidental.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

+ "\n" removal is to fix .h coding style. No extra newlines.

Event codegen very much relies on interface attributes and dictionary properties
etc. to match.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8438639 [details] [diff] [review]
patch

>+        while iface.identifier.name != "Event":

Why not just:

  while iface:

?  

But also, is this really simpler/better than:

  return needCx(None, self.member.signatures()[0][1], [], True)

?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Boris Zbarsky [:bz] from comment #4)
> Why not just:
> 
>   while iface:
> 
> ?  
Just because rest of the codegen tends to iterate up to Event


> 
> But also, is this really simpler/better than:
> 
>   return needCx(None, self.member.signatures()[0][1], [], True)

I have no idea what that does. Hard to read at least.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> I have no idea what that does.

It passes the list of our actual IDL arguments to needCx, which is how needCx is _meant_ to be used.  The fact that it even worked with an interface member list is just because both arguments and attributes happen to have the type in .type (which I've been planning to change for attributes, btw!) and because we only support attributes in codegenned events.

So how about this:

1)  Add an arguments() method to IDLMethod, which asserts that there is only one signature and returns that signatures arguments.

2)  Use self.member.arguments() here.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Need to still change .member name to be something which might hint what it is about, or
hide .member here and add arguments to CGNativeMember or something.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: hide the magic behind def arguments()

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8438668 [details] [diff] [review]
hide the magic behind def arguments()

>+    def arguments(self):
>+        return self.member.signatures()[0][1];

Oh, I meant adding arguments() to IDLMethod over in the parser.  But this is ok, I guess.

r=me

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Yeah, I realized you wanted arguments() to IDLMethod, but one would still need to use .member
which is too magical ;)

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Attachment: without ;

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/19ba96e89095

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/19ba96e89095