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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 654083006: DevTools: NetworkPanel: use optimistic JSON parser for response preview. (Closed)

Created:
6 years, 2 months ago by eustas
Modified:
6 years, 2 months ago
Reviewers:
vsevik
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

DevTools: NetworkPanel: use optimistic JSON parser for response preview. There are some legal tricks used to encode JSON. But JSON.parse rejects to parse it, whereas XHR response is correctly transformed to JSON. BUG=406900 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183821

Patch Set 1 #

Total comments: 6

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+170 lines, -61 lines) Patch
M LayoutTests/http/tests/inspector/network/network-preview-json.html View 1 1 chunk +21 lines, -37 lines 0 comments Download
M LayoutTests/http/tests/inspector/network/network-preview-json-expected.txt View 1 1 chunk +34 lines, -0 lines 0 comments Download
M Source/devtools/front_end/network/RequestJSONView.js View 1 1 chunk +111 lines, -23 lines 0 comments Download
M Source/devtools/front_end/network/RequestPreviewView.js View 1 1 chunk +4 lines, -1 line 0 comments Download

Messages

Total messages: 8 (2 generated)
eustas
6 years, 2 months ago (2014-10-15 13:45:14 UTC) #2
vsevik
lgtm, but let's add at least a few tests for this parser.
6 years, 2 months ago (2014-10-16 06:51:27 UTC) #3
vsevik
https://codereview.chromium.org/654083006/diff/1/Source/devtools/front_end/network/RequestJSONView.js File Source/devtools/front_end/network/RequestJSONView.js (right): https://codereview.chromium.org/654083006/diff/1/Source/devtools/front_end/network/RequestJSONView.js#newcode47 Source/devtools/front_end/network/RequestJSONView.js:47: WebInspector.RequestJSONView.buildJSON = function(text) buildObjectFromJSON https://codereview.chromium.org/654083006/diff/1/Source/devtools/front_end/network/RequestJSONView.js#newcode168 Source/devtools/front_end/network/RequestJSONView.js:168: return new WebInspector.ParsedJSON(WebInspector.RequestJSONView.buildJSON(text), ...
6 years, 2 months ago (2014-10-16 06:51:35 UTC) #4
eustas
Updated test https://codereview.chromium.org/654083006/diff/1/Source/devtools/front_end/network/RequestJSONView.js File Source/devtools/front_end/network/RequestJSONView.js (right): https://codereview.chromium.org/654083006/diff/1/Source/devtools/front_end/network/RequestJSONView.js#newcode47 Source/devtools/front_end/network/RequestJSONView.js:47: WebInspector.RequestJSONView.buildJSON = function(text) On 2014/10/16 06:51:35, vsevik ...
6 years, 2 months ago (2014-10-16 14:14:56 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/654083006/20001
6 years, 2 months ago (2014-10-16 14:16:10 UTC) #7
commit-bot: I haz the power
6 years, 2 months ago (2014-10-16 15:18:33 UTC) #8
Message was sent while issue was closed. 
Committed patchset #2 (id:20001) as 183821

Powered by Google App Engine
This is Rietveld 408576698