Refining the StackIterator callback interface.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Sep 2013 22:33:57 +0000 (22:33 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 4 Sep 2013 22:33:57 +0000 (22:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=120695.

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Introduce CallFrame::iterate() which instantiates a StackIterator and
invoke its iterate() method with the passed in functor. The only place
where the client code gets access to the StackIterator now is as an
argument to the client's functor.

* API/JSContextRef.cpp:
(JSContextCreateBacktrace):
* interpreter/CallFrame.cpp:
* interpreter/CallFrame.h:
(JSC::ExecState::iterate):
* interpreter/Interpreter.cpp:
(JSC::Interpreter::dumpRegisters):
(JSC::Interpreter::getStackTrace):
(JSC::Interpreter::unwind):
* interpreter/StackIterator.cpp:
(JSC::StackIterator::StackIterator):
(DebugPrintFrameFunctor::DebugPrintFrameFunctor):
(DebugPrintFrameFunctor::operator()):
(debugPrintCallFrame):
(debugPrintStack):
* interpreter/StackIterator.h:
(JSC::StackIterator::iterate):
* jsc.cpp:
(functionJSCStack):
* profiler/ProfileGenerator.cpp:
(JSC::ProfileGenerator::addParentForConsoleStart):
* runtime/JSFunction.cpp:
(JSC::retrieveArguments):
(JSC::RetrieveCallerFunctionFunctor::operator()):
(JSC::retrieveCallerFunction):
* runtime/JSGlobalObjectFunctions.cpp:
(JSC::globalFuncProtoGetter):
(JSC::globalFuncProtoSetter):
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorGetPrototypeOf):

Source/WebCore:

No new tests.

* bindings/js/JSXMLHttpRequestCustom.cpp:
(WebCore::SendFunctor::SendFunctor):
(WebCore::SendFunctor::line):
(WebCore::SendFunctor::url):
(WebCore::SendFunctor::operator()):
(WebCore::JSXMLHttpRequest::send):
* bindings/js/ScriptCallStackFactory.cpp:
(WebCore::createScriptCallStack):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@155075 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
Source/JavaScriptCore/API/JSContextRef.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/interpreter/CallFrame.cpp
Source/JavaScriptCore/interpreter/CallFrame.h
Source/JavaScriptCore/interpreter/Interpreter.cpp
Source/JavaScriptCore/interpreter/StackIterator.cpp
Source/JavaScriptCore/interpreter/StackIterator.h
Source/JavaScriptCore/jsc.cpp
Source/JavaScriptCore/profiler/ProfileGenerator.cpp
Source/JavaScriptCore/runtime/JSFunction.cpp
Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp
Source/JavaScriptCore/runtime/ObjectConstructor.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSXMLHttpRequestCustom.cpp
Source/WebCore/bindings/js/ScriptCallStackFactory.cpp

index 1e8f8df..fb806a5 100644 (file)
@@ -274,8 +274,7 @@ JSStringRef JSContextCreateBacktrace(JSContextRef ctx, unsigned maxStackSize)
 
     ASSERT(maxStackSize);
     BacktraceFunctor functor(builder, maxStackSize);
-    StackIterator iter = frame->begin();
-    iter.iterate(functor);
+    frame->iterate(functor);
 
     return OpaqueJSString::create(builder.toString()).leakRef();
 }
