[Webkit-unassigned] [Bug 37502] Web Inspector: Removes public callLocation API from ScriptCallStack and replaces with ability to get top 10 stack frames
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon May 17 11:10:08 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=37502
jaimeyap at google.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #55848|review+ |review-
Flag| |
--- Comment #15 from jaimeyap at google.com 2010-05-17 11:10:06 PST ---
(From update of attachment 55848)
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 59177)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,28 @@
> +2010-05-11 Jaime Yap <jaimeyap at google.com>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + Removes public callLocation API and utility context from ScriptCallStack.
> + Adds ability to grab the top 5 JavaScript stack frames for each timeline
> + record using new fast V8 stack trace API.
> + https://bugs.webkit.org/show_bug.cgi?id=37502
> +
> + * bindings/js/ScriptCallStack.cpp:
> + (WebCore::ScriptCallStack::stackTrace):
> + * bindings/js/ScriptCallStack.h:
> + * bindings/v8/ScriptArray.h:
> + (WebCore::ScriptArray::ScriptArray):
> + * bindings/v8/ScriptCallStack.cpp:
> + (WebCore::ScriptCallStack::callLocation):
> + (WebCore::ScriptCallStack::stackTrace):
> + * bindings/v8/ScriptCallStack.h:
> + * inspector/TimelineRecordFactory.cpp:
> + (WebCore::TimelineRecordFactory::createGenericRecord):
> + * inspector/front-end/TimelinePanel.js:
> + (WebInspector.TimelinePanel.FormattedRecord):
> + (WebInspector.TimelinePanel.FormattedRecord.prototype._generatePopupContent):
> + (WebInspector.TimelinePanel.FormattedRecord.prototype._getRecordDetails):
> +
> 2010-05-11 Mark Rowe <mrowe at apple.com>
>
> Fix the world.
> Index: WebCore/bindings/js/ScriptCallStack.cpp
> ===================================================================
> --- WebCore/bindings/js/ScriptCallStack.cpp (revision 59174)
> +++ WebCore/bindings/js/ScriptCallStack.cpp (working copy)
> @@ -101,7 +101,7 @@ void ScriptCallStack::initialize()
> m_initialized = true;
> }
>
> -bool ScriptCallStack::callLocation(String*, int*, String*)
> +bool ScriptCallStack::stackTrace(int, ScriptState*, ScriptArray&)
> {
> return false;
> }
> Index: WebCore/bindings/js/ScriptCallStack.h
> ===================================================================
> --- WebCore/bindings/js/ScriptCallStack.h (revision 59174)
> +++ WebCore/bindings/js/ScriptCallStack.h (working copy)
> @@ -31,6 +31,7 @@
> #ifndef ScriptCallStack_h
> #define ScriptCallStack_h
>
> +#include "ScriptArray.h"
> #include "ScriptCallFrame.h"
> #include "ScriptState.h"
> #include "ScriptString.h"
> @@ -53,7 +54,7 @@ namespace WebCore {
> // frame retrieval methods
> const ScriptCallFrame &at(unsigned);
> unsigned size();
> - static bool callLocation(String*, int*, String*);
> + static bool stackTrace(int, ScriptState*, ScriptArray&);
>
> private:
> void initialize();
> Index: WebCore/bindings/v8/ScriptArray.h
> ===================================================================
> --- WebCore/bindings/v8/ScriptArray.h (revision 59174)
> +++ WebCore/bindings/v8/ScriptArray.h (working copy)
> @@ -41,6 +41,7 @@ namespace WebCore {
> class ScriptArray : public ScriptObject {
> public:
> ScriptArray(ScriptState* scriptState, v8::Handle<v8::Array>);
> + ScriptArray() {};
> virtual ~ScriptArray() {}
>
> bool set(unsigned index, const ScriptObject&);
> Index: WebCore/bindings/v8/ScriptCallStack.cpp
> ===================================================================
> --- WebCore/bindings/v8/ScriptCallStack.cpp (revision 59174)
> +++ WebCore/bindings/v8/ScriptCallStack.cpp (working copy)
> @@ -31,6 +31,7 @@
> #include "config.h"
> #include "ScriptCallStack.h"
>
> +#include "ScriptScope.h"
> #include "ScriptController.h"
> #include "ScriptDebugServer.h"
> #include "V8Binding.h"
> @@ -41,8 +42,6 @@
>
> namespace WebCore {
>
> -v8::Persistent<v8::Context> ScriptCallStack::s_utilityContext;
> -
> ScriptCallStack* ScriptCallStack::create(const v8::Arguments& arguments, unsigned skipArgumentCount) {
> String sourceName;
> int sourceLineNumber;
> @@ -54,9 +53,22 @@ ScriptCallStack* ScriptCallStack::create
>
> bool ScriptCallStack::callLocation(String* sourceName, int* sourceLineNumber, String* functionName)
> {
> - if (!topStackFrame(*sourceName, *sourceLineNumber, *functionName))
> - return false;
> - *sourceLineNumber += 1;
> + v8::HandleScope scope;
> + v8::Context::Scope contextScope(v8::Context::GetCurrent());
> + v8::Handle<v8::StackTrace> stackTrace(v8::StackTrace::CurrentStackTrace(1));
> + if (stackTrace.IsEmpty())
> + return false;
> + if (stackTrace->GetFrameCount() <= 0) {
> + // Fallback to setting lineNumber to 0, and source and function name to "undefined".
> + *sourceName = toWebCoreString(v8::Undefined());
> + *sourceLineNumber = 0;
> + *functionName = toWebCoreString(v8::Undefined());
> + return true;
> + }
> + v8::Handle<v8::StackFrame> frame = stackTrace->GetFrame(0);
> + *sourceName = toWebCoreString(frame->GetScriptName());
> + *sourceLineNumber = frame->GetLineNumber();
> + *functionName = toWebCoreString(frame->GetFunctionName());
> return true;
> }
>
> @@ -78,66 +90,13 @@ const ScriptCallFrame& ScriptCallStack::
> return m_lastCaller;
> }
>
> -// Create the utility context for holding JavaScript functions used internally
> -// which are not visible to JavaScript executing on the page.
> -void ScriptCallStack::createUtilityContext()
> +bool ScriptCallStack::stackTrace(int frameLimit, ScriptState* state, ScriptArray& stackTrace)
> {
> - ASSERT(s_utilityContext.IsEmpty());
> -
> - v8::HandleScope scope;
> - v8::Handle<v8::ObjectTemplate> globalTemplate = v8::ObjectTemplate::New();
> - s_utilityContext = v8::Context::New(0, globalTemplate);
> - v8::Context::Scope contextScope(s_utilityContext);
> -
> - // Compile JavaScript function for retrieving the source line, the source
> - // name and the symbol name for the top JavaScript stack frame.
> - DEFINE_STATIC_LOCAL(const char*, topStackFrame,
> - ("function topStackFrame(exec_state) {"
> - " if (!exec_state.frameCount())"
> - " return undefined;"
> - " var frame = exec_state.frame(0);"
> - " var func = frame.func();"
> - " var scriptName;"
> - " if (func.resolved() && func.script())"
> - " scriptName = func.script().name();"
> - " return [scriptName, frame.sourceLine(), (func.name() || func.inferredName())];"
> - "}"));
> - v8::Script::Compile(v8::String::New(topStackFrame))->Run();
> -}
> -
> -bool ScriptCallStack::topStackFrame(String& sourceName, int& lineNumber, String& functionName)
> -{
> - v8::HandleScope scope;
> - v8::Handle<v8::Context> v8UtilityContext = utilityContext();
> - if (v8UtilityContext.IsEmpty())
> - return false;
> - v8::Context::Scope contextScope(v8UtilityContext);
> - v8::Handle<v8::Function> topStackFrame;
> - topStackFrame = v8::Local<v8::Function>::Cast(v8UtilityContext->Global()->Get(v8::String::New("topStackFrame")));
> - if (topStackFrame.IsEmpty())
> - return false;
> - v8::Handle<v8::Value> value = v8::Debug::Call(topStackFrame);
> - if (value.IsEmpty())
> - return false;
> - // If there is no top stack frame, we still return success, but fill the input params with defaults.
> - if (value->IsUndefined()) {
> - // Fallback to setting lineNumber to 0, and source and function name to "undefined".
> - sourceName = toWebCoreString(value);
> - lineNumber = 0;
> - functionName = toWebCoreString(value);
> - return true;
> - }
> - if (!value->IsArray())
> - return false;
> - v8::Local<v8::Object> jsArray = value->ToObject();
> - v8::Local<v8::Value> sourceNameValue = jsArray->Get(0);
> - v8::Local<v8::Value> lineNumberValue = jsArray->Get(1);
> - v8::Local<v8::Value> functionNameValue = jsArray->Get(2);
> - if (sourceNameValue.IsEmpty() || lineNumberValue.IsEmpty() || functionNameValue.IsEmpty())
> + ScriptScope scope(state);
> + v8::Handle<v8::StackTrace> trace(v8::StackTrace::CurrentStackTrace(frameLimit));
> + if (trace.IsEmpty() || !trace->GetFrameCount())
> return false;
> - sourceName = toWebCoreString(sourceNameValue);
> - lineNumber = lineNumberValue->Int32Value();
> - functionName = toWebCoreString(functionNameValue);
> + stackTrace = ScriptArray(state, trace->AsArray());
> return true;
> }
>
> Index: WebCore/bindings/v8/ScriptCallStack.h
> ===================================================================
> --- WebCore/bindings/v8/ScriptCallStack.h (revision 59174)
> +++ WebCore/bindings/v8/ScriptCallStack.h (working copy)
> @@ -31,6 +31,7 @@
> #ifndef ScriptCallStack_h
> #define ScriptCallStack_h
>
> +#include "ScriptArray.h"
> #include "ScriptCallFrame.h"
> #include "ScriptState.h"
> #include "ScriptValue.h"
> @@ -47,7 +48,16 @@ public:
> static ScriptCallStack* create(const v8::Arguments&, unsigned skipArgumentCount = 0);
> ~ScriptCallStack();
>
> - static bool callLocation(String* sourceName, int* sourceLineNumber, String* functionName);
> + // Returns false if there is no running JavaScript or if fetching the stack failed.
> + // Sets stackTrace to be an array of stack frame objects.
> + // A stack frame object looks like:
> + // {
> + // scriptName: <file name for the associated script resource>
> + // functionName: <name of the JavaScript function>
> + // lineNumber: <1 based line number>
> + // column: <1 based column offset on the line>
> + // }
> + static bool stackTrace(int frameLimit, ScriptState* state, ScriptArray& stackTrace);
>
> const ScriptCallFrame& at(unsigned) const;
> // FIXME: implement retrieving and storing call stack trace
> @@ -59,30 +69,10 @@ public:
> private:
> ScriptCallStack(const v8::Arguments& arguments, unsigned skipArgumentCount, String sourceName, int sourceLineNumber, String funcName);
>
> - // Function for retrieving the source name, line number and function name for the top
> - // JavaScript stack frame.
> - //
> - // It will return true if the caller information was successfully retrieved and written
> - // into the function parameters, otherwise the function will return false. It may
> - // fail due to a stack overflow in the underlying JavaScript implementation, handling
> - // of such exception is up to the caller.
> - static bool topStackFrame(String& sourceName, int& lineNumber, String& functionName);
> -
> - static void createUtilityContext();
> -
> - // Returns a local handle of the utility context.
> - static v8::Local<v8::Context> utilityContext()
> - {
> - if (s_utilityContext.IsEmpty())
> - createUtilityContext();
> - return v8::Local<v8::Context>::New(s_utilityContext);
> - }
> + static bool callLocation(String* sourceName, int* sourceLineNumber, String* functionName);
>
> ScriptCallFrame m_lastCaller;
> ScriptState* m_scriptState;
> -
> - // Utility context holding JavaScript functions used internally.
> - static v8::Persistent<v8::Context> s_utilityContext;
> };
>
> } // namespace WebCore
> Index: WebCore/inspector/TimelineRecordFactory.cpp
> ===================================================================
> --- WebCore/inspector/TimelineRecordFactory.cpp (revision 59174)
> +++ WebCore/inspector/TimelineRecordFactory.cpp (working copy)
> @@ -49,14 +49,9 @@ ScriptObject TimelineRecordFactory::crea
> ScriptObject record = frontend->newScriptObject();
> record.set("startTime", startTime);
>
> - String sourceName;
> - int sourceLineNumber;
> - String functionName;
> - if (ScriptCallStack::callLocation(&sourceName, &sourceLineNumber, &functionName) && sourceName != "undefined") {
> - record.set("callerScriptName", sourceName);
> - record.set("callerScriptLine", sourceLineNumber);
> - record.set("callerFunctionName", functionName);
> - }
> + ScriptArray stackTrace;
> + if (ScriptCallStack::stackTrace(5, frontend->scriptState(), stackTrace))
> + record.set("stackTrace", stackTrace);
> return record;
> }
>
> Index: WebCore/inspector/front-end/TimelinePanel.js
> ===================================================================
> --- WebCore/inspector/front-end/TimelinePanel.js (revision 59174)
> +++ WebCore/inspector/front-end/TimelinePanel.js (working copy)
> @@ -800,8 +800,7 @@ WebInspector.TimelinePanel.FormattedReco
> this._selfTime = this.endTime - this.startTime;
> this._lastChildEndTime = this.endTime;
> this.originalRecordForTests = record;
> - this.callerScriptName = record.callerScriptName;
> - this.callerScriptLine = record.callerScriptLine;
> + this.stackTrace = record.stackTrace;
> this.totalHeapSize = record.totalHeapSize;
> this.usedHeapSize = record.usedHeapSize;
>
> @@ -830,8 +829,11 @@ WebInspector.TimelinePanel.FormattedReco
> } else if (record.type === recordTypes.TimerFire) {
> var timerInstalledRecord = timerRecords[record.data.timerId];
> if (timerInstalledRecord) {
> - this.callSiteScriptName = timerInstalledRecord.callerScriptName;
> - this.callSiteScriptLine = timerInstalledRecord.callerScriptLine;
> + if (timerInstalledRecord.stackTrace) {
> + var callSite = timerInstalledRecord.stackTrace[0];
> + this.callSiteScriptName = callSite.scriptName;
> + this.callSiteScriptLine = callSite.lineNumber;
> + }
> this.timeout = timerInstalledRecord.timeout;
> this.singleShot = timerInstalledRecord.singleShot;
> }
> @@ -931,8 +933,10 @@ WebInspector.TimelinePanel.FormattedReco
> if (this.data.scriptName && this.type !== recordTypes.FunctionCall)
> contentHelper._appendLinkRow("Function Call", this.data.scriptName, this.data.scriptLine);
>
> - if (this.callerScriptName && this.type !== recordTypes.GCEvent)
> - contentHelper._appendLinkRow("Caller", this.callerScriptName, this.callerScriptLine);
> + if (this.stackTrace && this.type !== recordTypes.GCEvent) {
> + var callSite = this.stackTrace[0];
> + contentHelper._appendLinkRow("Caller", callSite.scriptName, callSite.lineNumber);
> + }
>
> if (this.usedHeapSize)
> contentHelper._appendTextRow("Used Heap Size", WebInspector.UIString("%s of %s", Number.bytesToString(this.usedHeapSize), Number.bytesToString(this.totalHeapSize)));
> @@ -955,7 +959,8 @@ WebInspector.TimelinePanel.FormattedReco
> return record.data.width + "\u2009\u00d7\u2009" + record.data.height;
> case WebInspector.TimelineAgent.RecordType.TimerInstall:
> case WebInspector.TimelineAgent.RecordType.TimerRemove:
> - return this.callerScriptName ? WebInspector.linkifyResourceAsNode(this.callerScriptName, "scripts", this.callerScriptLine, "", "") : record.data.timerId;
> + var callSite = this.stackTrace;
> + return callSite ? WebInspector.linkifyResourceAsNode(callSite.scriptName, "scripts", callSite.lineNumber, "", "") : record.data.timerId;
> case WebInspector.TimelineAgent.RecordType.ParseHTML:
> case WebInspector.TimelineAgent.RecordType.RecalculateStyles:
> return this.callerScriptName ? WebInspector.linkifyResourceAsNode(this.callerScriptName, "scripts", this.callerScriptLine, "", "") : null;
> Index: WebKit/chromium/DEPS
> ===================================================================
> --- WebKit/chromium/DEPS (revision 59174)
> +++ WebKit/chromium/DEPS (working copy)
> @@ -32,7 +32,7 @@
>
> vars = {
> 'chromium_svn': 'http://src.chromium.org/svn/trunk/src',
> - 'chromium_rev': '46805',
> + 'chromium_rev': '46991',
>
> 'pthreads-win32_rev': '26716',
> }
> Index: LayoutTests/ChangeLog
> ===================================================================
> --- LayoutTests/ChangeLog (revision 59177)
> +++ LayoutTests/ChangeLog (working copy)
> @@ -1,3 +1,17 @@
> +2010-05-11 Jaime Yap <jaimeyap at google.com>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + Test expectations changed due to new stackTrace field on timeline records.
> + https://bugs.webkit.org/show_bug.cgi?id=37502
> +
> + * inspector/timeline-test.js:
> + * platform/chromium-win/inspector/timeline-event-dispatch-expected.txt:
> + * platform/chromium-win/inspector/timeline-mark-timeline-expected.txt:
> + * platform/chromium-win/inspector/timeline-network-resource-expected.txt:
> + * platform/chromium-win/inspector/timeline-paint-expected.txt:
> + * platform/chromium-win/inspector/timeline-parse-html-expected.txt:
> +
> 2010-05-11 Dimitri Glazkov <dglazkov at chromium.org>
>
> Reviewed by Darin Adler.
> Index: LayoutTests/inspector/timeline-test.js
> ===================================================================
> --- LayoutTests/inspector/timeline-test.js (revision 59174)
> +++ LayoutTests/inspector/timeline-test.js (working copy)
> @@ -8,9 +8,7 @@ var timelineNonDeterministicProps = {
> identifier : 1,
> startTime : 1,
> width : 1,
> - callerScriptName: 1,
> - callerScriptLine: 1,
> - callerFunctionName: 1,
> + stackTrace: 1,
> url : 1,
> usedHeapSize: 1,
> totalHeapSize: 1,
> Index: LayoutTests/platform/chromium-win/inspector/timeline-event-dispatch-expected.txt
> ===================================================================
> --- LayoutTests/platform/chromium-win/inspector/timeline-event-dispatch-expected.txt (revision 59174)
> +++ LayoutTests/platform/chromium-win/inspector/timeline-event-dispatch-expected.txt (working copy)
> @@ -3,9 +3,7 @@ Tests the Timeline API instrumentation o
> Test Mouse Target
> EventDispatch Properties:
> + startTime : * DEFINED *
> -+ callerScriptName : * DEFINED *
> -+ callerScriptLine : * DEFINED *
> -+ callerFunctionName : * DEFINED *
> ++ stackTrace : * DEFINED *
> + data : {
> +- type : mousedown
> + }
> Index: LayoutTests/platform/chromium-win/inspector/timeline-mark-timeline-expected.txt
> ===================================================================
> --- LayoutTests/platform/chromium-win/inspector/timeline-mark-timeline-expected.txt (revision 59174)
> +++ LayoutTests/platform/chromium-win/inspector/timeline-mark-timeline-expected.txt (working copy)
> @@ -2,9 +2,7 @@ Tests the Timeline API mark feature
>
> MarkTimeline Properties:
> + startTime : * DEFINED *
> -+ callerScriptName : * DEFINED *
> -+ callerScriptLine : * DEFINED *
> -+ callerFunctionName : * DEFINED *
> ++ stackTrace : * DEFINED *
> + data : {
> +- message : MARK TIMELINE
> + }
> Index: LayoutTests/platform/chromium-win/inspector/timeline-network-resource-expected.txt
> ===================================================================
> --- LayoutTests/platform/chromium-win/inspector/timeline-network-resource-expected.txt (revision 59174)
> +++ LayoutTests/platform/chromium-win/inspector/timeline-network-resource-expected.txt (working copy)
> @@ -3,9 +3,7 @@ Tests the Timeline API instrumentation o
>
> ResourceSendRequest Properties:
> + startTime : * DEFINED *
> -+ callerScriptName : * DEFINED *
> -+ callerScriptLine : * DEFINED *
> -+ callerFunctionName : * DEFINED *
> ++ stackTrace : * DEFINED *
> + data : {
> +- identifier : * DEFINED *
> +- url : * DEFINED *
> Index: LayoutTests/platform/chromium-win/inspector/timeline-paint-expected.txt
> ===================================================================
> --- LayoutTests/platform/chromium-win/inspector/timeline-paint-expected.txt (revision 59174)
> +++ LayoutTests/platform/chromium-win/inspector/timeline-paint-expected.txt (working copy)
> @@ -2,9 +2,7 @@ Tests the Timeline API instrumentation o
>
> Paint Properties:
> + startTime : * DEFINED *
> -+ callerScriptName : * DEFINED *
> -+ callerScriptLine : * DEFINED *
> -+ callerFunctionName : * DEFINED *
> ++ stackTrace : * DEFINED *
> + data : {
> +- x : 0
> +- y : 0
> Index: LayoutTests/platform/chromium-win/inspector/timeline-parse-html-expected.txt
> ===================================================================
> --- LayoutTests/platform/chromium-win/inspector/timeline-parse-html-expected.txt (revision 59174)
> +++ LayoutTests/platform/chromium-win/inspector/timeline-parse-html-expected.txt (working copy)
> @@ -2,9 +2,7 @@ Tests the Timeline API instrumentation o
>
> ParseHTML Properties:
> + startTime : * DEFINED *
> -+ callerScriptName : * DEFINED *
> -+ callerScriptLine : * DEFINED *
> -+ callerFunctionName : * DEFINED *
> ++ stackTrace : * DEFINED *
> + data : {
> +- length : 9
> +- startLine : 0
R-ing this obsolete patch so that it no longer shows up in commit queue.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list