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

Issue 6592027: Update NetLog in preparation for late binding of HttpStream jobs to requests. (Closed)

Created:
9 years, 10 months ago by willchan no longer on Chromium
Modified:
9 years, 7 months ago
Reviewers:
Mike Belshe, eroman, mmenke
CC:
chromium-reviews, cbentzel+watch_chromium.org, arv (Not doing code reviews), darin-cc_chromium.org
Visibility:
Public.

Description

Update NetLog in preparation for late binding of HttpStream jobs to requests. BUG=54371, 42669 TEST=Open up about:net-internals#Events and make sure HTTP_STREAM_JOBs appear and are tied together to URL_REQUESTs. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76287

Patch Set 1 #

Patch Set 2 : Fix tests. #

Patch Set 3 : Fix lint. #

Total comments: 3

Patch Set 4 : Address nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+198 lines, -110 lines) Patch
M chrome/browser/net/passive_log_collector.h View 9 chunks +23 lines, -16 lines 0 comments Download
M chrome/browser/net/passive_log_collector.cc View 5 chunks +37 lines, -4 lines 0 comments Download
M chrome/browser/net/passive_log_collector_unittest.cc View 1 2 12 chunks +79 lines, -79 lines 0 comments Download
M chrome/browser/resources/net_internals/sourceentry.js View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M net/base/host_resolver_impl.cc View 1 chunk +1 line, -1 line 0 comments Download
M net/base/net_log_event_type_list.h View 2 chunks +18 lines, -1 line 0 comments Download
M net/base/net_log_source_type_list.h View 1 chunk +2 lines, -1 line 0 comments Download
M net/http/http_stream_factory_impl.cc View 2 chunks +3 lines, -2 lines 0 comments Download
M net/http/http_stream_factory_impl_job.h View 2 chunks +2 lines, -0 lines 0 comments Download
M net/http/http_stream_factory_impl_job.cc View 1 2 3 4 chunks +10 lines, -2 lines 0 comments Download
M net/http/http_stream_factory_impl_request.h View 5 chunks +9 lines, -2 lines 0 comments Download
M net/http/http_stream_factory_impl_request.cc View 3 chunks +13 lines, -2 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
willchan no longer on Chromium
mbelshe: Everything mmenke/eroman: Either one of you, please verify PassiveLogCollector and net_internals look good.
9 years, 10 months ago (2011-02-26 01:48:13 UTC) #1
Mike Belshe
LGTM, except it looks like the bots caught something.
9 years, 10 months ago (2011-02-26 04:47:38 UTC) #2
willchan no longer on Chromium
No bug. The tests were assuming that URL_REQUESTS bound to SOCKETs. No longer true, since ...
9 years, 10 months ago (2011-02-26 19:59:08 UTC) #3
mmenke
9 years, 9 months ago (2011-02-28 15:57:28 UTC) #4
LGTM, but probably best to have Eric take a look at the PassiveLogCollector
stuff.

http://codereview.chromium.org/6592027/diff/4002/chrome/browser/resources/net...
File chrome/browser/resources/net_internals/sourceentry.js (right):

http://codereview.chromium.org/6592027/diff/4002/chrome/browser/resources/net...
chrome/browser/resources/net_internals/sourceentry.js:250: case
LogSourceType.HTTP_STREAM_JOB:
nit:  Might as well merge this with LogSourceType.URL_REQUEST and
LogSourceType.SOCKET_STREAM above.

http://codereview.chromium.org/6592027/diff/4002/net/http/http_stream_factory...
File net/http/http_stream_factory_impl_job.cc (right):

http://codereview.chromium.org/6592027/diff/4002/net/http/http_stream_factory...
net/http/http_stream_factory_impl_job.cc:49: class JobCreationParameters :
public NetLog::EventParameters {
This class isn't needed - see below.

http://codereview.chromium.org/6592027/diff/4002/net/http/http_stream_factory...
net/http/http_stream_factory_impl_job.cc:401:
request_info.url.GetOrigin().spec())));
You can just use:
new NetLogStringParameter("url", request_info.url.GetOrigin().spec());

Powered by Google App Engine
This is Rietveld 408576698