Hide .crdownload files and prevent users from changing extension to it.

Hide .crdownload files and prevent users from changing extension to it.

BUG=409490
TEST=manually

Committed: https://crrev.com/6e7df01d4614df70ec2a1418a6b36717e21c85f3
Cr-Commit-Position: refs/heads/master@{#293477}

[fukino, 2014-09-03 09:06:34]

fukino@chromium.org changed reviewers:
+ yoshiki@chromium.org

[fukino, 2014-09-03 09:07:26]

@yoshiki, could you take a look? I have a question about string ID. Thanks in
advance!

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/common/js/util.js (right):

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/common/js/util.js:1348: return
Promise.reject(str('ERROR_RESERVED_NAME'));
Though the .crdownload will be hidden, the message ERROR_RESERVED_NAME looks
more appropriate than ERROR_HIDDEN_NAME.

Can I use ERROR_RESERVED_NAME for the *.crdownload, or should I modify
ERROR_HIDDEN_NAME or create new message?

[yoshiki, 2014-09-04 03:39:16]

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/common/js/util.js (right):

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/common/js/util.js:1348: return
Promise.reject(str('ERROR_RESERVED_NAME'));
On 2014/09/03 09:07:26, fukino wrote:
> Though the .crdownload will be hidden, the message ERROR_RESERVED_NAME looks
> more appropriate than ERROR_HIDDEN_NAME.
> 
> Can I use ERROR_RESERVED_NAME for the *.crdownload, or should I modify
> ERROR_HIDDEN_NAME or create new message?

ERROR_HIDDEN_NAME sounds good for me. If you are not sure, please ask PM.

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/foreground/js/directory_contents.js (right):

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_contents.js:380:
!/\.crdownload$/i.test(entry.name);
The following should be faster. This creates neither new object nor new
substring.

var suffix = '.crdownload';
(entry.name.indexOf(suffix, entry.name.length - suffix.length) !== -1)

[fukino, 2014-09-05 02:59:31]

Thank you for comments!

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/common/js/util.js (right):

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/common/js/util.js:1348: return
Promise.reject(str('ERROR_RESERVED_NAME'));
On 2014/09/04 03:39:15, yoshiki wrote:
> On 2014/09/03 09:07:26, fukino wrote:
> > Though the .crdownload will be hidden, the message ERROR_RESERVED_NAME looks
> > more appropriate than ERROR_HIDDEN_NAME.
> > 
> > Can I use ERROR_RESERVED_NAME for the *.crdownload, or should I modify
> > ERROR_HIDDEN_NAME or create new message?
> 
> ERROR_HIDDEN_NAME sounds good for me. If you are not sure, please ask PM.

I confirmed with PM that we can use ERROR_RESERVED_NAME.

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/foreground/js/directory_contents.js (right):

https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_contents.js:380:
!/\.crdownload$/i.test(entry.name);
On 2014/09/04 03:39:15, yoshiki wrote:
> The following should be faster. This creates neither new object nor new
> substring.
> 
> var suffix = '.crdownload';
> (entry.name.indexOf(suffix, entry.name.length - suffix.length) !== -1)

According to http://jsperf.com/regexp-test-search-vs-indexof/52, regexp is
slightly faster than indexOf, and I confirmed locally that it is true for our
case.

Given that regexp is not slower, regexp looks simpler for me, so I'd like to use
it.
Is it OK with you?

[yoshiki, 2014-09-05 05:36:17]

On 2014/09/05 02:59:31, fukino wrote:
> Thank you for comments!
> 
>
https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
> File ui/file_manager/file_manager/common/js/util.js (right):
> 
>
https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
> ui/file_manager/file_manager/common/js/util.js:1348: return
> Promise.reject(str('ERROR_RESERVED_NAME'));
> On 2014/09/04 03:39:15, yoshiki wrote:
> > On 2014/09/03 09:07:26, fukino wrote:
> > > Though the .crdownload will be hidden, the message ERROR_RESERVED_NAME
looks
> > > more appropriate than ERROR_HIDDEN_NAME.
> > > 
> > > Can I use ERROR_RESERVED_NAME for the *.crdownload, or should I modify
> > > ERROR_HIDDEN_NAME or create new message?
> > 
> > ERROR_HIDDEN_NAME sounds good for me. If you are not sure, please ask PM.
> 
> I confirmed with PM that we can use ERROR_RESERVED_NAME.
> 
>
https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
> File ui/file_manager/file_manager/foreground/js/directory_contents.js (right):
> 
>
https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
> ui/file_manager/file_manager/foreground/js/directory_contents.js:380:
> !/\.crdownload$/i.test(entry.name);
> On 2014/09/04 03:39:15, yoshiki wrote:
> > The following should be faster. This creates neither new object nor new
> > substring.
> > 
> > var suffix = '.crdownload';
> > (entry.name.indexOf(suffix, entry.name.length - suffix.length) !== -1)
> 
> According to http://jsperf.com/regexp-test-search-vs-indexof/52, regexp is
> slightly faster than indexOf, and I confirmed locally that it is true for our
> case.
> 
> Given that regexp is not slower, regexp looks simpler for me, so I'd like to
use
> it.
> Is it OK with you?

This is hot code and we should consider about performance at most.

I did the benchmark and confirms that the regexp is faster.
http://jsperf.com/endswith-regexp-vs-lastindexof

Could you make the creation of regexp object out of the loop, like
"endsWithExt1" in the jspref? I means the code like following.

var regexp = /.../;
this.addFiler(...., function(filename) { return regexp.test(filename); });

[fukino, 2014-09-05 06:10:49]

On 2014/09/05 05:36:17, yoshiki wrote:
> On 2014/09/05 02:59:31, fukino wrote:
> > Thank you for comments!
> > 
> >
>
https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
> > File ui/file_manager/file_manager/common/js/util.js (right):
> > 
> >
>
https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
> > ui/file_manager/file_manager/common/js/util.js:1348: return
> > Promise.reject(str('ERROR_RESERVED_NAME'));
> > On 2014/09/04 03:39:15, yoshiki wrote:
> > > On 2014/09/03 09:07:26, fukino wrote:
> > > > Though the .crdownload will be hidden, the message ERROR_RESERVED_NAME
> looks
> > > > more appropriate than ERROR_HIDDEN_NAME.
> > > > 
> > > > Can I use ERROR_RESERVED_NAME for the *.crdownload, or should I modify
> > > > ERROR_HIDDEN_NAME or create new message?
> > > 
> > > ERROR_HIDDEN_NAME sounds good for me. If you are not sure, please ask PM.
> > 
> > I confirmed with PM that we can use ERROR_RESERVED_NAME.
> > 
> >
>
https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
> > File ui/file_manager/file_manager/foreground/js/directory_contents.js
(right):
> > 
> >
>
https://codereview.chromium.org/528263002/diff/1/ui/file_manager/file_manager...
> > ui/file_manager/file_manager/foreground/js/directory_contents.js:380:
> > !/\.crdownload$/i.test(entry.name);
> > On 2014/09/04 03:39:15, yoshiki wrote:
> > > The following should be faster. This creates neither new object nor new
> > > substring.
> > > 
> > > var suffix = '.crdownload';
> > > (entry.name.indexOf(suffix, entry.name.length - suffix.length) !== -1)
> > 
> > According to http://jsperf.com/regexp-test-search-vs-indexof/52, regexp is
> > slightly faster than indexOf, and I confirmed locally that it is true for
our
> > case.
> > 
> > Given that regexp is not slower, regexp looks simpler for me, so I'd like to
> use
> > it.
> > Is it OK with you?
> 
> This is hot code and we should consider about performance at most.
> 
> I did the benchmark and confirms that the regexp is faster.
> http://jsperf.com/endswith-regexp-vs-lastindexof
> 
> Could you make the creation of regexp object out of the loop, like
> "endsWithExt1" in the jspref? I means the code like following.
> 
> var regexp = /.../;
> this.addFiler(...., function(filename) { return regexp.test(filename); });

Thank you for confirming on the jsperf!
Moved the creation of regexp object accordingly.

[yoshiki, 2014-09-05 06:22:47]

lgtm

[fukino, 2014-09-05 06:34:51]

On 2014/09/05 06:22:47, yoshiki wrote:
> lgtm

Thanks!

[fukino, 2014-09-05 06:34:55]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2014-09-05 06:35:59]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fukino@chromium.org/528263002/20001

[commit-bot: I haz the power, 2014-09-05 08:25:53]

Committed patchset #2 (id:20001) as 5193ef8fba9dcf0e9bb649e4faaec496b93f8675

[commit-bot: I haz the power, 2014-09-10 03:38:02]

Patchset 2 (id:??) landed as
https://crrev.com/6e7df01d4614df70ec2a1418a6b36717e21c85f3
Cr-Commit-Position: refs/heads/master@{#293477}