Bug 127543 - Move JavaScriptCallFrame and ScriptDebugServer into JavaScriptCore for inspector
Summary: Move JavaScriptCallFrame and ScriptDebugServer into JavaScriptCore for inspector
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-23 21:32 PST by Joseph Pecoraro
Modified: 2014-01-24 20:38 PST (History)
10 users (show)

See Also:


Attachments
[PATCH] Part 1 - JavaScriptCallFrame (74.98 KB, patch)
2014-01-24 17:15 PST, Joseph Pecoraro
ggaren: review+
Details | Formatted Diff | Diff
[PATCH] Part 2 - ScriptDebugServer (46.58 KB, patch)
2014-01-24 17:15 PST, Joseph Pecoraro
ggaren: review+
Details | Formatted Diff | Diff
[PATCH] For Bots 1 (115.25 KB, patch)
2014-01-24 17:15 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] For Bots 2 (116.97 KB, patch)
2014-01-24 19:23 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2014-01-23 21:32:48 PST
Needed for InspectorDebuggerAgent (e.g. setting breakpoints, stepping through source code, evaluating JavaScript on call frames).

This is a non-tivial move. It makes these changes:
- Move JavaScriptCallFrame from being idl generated to native JS.
- Make some minor changes to ScriptDebugServer so that it doesn't rely on WebCore knowledge.
Comment 1 Joseph Pecoraro 2014-01-24 17:15:00 PST
Created attachment 222171 [details]
[PATCH] Part 1 - JavaScriptCallFrame
Comment 2 Joseph Pecoraro 2014-01-24 17:15:22 PST
Created attachment 222172 [details]
[PATCH] Part 2 - ScriptDebugServer
Comment 3 Joseph Pecoraro 2014-01-24 17:15:59 PST
Created attachment 222173 [details]
[PATCH] For Bots 1
Comment 4 WebKit Commit Bot 2014-01-24 17:17:22 PST
Attachment 222171 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:76:  GLOBAL_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:77:  LOCAL_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:78:  WITH_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:79:  CLOSURE_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:80:  CATCH_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 5 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 WebKit Commit Bot 2014-01-24 17:18:59 PST
Attachment 222173 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/ScriptDebugServer.h:89:  The parameter name "callback" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:76:  GLOBAL_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:77:  LOCAL_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:78:  WITH_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:79:  CLOSURE_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:80:  CATCH_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Geoffrey Garen 2014-01-24 17:23:30 PST
Comment on attachment 222171 [details]
[PATCH] Part 1 - JavaScriptCallFrame

r=me
Comment 7 Geoffrey Garen 2014-01-24 17:23:51 PST
Comment on attachment 222172 [details]
[PATCH] Part 2 - ScriptDebugServer

r=me
Comment 8 Geoffrey Garen 2014-01-24 17:27:08 PST
Comment on attachment 222171 [details]
[PATCH] Part 1 - JavaScriptCallFrame

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

> Source/JavaScriptCore/inspector/JSJavaScriptCallFramePrototype.cpp:66
> +#define JSC_NATIVE_NON_INDEX_ACCESSOR(jsName, cppName, attributes) \

Any reason this can't be an inline function?
Comment 9 Joseph Pecoraro 2014-01-24 17:40:29 PST
(In reply to comment #8)
> (From update of attachment 222171 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=222171&action=review
> 
> > Source/JavaScriptCore/inspector/JSJavaScriptCallFramePrototype.cpp:66
> > +#define JSC_NATIVE_NON_INDEX_ACCESSOR(jsName, cppName, attributes) \
> 
> Any reason this can't be an inline function?

This, like the other JSC_NATIVE* macros, assumes you have "exec, globalObject, and vm" variables in scope with those names. So turning this into inline function would double the number of arguments to JSC_NATIVE_NON_INDEX_ACCESSOR making it a bit harder to read. However, it would get rid of the need for the anonymous scope in the macro. I like the easier to read call site matching JSC_NATIVE_FUNCTION.

Would an inline function be preferred?
Comment 10 Joseph Pecoraro 2014-01-24 19:23:42 PST
Created attachment 222185 [details]
[PATCH] For Bots 2

Apparently includes between debug and release are different! Adding some needed for Release builds.
Comment 11 WebKit Commit Bot 2014-01-24 19:24:47 PST
Attachment 222185 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/inspector/ScriptDebugServer.h:89:  The parameter name "callback" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:76:  GLOBAL_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:77:  LOCAL_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:78:  WITH_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:79:  CLOSURE_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/inspector/JSJavaScriptCallFrame.h:80:  CATCH_SCOPE is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 6 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Joseph Pecoraro 2014-01-24 20:38:37 PST
Landed <http://trac.webkit.org/changeset/162757>.