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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1798173003: [DevTools] Added serverIPaddress to HAR output (Closed)

Created:
4 years, 9 months ago by Allada-Google
Modified:
4 years, 9 months ago
Reviewers:
caseq, alph, pfeldman, allada
CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, sergeyv+blink_chromium.org, pfeldman, kozyatinskiy+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[DevTools] Added serverIPaddress to HAR output JSON format for the HAR output file should now have the IP address as referenced in v1.2 of the HAR spec. (https://dvcs.w3.org/hg/webperf/raw-file/tip/specs/HAR/Overview.html) R=alph BUG=481358

Patch Set 1 #

Total comments: 1

Patch Set 2 : Added tests and fixed bug #

Patch Set 3 : Fixed format issue of using single vs double quotes #

Total comments: 1

Patch Set 4 : Updated Tests #

Total comments: 6

Patch Set 5 : Tests now use values intead of types #

Patch Set 6 : Fixed a couple files that were wrong #

Total comments: 1

Messages

Total messages: 16 (5 generated)
Allada-Google
PTL
4 years, 9 months ago (2016-03-14 23:47:47 UTC) #1
pfeldman
https://codereview.chromium.org/1798173003/diff/1/third_party/WebKit/Source/devtools/front_end/sdk/HAREntry.js File third_party/WebKit/Source/devtools/front_end/sdk/HAREntry.js (right): https://codereview.chromium.org/1798173003/diff/1/third_party/WebKit/Source/devtools/front_end/sdk/HAREntry.js#newcode59 third_party/WebKit/Source/devtools/front_end/sdk/HAREntry.js:59: serverIPAddress: this._request.remoteAddress().split(':')[0] ":", also, we'd want a test with ...
4 years, 9 months ago (2016-03-15 00:23:51 UTC) #3
Allada-Google
PTL
4 years, 9 months ago (2016-03-15 21:46:58 UTC) #4
caseq
https://codereview.chromium.org/1798173003/diff/40001/third_party/WebKit/LayoutTests/inspector/network/network-request-server-ip-address.html File third_party/WebKit/LayoutTests/inspector/network/network-request-server-ip-address.html (right): https://codereview.chromium.org/1798173003/diff/40001/third_party/WebKit/LayoutTests/inspector/network/network-request-server-ip-address.html#newcode8 third_party/WebKit/LayoutTests/inspector/network/network-request-server-ip-address.html:8: return new Promise(function (resolve) { Why does it have ...
4 years, 9 months ago (2016-03-15 21:50:11 UTC) #6
Allada-Google
PTL - Adjusted the existing tests instead
4 years, 9 months ago (2016-03-16 00:05:35 UTC) #7
pfeldman
https://codereview.chromium.org/1798173003/diff/60001/third_party/WebKit/LayoutTests/http/tests/inspector/resource-har-conversion-expected.txt File third_party/WebKit/LayoutTests/http/tests/inspector/resource-har-conversion-expected.txt (right): https://codereview.chromium.org/1798173003/diff/60001/third_party/WebKit/LayoutTests/http/tests/inspector/resource-har-conversion-expected.txt#newcode44 third_party/WebKit/LayoutTests/http/tests/inspector/resource-har-conversion-expected.txt:44: serverIPAddress : <string> Lets dump the actual value. https://codereview.chromium.org/1798173003/diff/60001/third_party/WebKit/Source/devtools/front_end/sdk/HAREntry.js ...
4 years, 9 months ago (2016-03-16 00:10:26 UTC) #8
allada
PTL https://codereview.chromium.org/1798173003/diff/60001/third_party/WebKit/LayoutTests/http/tests/inspector/resource-har-conversion-expected.txt File third_party/WebKit/LayoutTests/http/tests/inspector/resource-har-conversion-expected.txt (right): https://codereview.chromium.org/1798173003/diff/60001/third_party/WebKit/LayoutTests/http/tests/inspector/resource-har-conversion-expected.txt#newcode44 third_party/WebKit/LayoutTests/http/tests/inspector/resource-har-conversion-expected.txt:44: serverIPAddress : <string> On 2016/03/16 00:10:26, pfeldman wrote: ...
4 years, 9 months ago (2016-03-16 23:28:03 UTC) #10
pfeldman
lgtm
4 years, 9 months ago (2016-03-17 00:26:38 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1798173003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1798173003/100001
4 years, 9 months ago (2016-03-21 19:27:27 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/198384)
4 years, 9 months ago (2016-03-21 20:52:38 UTC) #15
alph
4 years, 9 months ago (2016-03-21 23:09:55 UTC) #16
Message was sent while issue was closed. 
https://codereview.chromium.org/1798173003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/sdk/HAREntry.js (right):

https://codereview.chromium.org/1798173003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/sdk/HAREntry.js:54: if
(portPositionInString)
!== -1

Powered by Google App Engine
This is Rietveld 408576698