Closed
Bug 676281
Opened 13 years ago
Closed 12 years ago
[jsdbg2] Debugger.prototype.findScripts
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: jorendorff, Assigned: jimb)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 4 obsolete files)
11.59 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
Debuggers need an API to get all existing scripts for a given debuggee. (That is, Debugger.Script objects for all existing scripts that could conceivably be the .script of a Frame whose scope includes a given debuggee global object.) Steps 1-3 below are necessary because there is currently no way to create a Debugger.Script object for a function script or eval script, given only a pointer to the JSScript. We currently need a holder object. That has to change. Plan: 1. Populate JSScript::object for eval scripts and JS_Evaluate* just like we do for JS_CompileScript. Stop destroying those scripts eagerly; let GC collect them. 2. Make function scripts populate JSScript::object (pointing to the model Function JSObject). 3. Remove all the "unheld script" nonsense and the passing around of scope objects. 4. Add Debugger.prototype.getAllScripts; it'll use the JSCompartment's circular linked list of all scripts.
Reporter | ||
Updated•13 years ago
|
Depends on: 665167
Summary: Debugger.prototype.getAllScripts → [jsdbg2] Debugger.prototype.getAllScripts
Reporter | ||
Comment 1•13 years ago
|
||
Assignee: general → jorendorff
Attachment #551123 -
Flags: review?(jimb)
Reporter | ||
Comment 2•13 years ago
|
||
Add code and tests for invalid arguments. D'oh.
Attachment #551123 -
Attachment is obsolete: true
Attachment #551123 -
Flags: review?(jimb)
Attachment #551165 -
Flags: review?(jimb)
Assignee | ||
Comment 3•13 years ago
|
||
Given that JSScripts no longer need holder objects, does this patch need to be revised?
Assignee | ||
Comment 4•12 years ago
|
||
This patch needs to be rewritten for the new JSScript arrangement. Cribbing from other stuff I see, it seems like the answer would involve this sort of magic: for (gc::CellIter i(cx, this, gc::FINALIZE_SCRIPT); !i.done(); i.next()) {
Reporter | ||
Updated•12 years ago
|
Attachment #551165 -
Flags: review?(jimb)
Comment 5•12 years ago
|
||
Will getAllScripts() include worker scripts as well? I mean scripts added in a page with new Worker("worker.js").
Comment 6•12 years ago
|
||
(In reply to Panos Astithas [:past] from comment #5) > Will getAllScripts() include worker scripts as well? I mean scripts added in > a page with new Worker("worker.js"). Would be nice to have, but we could accept a follow-up for that.
Reporter | ||
Comment 7•12 years ago
|
||
(In reply to Panos Astithas [:past] from comment #5) > Will getAllScripts() include worker scripts as well? I mean scripts added in > a page with new Worker("worker.js"). No, because those scripts are in a different thread. The way the API is designed you'll have a Debugger object in that thread, too. But it needs more work that we haven't started yet. I'll chat with jimb today and we'll see if we can get some bugs filed for it.
Comment 8•12 years ago
|
||
(In reply to Jason Orendorff [:jorendorff] from comment #7) > (In reply to Panos Astithas [:past] from comment #5) > > Will getAllScripts() include worker scripts as well? I mean scripts added in > > a page with new Worker("worker.js"). > > No, because those scripts are in a different thread. > > The way the API is designed you'll have a Debugger object in that thread, > too. But it needs more work that we haven't started yet. I'll chat with jimb > today and we'll see if we can get some bugs filed for it. that sounds like we'll need some corresponding UI to go along with it. If more than one thread, display a thread list near the stack.
Assignee | ||
Comment 9•12 years ago
|
||
Change title to reflect new spec, draft on this branch in the debugge docs GitHub repo: https://github.com/jimblandy/DebuggerDocs/tree/script-enumeration
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Summary: [jsdbg2] Debugger.prototype.getAllScripts → [jsdbg2] Debugger.prototype.findScripts
Assignee | ||
Updated•12 years ago
|
Assignee: jorendorff → jimb
Assignee | ||
Comment 10•12 years ago
|
||
First draft. Doesn't return eval scripts; no 'query' parameter.
Assignee | ||
Updated•12 years ago
|
Attachment #551165 -
Attachment is obsolete: true
Assignee | ||
Comment 11•12 years ago
|
||
The scripts are unrooted from the point that we leave the AutoLockGC region until we put them in the final array. We need an AutoScriptVector or something of that ilk.
Assignee | ||
Comment 12•12 years ago
|
||
This version eliminates the locking around CellIter (billm said it was unnecessary), and defines an AutoScriptVector type to hold the list we're building.
Attachment #601826 -
Flags: review?(jorendorff)
Assignee | ||
Updated•12 years ago
|
Attachment #601799 -
Attachment is obsolete: true
Assignee | ||
Comment 13•12 years ago
|
||
Try run: https://tbpl.mozilla.org/?tree=Try&rev=c67f964d3672
Assignee | ||
Comment 14•12 years ago
|
||
Third draft. Finds eval scripts. No 'query' parameter support. Try run: https://tbpl.mozilla.org/?tree=Try&rev=e40b0d124aef
Attachment #601826 -
Attachment is obsolete: true
Attachment #601826 -
Flags: review?(jorendorff)
Attachment #602203 -
Flags: review?(jorendorff)
Reporter | ||
Comment 15•12 years ago
|
||
Comment on attachment 602203 [details] [diff] [review] Implement Debugger.prototype.findScripts. Great! In vm/Debugger.cpp: >+ JSScript *script = fri.fp()->script(); >+ /* Style nit: blank line before that comment.
Attachment #602203 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 16•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8809550cb08
Target Milestone: --- → mozilla13
Assignee | ||
Updated•12 years ago
|
Flags: in-testsuite+
Assignee | ||
Comment 17•12 years ago
|
||
(In reply to Jim Blandy :jimb from comment #16) That is the wrong changeset id. Hmm.
Assignee | ||
Comment 18•12 years ago
|
||
Here's the real mozilla-inbound changeset ID: https://hg.mozilla.org/integration/mozilla-inbound/rev/6692fb9f8f21
Comment 19•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6692fb9f8f21
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•