Bug 21180 - console.trace should show file and line number for each function in the stack
: console.trace should show file and line number for each function in the stack
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Yury Semikhatsky
Depends on: 22758
Blocks: 22556
  Show dependency treegraph
Reported: 2008-09-27 14:58 PDT by Timothy Hatcher
Modified: 2010-08-27 06:36 PDT (History)
13 users (show)

See Also:

Patch (29.89 KB, patch)
2010-07-16 03:59 PDT, Yury Semikhatsky
no flags Details | Formatted Diff | Diff
Patch (28.67 KB, patch)
2010-07-16 06:37 PDT, Yury Semikhatsky
pfeldman: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timothy Hatcher 2008-09-27 14:58:08 PDT
The file and line number should be shown next to each function name. Right now anonymous functions are impossible to distinguish.

The line number should be the point where the execution called the next function in the stack, not the line where the function was declared.
Comment 1 Yury Semikhatsky 2010-07-16 03:59:36 PDT
Created attachment 61791 [details]
Comment 2 Pavel Feldman 2010-07-16 04:23:26 PDT
Comment on attachment 61791 [details]

Formally correct, but I'd like you to use TreeOutline instead of a custom list.

 +      if (m_stackTrace.get().IsEmpty())
Do we need a handle to stack trace here?

 +      OwnPtr<ScriptCallFrame> m_topFrame;
Can we make it value type?

 +      v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(20000);

 +              frames.set(i, m_frames.at(i).createFrontendObject(frontend));

 +                  var urlElement = this._createSourceUrlLink(this.url, this.line);

 +                  var ol = this._createStackTraceElement();
You should use TreeOutline and TreeItem here.
Comment 3 Yury Semikhatsky 2010-07-16 06:37:22 PDT
Created attachment 61800 [details]
Comment 4 Yury Semikhatsky 2010-07-16 06:42:08 PDT
Addressed all the comments.
Comment 5 Pavel Feldman 2010-07-16 06:47:01 PDT
Comment on attachment 61800 [details]

Please test well prior to landing.
Comment 6 Yury Semikhatsky 2010-07-16 08:31:02 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/http/tests/inspector/console-tests.js
	A	LayoutTests/inspector/console-trace-expected.txt
	A	LayoutTests/inspector/console-trace.html
	M	WebCore/ChangeLog
	M	WebCore/bindings/js/ScriptCallStack.cpp
	M	WebCore/bindings/v8/ScriptCallFrame.cpp
	M	WebCore/bindings/v8/ScriptCallFrame.h
	M	WebCore/bindings/v8/ScriptCallStack.cpp
	M	WebCore/bindings/v8/ScriptCallStack.h
	M	WebCore/bindings/v8/custom/V8ConsoleCustom.cpp
	M	WebCore/inspector/ConsoleMessage.cpp
	M	WebCore/inspector/ConsoleMessage.h
	M	WebCore/inspector/front-end/ConsoleView.js
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/page/Console.idl
Committed r63548
Comment 7 WebKit Review Bot 2010-07-16 08:45:06 PDT
http://trac.webkit.org/changeset/63548 might have broken Chromium Linux Release
Comment 8 Simon Hausmann 2010-08-25 01:17:00 PDT
Revision r63548 cherry-picked into qtwebkit-2.1 with commit ca8729b0b71bda22694cb564737ef520765c6ae5
Comment 9 Ademar Reis 2010-08-26 11:04:28 PDT
This bug is marked as RESOLVED/FIXED but it depends on an open-bug: #13646

Should the dependency be removed? Maybe bug #13646 should block the qtwebkit-2.1 release bug instead? (bug #39121)