541456 - change ConvertSupportsTojsvals not to use js_AllocStack

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Currently nsJSContext::CallEventHandler calls JS_CallFunctionValue with a js_AllocStack'd argument array.  This conflicts with upcoming changes which restrict how the stack is used.  Additionally, this seems wasteful because JS_CallFunctionValue calls js_InternalInvoke which just js_AllocStack/memcpy's a *second* arg vector.  So, should be an all-around win.

Comment 1

[Luke Wagner [:luke]]

I spoke too soon.  Looking at the control flow, it appears that non-js_AllocStack'd data can also be used to call JS_CallFunctionValue.  I'll use the tempPool and an auto temp value rooter instead.

Comment 2

[Luke Wagner [:luke]]

Attachment: basic idea

Here's the basic idea.

Added a snappy new js::LazilyConstructed template to jstl.h so that I can write:

js::LazilyConstructed<JSAutoTempValueRooter> tvr;
...
if (...) {
  tvr.construct(cx, argv, argc);
}

and have tvr do the right thing.

Comment 3

[Luke Wagner [:luke]]

Attachment: for review

Comment 4

[Johnny Stenback (:jst)]

Comment on attachment 423368 [details] [diff] [review]
for review

Looks good, r=jst!

Comment 5

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/2b3944b2a5c2

Comment 6

[Johnny Stenback (:jst)]

Luke, anything left to do here, should this bug be marked fixed now?

Comment 7

[Luke Wagner [:luke]]

Sayre usually merges to m-c, posts the cset and then marks as fixed.

Comment 8

[Luke Wagner [:luke]]

http://hg.mozilla.org/mozilla-central/rev/2b3944b2a5c2