1754493 - Check JS_HAZ_CAN_RUN_SCRIPT annotations when deciding whether an IDL method can be implemented in JS

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P3)

Description

[Steve Fink [:sfink] [:s:]]

Make use of the work in bug 1347999. I think it's valid to say now that only methods on an nsISupports subclass that are marked JS_HAZ_CAN_RUN_SCRIPT can indeed be implemented in JS.

Comment 1

[Andrew McCreight [:mccr8]]

While I think this should be generally sound, there's one tricky case here, which is when an interface is [scriptable] and not [builtinclass], but a method or attribute is marked [noscript]. An example of this is nsIInputStream::read.

In this case, the method can't be implemented in JS, so we don't mark it with JS_HAZ_CAN_RUN_SCRIPT.

Nika pointed me at the piece of code in nsXPCWrappedJS::CallMethod() that intercepts and rejects attempts to call into JS:

https://searchfox.org/mozilla-central/rev/b697834e78a3ef7613e2fa57c07624b1d9d1c909/js/xpconnect/src/XPCWrappedJSClass.cpp#793-795

However, we do run AutoEntryScript before that check, so if that can GC, then it means we can still GC in this case. Of course, the easy fix is to move the IsReflectable check earlier before we do anything that can GC.

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1754493 - Use JS_HAZ_CAN_RUN_SCRIPT to determine whether an IDL method could be implemented in JS

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:sfink, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5fa8d91c0c37
Use JS_HAZ_CAN_RUN_SCRIPT to determine whether an IDL method could be implemented in JS r=jonco,mccr8

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/5fa8d91c0c37