Last Comment Bug 901098 - Trace actor "callsite" trace type actually returns source location
: Trace actor "callsite" trace type actually returns source location
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: x86 Mac OS X
-- normal (vote)
: Firefox 25
Assigned To: Jake Bailey
: J. Ryan Stinnett [:jryans] (use ni?)
Depends on:
Blocks: 887024
  Show dependency treegraph
Reported: 2013-08-02 13:48 PDT by Jake Bailey
Modified: 2013-08-05 01:10 PDT (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (6.29 KB, patch)
2013-08-02 13:53 PDT, Jake Bailey
no flags Details | Diff | Splinter Review
Patch (9.40 KB, patch)
2013-08-02 16:35 PDT, Jake Bailey
nfitzgerald: review+
Details | Diff | Splinter Review

Description User image Jake Bailey 2013-08-02 13:48:39 PDT
The trace actor has a "callsite" trace type, but it returns the current position in execution, i.e. the first character of the executing function. There should be a separate "location" trace type for source location, and "callsite" should return the actual call site of the executing function.
Comment 1 User image Jake Bailey 2013-08-02 13:53:01 PDT
Created attachment 785182 [details] [diff] [review]
Comment 2 User image Jake Bailey 2013-08-02 13:58:31 PDT
Try push:
Comment 3 User image Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] 2013-08-02 14:37:08 PDT
Comment on attachment 785182 [details] [diff] [review]

Review of attachment 785182 [details] [diff] [review]:

Can you file a follow up to use Debugger.Script.prototype.startLine and Debugger.Script.prototype.startColumn (which doesn't exist yet, so file a bug for it please) instead of the line/col of the frame?

::: toolkit/devtools/server/tests/unit/test_trace_actor-05.js
@@ +85,5 @@
> +      do_check_eq(typeof packets[1].callsite.column, "number",
> +                  'anonymous function callsite should have column');
> +      do_check_true(!isNaN(packets[1].callsite.column),
> +                    'anonymous function callsite column should be a number');
> +

Can we do better than just checking that line and column are numbers? Can we actually check that these things are the callsite vs definition location so that we don't repeat this regression?
Comment 4 User image Jake Bailey 2013-08-02 16:35:15 PDT
Created attachment 785270 [details] [diff] [review]
Comment 5 User image Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] 2013-08-02 17:27:43 PDT
Comment 6 User image Tim Taubert [:ttaubert] 2013-08-05 01:10:42 PDT

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