index 91b1ea5..66a180a 100644 (file)
@@ -1,3 +1,46 @@
+2013-09-04  Mark Lam  <mark.lam@apple.com>
+
+        Refining the StackIterator callback interface.
+        https://bugs.webkit.org/show_bug.cgi?id=120695.
+
+        Reviewed by Geoffrey Garen.
+
+        Introduce CallFrame::iterate() which instantiates a StackIterator and
+        invoke its iterate() method with the passed in functor. The only place
+        where the client code gets access to the StackIterator now is as an
+        argument to the client's functor.
+
+        * API/JSContextRef.cpp:
+        (JSContextCreateBacktrace):
+        * interpreter/CallFrame.cpp:
+        * interpreter/CallFrame.h:
+        (JSC::ExecState::iterate):
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::dumpRegisters):
+        (JSC::Interpreter::getStackTrace):
+        (JSC::Interpreter::unwind):
+        * interpreter/StackIterator.cpp:
+        (JSC::StackIterator::StackIterator):
+        (DebugPrintFrameFunctor::DebugPrintFrameFunctor):
+        (DebugPrintFrameFunctor::operator()):
+        (debugPrintCallFrame):
+        (debugPrintStack):
+        * interpreter/StackIterator.h:
+        (JSC::StackIterator::iterate):
+        * jsc.cpp:
+        (functionJSCStack):
+        * profiler/ProfileGenerator.cpp:
+        (JSC::ProfileGenerator::addParentForConsoleStart):
+        * runtime/JSFunction.cpp:
+        (JSC::retrieveArguments):
+        (JSC::RetrieveCallerFunctionFunctor::operator()):
+        (JSC::retrieveCallerFunction):
+        * runtime/JSGlobalObjectFunctions.cpp:
+        (JSC::globalFuncProtoGetter):
+        (JSC::globalFuncProtoSetter):
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorGetPrototypeOf):
+
 2013-09-04  Benjamin Poulain  <benjamin@webkit.org>
 
         JSGenericTypedArrayViewConstructor.h is referenced twice in the XCode project build section, causing warnings
index 44c8598..89f6541 100644 (file)
@@ -97,10 +97,4 @@ Register* CallFrame::frameExtentInternal()
     return registers() + codeBlock->m_numCalleeRegisters;
 }
 
-StackIterator CallFrame::begin(StackIterator::FrameFilter filter)
-{
-    ASSERT(this);
-    return StackIterator(this, filter);
-}
-
 } // namespace JSC
