Remove IsCallable check for constructor

[dminor]

From my initial telemetry, I think requiring IsCallable(constructor) when the optional constructor argument is present will be web compatible, but I don't have a strong opinion about whether it's worth the bother. Setting up this PR in case we decide to remove this check.

[mgaudet]

SGTM

[dead-claudia]

Question: does your telemetry include calls like Error.captureStackTrace(error) or Error.captureStackTrace(error, null), where the constructor is clearly intended to be omitted? Or does it only include stuff like Error.captureStackTrace(error, "foo") and Error.captureStackTrace(error, false)?

Edit: I'm going based off this slide from the agenda post: https://docs.google.com/presentation/d/1RGNDcJee_6N2II0SeGyMVglvjhJbuqDX02LLzBqyCWI/mobilepresent?slide=id.g36c5e1494e7_0_9

[dminor]

Question: does your telemetry include calls like Error.captureStackTrace(error) or Error.captureStackTrace(error, null), where the constructor is clearly intended to be omitted? Or does it only include stuff like Error.captureStackTrace(error, "foo") and Error.captureStackTrace(error, false)?

Edit: I'm going based off this slide from the agenda post: https://docs.google.com/presentation/d/1RGNDcJee_6N2II0SeGyMVglvjhJbuqDX02LLzBqyCWI/mobilepresent?slide=id.g36c5e1494e7_0_9

The actual check is here. If constructor is not undefined, we check to see if it's callable. So we would count Error.captureStackTrace(error, null) as passing in a non-callable constructor.

[dead-claudia]

I'm curious how the telemetry changes if you exclude null and undefined constructor arguments. 0.8% of calls is still a lot of calls, but I suspect a majority of the calls with a non-callable constructor argument are probably just one of those two. Plus, this PR explicitly ignores undefined in particular, and your Firefox metrics code doesn't check for that to exclude it.

[dminor]

To me it seems kind of strange that people would use Error.captureStackTrace(error, undefined) or Error.captureStackTrace(error, null) rather than just not passing in the constructor argument, but you're right, we could modify the telemetry to handle those cases separately.

That said, maybe we should just land this PR and remove the check, I'm not sure if it's worth potentially causing problems for people. I don't think we'd special case undefined or null in the specification text, for instance.

[dead-claudia]

To me it seems kind of strange that people would use Error.captureStackTrace(error, undefined) or Error.captureStackTrace(error, null) rather than just not passing in the constructor argument, but you're right, we could modify the telemetry to handle those cases separately.

It seems equally strange to me as well (especially for null), but I see similar idioms all the time in the wild in Stack Overflow and various websites, in code written by less JS-savvy web developers who mainly focus on HTML, CSS, and backend stuff. That's why I called it out.