Last Comment Bug 676281 - [jsdbg2] Debugger.prototype.findScripts
: [jsdbg2] Debugger.prototype.findScripts
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: All All
: -- normal (vote)
: mozilla13
Assigned To: Jim Blandy :jimb
:
Mentors:
Depends on: 665167
Blocks: js::dbg2 670002 723563 733461
  Show dependency treegraph
 
Reported: 2011-08-03 09:30 PDT by Jason Orendorff [:jorendorff]
Modified: 2012-03-10 02:51 PST (History)
9 users (show)
jimb: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (12.32 KB, patch)
2011-08-05 12:30 PDT, Jason Orendorff [:jorendorff]
no flags Details | Diff | Review
v2 (14.56 KB, patch)
2011-08-05 14:43 PDT, Jason Orendorff [:jorendorff]
no flags Details | Diff | Review
Implement Debugger.prototype.findScripts. (8.25 KB, patch)
2012-02-29 15:32 PST, Jim Blandy :jimb
no flags Details | Diff | Review
Implement Debugger.prototype.findScripts. (9.07 KB, patch)
2012-02-29 17:07 PST, Jim Blandy :jimb
no flags Details | Diff | Review
Implement Debugger.prototype.findScripts. (11.59 KB, patch)
2012-03-01 16:54 PST, Jim Blandy :jimb
jorendorff: review+
Details | Diff | Review

Description Jason Orendorff [:jorendorff] 2011-08-03 09:30:43 PDT
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.
Comment 1 Jason Orendorff [:jorendorff] 2011-08-05 12:30:48 PDT
Created attachment 551123 [details] [diff] [review]
v1
Comment 2 Jason Orendorff [:jorendorff] 2011-08-05 14:43:16 PDT
Created attachment 551165 [details] [diff] [review]
v2

Add code and tests for invalid arguments. D'oh.
Comment 3 Jim Blandy :jimb 2011-11-08 15:33:29 PST
Given that JSScripts no longer need holder objects, does this patch need to be revised?
Comment 4 Jim Blandy :jimb 2011-12-19 18:44:38 PST
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()) {
Comment 5 Panos Astithas [:past] 2012-02-01 03:51:09 PST
Will getAllScripts() include worker scripts as well? I mean scripts added in a page with new Worker("worker.js").
Comment 6 Rob Campbell [:rc] (:robcee) 2012-02-01 10:34:09 PST
(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.
Comment 7 Jason Orendorff [:jorendorff] 2012-02-02 04:57:50 PST
(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 Rob Campbell [:rc] (:robcee) 2012-02-06 06:45:28 PST
(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.
Comment 9 Jim Blandy :jimb 2012-02-29 15:29:46 PST
Change title to reflect new spec, draft on this branch in the debugge docs GitHub repo:
https://github.com/jimblandy/DebuggerDocs/tree/script-enumeration
Comment 10 Jim Blandy :jimb 2012-02-29 15:32:21 PST
Created attachment 601799 [details] [diff] [review]
Implement Debugger.prototype.findScripts.

First draft. Doesn't return eval scripts; no 'query' parameter.
Comment 11 Jim Blandy :jimb 2012-02-29 15:46:31 PST
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.
Comment 12 Jim Blandy :jimb 2012-02-29 17:07:25 PST
Created attachment 601826 [details] [diff] [review]
Implement Debugger.prototype.findScripts.

This version eliminates the locking around CellIter (billm said it was unnecessary), and defines an AutoScriptVector type to hold the list we're building.
Comment 13 Jim Blandy :jimb 2012-02-29 17:09:57 PST
Try run: https://tbpl.mozilla.org/?tree=Try&rev=c67f964d3672
Comment 14 Jim Blandy :jimb 2012-03-01 16:54:29 PST
Created attachment 602203 [details] [diff] [review]
Implement Debugger.prototype.findScripts.

Third draft. Finds eval scripts. No 'query' parameter support.
Try run: https://tbpl.mozilla.org/?tree=Try&rev=e40b0d124aef
Comment 15 Jason Orendorff [:jorendorff] 2012-03-02 09:15:42 PST
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.
Comment 17 Jim Blandy :jimb 2012-03-09 17:33:12 PST
(In reply to Jim Blandy :jimb from comment #16)

That is the wrong changeset id. Hmm.
Comment 18 Jim Blandy :jimb 2012-03-09 18:25:37 PST
Here's the real mozilla-inbound changeset ID:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6692fb9f8f21

Note You need to log in before you can comment on or make changes to this bug.