FTP: add net-internals logging.

FTP: add net-internals logging.

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

[eroman, 2012-11-06 23:38:24]

looking!

note that the tryresults suggest a compile failure.

[eroman, 2012-11-06 23:49:29]

LGTM after these comments

https://codereview.chromium.org/11377007/diff/1/net/base/net_log_event_type_l...
File net/base/net_log_event_type_list.h (right):

https://codereview.chromium.org/11377007/diff/1/net/base/net_log_event_type_l...
net/base/net_log_event_type_list.h:1733: EVENT_TYPE(FTP_COMMAND_SENT)
Can you add some comments on what the parameters look like? (using similar
format to the ones above).

https://codereview.chromium.org/11377007/diff/1/net/ftp/ftp_ctrl_response_buf...
File net/ftp/ftp_ctrl_response_buffer.cc (right):

https://codereview.chromium.org/11377007/diff/1/net/ftp/ftp_ctrl_response_buf...
net/ftp/ftp_ctrl_response_buffer.cc:100:
base::Bind(&NetLogFtpCtrlResponseCallback, result));
Note that this line will cause |result| to be copied.

In general we try to avoid this for the NetLogging.

Instead, you can do ConstRef(result). Or you can change the parameter to be
|const FtpCtrlResponse*|. The preferred style is the latter, since it is easier
to not mess up.

[Paweł Hajdan Jr., 2012-11-07 00:42:55]

https://codereview.chromium.org/11377007/diff/1/net/base/net_log_event_type_l...
File net/base/net_log_event_type_list.h (right):

https://codereview.chromium.org/11377007/diff/1/net/base/net_log_event_type_l...
net/base/net_log_event_type_list.h:1733: EVENT_TYPE(FTP_COMMAND_SENT)
On 2012/11/06 23:49:29, eroman wrote:
> Can you add some comments on what the parameters look like? (using similar
> format to the ones above).

Done.

https://codereview.chromium.org/11377007/diff/1/net/ftp/ftp_ctrl_response_buf...
File net/ftp/ftp_ctrl_response_buffer.cc (right):

https://codereview.chromium.org/11377007/diff/1/net/ftp/ftp_ctrl_response_buf...
net/ftp/ftp_ctrl_response_buffer.cc:100:
base::Bind(&NetLogFtpCtrlResponseCallback, result));
On 2012/11/06 23:49:29, eroman wrote:
> Note that this line will cause |result| to be copied.
> 
> In general we try to avoid this for the NetLogging.
> 
> Instead, you can do ConstRef(result). Or you can change the parameter to be
> |const FtpCtrlResponse*|. The preferred style is the latter, since it is
easier
> to not mess up.

Done.

[eroman, 2012-11-07 01:39:13]

lgtm

[commit-bot: I haz the power, 2012-11-07 02:08:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/phajdan.jr@chromium.org/11377007/5002

[commit-bot: I haz the power, 2012-11-07 12:06:04]

Change committed as 166387