Last Comment Bug 901098 - Trace actor "callsite" trace type actually returns source location
: Trace actor "callsite" trace type actually returns source location
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: Firefox 25
Assigned To: Jake Bailey
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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 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 Jake Bailey 2013-08-02 13:53:01 PDT
Created attachment 785182 [details] [diff] [review]
Patch
Comment 2 Jake Bailey 2013-08-02 13:58:31 PDT
Try push: https://tbpl.mozilla.org/?tree=Try&rev=1dbf0a1bc93c
Comment 3 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2013-08-02 14:37:08 PDT
Comment on attachment 785182 [details] [diff] [review]
Patch

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 Jake Bailey 2013-08-02 16:35:15 PDT
Created attachment 785270 [details] [diff] [review]
Patch
Comment 5 Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] 2013-08-02 17:27:43 PDT
https://hg.mozilla.org/integration/fx-team/rev/290389a3cbe7
Comment 6 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2013-08-05 01:10:42 PDT
https://hg.mozilla.org/mozilla-central/rev/290389a3cbe7

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