Added NetLog statements for Canonical name lookup.

Added NetLog statements for Canonical name lookup.

BUG=None
TEST=None

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

[willchan no longer on Chromium, 2010-04-14 17:17:01]

wtc: didn't we say we weren't going to add more TRACE statements?  I'm inclined
to delete them.

[cbentzel, 2010-04-14 17:24:44]

yeah, I was unclear what the state of TRACE and netlog was.

On Wed, Apr 14, 2010 at 1:17 PM, <willchan@chromium.org> wrote:

> wtc: didn't we say we weren't going to add more TRACE statements?  I'm
> inclined
> to delete them.
>
>
> http://codereview.chromium.org/1629016/show
>

[willchan no longer on Chromium, 2010-04-14 17:36:29]

NetLog is the new hotness.  I think mbelshe/erikkay used TRACE stuff
for some manual performance tests a long time ago.  The data in NetLog
should capture all that information already though.

On Wed, Apr 14, 2010 at 10:24 AM, Chris Bentzel <cbentzel@chromium.org> wrote:
> yeah, I was unclear what the state of TRACE and netlog was.
>
> On Wed, Apr 14, 2010 at 1:17 PM, <willchan@chromium.org> wrote:
>>
>> wtc: didn't we say we weren't going to add more TRACE statements?  I'm
>> inclined
>> to delete them.
>>
>> http://codereview.chromium.org/1629016/show
>
>

[Erik does not do reviews, 2010-04-14 18:03:34]

+mbelshe

Mike was using this stuff more recently.  I haven't touched it in a while,
and I think that the recent work that's been going on with the devtools
timeline and speed tracer are the future for the intent behind TRACE and I'd
be happy to see TRACE replaced with calls that can hook into those tools.

Erik


On Wed, Apr 14, 2010 at 10:36 AM, William Chan (陈智昌)
<willchan@chromium.org>wrote:

> NetLog is the new hotness.  I think mbelshe/erikkay used TRACE stuff
> for some manual performance tests a long time ago.  The data in NetLog
> should capture all that information already though.
>
> On Wed, Apr 14, 2010 at 10:24 AM, Chris Bentzel <cbentzel@chromium.org>
> wrote:
> > yeah, I was unclear what the state of TRACE and netlog was.
> >
> > On Wed, Apr 14, 2010 at 1:17 PM, <willchan@chromium.org> wrote:
> >>
> >> wtc: didn't we say we weren't going to add more TRACE statements?  I'm
> >> inclined
> >> to delete them.
> >>
> >> http://codereview.chromium.org/1629016/show
> >
> >
>

[willchan no longer on Chromium, 2010-04-15 00:06:20]

This LGTM if you drop the TRACE statements.

In answer to erikkay's comments about Speed Tracer / DevTools, we plan to look
into hooking NetLog into those tools so that they have visibility into network
stack logging.

[cbentzel, 2010-04-15 15:12:07]

The new patchset eliminates the TRACE statements. Thanks for the review.

[eroman, 2010-04-15 17:27:37]

LGTM.

Regarding speed tracer/dev tools: I spoke with Pavel about this recently, and we
are going to start by putting a subset of the information on the dev tools page
(probably the basics, like dns resolve time, tcp connection time, cache
retrieval time, transfer time). For the more complicated full set of data, we
have have a more specialized network-internals visualizer.

[wtc, 2010-04-16 01:13:46]

LGTM.

Remember to remove "and TRACE" from the Description of your
CL.

http://codereview.chromium.org/1629016/diff/7001/8001
File net/base/net_log_event_type_list.h (right):

http://codereview.chromium.org/1629016/diff/7001/8001#newcode187
net/base/net_log_event_type_list.h:187: // Measures the time to resolve the
canonical name for Negotiate scheme.
Nit: Negotiate scheme => HTTP Negotiate auth scheme

"Negotiate" is an obscure term outside our circle.

Nit: HTTP_TRANSACTION_RESOLVE_CANONICAL =>
     HTTP_TRANSACTION_RESOLVE_CANONICAL_NAME or
     HTTP_TRANSACTION_RESOLVE_CANONNAME

http://codereview.chromium.org/1629016/diff/7001/8002
File net/http/http_network_transaction.cc (right):

http://codereview.chromium.org/1629016/diff/7001/8002#newcode578
net/http/http_network_transaction.cc:578:
net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_RESOLVE_CANONICAL);
Can you add
    DCHECK_EQ(OK, rv);
before this line?

[cbentzel, 2010-04-16 15:11:00]

wtc: the newest patchset includes your suggestions.

http://codereview.chromium.org/1629016/diff/7001/8001
File net/base/net_log_event_type_list.h (right):

http://codereview.chromium.org/1629016/diff/7001/8001#newcode187
net/base/net_log_event_type_list.h:187: // Measures the time to resolve the
canonical name for Negotiate scheme.
On 2010/04/16 01:13:46, wtc wrote:
> Nit: Negotiate scheme => HTTP Negotiate auth scheme
> 
> "Negotiate" is an obscure term outside our circle.
> 
> Nit: HTTP_TRANSACTION_RESOLVE_CANONICAL =>
>      HTTP_TRANSACTION_RESOLVE_CANONICAL_NAME or
>      HTTP_TRANSACTION_RESOLVE_CANONNAME

Done.

http://codereview.chromium.org/1629016/diff/7001/8002
File net/http/http_network_transaction.cc (right):

http://codereview.chromium.org/1629016/diff/7001/8002#newcode578
net/http/http_network_transaction.cc:578:
net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_RESOLVE_CANONICAL);
On 2010/04/16 01:13:46, wtc wrote:
> Can you add
>     DCHECK_EQ(OK, rv);
> before this line?

Done.