Chromium Code Reviews

Issue 466243002: Support merged Dart-JS callstacks (Closed)

Created:
6 years, 4 months ago by Jacob
Modified:
5 years, 8 months ago
Reviewers:
vsm, rmacnak
CC:
reviews+dom_dartlang.org
Visibility:
Public.

Description

Support async callstacks BUG=

Patch Set 1 #

Total comments: 20

Patch Set 2 : ptal #

Patch Set 3 : #

Patch Set 4 : fix #

Patch Set 5 : #

Total comments: 16

Patch Set 6 : ptal #

Total comments: 2

Patch Set 7 : switched to framePointerHigh and framePointerLow #

Patch Set 8 : #

Unified diffs Side-by-side diffs Stats (+440 lines, -144 lines)
A LayoutTests/dart/inspector/merged-callstack.dart View 1 chunk +27 lines, -0 lines 0 comments
A + LayoutTests/dart/inspector/merged-callstack.html View 3 chunks +4 lines, -29 lines 0 comments
A LayoutTests/dart/inspector/merged-callstack.js View 1 chunk +7 lines, -0 lines 0 comments
A LayoutTests/dart/inspector/merged-callstack-expected.txt View 1 chunk +114 lines, -0 lines 0 comments
M Source/bindings/common/StackTrace.h View 3 chunks +27 lines, -0 lines 0 comments
A + Source/bindings/common/StackTrace.cpp View 1 chunk +16 lines, -17 lines 0 comments
M Source/bindings/common/common.gypi View 1 chunk +1 line, -0 lines 0 comments
M Source/bindings/dart/DartInjectedScript.cpp View 9 chunks +22 lines, -8 lines 0 comments
M Source/bindings/dart/DartScriptDebugServer.h View 3 chunks +7 lines, -6 lines 0 comments
M Source/bindings/dart/DartScriptDebugServer.cpp View 9 chunks +40 lines, -24 lines 0 comments
M Source/bindings/v8/DebuggerScript.js View 3 chunks +12 lines, -0 lines 0 comments
M Source/bindings/v8/ScriptDebugServer.h View 4 chunks +8 lines, -8 lines 0 comments
M Source/bindings/v8/ScriptDebugServer.cpp View 4 chunks +12 lines, -10 lines 0 comments
M Source/core/inspector/AsyncCallStackTracker.h View 3 chunks +10 lines, -10 lines 0 comments
M Source/core/inspector/AsyncCallStackTracker.cpp View 8 chunks +8 lines, -8 lines 0 comments
M Source/core/inspector/InjectedScriptExterns.js View 1 chunk +4 lines, -0 lines 0 comments
M Source/core/inspector/InjectedScriptSource.js View 2 chunks +9 lines, -1 line 0 comments
M Source/core/inspector/InspectorDebuggerAgent.h View 4 chunks +5 lines, -3 lines 0 comments
M Source/core/inspector/InspectorDebuggerAgent.cpp View 13 chunks +76 lines, -18 lines 0 comments
M Source/core/inspector/JavaScriptCallFrame.h View 2 chunks +4 lines, -0 lines 0 comments
M Source/core/inspector/JavaScriptCallFrame.cpp View 2 chunks +21 lines, -0 lines 0 comments
M Source/core/inspector/JavaScriptCallFrame.idl View 1 chunk +2 lines, -0 lines 0 comments
M Source/core/inspector/ScriptDebugListener.h View 1 chunk +1 line, -1 line 0 comments
M Source/devtools/protocol.json View 1 chunk +3 lines, -1 line 0 comments

Messages

