Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(256)

Issue 1548503002: net: extract GetDirectoryListingXXX functions into directory_listing.* (Closed)

Created:
5 years ago by tfarina
Modified:
4 years, 11 months ago
Reviewers:
Charlie Reis, eroman, dcheng
CC:
cbentzel+watch_chromium.org, chromium-reviews, Charlie Reis, jshin+watch_chromium.org, jungshik at Google, nhiroki
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

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}

Patch Set 1 #

Total comments: 5

Patch Set 2 : fixes #

Patch Set 3 : back with directory_listing.cc only #

Patch Set 4 : REBASE #

Patch Set 5 : fix fo storage #

Patch Set 6 : content fix #

Patch Set 7 : REBASE #

Unified diffs Side-by-side diffs Delta from patch set Stats (+109 lines, -229 lines) Patch
M content/child/ftp_directory_listing_response_delegate.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M net/BUILD.gn View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M net/DEPS View 1 2 3 1 chunk +5 lines, -6 lines 0 comments Download
A net/base/directory_listing.h View 1 chunk +44 lines, -0 lines 0 comments Download
A + net/base/directory_listing.cc View 1 2 1 chunk +24 lines, -2 lines 0 comments Download
A + net/base/directory_listing_unittest.cc View 1 2 chunks +2 lines, -7 lines 0 comments Download
M net/base/net_util.h View 1 2 3 1 chunk +0 lines, -22 lines 0 comments Download
M net/base/net_util.cc View 1 2 3 3 chunks +0 lines, -21 lines 0 comments Download
D net/base/net_util_icu.cc View 1 chunk +0 lines, -56 lines 0 comments Download
D net/base/net_util_icu_unittest.cc View 1 chunk +0 lines, -81 lines 0 comments Download
M net/net.gyp View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M net/net.gypi View 1 2 3 4 5 6 16 chunks +30 lines, -29 lines 0 comments Download
M net/url_request/url_request_file_dir_job.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M net/url_request/url_request_unittest.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M storage/browser/fileapi/file_system_dir_url_request_job.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 30 (10 generated)
tfarina
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. ...
5 years ago (2015-12-21 20:47:47 UTC) #1
eroman
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 ...
5 years ago (2015-12-21 21:17:35 UTC) #2
tfarina
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 ...
5 years ago (2015-12-21 22:20:28 UTC) #3
mmenke
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 > ...
5 years ago (2015-12-22 16:39:59 UTC) #4
mmenke
On 2015/12/22 16:39:59, mmenke (OOO Dec 19 to Jan 3) wrote: > On 2015/12/21 22:20:28, ...
5 years ago (2015-12-22 16:40:40 UTC) #5
tfarina
On 2015/12/22 16:40:40, mmenke (OOO Dec 19 to Jan 3) wrote: > On 2015/12/22 16:39:59, ...
4 years, 11 months ago (2016-01-04 22:48:01 UTC) #6
tfarina
Matt, which "FTP enabled compile flag" you say? There is no enable_ftp flag.
4 years, 11 months ago (2016-01-04 23:21:25 UTC) #8
mmenke
On 2016/01/04 23:21:25, tfarina wrote: > Matt, which "FTP enabled compile flag" you say? There ...
4 years, 11 months ago (2016-01-04 23:39:36 UTC) #9
tfarina
On 2016/01/04 23:39:36, mmenke wrote: > On 2016/01/04 23:21:25, tfarina wrote: > > Matt, which ...
4 years, 11 months ago (2016-01-06 15:37:02 UTC) #10
eroman
lgtm
4 years, 11 months ago (2016-01-06 19:29:54 UTC) #11
tfarina
TBRing OWNERS: content -> creis storage -> nhiroki base/i18n -> jshin
4 years, 11 months ago (2016-01-06 20:31:10 UTC) #12
commit-bot: I haz the power
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
4 years, 11 months ago (2016-01-06 20:33:27 UTC) #16
commit-bot: I haz the power
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_android/builds/4637)
4 years, 11 months ago (2016-01-06 20:51:07 UTC) #18
commit-bot: I haz the power
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
4 years, 11 months ago (2016-01-06 20:53:01 UTC) #20
Charlie Reis
content/ LGTM
4 years, 11 months ago (2016-01-06 21:00:13 UTC) #22
commit-bot: I haz the power
Committed patchset #7 (id:120001)
4 years, 11 months ago (2016-01-06 21:48:26 UTC) #24
commit-bot: I haz the power
Patchset 7 (id:??) landed as https://crrev.com/43a416ba67503d45293a301959ea428d0c0fc3bb Cr-Commit-Position: refs/heads/master@{#367926}
4 years, 11 months ago (2016-01-06 21:49:42 UTC) #26
dcheng
This is failing to build on one of the Android bots: https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Data%20Reduction%20Proxy%20Builder/builds/1005 Mind fixing that?
4 years, 11 months ago (2016-01-06 22:58:45 UTC) #28
tfarina
On 2016/01/06 22:58:45, dcheng wrote: > This is failing to build on one of the ...
4 years, 11 months ago (2016-01-06 23:02:02 UTC) #29
tfarina
4 years, 11 months ago (2016-01-06 23:08:23 UTC) #30
Message was sent while issue was closed.
Daniel, Eric, I put a tentative fix for this here ->
https://codereview.chromium.org/1562073003

Powered by Google App Engine
This is Rietveld 408576698