Bug 64981 - export JSContextGetBacktrace as SPI in JSContextRefPrivate.h
Summary: export JSContextGetBacktrace as SPI in JSContextRefPrivate.h
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-21 15:04 PDT by Sommer Panage
Modified: 2011-12-21 15:19 PST (History)
5 users (show)

See Also:


Attachments
path to export JSContextGetBacktrace functionality as private SPI (4.98 KB, patch)
2011-07-21 15:15 PDT, Sommer Panage
ggaren: review-
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
same backtrace patch with changes suggested by Geoffrey Garen integrated (4.91 KB, patch)
2011-07-22 15:09 PDT, Sommer Panage
oliver: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
same backtrace patch with tabs removed from ChangeLog (4.96 KB, patch)
2011-07-22 15:35 PDT, Sommer Panage
oliver: review-
gyuyoung.kim: commit-queue-
Details | Formatted Diff | Diff
Same patch, getting rid of sprintf for cleaner UString::number() use instead (5.01 KB, patch)
2011-07-22 17:00 PDT, Sommer Panage
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sommer Panage 2011-07-21 15:04:17 PDT
UIAutomation for iOS would like to support a Javascript backtrace in our error logs. Currently, the C API does not provide the tools to do this. However, the private API does expose the necessary functionality to get a backtrace (via Interpreter::retrieveLastCaller). We recognize this information may result in failure in the cases of programs run by 'veal', stack frames beneath host function call frames, and in programs run from other programs. Thus, we propose exporting our JSContextGetBacktrace in JSContextRefPrivate.h. This will provide us with the tools we need while not advertising an API that isn't really ready for full use.
Comment 1 Sommer Panage 2011-07-21 15:04:43 PDT
The patch will be added to this bug report shortly.
Comment 2 Sommer Panage 2011-07-21 15:15:28 PDT
Created attachment 101648 [details]
path to export JSContextGetBacktrace functionality as private SPI
Comment 3 Sommer Panage 2011-07-21 15:15:45 PDT
Patch attached.
Comment 4 Alexey Proskuryakov 2011-07-22 10:53:09 PDT
This patch is not marked for review. Did you want it to be reviewed?

Please see <http://www.webkit.org/coding/contributing.html> for information about contributing code to WebKit.
Comment 5 Oliver Hunt 2011-07-22 10:57:13 PDT
Comment on attachment 101648 [details]
path to export JSContextGetBacktrace functionality as private SPI

We're working on a better mechanism for getting a stacktrace.  Once that's in place any API of this kind will be able to use that.
Comment 6 Sommer Panage 2011-07-22 11:11:00 PDT
Yes, I apologize as, we do want the patch reviewed. 

We recognize that there is work underway to get a stack trace into the API. Of course, once that's in place, we would be able to utilize it for our purposes. However, at this point, it is our understanding that this work is in progress and may not be available for some time. As we very much need a solution now, we were hoping to get this in place as, at least, a temporary fix until better methods are made available.
Comment 7 Oliver Hunt 2011-07-22 13:47:44 PDT
Comment on attachment 101648 [details]
path to export JSContextGetBacktrace functionality as private SPI

We'll migrate to the better implementation in future
Comment 8 WebKit Review Bot 2011-07-22 13:50:29 PDT
Comment on attachment 101648 [details]
path to export JSContextGetBacktrace functionality as private SPI

Rejecting attachment 101648 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2

Last 500 characters of output:
Parsed 4 diffs from patch file(s).
patching file Source/JavaScriptCore/ChangeLog-PEP
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file Source/JavaScriptCore/ChangeLog-PEP.rej
patching file Source/JavaScriptCore/JavaScriptCore.exp
patching file Source/JavaScriptCore/API/JSContextRef.cpp
patching file Source/JavaScriptCore/API/JSContextRefPrivate.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Oliver Hunt', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/9232141
Comment 9 Geoffrey Garen 2011-07-22 13:52:39 PDT
Comment on attachment 101648 [details]
path to export JSContextGetBacktrace functionality as private SPI

r- because this patch doesn't apply cleanly.

> +++ Source/JavaScriptCore/ChangeLog-PEP	(working copy)

To apply cleanly, you need to patch JavaScriptCore/ChangeLog. (There is no ChangeLog-PEP.)

> failure in the cases of programs run by 'veal'

veal! ;)
Comment 10 Geoffrey Garen 2011-07-22 13:56:50 PDT
One more comment:

> JSContextGetBacktrace

Since this function allocates a string the the caller must release, it should be called "JSContextCreateBacktrace", not "JSContextGetBacktrace". (Same naming conventions as CF.)
Comment 11 Sommer Panage 2011-07-22 15:09:12 PDT
Created attachment 101773 [details]
same backtrace patch with changes suggested by Geoffrey Garen integrated
Comment 12 WebKit Review Bot 2011-07-22 15:09:53 PDT
Comment on attachment 101773 [details]
same backtrace patch with changes suggested by Geoffrey Garen integrated

Rejecting attachment 101773 [details] from review queue.

panage@apple.com does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.
Comment 13 WebKit Review Bot 2011-07-22 15:26:02 PDT
Comment on attachment 101773 [details]
same backtrace patch with changes suggested by Geoffrey Garen integrated

Rejecting attachment 101773 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2

Last 500 characters of output:
47ac38bc1954a95bbb76e9cf2258dfa2fd0f625d
r91611 = 788227ec3d4f823dac8b3804ad12ab188a0f557e
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/9230224
Comment 14 Sommer Panage 2011-07-22 15:35:03 PDT
Created attachment 101780 [details]
same backtrace patch with tabs removed from ChangeLog

Some tabs seem to have snuck into the change log. Trying again.
Comment 15 Gyuyoung Kim 2011-07-22 16:44:27 PDT
Comment on attachment 101780 [details]
same backtrace patch with tabs removed from ChangeLog

Attachment 101780 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9233218
Comment 16 Oliver Hunt 2011-07-22 16:46:52 PDT
Comment on attachment 101780 [details]
same backtrace patch with tabs removed from ChangeLog

View in context: https://bugs.webkit.org/attachment.cgi?id=101780&action=review

> Source/JavaScriptCore/API/JSContextRef.cpp:211
> +        char levelStr[6];
> +        sprintf(levelStr, "#%d ", count);

should be snprintf if anything i guess, but...

> Source/JavaScriptCore/API/JSContextRef.cpp:226
> +        builder.append(levelStr);

Can you not just use UString::number() here instead?
Comment 17 Sommer Panage 2011-07-22 16:58:33 PDT
I most certainly can use UString::number() -- submitting the change.
Comment 18 Sommer Panage 2011-07-22 17:00:48 PDT
Created attachment 101796 [details]
Same patch, getting rid of sprintf for cleaner UString::number() use instead
Comment 19 WebKit Review Bot 2011-07-22 17:19:20 PDT
Comment on attachment 101796 [details]
Same patch, getting rid of sprintf for cleaner UString::number() use instead

Clearing flags on attachment: 101796

Committed r91627: <http://trac.webkit.org/changeset/91627>
Comment 20 Eric Seidel (no email) 2011-12-21 15:19:34 PST
Looks landed.  If that's incorrect, please open a new bug per additional patch you'd like to land.