Total messages: 9 (0 generated)
Jacob
Sending out for some overnight reviewing. I still owe a layouttest that shows mixed Dart-JS ...
6 years, 4 months ago (2014-08-13 03:54:58 UTC) #1
vsm
https://chromiumcodereview.appspot.com/466243002/diff/1/Source/bindings/common/StackTrace.cpp File Source/bindings/common/StackTrace.cpp (right): https://chromiumcodereview.appspot.com/466243002/diff/1/Source/bindings/common/StackTrace.cpp#newcode94 Source/bindings/common/StackTrace.cpp:94: int64_t StackTraceTimestampTracker::getTimestamp(size_t frame) Can you add a comment here? ...
6 years, 4 months ago (2014-08-13 14:26:46 UTC) #2
Jacob
ptal https://chromiumcodereview.appspot.com/466243002/diff/1/Source/bindings/common/StackTrace.cpp File Source/bindings/common/StackTrace.cpp (right): https://chromiumcodereview.appspot.com/466243002/diff/1/Source/bindings/common/StackTrace.cpp#newcode94 Source/bindings/common/StackTrace.cpp:94: int64_t StackTraceTimestampTracker::getTimestamp(size_t frame) On 2014/08/13 14:26:45, vsm wrote: ...
6 years, 4 months ago (2014-08-13 21:36:16 UTC) #3
rmacnak
https://chromiumcodereview.appspot.com/466243002/diff/1/Source/bindings/dart/DartScriptDebugServer.h File Source/bindings/dart/DartScriptDebugServer.h (right): https://chromiumcodereview.appspot.com/466243002/diff/1/Source/bindings/dart/DartScriptDebugServer.h#newcode197 Source/bindings/dart/DartScriptDebugServer.h:197: virtual StackTrace currentCallFramesForVm(); currentStackWithForeignFrames, currentStackWithoutForeignFrames?
6 years, 4 months ago (2014-08-14 23:21:08 UTC) #4
Jacob
https://chromiumcodereview.appspot.com/466243002/diff/1/Source/bindings/dart/DartScriptDebugServer.h File Source/bindings/dart/DartScriptDebugServer.h (right): https://chromiumcodereview.appspot.com/466243002/diff/1/Source/bindings/dart/DartScriptDebugServer.h#newcode197 Source/bindings/dart/DartScriptDebugServer.h:197: virtual StackTrace currentCallFramesForVm(); On 2014/08/14 23:21:08, rmacnak wrote: > ...
6 years, 4 months ago (2014-08-15 16:23:01 UTC) #5
vsm
https://chromiumcodereview.appspot.com/466243002/diff/80001/Source/bindings/common/StackTrace.cpp File Source/bindings/common/StackTrace.cpp (right): https://chromiumcodereview.appspot.com/466243002/diff/80001/Source/bindings/common/StackTrace.cpp#newcode93 Source/bindings/common/StackTrace.cpp:93: m_v8ScriptState = other.v8ScriptState(); What if this StackTrace already hasJavaScript ...
6 years, 3 months ago (2014-08-29 08:42:08 UTC) #6
Jacob
Refactored this CL significantly to sort based on frame_pointers rather than tracking timestamps. The logic ...
6 years, 3 months ago (2014-09-05 23:35:56 UTC) #7
rmacnak
https://chromiumcodereview.appspot.com/466243002/diff/100001/Source/bindings/dart/DartInjectedScript.cpp File Source/bindings/dart/DartInjectedScript.cpp (right): https://chromiumcodereview.appspot.com/466243002/diff/100001/Source/bindings/dart/DartInjectedScript.cpp#newcode1322 Source/bindings/dart/DartInjectedScript.cpp:1322: result = Dart_ActivationFrameGetFramePointer(frame, &framePointer); Why not represent the frame ...
6 years, 3 months ago (2014-09-08 22:51:44 UTC) #8
Jacob
6 years, 3 months ago (2014-09-10 17:58:56 UTC) #9
https://codereview.chromium.org/466243002/diff/100001/Source/bindings/dart/Da...
File Source/bindings/dart/DartInjectedScript.cpp (right):

https://codereview.chromium.org/466243002/diff/100001/Source/bindings/dart/Da...
Source/bindings/dart/DartInjectedScript.cpp:1322: result =
Dart_ActivationFrameGetFramePointer(frame, &framePointer);
On 2014/09/08 22:51:44, rmacnak wrote:
> Why not represent the frame pointer as a string or as high and low halves in
> JavaScript?

I've switched to high and low halves instead of trying to be clever.

Powered by Google App Engine