net: extract GetDirectoryListingXXX functions into directory_listing.*

net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
TBR=creis@chromium.org,nhiroki@chromium.org,jshin@chromium.org

Committed: https://crrev.com/43a416ba67503d45293a301959ea428d0c0fc3bb
Cr-Commit-Position: refs/heads/master@{#367926}

[tfarina, 2015-12-21 20:47:47]

https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn
File net/BUILD.gn (left):

https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn#oldcode438
net/BUILD.gn:438: "base/net_util_icu.cc",
looks like I will have problem with this. So, will I have to have a
directory_listing_icu.cc?

https://codereview.chromium.org/1548503002/diff/1/net/base/directory_listing_...
File net/base/directory_listing_unittest.cc (right):

https://codereview.chromium.org/1548503002/diff/1/net/base/directory_listing_...
net/base/directory_listing_unittest.cc:5: #include "net/base/net_util.h"
note for myself: update this!

https://codereview.chromium.org/1548503002/diff/1/net/base/directory_listing_...
net/base/directory_listing_unittest.cc:28: TEST(NetUtilTest,
GetDirectoryListingEntry) {
note for myself: update this!

[eroman, 2015-12-21 21:17:35]

https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn
File net/BUILD.gn (left):

https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn#oldcode438
net/BUILD.gn:438: "base/net_util_icu.cc",
On 2015/12/21 20:47:47, tfarina wrote:
> looks like I will have problem with this. So, will I have to have a
> directory_listing_icu.cc?

My expectation is that GetDirectoryListingHeader() will just be needed from the
same configurations as GetDirectoryListingEntry(), so grouping them in the same
file and conditionally building _should_ be fine.

@mmenke: Do you want the renamed file to be called directory_listing_icu.cc
rather than directory_listing.cc ?

[tfarina, 2015-12-21 22:20:28]

https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn
File net/BUILD.gn (left):

https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn#oldcode438
net/BUILD.gn:438: "base/net_util_icu.cc",
On 2015/12/21 21:17:35, eroman wrote:
> On 2015/12/21 20:47:47, tfarina wrote:
> > looks like I will have problem with this. So, will I have to have a
> > directory_listing_icu.cc?
> 
> My expectation is that GetDirectoryListingHeader() will just be needed from
the
> same configurations as GetDirectoryListingEntry(), so grouping them in the
same
> file and conditionally building _should_ be fine.
> 
I got back with directory_listing.cc. I'm unsure how best to build it
conditionally though.

[mmenke, 2015-12-22 16:39:59]

On 2015/12/21 22:20:28, tfarina wrote:
> https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn
> File net/BUILD.gn (left):
> 
> https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn#oldcode438
> net/BUILD.gn:438: "base/net_util_icu.cc",
> On 2015/12/21 21:17:35, eroman wrote:
> > On 2015/12/21 20:47:47, tfarina wrote:
> > > looks like I will have problem with this. So, will I have to have a
> > > directory_listing_icu.cc?
> > 
> > My expectation is that GetDirectoryListingHeader() will just be needed from
> the
> > same configurations as GetDirectoryListingEntry(), so grouping them in the
> same
> > file and conditionally building _should_ be fine.
> > 
> I got back with directory_listing.cc. I'm unsure how best to build it
> conditionally though.

If it's only used by the FTP code, could we just gate it on the FTP enabled
compile flag?

[mmenke, 2015-12-22 16:40:40]

On 2015/12/22 16:39:59, mmenke (OOO Dec 19 to Jan 3) wrote:
> On 2015/12/21 22:20:28, tfarina wrote:
> > https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn
> > File net/BUILD.gn (left):
> > 
> > https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn#oldcode438
> > net/BUILD.gn:438: "base/net_util_icu.cc",
> > On 2015/12/21 21:17:35, eroman wrote:
> > > On 2015/12/21 20:47:47, tfarina wrote:
> > > > looks like I will have problem with this. So, will I have to have a
> > > > directory_listing_icu.cc?
> > > 
> > > My expectation is that GetDirectoryListingHeader() will just be needed
from
> > the
> > > same configurations as GetDirectoryListingEntry(), so grouping them in the
> > same
> > > file and conditionally building _should_ be fine.
> > > 
> > I got back with directory_listing.cc. I'm unsure how best to build it
> > conditionally though.
> 
> If it's only used by the FTP code, could we just gate it on the FTP enabled
> compile flag?

Could also toss FTP in its file name.

[tfarina, 2016-01-04 22:48:01]

On 2015/12/22 16:40:40, mmenke (OOO Dec 19 to Jan 3) wrote:
> On 2015/12/22 16:39:59, mmenke (OOO Dec 19 to Jan 3) wrote:
> > On 2015/12/21 22:20:28, tfarina wrote:
> > > https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn
> > > File net/BUILD.gn (left):
> > > 
> > > https://codereview.chromium.org/1548503002/diff/1/net/BUILD.gn#oldcode438
> > > net/BUILD.gn:438: "base/net_util_icu.cc",
> > > On 2015/12/21 21:17:35, eroman wrote:
> > > > On 2015/12/21 20:47:47, tfarina wrote:
> > > > > looks like I will have problem with this. So, will I have to have a
> > > > > directory_listing_icu.cc?
> > > > 
> > > > My expectation is that GetDirectoryListingHeader() will just be needed
> from
> > > the
> > > > same configurations as GetDirectoryListingEntry(), so grouping them in
the
> > > same
> > > > file and conditionally building _should_ be fine.
> > > > 
> > > I got back with directory_listing.cc. I'm unsure how best to build it
> > > conditionally though.
> > 
> > If it's only used by the FTP code, could we just gate it on the FTP enabled
> > compile flag?
> 
> Could also toss FTP in its file name.

Would ftp_directory_listing be fine?

[tfarina, 2016-01-04 23:20:30]

Description was changed from

==========
net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
==========

to

==========
net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
==========

[tfarina, 2016-01-04 23:21:25]

Matt, which "FTP enabled compile flag" you say? There is no enable_ftp flag.

[mmenke, 2016-01-04 23:39:36]

On 2016/01/04 23:21:25, tfarina wrote:
> Matt, which "FTP enabled compile flag" you say? There is no enable_ftp flag.

disable_ftp_support

[tfarina, 2016-01-06 15:37:02]

On 2016/01/04 23:39:36, mmenke wrote:
> On 2016/01/04 23:21:25, tfarina wrote:
> > Matt, which "FTP enabled compile flag" you say? There is no enable_ftp flag.
> 
> disable_ftp_support

Matt, this is gated behind disable_file_support.

Is Eric on vacation?

Can I get a review now?

[eroman, 2016-01-06 19:29:54]

lgtm

[tfarina, 2016-01-06 20:31:10]

TBRing OWNERS:

content -> creis
storage -> nhiroki
base/i18n -> jshin

[tfarina, 2016-01-06 20:31:39]

Description was changed from

==========
net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
==========

to

==========
net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
TBR=creis@chromium.org,nhiroki@chromium.org,jshin@chromium.org
==========

[tfarina, 2016-01-06 20:31:48]

The CQ bit was checked by tfarina@chromium.org

[tfarina, 2016-01-06 20:31:49]

The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org
Link to the patchset: https://codereview.chromium.org/1548503002/#ps120001
(title: "REBASE")

[commit-bot: I haz the power, 2016-01-06 20:33:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1548503002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1548503002/120001

[commit-bot: I haz the power, 2016-01-06 20:51:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-06 20:51:07]

Try jobs failed on following builders:
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[tfarina, 2016-01-06 20:51:40]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2016-01-06 20:53:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1548503002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1548503002/120001

[Charlie Reis, 2016-01-06 21:00:12]

creis@chromium.org changed reviewers:
+ creis@chromium.org

[Charlie Reis, 2016-01-06 21:00:13]

content/ LGTM

[commit-bot: I haz the power, 2016-01-06 21:48:25]

Description was changed from

==========
net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
TBR=creis@chromium.org,nhiroki@chromium.org,jshin@chromium.org
==========

to

==========
net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
TBR=creis@chromium.org,nhiroki@chromium.org,jshin@chromium.org
==========

[commit-bot: I haz the power, 2016-01-06 21:48:26]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-01-06 21:49:41]

Description was changed from

==========
net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
TBR=creis@chromium.org,nhiroki@chromium.org,jshin@chromium.org
==========

to

==========
net: extract GetDirectoryListingXXX functions into directory_listing.*

This patch moves these function from net_util into a new component
(directory_listing.*) as it seems a more suitable place than the generic
net_util.h header.

BUG=488531
R=eroman@chromium.org
TBR=creis@chromium.org,nhiroki@chromium.org,jshin@chromium.org

Committed: https://crrev.com/43a416ba67503d45293a301959ea428d0c0fc3bb
Cr-Commit-Position: refs/heads/master@{#367926}
==========

[commit-bot: I haz the power, 2016-01-06 21:49:42]

Patchset 7 (id:??) landed as
https://crrev.com/43a416ba67503d45293a301959ea428d0c0fc3bb
Cr-Commit-Position: refs/heads/master@{#367926}

[dcheng, 2016-01-06 22:58:45]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2016-01-06 22:58:45]

This is failing to build on one of the Android bots:
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Dat...

Mind fixing that?

[tfarina, 2016-01-06 23:02:02]

On 2016/01/06 22:58:45, dcheng wrote:
> This is failing to build on one of the Android bots:
>
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Dat...
> 
> Mind fixing that?

Sure, looking.

[tfarina, 2016-01-06 23:08:23]

Daniel, Eric, I put a tentative fix for this here ->
https://codereview.chromium.org/1562073003