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

Issue 667393002: [XHR] No need to remember line number and URL on send() for inspector (Closed)

Created:
6 years, 2 months ago by tyoshino (SeeGerritForStatus)
Modified:
6 years, 2 months ago
Reviewers:
eustas, sof
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, Inactive, devtools-reviews_chromium.org, arv+blink, sergeyv+blink_chromium.org, aandrey+blink_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

[XHR] No need to remember line number and URL on send() for inspector XMLHttpRequest implementation has methods to set line number and URL of send() method call to pass to InspectorInstrumentation::didFinishXHRLoading(), but they are not used. We can just remove this code. See below. Since WebInspector.ConsoleMessage gets the corresponding request object in its constructor using the requestId parameter and extracts the line number and URL of send() method call, we don't need to remember and pass them from the XMLHttpRequest class as far as we pass the request identifier given on XMLHttpRequest::didFinishLoading() call to InspectorInstrumentation::didFinishXHRLoading(). R=sigbjornf,eustas BUG=none Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184163

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed #3 #

Patch Set 3 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -13 lines) Patch
M Source/core/inspector/InspectorConsoleAgent.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/core/inspector/InspectorConsoleAgent.cpp View 1 2 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/inspector/InspectorInstrumentation.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/inspector/InspectorResourceAgent.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/inspector/InspectorResourceAgent.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/xml/XMLHttpRequest.h View 1 2 2 chunks +0 lines, -5 lines 0 comments Download
M Source/core/xml/XMLHttpRequest.cpp View 1 2 chunks +1 line, -2 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
tyoshino (SeeGerritForStatus)
6 years, 2 months ago (2014-10-22 06:23:34 UTC) #2
sof
lgtm https://codereview.chromium.org/667393002/diff/1/Source/core/xml/XMLHttpRequest.cpp File Source/core/xml/XMLHttpRequest.cpp (right): https://codereview.chromium.org/667393002/diff/1/Source/core/xml/XMLHttpRequest.cpp#newcode73 Source/core/xml/XMLHttpRequest.cpp:73: #include "wtf/text/WTFString.h" Accidental addition?
6 years, 2 months ago (2014-10-22 06:35:41 UTC) #3
tyoshino (SeeGerritForStatus)
https://codereview.chromium.org/667393002/diff/1/Source/core/xml/XMLHttpRequest.cpp File Source/core/xml/XMLHttpRequest.cpp (right): https://codereview.chromium.org/667393002/diff/1/Source/core/xml/XMLHttpRequest.cpp#newcode73 Source/core/xml/XMLHttpRequest.cpp:73: #include "wtf/text/WTFString.h" On 2014/10/22 06:35:41, sof wrote: > Accidental ...
6 years, 2 months ago (2014-10-22 06:53:57 UTC) #4
eustas
lgtm This seems to be artifacts of old Apple Webkit.
6 years, 2 months ago (2014-10-22 07:48:08 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/667393002/40001
6 years, 2 months ago (2014-10-22 08:47:27 UTC) #7
commit-bot: I haz the power
6 years, 2 months ago (2014-10-22 10:52:57 UTC) #8
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as 184163

Powered by Google App Engine
This is Rietveld 408576698