Closed
Bug 732653
Opened 12 years ago
Closed 12 years ago
Remove JS_FrameIterator
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dvander, Assigned: dvander)
References
(Blocks 1 open bug)
Details
Attachments
(3 files, 2 obsolete files)
7.90 KB,
patch
|
luke
:
review+
mrbkap
:
review+
|
Details | Diff | Splinter Review |
19.07 KB,
patch
|
bholley
:
review+
luke
:
review+
|
Details | Diff | Splinter Review |
12.35 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
This API call is incompatible with IonMonkey, which does not create JSStackFrames. Most uses of this in the browser just need basic information about the topmost frame. For more special uses (like in caps), we can preserve the API (it'll be renamed to something like JS_BrokenFrameIterator) - it'll still work as intended, since Ion stack traces won't cross globals - but the actual iteration will be eliding optimized frames.
Assignee | ||
Comment 1•12 years ago
|
||
This is a somewhat ugly but safer C API for iterating and querying stack frames. XPCDebug and jsd's uses of JS_FrameIterator can go away with this, I think.
Assignee: general → dvander
Status: NEW → ASSIGNED
Comment 2•12 years ago
|
||
I think you meant to qref before attaching.
Assignee | ||
Comment 3•12 years ago
|
||
This patch introduces a new API, that returns an array of stack frame descriptions. This covers exactly what XPCStack needs.
Attachment #603141 -
Attachment is obsolete: true
Assignee | ||
Comment 4•12 years ago
|
||
This patch removes a bunch of complex frame snooping code from js/xpconnect/src to js/src. The functionality should be identical, I just cleaned it up.
Assignee | ||
Comment 5•12 years ago
|
||
Comment on attachment 614659 [details] [diff] [review] remove JS_FrameIterator use from XPCStack Luke for JS changes
Attachment #614659 -
Flags: review?(luke)
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 614659 [details] [diff] [review] remove JS_FrameIterator use from XPCStack Bobby for XPC changes
Attachment #614659 -
Flags: review?(bobbyholley+bmo)
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #614938 -
Attachment is obsolete: true
Attachment #614964 -
Flags: review?(bobbyholley+bmo)
Assignee | ||
Updated•12 years ago
|
Attachment #614964 -
Flags: review?(luke)
Assignee | ||
Updated•12 years ago
|
Attachment #614964 -
Attachment description: patch → move FormatJSStack to js/src
Assignee | ||
Comment 8•12 years ago
|
||
These remaining uses are either going away from CPG or are guaranteed not to run with live IonMonkey code.
Attachment #614969 -
Flags: review?(luke)
Updated•12 years ago
|
Attachment #614659 -
Flags: review?(luke) → review+
Comment 9•12 years ago
|
||
Comment on attachment 614964 [details] [diff] [review] move FormatJSStack to js/src Review of attachment 614964 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsdbgapi.h @@ +105,5 @@ > > +extern JS_PUBLIC_API(char *) > +FormatStackDump(JSContext *cx, char *buf, > + JSBool showArgs, JSBool showLocals, > + JSBool showThisProps); Can you align the parameters? Also I think you can remove 'buf'.
Attachment #614964 -
Flags: review?(luke) → review+
Comment 10•12 years ago
|
||
Can we assert against IM somehow in JS_BrokenFrameIterator? The caps fixup has always been a 'when we have time', sort of thing. So if we have a more serious deadline, we need to make the dependency very explicit (and possibly get someone in the loop whose job it is to keep track of these things).
Updated•12 years ago
|
Attachment #614964 -
Flags: review?(bobbyholley+bmo) → review+
Comment 11•12 years ago
|
||
Comment on attachment 614659 [details] [diff] [review] remove JS_FrameIterator use from XPCStack I can't really get my head around the CreateStack rewrite, mostly because I've never looked at this code before and the diff isn't very comprehensible. Bouncing to Blake to see if he wants to do it. I'd also be fine with luke reviewing this part.
Attachment #614659 -
Flags: review?(bobbyholley+bmo) → review?(mrbkap)
Comment 12•12 years ago
|
||
Comment on attachment 614659 [details] [diff] [review] remove JS_FrameIterator use from XPCStack Review of attachment 614659 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCStack.cpp @@ +134,2 @@ > > + JS::StackDescription* desc = JS::DescribeStack(cx, MAX_FRAMES); Is it worth exposing a RAII helper to auto-destroy this as well? @@ +138,2 @@ > > + for (unsigned i = 0; i < desc->nframes && self; i++) { I'd prefer to use a more explicitly-typed integer type here.
Attachment #614659 -
Flags: review?(mrbkap) → review+
Updated•12 years ago
|
Attachment #614969 -
Flags: review?(luke) → review+
Comment 13•12 years ago
|
||
Is there an ETA for this landing? It seems like it's been a month or so with no movement. I'm trying to get rid of js_GetTopStackFrame uses from the code for Ion, and JS_FrameIterator seems to be the cause of basically every orange on TBPL that I'm getting.
Comment 14•12 years ago
|
||
Well, yes. You might want to follow bug 757046. I also just landed bug 754202, which was part of the battle. What's the current ETA for IM?
Comment 15•12 years ago
|
||
End of June. I'll watch this for movement and take care of more pressing fuzz bugs in the meantime.
Assignee | ||
Comment 16•12 years ago
|
||
Rebased and rolled up into one patch: https://hg.mozilla.org/projects/ionmonkey/rev/9df5b5265826 Luckily, it looks like a lot of uses went away on their own as post-CPG work. Cool!
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 17•12 years ago
|
||
Comment on attachment 614969 [details] [diff] [review] "remove" the API Review of attachment 614969 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsdbgapi.h @@ +251,5 @@ > > /* > + * This function does not work when IonMonkey is active. It remains for legacy > + * code: caps/principal clamping, which will be removed shortly after > + * compartment-per-global, and jsd, which can only be used when IonMonkey is Heh, how shortly?
You need to log in
before you can comment on or make changes to this bug.
Description
•