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

Issue 1746012: More cleanup of net_log.h (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, pam+watch_chromium.org, John Grabowski, cbentzel+watch_chromium.org, darin-cc_chromium.org, PaweĊ‚ Hajdan Jr.
Visibility:
Public.

Description

More cleanup to address TODOs in net_log.h.

* Removes 9 methods: AddEventWithParameters, BeginEventWithParameters, EndEventWithParameters, BeginEventWithString, BeginEventWithInteger, AddEventWithString, AddEventWithInteger, EndEventWithParameters, EndEventWithInteger. This was becoming ridiculous, instead made the EventParameters* a required parameter.
* Moves CapturingBoundNetLog / CapturingNetLog to its own file.

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

Patch Set 1 #

Total comments: 8

Patch Set 2 : Address willchan's comments #

Patch Set 3 : fixes for windows compile #

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+284 lines, -817 lines) Lint Patch
A + net/base/capturing_net_log.h View 1 3 chunks +4 lines, -237 lines 0 comments 1 errors Download
A + net/base/capturing_net_log.cc View 1 1 chunk +3 lines, -143 lines 0 comments 0 errors Download
M net/base/host_resolver_impl.h View 1 2 1 chunk +1 line, -0 lines 0 comments 0 errors Download
M net/base/host_resolver_impl.cc View 1 2 4 chunks +10 lines, -10 lines 0 comments 0 errors Download
M net/base/net_log.h View 1 2 4 chunks +17 lines, -118 lines 0 comments 0 errors Download
M net/base/net_log.cc View 1 2 5 chunks +13 lines, -96 lines 0 comments 0 errors Download
M net/base/net_log_unittest.h View 1 2 1 chunk +1 line, -1 line 0 comments 0 errors Download
M net/base/net_log_util.h View 1 2 1 chunk +1 line, -1 line 0 comments 0 errors Download
M net/http/http_cache_transaction.cc View 1 2 10 chunks +12 lines, -12 lines 0 comments 0 errors Download
M net/http/http_network_transaction.cc View 1 2 2 chunks +17 lines, -16 lines 0 comments 0 errors Download
M net/http/http_stream_parser.cc View 1 2 1 chunk +2 lines, -2 lines 0 comments 0 errors Download
M net/net.gyp View 1 2 1 chunk +2 lines, -0 lines 0 comments 0 errors Download
M net/proxy/init_proxy_resolver.cc View 1 2 7 chunks +15 lines, -14 lines 0 comments 0 errors Download
M net/proxy/proxy_resolver_v8.cc View 1 2 4 chunks +8 lines, -8 lines 0 comments 0 errors Download
M net/proxy/proxy_service.cc View 1 2 9 chunks +16 lines, -13 lines 0 comments 0 errors Download
M net/proxy/single_threaded_proxy_resolver.cc View 1 2 3 chunks +3 lines, -3 lines 0 comments 0 errors Download
M net/proxy/single_threaded_proxy_resolver_unittest.cc View 1 2 1 chunk +1 line, -1 line 0 comments 0 errors Download
M net/socket/client_socket_pool_base.cc View 1 2 15 chunks +39 lines, -33 lines 0 comments 0 errors Download
net/socket/socks5_client_socket.cc View 1 2 10 chunks +26 lines, -24 lines 0 comments 0 errors Download
M net/socket/socks_client_socket.cc View 1 2 2 chunks +3 lines, -3 lines 0 comments 0 errors Download
M net/socket/ssl_client_socket_mac.cc View 1 2 4 chunks +5 lines, -5 lines 0 comments 0 errors Download
M net/socket/ssl_client_socket_nss.cc View 3 chunks +5 lines, -5 lines 0 comments 0 errors Download
M net/socket/ssl_client_socket_win.cc View 3 chunks +4 lines, -4 lines 0 comments 0 errors Download
M net/socket/tcp_client_socket_libevent.cc View 1 2 9 chunks +13 lines, -13 lines 0 comments 0 errors Download
M net/socket/tcp_client_socket_pool.cc View 1 2 1 chunk +6 lines, -5 lines 0 comments 0 errors Download
net/socket/tcp_client_socket_win.cc View 1 2 8 chunks +14 lines, -14 lines 0 comments 0 errors Download
M net/socket_stream/socket_stream.cc View 1 2 8 chunks +11 lines, -10 lines 0 comments 0 errors Download
M net/spdy/spdy_network_transaction.cc View 1 2 1 chunk +9 lines, -8 lines 0 comments 0 errors Download
M net/spdy/spdy_session.cc View 1 2 1 chunk +1 line, -1 line 0 comments 0 errors Download
M net/spdy/spdy_stream.cc View 1 2 2 chunks +8 lines, -8 lines 0 comments 0 errors Download
M net/url_request/url_request.cc View 1 2 4 chunks +14 lines, -9 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 3
eroman
3 years, 11 months ago #1
willchan
LGTM http://codereview.chromium.org/1746012/diff/1/25 File net/base/capturing_net_log.h (left): http://codereview.chromium.org/1746012/diff/1/25#oldcode11 net/base/capturing_net_log.h:11: #include "base/basictypes.h" You need this include for DISALLOW_COPY_AND_ASSIGN. ...
3 years, 11 months ago #2
eroman
3 years, 11 months ago #3
Thanks for the review

http://codereview.chromium.org/1746012/diff/1/25
File net/base/capturing_net_log.h (left):

http://codereview.chromium.org/1746012/diff/1/25#oldcode11
net/base/capturing_net_log.h:11: #include "base/basictypes.h"
On 2010/04/28 17:15:37, willchan wrote:
> You need this include for DISALLOW_COPY_AND_ASSIGN.

Done.

http://codereview.chromium.org/1746012/diff/1/25
File net/base/capturing_net_log.h (right):

http://codereview.chromium.org/1746012/diff/1/25#newcode44
net/base/capturing_net_log.h:44: explicit CapturingNetLog(size_t
max_num_entries)
On 2010/04/28 17:15:37, willchan wrote:
> Nit: In general I prefer having these constructors in the .cc.  Minimizes the
> inlining of the constructor which is non-trivially sized (the EntryList
> constructor will be large).

Done.

http://codereview.chromium.org/1746012/diff/1/30
File net/base/net_log.cc (right):

http://codereview.chromium.org/1746012/diff/1/30#newcode6
net/base/net_log.cc:6: #include "base/logging.h"
On 2010/04/28 17:15:37, willchan wrote:
> I don't think you need this include?  Dunno where logging.h is used here. 
Same
> with string_util.h.

Done.

http://codereview.chromium.org/1746012/diff/1/27
File net/base/net_log.h (right):

http://codereview.chromium.org/1746012/diff/1/27#newcode162
net/base/net_log.h:162: // currently bound source, current time, and a fixed
CapturePhase.
On 2010/04/28 17:15:37, willchan wrote:
> CapturePhase?  What's that?

Done.
Sign in to reply to this message.

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