Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(492)

Issue 2515133003: [debug-wrapper] Migrate wasm/frame-inspection test (Closed)

Created:
4 years, 1 month ago by jgruber
Modified:
4 years, 1 month ago
Reviewers:
Yang
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[debug-wrapper] Migrate wasm/frame-inspection test Wasm frames are special in that they have a non-integer script id in inspector. The way we treat script ids currently is a bit of a mess - our runtime functions expected integer IDs while inspector has string IDs (which contain integers, except for Wasm frames). This will need to be cleaned up once more Wasm tests are added. The meaning of line/column numbers has also changed; the old JS debug API encoded the function index and byte offset into line/column numbers, while inspector-based API actually translates into lines/columns in the disassembly. BUG=v8:5530 Committed: https://crrev.com/9eec1c86179f82d776cc91f7f320943fa4c341a4 Cr-Commit-Position: refs/heads/master@{#41182}

Patch Set 1 #

Patch Set 2 : Adjust expectations #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -99 lines) Patch
A + test/debugger/debug/wasm/frame-inspection.js View 1 3 chunks +6 lines, -15 lines 0 comments Download
M test/debugger/test-api.js View 1 2 chunks +3 lines, -3 lines 0 comments Download
D test/mjsunit/wasm/frame-inspection.js View 1 chunk +0 lines, -81 lines 0 comments Download

Messages

Total messages: 18 (11 generated)
jgruber
4 years, 1 month ago (2016-11-21 10:43:22 UTC) #6
Yang
On 2016/11/21 10:43:22, jgruber wrote: lgtm.
4 years, 1 month ago (2016-11-22 08:10:34 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2515133003/1
4 years, 1 month ago (2016-11-22 12:45:17 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/builds/12843) v8_linux64_asan_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, ...
4 years, 1 month ago (2016-11-22 12:59:26 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2515133003/20001
4 years, 1 month ago (2016-11-22 13:27:36 UTC) #14
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 1 month ago (2016-11-22 13:58:54 UTC) #16
commit-bot: I haz the power
4 years, 1 month ago (2016-11-22 13:59:09 UTC) #18
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/9eec1c86179f82d776cc91f7f320943fa4c341a4
Cr-Commit-Position: refs/heads/master@{#41182}

Powered by Google App Engine
This is Rietveld 408576698