Closed
Bug 894854
Opened 11 years ago
Closed 11 years ago
Add env var that disables censoring of self-hosted script frames
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: pnkfelix, Assigned: pnkfelix)
References
(Blocks 1 open bug)
Details
(Whiteboard: [js:t])
Attachments
(1 file, 3 obsolete files)
1.90 KB,
patch
|
pnkfelix
:
review+
|
Details | Diff | Splinter Review |
We currently take pains to skip over the frames from self-hosted javascript code when building up stack traces. That makes sense for inter-operation with tools that want to inspect the stack, where we want to hide our internal implementation details. But when one is debugging the self-hosted code itself, skipping over those frames is throwing out useful information. So we should provide some way for that information to be reported; not by default, but when used in tandem with some combination of: * A debug-build of spidermonkey, and/or * A shell build, and/or * Some environment variable setting.
Assignee | ||
Comment 1•11 years ago
|
||
This is probably not what we actually want. I know I'm inviting a bike shed discussion, but since I don't actually care what constraints we add to this "functionality", go ahead, pile on what shades of paint you want to see here.
Attachment #777034 -
Flags: feedback?(till)
Comment 2•11 years ago
|
||
Sorry, should've filed this yesterday. We can't make it hinge on debug mode, as that would make testing of at least some addons (such as GreaseMonkey) impossible. And in general, would be a pretty severe change of behavior between release and debug. Enabling it only in the shell would work, but it would also be pretty unfortunate: there'd be no way to debug self-hosting issues that one can reproduce only in the browser. Plus, again, introduce a noticeable difference in behavior between two environments: the shell and the browser, in this case. We already have too many of those. Thus, an env var is probably the way to go.
Blocks: 784288
OS: Mac OS X → All
Hardware: x86 → All
Summary: self-hosted errors should be identified by line number in debug (and/or shell?) → Add env var that disables censoring of self-hosted script frames
Whiteboard: [js:t]
Comment 3•11 years ago
|
||
Comment on attachment 777034 [details] [diff] [review] patch A v1: DEBUG guard alone Review of attachment 777034 [details] [diff] [review]: ----------------------------------------------------------------- Clearing feedback based on the previous comment.
Attachment #777034 -
Flags: feedback?(till)
Assignee | ||
Comment 4•11 years ago
|
||
Attachment #777034 -
Attachment is obsolete: true
Attachment #777075 -
Flags: review?(till)
Assignee | ||
Updated•11 years ago
|
Assignee: general → pnkfelix
Assignee | ||
Comment 5•11 years ago
|
||
Not asking for review yet because this is not tested beyond checking it compile; just checkpointing here; will sanity-check later today.
Attachment #777075 -
Attachment is obsolete: true
Attachment #777075 -
Flags: review?(till)
Comment 6•11 years ago
|
||
Comment on attachment 777092 [details] [diff] [review] patch A v3: cached env var and debug guard both Review of attachment 777092 [details] [diff] [review]: ----------------------------------------------------------------- I know you didn't ask for a review, but if your testing shows this to work as expected, I'm fine with it. My only quibble is the long name for the env var: I'm 100% certain I won't be able to remember that. How about MOZ_SHOW_SH_FRAMES, or even just SHOW_SH_FRAMES? (Little danger of that colliding with anything, I'd say.)
Attachment #777092 -
Flags: review+
Assignee | ||
Comment 7•11 years ago
|
||
till notes that it would be good to update this page: https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/self-hosting in tandem with landing any patch for this bug.
Assignee | ||
Comment 8•11 years ago
|
||
inherited r+ from till. a bit of bikeshedding in pjs: http://logbot.glob.com.au/?c=mozilla%23pjs#c35581 yielded the environment variable name that we settled on: MOZ_SHOW_ALL_JS_FRAMES
Attachment #777092 -
Attachment is obsolete: true
Attachment #777177 -
Flags: review+
Assignee | ||
Comment 9•11 years ago
|
||
try run: try: -b do -p all -u all[x86] -t none https://tbpl.mozilla.org/?tree=Try&rev=5368efc761a4 (all green)
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5e29b812885c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in
before you can comment on or make changes to this bug.
Description
•