Remove unused sorting options from DirectoryLister.

DATE and  FULL_PATH type sorts are not used anywhere. Date is not even
tested anywhere. Removed those from all related files. Recursive is
used only with NO_SORT so removed the option to use it in
combination with sorting.

Committed: https://crrev.com/3396ad31e40ceef8d08cef11a91da9ee15ff6159
Cr-Commit-Position: refs/heads/master@{#326754}

[olli.syrjala, 2015-04-22 08:08:21]

olli.syrjala@intel.com changed reviewers:
+ mmenke@chromium.org

[olli.syrjala, 2015-04-22 08:09:12]

Could you review?

[mmenke, 2015-04-22 14:57:20]

https://codereview.chromium.org/1096323003/diff/1/net/base/directory_lister_u...
File net/base/directory_lister_unittest.cc (left):

https://codereview.chromium.org/1096323003/diff/1/net/base/directory_lister_u...
net/base/directory_lister_unittest.cc:221: ListerDelegate delegate(true);
This was the only recursive test.  I can't sign off on this without a new
recursive test.  Note that this CL currently leaves CheckRecursiveSort in the
test fixture as dead code.

[olli.syrjala, 2015-04-23 12:50:46]

olli.syrjala@intel.com changed reviewers:
+ sky@chromium.org

[olli.syrjala, 2015-04-23 13:11:07]

Added sky to reviewers because changed function call in owned file

Thanks for comments. I realized that the recursive is used only with NO_SORT, so
other possibilities are now removed. One variable is renamed because now that
variable also tells if recursive is wanted. Non-recursive NO_SORT could be also
removed, but that will not clean the code almost at all.

[mmenke, 2015-04-23 14:57:00]

LGTM, modulo nits.  Think it's reasonable to remove sort+recursive, since no
one's using it.

https://codereview.chromium.org/1096323003/diff/20001/chrome/browser/file_sel...
File chrome/browser/file_select_helper.cc (right):

https://codereview.chromium.org/1096323003/diff/20001/chrome/browser/file_sel...
chrome/browser/file_select_helper.cc:197: entry->delegate_.get()));
nit:  Should now use 4-space indent here (4 spaces relative to entry)

https://codereview.chromium.org/1096323003/diff/20001/net/base/directory_list...
File net/base/directory_lister.cc (right):

https://codereview.chromium.org/1096323003/diff/20001/net/base/directory_list...
net/base/directory_lister.cc:130: if (type_ != NO_SORT_RECURSIVE) {
optional:  Suggest flipping order here (Seems more natural to do the == case
first)

https://codereview.chromium.org/1096323003/diff/20001/net/base/directory_list...
File net/base/directory_lister.h (right):

https://codereview.chromium.org/1096323003/diff/20001/net/base/directory_list...
net/base/directory_lister.h:48: // ALPHA_DIRS_FIRST is the default  listing
type:
nit:  Remove extra space before listing

https://codereview.chromium.org/1096323003/diff/20001/net/base/directory_list...
net/base/directory_lister.h:49: //   directories first in name order, then files
by name order
Mind adding a comment about NO_SORT_RECURSIVE?  (Just something along the lines
of "List is only recursive if listing type is NO_SORT_RECURSIVE.")

https://codereview.chromium.org/1096323003/diff/20001/net/base/directory_list...
File net/base/directory_lister_unittest.cc (right):

https://codereview.chromium.org/1096323003/diff/20001/net/base/directory_list...
net/base/directory_lister_unittest.cc:68: if ((type_ ==
DirectoryLister::ALPHA_DIRS_FIRST))
nit:  Remove extra set of parentheses.

[mmenke, 2015-04-23 16:05:35]

One other thing:  Suggest you clean up the description a little.  No need to
include my comment, for instance.  Here's my suggestion, feel free to modify it
as you like:

Remove unused sorting options from DirectoryLister.

DATE and  FULL_PATH type sorts are not used anywhere. Date is not even tested
anywhere. Removed those from all related files. Recursive is used only with
NO_SORT so removed the option to use it in combination with sorting.

[sky, 2015-04-23 16:40:40]

LGTM

https://codereview.chromium.org/1096323003/diff/20001/chrome/browser/file_sel...
File chrome/browser/file_select_helper.cc (right):

https://codereview.chromium.org/1096323003/diff/20001/chrome/browser/file_sel...
chrome/browser/file_select_helper.cc:197: entry->delegate_.get()));
On 2015/04/23 14:57:00, mmenke (Out 4-27 to 5-1) wrote:
> nit:  Should now use 4-space indent here (4 spaces relative to entry)

Just run git cl format. It should do the right thing.

[olli.syrjala, 2015-04-24 06:47:33]

The CQ bit was checked by olli.syrjala@intel.com

[olli.syrjala, 2015-04-24 06:47:34]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org,
mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/1096323003/#ps60001
(title: "fixed nits")

[commit-bot: I haz the power, 2015-04-24 06:47:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1096323003/60001

[commit-bot: I haz the power, 2015-04-24 07:56:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-24 07:57:55]

Patchset 4 (id:??) landed as
https://crrev.com/3396ad31e40ceef8d08cef11a91da9ee15ff6159
Cr-Commit-Position: refs/heads/master@{#326754}