Closed Bug 541456 Opened 10 years ago Closed 10 years ago

change ConvertSupportsTojsvals not to use js_AllocStack


(Core :: JavaScript Engine, defect)

Not set





(Reporter: luke, Assigned: luke)



(Whiteboard: fixed-in-tracemonkey)


(1 file, 1 obsolete file)

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.
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.
Summary: change nsJSContext::CallEventHandler to use js_Invoke → change ConvertSupportsTojsvals not to use js_AllocStack
Attached patch basic idea (obsolete) — Splinter Review
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.
Attached patch for reviewSplinter Review
Attachment #423109 - Attachment is obsolete: true
Attachment #423368 - Flags: review?
Attachment #423368 - Flags: review? → review?(jst)
Attachment #423368 - Flags: review?(jst) → review+
Comment on attachment 423368 [details] [diff] [review]
for review

Looks good, r=jst!
Whiteboard: fixed-in-tracemonkey
Luke, anything left to do here, should this bug be marked fixed now?
Sayre usually merges to m-c, posts the cset and then marks as fixed.
Blocks: 545044
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.