Add support for attaching custom parameters to NetLog events.

Add support for attaching custom parameters to NetLog events.

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

[willchan no longer on Chromium, 2010-04-09 00:14:41]

LGTM

The casting makes me a bit nervous.  I'm going to think about whether or not we
can come up with a typesafe solution.

http://codereview.chromium.org/1556018/diff/20001/21018
File chrome/browser/net/passive_log_collector.cc (right):

http://codereview.chromium.org/1556018/diff/20001/21018#newcode276
chrome/browser/net/passive_log_collector.cc:276: // Note: we look at the first
*two* entries, since the outter REQUEST_ALIVE
s/outter/outer/

http://codereview.chromium.org/1556018/diff/20001/21009
File net/base/net_log.h (right):

http://codereview.chromium.org/1556018/diff/20001/21009#newcode31
net/base/net_log.h:31: //  TODO(eroman): Remove the 'const' qualitifer from the
BoundNetLog methods.
You've got one too many whitespaces before the TODO.  It's setting off lint
errors.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode92
net/base/net_log.h:92: };
DISALLOW_COPY_AND_ASSIGN

http://codereview.chromium.org/1556018/diff/20001/21009#newcode110
net/base/net_log.h:110: EventParameters* extra_parameters) = 0;
This should be const.  Is it non-const because of our dumb RefCounted
implementation?  I'm going to fix that.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode127
net/base/net_log.h:127: DISALLOW_COPY_AND_ASSIGN(NetLog);
You need basictypes.h for this.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode192
net/base/net_log.h:192: explicit NetLogStringParameter(const std::string& str) :
str_(str) {}
This constructor should be defined in the .cc.  basic_string's constructor is
non-trivial and inlined in many stl implementations.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode194
net/base/net_log.h:194: const std::string& string() const {
I think all these accessors should be value() and value_ instead of type()
type_.  That way, if the types change, then we don't need to change the calling
code.

I thought briefly about whether or not we'd want to simply templatize this. 
It's possible we may be able to achieve some safety wrt the casting we'll have
to do with EventParameter.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode219
net/base/net_log.h:219: int integer_;
const

http://codereview.chromium.org/1556018/diff/20001/21009#newcode238
net/base/net_log.h:238: const char* literal_;
const char* const

[eroman, 2010-04-09 03:45:13]

http://codereview.chromium.org/1556018/diff/20001/21018
File chrome/browser/net/passive_log_collector.cc (right):

http://codereview.chromium.org/1556018/diff/20001/21018#newcode276
chrome/browser/net/passive_log_collector.cc:276: // Note: we look at the first
*two* entries, since the outter REQUEST_ALIVE
On 2010/04/09 00:14:41, willchan wrote:
> s/outter/outer/

Done.

http://codereview.chromium.org/1556018/diff/20001/21009
File net/base/net_log.h (right):

http://codereview.chromium.org/1556018/diff/20001/21009#newcode31
net/base/net_log.h:31: //  TODO(eroman): Remove the 'const' qualitifer from the
BoundNetLog methods.
On 2010/04/09 00:14:41, willchan wrote:
> You've got one too many whitespaces before the TODO.  It's setting off lint
> errors.

Done.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode92
net/base/net_log.h:92: };
On 2010/04/09 00:14:41, willchan wrote:
> DISALLOW_COPY_AND_ASSIGN

Done.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode110
net/base/net_log.h:110: EventParameters* extra_parameters) = 0;
On 2010/04/09 00:14:41, willchan wrote:
> This should be const.  Is it non-const because of our dumb RefCounted
> implementation?  I'm going to fix that.

Correct. |extra_parameters| should not be mutable, to facilitate sharing it
across observers (and possibly in the future threads).

http://codereview.chromium.org/1556018/diff/20001/21009#newcode127
net/base/net_log.h:127: DISALLOW_COPY_AND_ASSIGN(NetLog);
On 2010/04/09 00:14:41, willchan wrote:
> You need basictypes.h for this.

Done.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode192
net/base/net_log.h:192: explicit NetLogStringParameter(const std::string& str) :
str_(str) {}
On 2010/04/09 00:14:41, willchan wrote:
> This constructor should be defined in the .cc.  basic_string's constructor is
> non-trivial and inlined in many stl implementations.

Done.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode194
net/base/net_log.h:194: const std::string& string() const {
On 2010/04/09 00:14:41, willchan wrote:
> I think all these accessors should be value() and value_ instead of type()
> type_.  That way, if the types change, then we don't need to change the
calling
> code.
> 
> I thought briefly about whether or not we'd want to simply templatize this. 
> It's possible we may be able to achieve some safety wrt the casting we'll have
> to do with EventParameter.

Done.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode219
net/base/net_log.h:219: int integer_;
On 2010/04/09 00:14:41, willchan wrote:
> const

Done.

http://codereview.chromium.org/1556018/diff/20001/21009#newcode238
net/base/net_log.h:238: const char* literal_;
On 2010/04/09 00:14:41, willchan wrote:
> const char* const

Done.