FTP: fix directory listing parser for ftp.usa.hp.com

FTP: fix directory listing parser for ftp.usa.hp.com

BUG=56547
TEST=net_unittests

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

[eroman, 2010-09-27 18:44:55]

LGTM

http://codereview.chromium.org/3448029/diff/1/9
File net/ftp/ftp_directory_listing_buffer_unittest.cc (right):

http://codereview.chromium.org/3448029/diff/1/9#newcode63
net/ftp/ftp_directory_listing_buffer_unittest.cc:63:
ASSERT_TRUE(base::Time::FromString(L"Tue, 15 Nov 1994 12:45:26",
What is this change about?

http://codereview.chromium.org/3448029/diff/1/11
File net/ftp/ftp_directory_listing_parser_hprc.h (right):

http://codereview.chromium.org/3448029/diff/1/11#newcode25
net/ftp/ftp_directory_listing_parser_hprc.h:25: virtual FtpServerType
GetServerType() const { return SERVER_HPRC; }
Please put this in the .cc file (to avoid header bloat).

http://codereview.chromium.org/3448029/diff/1/11#newcode38
net/ftp/ftp_directory_listing_parser_hprc.h:38:
DISALLOW_COPY_AND_ASSIGN(FtpDirectoryListingParserHprc);
nit: can you explicitly include base/basictypes.h for the macro?

[Paweł Hajdan Jr., 2010-09-28 09:25:50]

http://codereview.chromium.org/3448029/diff/1/9
File net/ftp/ftp_directory_listing_buffer_unittest.cc (right):

http://codereview.chromium.org/3448029/diff/1/9#newcode63
net/ftp/ftp_directory_listing_buffer_unittest.cc:63:
ASSERT_TRUE(base::Time::FromString(L"Tue, 15 Nov 1994 12:45:26",
On 2010/09/27 18:44:55, eroman wrote:
> What is this change about?

If I specify GMT, the time calculation is timezone-dependent (and the resulting
time may have different hour value than expected). If I omit it, the local
timezone is used, which results in the expected hour value. For some reason no
existing test has exposed this before.

http://codereview.chromium.org/3448029/diff/1/11
File net/ftp/ftp_directory_listing_parser_hprc.h (right):

http://codereview.chromium.org/3448029/diff/1/11#newcode25
net/ftp/ftp_directory_listing_parser_hprc.h:25: virtual FtpServerType
GetServerType() const { return SERVER_HPRC; }
On 2010/09/27 18:44:55, eroman wrote:
> Please put this in the .cc file (to avoid header bloat).

Done.

http://codereview.chromium.org/3448029/diff/1/11#newcode38
net/ftp/ftp_directory_listing_parser_hprc.h:38:
DISALLOW_COPY_AND_ASSIGN(FtpDirectoryListingParserHprc);
On 2010/09/27 18:44:55, eroman wrote:
> nit: can you explicitly include base/basictypes.h for the macro?

Done. Also explicitly #included base/time.h for current_time_ member variable.

[eroman, 2010-09-29 21:21:32]

lgtm