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.
Created attachment 208347 [details] the patch
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
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
(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.
(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.
Created attachment 208353 [details] updated patch: use "goto", and ErrorConstructor changes landed separately.
Thanks for the review. Revised patch landed in r153825: <http://trac.webkit.org/r153825>.