1618865 - Consider making webidl method calls that have extra, unrecognized parameters warn in the console when used by developers

Status: NEW

(Core :: DOM: Bindings (WebIDL), enhancement, P3)

Description

[:Gijs (he/him)]

I was helping someone debug an issue with getUserMedia, where it turned out they were passing the callback function as a second argument, instead of a promise handler.

If they'd just passed the wrong type of argument to the method, the console would have told them something informative like "argument 3 of call to foo() should be ...". It would be nice if we did something similar (perhaps only if devtools are in use?) when passing extraneous parameters to methods (instead of just ignoring them, which is what happens today).

Comment 1

[Boris Zbarsky [:bzbarsky]]

So, implementing this is pretty straightforward. On the assumption that we don't want to do this for setters (because if someone .calls a setter function with multiple args... meh), we can modify CGPerSignatureCall in Codegen.py to spit out a block like so in the not getter and not setter case:

  if (args.length() > argcount) {
    // Call some out-of-line binding method to issue a console warning, because I don't see JSAPI for warnings.
  }

where argcount is the argument count for the signature. We'd need to not do this if the signature is variadic, of course.

The main drawbacks are:

• Some increase in codesize; can be measured with bloaty pretty easily.
• Possible false positives in practice; would need to be experimented with.

Thoughts?

Comment 2

[Boris Zbarsky [:bzbarsky]]

because I don't see JSAPI for warnings

I asked on #spidermonkey and we have two options there:

• JS::WarnUTF8
• Something like JS_ReportErrorFlagsAndNumberASCII(cx, JSREPORT_WARNING, GetErrorMessage, nullptr, <error-code-goes-here>)

Building this on top of JS::WarnUTF8 should be pretty easy.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: prototype. This needs a bunch of cleanup as follows:

This is a proof-of-concept based on bug 1618011.

It adds about 82KB of codesize and about 23KB of readonly string data...

On the bright side, I guess, it catches calls to the TextEncoder constructor with
useless arguments in toolkit/components/osfile/modules/osfile_native.jsm and
toolkit/components/utils/Sampling.jsm.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Merged on top of changes to preceding patches

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated on top of final bug 1618865 changes