index 698d940..cf82894 100644 (file)
@@ -283,7 +283,14 @@ namespace JSC  {
 #endif
         CallFrame* callerFrameNoFlags() { return callerFrame()->removeHostCallFrameFlag(); }
 
-        JS_EXPORT_PRIVATE StackIterator begin(StackIterator::FrameFilter = 0);
+        // CallFrame::iterate() expects a Functor that implements the following method:
+        //     StackIterator::Status operator()(StackIterator&);
+
+        template <typename Functor> void iterate(Functor& functor)
+        {
+            StackIterator iter(this);
+            iter.iterate<Functor>(functor);
+        }
 
     private:
         static const intptr_t HostCallFrameFlag = 1;
index 1630f77..b00a2a0 100644 (file)
@@ -339,8 +339,7 @@ void Interpreter::dumpRegisters(CallFrame* callFrame)
 #endif
 
     DumpRegisterFunctor functor(it);
-    StackIterator iter = callFrame->begin();
-    iter.iterate(functor);
+    callFrame->iterate(functor);
 
     dataLogF("[CodeBlock]                | %10p | %p \n", it, callFrame->codeBlock());
     ++it;
@@ -552,8 +551,7 @@ void Interpreter::getStackTrace(Vector<StackFrame>& results, size_t maxStackSize
         return;
 
     GetStackTraceFunctor functor(vm, results, maxStackSize);
-    StackIterator iter = callFrame->begin();
-    iter.iterate(functor);
+    callFrame->iterate(functor);
 }
 
 JSString* Interpreter::stackTraceAsString(ExecState* exec, Vector<StackFrame> stackTrace)
@@ -640,8 +638,7 @@ NEVER_INLINE HandlerInfo* Interpreter::unwind(CallFrame*& callFrame, JSValue& ex
     VM& vm = callFrame->vm();
     ASSERT(callFrame == vm.topCallFrame);
     UnwindFunctor functor(callFrame, exceptionValue, isTermination, codeBlock, handler);
-    StackIterator iter = callFrame->begin();
-    iter.iterate(functor);
+    callFrame->iterate(functor);
     if (!handler)
         return 0;
 
index d846a18..6025687 100644 (file)
 
 namespace JSC {
 
-StackIterator::StackIterator(CallFrame* startFrame, StackIterator::FrameFilter filter)
+StackIterator::StackIterator(CallFrame* startFrame)
     : m_startFrame(startFrame)
-    , m_filter(filter)
 {
     resetIterator();
 }
 
-size_t StackIterator::numberOfFrames()
-{
-    int savedFrameIndex = m_frame.index();
-    resetIterator();
-    while (m_frame.callFrame())
-        gotoNextFrameWithFilter();
-    size_t numberOfFrames = m_frame.index();
-
-    resetIterator();
-    gotoFrameAtIndex(savedFrameIndex);
-
-    return numberOfFrames;
-}
-
-void StackIterator::gotoFrameAtIndex(size_t index)
-{
-    while (m_frame.callFrame() && (m_frame.index() != index))
-        gotoNextFrameWithFilter();
-}
-
 void StackIterator::gotoNextFrame()
 {
 #if ENABLE(DFG_JIT)
@@ -75,17 +54,6 @@ void StackIterator::gotoNextFrame()
         readFrame(m_frame.callerFrame());
 }
 
-void StackIterator::gotoNextFrameWithFilter()
-{
-    ASSERT(m_frame.callFrame());
-    while (m_frame.callFrame()) {
-        gotoNextFrame();
-        if (!m_frame.callFrame() || !m_filter || !m_filter(&m_frame))
-            break;
-    }
-    m_frame.m_index++;
-}
-
 void StackIterator::resetIterator()
 {
     m_frame.m_index = 0;
@@ -455,29 +423,42 @@ using JSC::StackIterator;
 void debugPrintCallFrame(JSC::CallFrame*);
 void debugPrintStack(JSC::CallFrame* topCallFrame);
 
-void debugPrintCallFrame(JSC::CallFrame* callFrame)
-{
-    if (!callFrame)
-        return;
-    StackIterator iter = callFrame->begin();
-    iter->print(2);
-}
-
-class DebugPrintStackFunctor {
+class DebugPrintFrameFunctor {
 public:
+    enum Action {
+        PrintOne,
+        PrintAll
+    };
+
+    DebugPrintFrameFunctor(Action action)
+        : m_action(action)
+    {
+    }
+
     StackIterator::Status operator()(StackIterator& iter)
     {
         iter->print(2);
-        return StackIterator::Continue;
+        return m_action == PrintAll ? StackIterator::Continue : StackIterator::Done;
     }
+
+private:
+    Action m_action;
 };
 
+void debugPrintCallFrame(JSC::CallFrame* callFrame)
+{
+    if (!callFrame)
+        return;
+    DebugPrintFrameFunctor functor(DebugPrintFrameFunctor::PrintOne);
+    callFrame->iterate(functor);
+}
+
 void debugPrintStack(JSC::CallFrame* topCallFrame)
 {
     if (!topCallFrame)
         return;
-    DebugPrintStackFunctor functor;
-    StackIterator iter = topCallFrame->begin();
-    iter.iterate(functor);
+    DebugPrintFrameFunctor functor(DebugPrintFrameFunctor::PrintAll);
+    topCallFrame->iterate(functor);
 }
+
 #endif // !NDEBUG
index ac5dd4e..8130910 100644 (file)
@@ -100,14 +100,11 @@ public:
 #if ENABLE(DFG_JIT)
         InlineCallFrame* m_inlineCallFrame;
 #endif
-
         CallFrame* m_callFrame;
 
         friend class StackIterator;
     };
 
-    typedef bool (*FrameFilter)(Frame*);
-
     enum Status {
         Continue = 0,
         Done = 1
@@ -122,21 +119,17 @@ public:
             Status status = functor(*this);
             if (status != Continue)
                 break;
-            gotoNextFrameWithFilter();
+            gotoNextFrame();
         }
     }
 
-    JS_EXPORT_PRIVATE size_t numberOfFrames();
-
     Frame& operator*() { return m_frame; }
     ALWAYS_INLINE Frame* operator->() { return &m_frame; }
 
 private:
-    JS_EXPORT_PRIVATE StackIterator(CallFrame* startFrame, FrameFilter = 0);
+    JS_EXPORT_PRIVATE StackIterator(CallFrame* startFrame);
 
-    void gotoFrameAtIndex(size_t frameIndex);
-    void gotoNextFrame();
-    JS_EXPORT_PRIVATE void gotoNextFrameWithFilter();
+    JS_EXPORT_PRIVATE void gotoNextFrame();
     void resetIterator();
 
     void readFrame(CallFrame*);
@@ -146,7 +139,6 @@ private:
 #endif
 
     CallFrame* m_startFrame;
-    FrameFilter m_filter;
     Frame m_frame;
 
     friend class ExecState;
index 708a2fa..eeeee80 100644 (file)
@@ -349,8 +349,7 @@ EncodedJSValue JSC_HOST_CALL functionJSCStack(ExecState* exec)
     trace.appendLiteral("--> Stack trace:\n");
 
     FunctionJSCStackFunctor functor(trace);
-    StackIterator iter = exec->begin();
-    iter.iterate(functor);
+    exec->iterate(functor);
     fprintf(stderr, "%s", trace.toString().utf8().data());
     return JSValue::encode(jsUndefined());
 }
index 6a970f4..679be30 100644 (file)
@@ -98,8 +98,7 @@ private:
 void ProfileGenerator::addParentForConsoleStart(ExecState* exec)
 {
     AddParentForConsoleStartFunctor functor(exec, m_head, m_currentNode);
-    StackIterator iter = exec->begin();
-    iter.iterate(functor);
+    exec->iterate(functor);
 
     if (!functor.foundParent()) {
         m_currentNode = ProfileNode::create(exec, LegacyProfiler::createCallIdentifier(exec, JSValue(), String(), 0), m_head.get(), m_head.get());
index 898e71f..66ce9d2 100644 (file)
@@ -210,8 +210,7 @@ private:
 static JSValue retrieveArguments(ExecState* exec, JSFunction* functionObj)
 {
     RetrieveArgumentsFunctor functor(functionObj);
-    StackIterator iter = exec->begin();
-    iter.iterate(functor);
+    exec->iterate(functor);
     return functor.result();
 }
 
@@ -223,13 +222,6 @@ JSValue JSFunction::argumentsGetter(ExecState* exec, JSValue slotBase, PropertyN
     return retrieveArguments(exec, thisObj);
 }
 
-static bool skipOverBoundFunctions(StackIterator::Frame* frame)
-{
-    JSObject* callee = frame->callee();
-    bool shouldSkip = callee ? callee->inherits(JSBoundFunction::info()) : false;
-    return shouldSkip;
-}
-
 class RetrieveCallerFunctionFunctor {
 public:
     RetrieveCallerFunctionFunctor(JSFunction* functionObj)
@@ -245,6 +237,10 @@ public:
     StackIterator::Status operator()(StackIterator& iter)
     {
         JSObject* callee = iter->callee();
+
+        if (callee && callee->inherits(JSBoundFunction::info()))
+            return StackIterator::Continue;
+
         if (!m_hasFoundFrame && (callee != m_targetCallee))
             return StackIterator::Continue;
 
@@ -269,8 +265,7 @@ private:
 static JSValue retrieveCallerFunction(ExecState* exec, JSFunction* functionObj)
 {
     RetrieveCallerFunctionFunctor functor(functionObj);
-    StackIterator iter = exec->begin(skipOverBoundFunctions);
-    iter.iterate(functor);
+    exec->iterate(functor);
     return functor.result();
 }
 
index 3b569ee..1d0d50a 100644 (file)
@@ -744,8 +744,7 @@ EncodedJSValue JSC_HOST_CALL globalFuncProtoGetter(ExecState* exec)
         return JSValue::encode(exec->thisValue().synthesizePrototype(exec));
 
     GlobalFuncProtoGetterFunctor functor(thisObject);
-    StackIterator iter = exec->begin();
-    iter.iterate(functor);
+    exec->iterate(functor);
     return functor.result();
 }
 
@@ -788,8 +787,7 @@ EncodedJSValue JSC_HOST_CALL globalFuncProtoSetter(ExecState* exec)
         return JSValue::encode(jsUndefined());
 
     GlobalFuncProtoSetterFunctor functor(thisObject);
-    StackIterator iter = exec->begin();
-    iter.iterate(functor);
+    exec->iterate(functor);
     if (!functor.allowsAccess())
         return JSValue::encode(jsUndefined());
 
index bf1e266..c6b5ce5 100644 (file)
@@ -166,8 +166,7 @@ EncodedJSValue JSC_HOST_CALL objectConstructorGetPrototypeOf(ExecState* exec)
         return throwVMError(exec, createTypeError(exec, ASCIILiteral("Requested prototype of a value that is not an object.")));
     JSObject* object = asObject(exec->argument(0));
     ObjectConstructorGetPrototypeOfFunctor functor(object);
-    StackIterator iter = exec->begin();
-    iter.iterate(functor);
+    exec->iterate(functor);
     return functor.result();
 }
 
index c06154c..66aa694 100644 (file)
@@ -1,3 +1,21 @@
+2013-09-04  Mark Lam  <mark.lam@apple.com>
+
+        Refining the StackIterator callback interface.
+        https://bugs.webkit.org/show_bug.cgi?id=120695.
+
+        Reviewed by Geoffrey Garen.
+
+        No new tests.
+
+        * bindings/js/JSXMLHttpRequestCustom.cpp:
+        (WebCore::SendFunctor::SendFunctor):
+        (WebCore::SendFunctor::line):
+        (WebCore::SendFunctor::url):
+        (WebCore::SendFunctor::operator()):
+        (WebCore::JSXMLHttpRequest::send):
+        * bindings/js/ScriptCallStackFactory.cpp:
+        (WebCore::createScriptCallStack):
+
 2013-09-04  Andreas Kling  <akling@apple.com>
 
         Remove unnecessary RenderView.h inclusion from headers.
index acaf1d2..061790c 100644 (file)
@@ -116,26 +116,32 @@ class SendFunctor {
 public:
     SendFunctor()
         : m_hasSkippedFirstFrame(false)
-        , m_hasViableFrame(false)
+        , m_line(0)
     {
     }
 
-    bool hasViableFrame() const { return m_hasViableFrame; }
+    unsigned line() const { return m_line; }
+    String url() const { return m_url; }
 
-    StackIterator::Status operator()(StackIterator&)
+    StackIterator::Status operator()(StackIterator& iter)
     {
         if (!m_hasSkippedFirstFrame) {
             m_hasSkippedFirstFrame = true;
             return StackIterator::Continue;
         }
 
-        m_hasViableFrame = true;
+        unsigned line = 0;
+        unsigned unusedColumn = 0;
+        iter->computeLineAndColumn(line, unusedColumn);
+        m_line = line;
+        m_url = iter->sourceURL();
         return StackIterator::Done;
     }
 
 private:
     bool m_hasSkippedFirstFrame;
-    bool m_hasViableFrame;
+    unsigned m_line;
+    String m_url;
 };
 
 JSValue JSXMLHttpRequest::send(ExecState* exec)
@@ -165,18 +171,9 @@ JSValue JSXMLHttpRequest::send(ExecState* exec)
     }
 
     SendFunctor functor;
-    StackIterator iter = exec->begin();
-    iter.iterate(functor);
-    if (functor.hasViableFrame()) {
-        unsigned line = 0;
-        unsigned unusuedColumn = 0;
-        iter->computeLineAndColumn(line, unusuedColumn);
-        impl()->setLastSendLineNumber(line);
-        impl()->setLastSendURL(iter->sourceURL());
-    } else {
-        impl()->setLastSendLineNumber(0);
-        impl()->setLastSendURL(String());
-    }
+    exec->iterate(functor);
+    impl()->setLastSendLineNumber(functor.line());
+    impl()->setLastSendURL(functor.url());
     setDOMException(exec, ec);
     return jsUndefined();
 }
index d1dc756..e94e1e7 100644 (file)
@@ -87,8 +87,7 @@ PassRefPtr<ScriptCallStack> createScriptCallStack(size_t maxStackSize, bool empt
     if (JSC::ExecState* exec = JSMainThreadExecState::currentState()) {
         CallFrame* frame = exec->vm().topCallFrame;
         CreateScriptCallStackFunctor functor(frames, maxStackSize);
-        StackIterator iter = frame->begin();
-        iter.iterate(functor);
+        frame->iterate(functor);
     }
     if (frames.isEmpty() && !emptyIsAllowed) {
         // No frames found. It may happen in the case where
@@ -144,10 +143,12 @@ PassRefPtr<ScriptCallStack> createScriptCallStack(JSC::ExecState* exec, size_t m
     Vector<ScriptCallFrame> frames;
     ASSERT(exec);
     CallFrame* frame = exec->vm().topCallFrame;
-    StackIterator iter = frame->begin();
-    size_t numberOfFrames = iter.numberOfFrames();
-    CreateScriptCallStackForConsoleFunctor functor(numberOfFrames > 1, maxStackSize, frames);
-    iter.iterate(functor);
+    CreateScriptCallStackForConsoleFunctor functor(true, maxStackSize, frames);
+    frame->iterate(functor);
+    if (frames.isEmpty()) {
+        CreateScriptCallStackForConsoleFunctor functor(false, maxStackSize, frames);
+        frame->iterate(functor);
+    }
     return ScriptCallStack::create(frames);
 }