WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 119575
Restoring use of StackIterator instead of Interpreter::getStacktrace()
https://bugs.webkit.org/show_bug.cgi?id=119575
Summary
Restoring use of StackIterator instead of Interpreter::getStacktrace()
Mark Lam
Reported
2013-08-08 08:41:35 PDT
This change set reverts
r153383
(
https://bugs.webkit.org/show_bug.cgi?id=119141
) by adding a numberOfFrames() method to the StackIterator class. This enables us to achieve the createScriptCallStack() semantics that the inspector expects. This fixes the issue in
https://bugs.webkit.org/show_bug.cgi?id=119141
, and allows us to pass the "plugins" layout tests. It is questionable whether it is appropriate for the inspector to expect those semantics, but that's a question for another day. This change set also makes Interpreter::getStackTrace() private again so that "unauthorized" code is not encouraged to call it. This results in "moving" some ErrorConstructor and NativeErrorConstructor functions under the Interpreter.
Attachments
the patch
(12.46 KB, patch)
2013-08-08 08:52 PDT
,
Mark Lam
oliver
: review+
Details
Formatted Diff
Diff
updated patch: use "goto", and ErrorConstructor changes landed separately.
(6.85 KB, patch)
2013-08-08 09:52 PDT
,
Mark Lam
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Mark Lam
Comment 1
2013-08-08 08:52:16 PDT
Created
attachment 208347
[details]
the patch
Oliver Hunt
Comment 2
2013-08-08 09:01:14 PDT
Comment on
attachment 208347
[details]
the patch View in context:
https://bugs.webkit.org/attachment.cgi?id=208347&action=review
> Source/JavaScriptCore/ChangeLog:19 > + (JSC::StackIterator::goToNextFrame): Renamed from gotoNextFrame.
goto is a single word so the old capitalization was correct
> Source/JavaScriptCore/ChangeLog:32 > + (JSC::ErrorConstructor::getCallData): > + - Don't want ErrorConstructor to call Interpreter::getStackTrace() > + directly. So, we moved the helper functions into the Interpreter > + class.
Haven't read the full patch yet, but if possible it would be nice to have changes that should not effect behavior moved into a separate bug, and landed earlier if possible
Oliver Hunt
Comment 3
2013-08-08 09:04:06 PDT
Comment on
attachment 208347
[details]
the patch View in context:
https://bugs.webkit.org/attachment.cgi?id=208347&action=review
> Source/JavaScriptCore/interpreter/Interpreter.h:229 > + static EncodedJSValue JSC_HOST_CALL constructWithErrorConstructor(ExecState*); > + static EncodedJSValue JSC_HOST_CALL callErrorConstructor(ExecState*); > + static EncodedJSValue JSC_HOST_CALL constructWithNativeErrorConstructor(ExecState*); > + static EncodedJSValue JSC_HOST_CALL callNativeErrorConstructor(ExecState*); > +
I like this, but it seems like this can be a separate patch, r+ for anding it separately
Mark Lam
Comment 4
2013-08-08 09:05:26 PDT
(In reply to
comment #2
)
> (From update of
attachment 208347
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=208347&action=review
> > > Source/JavaScriptCore/ChangeLog:19 > > + (JSC::StackIterator::goToNextFrame): Renamed from gotoNextFrame. > > goto is a single word so the old capitalization was correct
Hmmm ... not in English AFAIK, but I'm ok with sticking with "goto" if there are no objection from anyone else. (In reply to
comment #3
)
> (From update of
attachment 208347
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=208347&action=review
> > > Source/JavaScriptCore/interpreter/Interpreter.h:229 > > + static EncodedJSValue JSC_HOST_CALL constructWithErrorConstructor(ExecState*); > > + static EncodedJSValue JSC_HOST_CALL callErrorConstructor(ExecState*); > > + static EncodedJSValue JSC_HOST_CALL constructWithNativeErrorConstructor(ExecState*); > > + static EncodedJSValue JSC_HOST_CALL callNativeErrorConstructor(ExecState*); > > + > > I like this, but it seems like this can be a separate patch, r+ for anding it separately
Will land this separately.
Oliver Hunt
Comment 5
2013-08-08 09:24:03 PDT
(In reply to
comment #4
)
> (In reply to
comment #2
) > > (From update of
attachment 208347
[details]
[details]) > > View in context:
https://bugs.webkit.org/attachment.cgi?id=208347&action=review
> > > > > Source/JavaScriptCore/ChangeLog:19 > > > + (JSC::StackIterator::goToNextFrame): Renamed from gotoNextFrame. > > > > goto is a single word so the old capitalization was correct > > Hmmm ... not in English AFAIK, but I'm ok with sticking with "goto" if there are no objection from anyone else.
goto doesn't get flagged as wrong by spellcheck or autocorrect :D
> > (In reply to
comment #3
) > > (From update of
attachment 208347
[details]
[details]) > > View in context:
https://bugs.webkit.org/attachment.cgi?id=208347&action=review
> > > > > Source/JavaScriptCore/interpreter/Interpreter.h:229 > > > + static EncodedJSValue JSC_HOST_CALL constructWithErrorConstructor(ExecState*); > > > + static EncodedJSValue JSC_HOST_CALL callErrorConstructor(ExecState*); > > > + static EncodedJSValue JSC_HOST_CALL constructWithNativeErrorConstructor(ExecState*); > > > + static EncodedJSValue JSC_HOST_CALL callNativeErrorConstructor(ExecState*); > > > + > > > > I like this, but it seems like this can be a separate patch, r+ for anding it separately > > Will land this separately.
Mark Lam
Comment 6
2013-08-08 09:52:32 PDT
Created
attachment 208353
[details]
updated patch: use "goto", and ErrorConstructor changes landed separately.
Mark Lam
Comment 7
2013-08-08 09:57:15 PDT
Thanks for the review. Revised patch landed in
r153825
: <
http://trac.webkit.org/r153825
>.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug