change ConvertSupportsTojsvals not to use js_AllocStack

RESOLVED FIXED

Status

()

RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
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.
(Assignee)

Comment 1

9 years ago
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
(Assignee)

Comment 2

9 years ago
Created attachment 423109 [details] [diff] [review]
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.
(Assignee)

Comment 3

9 years ago
Created attachment 423368 [details] [diff] [review]
for review
Attachment #423109 - Attachment is obsolete: true
Attachment #423368 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #423368 - Flags: review? → review?(jst)

Updated

9 years ago
Attachment #423368 - Flags: review?(jst) → review+
Comment on attachment 423368 [details] [diff] [review]
for review

Looks good, r=jst!
(Assignee)

Comment 5

9 years ago
http://hg.mozilla.org/tracemonkey/rev/2b3944b2a5c2
Whiteboard: fixed-in-tracemonkey
Luke, anything left to do here, should this bug be marked fixed now?
(Assignee)

Comment 7

9 years ago
Sayre usually merges to m-c, posts the cset and then marks as fixed.
(Assignee)

Updated

9 years ago
Blocks: 545044
(Assignee)

Comment 8

9 years ago
http://hg.mozilla.org/mozilla-central/rev/2b3944b2a5c2
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.