Chromium Code Reviews
Help | Chromium Project | Sign in
(103)

Issue 2108003: Add the URLRequest's method and load flags to the NetLog. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 11 months ago by eroman
Modified:
2 years, 10 months ago
Reviewers:
willchan
CC:
chromium-reviews_chromium.org, cbentzel+watch_chromium.org, darin-cc_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Add the URLRequest's method and load flags to the NetLog.

BUG=37421
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=47321

Patch Set 1 #

Patch Set 2 : Add the unittest file #

Patch Set 3 : Add missing files #

Patch Set 4 : Fix some comments #

Patch Set 5 : change original_url_ --> url_ #

Total comments: 4

Patch Set 6 : address willchan's comments #

Patch Set 7 : fix test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+80 lines, -21 lines) Lint Patch
M chrome/browser/net/passive_log_collector.cc View 1 2 3 4 5 2 chunks +3 lines, -0 lines 0 comments 0 errors Download
M chrome/browser/net/passive_log_collector_unittest.cc View 2 3 4 5 6 6 chunks +12 lines, -11 lines 0 comments 0 errors Download
M net/base/net_log.h View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments 0 errors Download
M net/base/net_log_event_type_list.h View 1 2 3 4 5 5 chunks +9 lines, -7 lines 0 comments 0 errors Download
M net/net.gyp View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments 0 errors Download
M net/url_request/url_request.cc View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments 0 errors Download
A net/url_request/url_request_netlog_params.h View 3 1 chunk +32 lines, -0 lines 0 comments 1 errors Download
A net/url_request/url_request_netlog_params.cc View 1 chunk +20 lines, -0 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 4
eroman
3 years, 11 months ago #1
eroman
switched to using url_ instead of original_url_ (so the new URL is displayed when doing ...
3 years, 11 months ago #2
willchan
LGTM http://codereview.chromium.org/2108003/diff/24001/25004 File net/base/net_log_event_type_list.h (right): http://codereview.chromium.org/2108003/diff/24001/25004#newcode261 net/base/net_log_event_type_list.h:261: // "method": <The method ("POST" or "GET" or ...
3 years, 11 months ago #3
eroman
3 years, 11 months ago #4
Thanks for the review.

http://codereview.chromium.org/2108003/diff/24001/25004
File net/base/net_log_event_type_list.h (right):

http://codereview.chromium.org/2108003/diff/24001/25004#newcode261
net/base/net_log_event_type_list.h:261: //      "method": <The method ("POST" or
"GET" or "HEAD" etc..)>,
On 2010/05/14 19:13:59, willchan wrote:
> Your commas are inconsistent (within this block and also with the file [none
of
> my multi key params are comma delimited]).  I think you should just ditch them
> or make everything else consistent with yours (You should then add a comma at
> the end of the line before this one).

Done.
I added commas where they are missing (the intent with this formatting is to
match JSON).

http://codereview.chromium.org/2108003/diff/24001/25007
File net/url_request/url_request_netlog_params.cc (right):

http://codereview.chromium.org/2108003/diff/24001/25007#newcode15
net/url_request/url_request_netlog_params.cc:15: dict->SetString(L"url",
url_.possibly_invalid_spec());
On 2010/05/14 19:13:59, willchan wrote:
> Is the URL ever allowed to be invalid here?  I forget when we do the check.

I prefer to be on the safe side since this is for logging purposes, and I want
it to work even if some other expectation is wrong.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6