Do not call stat() when reading directories via File API.

Do not call stat() when reading directories via File API.

It's very heavy, and on the JavaScript side the metadata is discarded
anyway, and another call is used to obtain it.

TEST=All existing tests should pass. Tested manually with MTP and
     chrome://quota-internals, then filesystem://.../isolated in the browser.
BUG=558191
Committed: https://crrev.com/4c70fc9c006c7d5a3099cac3842cef8b411d2a49
Cr-Commit-Position: refs/heads/master@{#359768}

[mtomasz, 2015-11-12 05:48:30]

The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-12 05:48:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1432403003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1432403003/40001

[mtomasz, 2015-11-12 05:49:16]

Description was changed from

==========
Do not call stat() when reading directories via File API.

It's very heavy, and on the JavaScript side the metadata is discarded
anyway, and another call is used to obtain it.

TEST=All existing tests should pass. Tested manually with MTP and
     chrome://quota-internals, then filesystem://.../isolated in the browser.
BUG=None
==========

to

==========
Do not call stat() when reading directories via File API.

It's very heavy, and on the JavaScript side the metadata is discarded
anyway, and another call is used to obtain it.

TEST=All existing tests should pass. Tested manually with MTP and
     chrome://quota-internals, then filesystem://.../isolated in the browser.
BUG=None
==========

[mtomasz, 2015-11-12 05:49:16]

mtomasz@chromium.org changed reviewers:
+ thestig@chromium.org, tzik@chromium.org

[mtomasz, 2015-11-12 05:50:25]

mtomasz@chromium.org changed reviewers:
+ hirono@chromium.org

[mtomasz, 2015-11-12 05:50:25]

@thestig: PTAL at chrome/browser/media_galleries/*
@tzik: PTAL at storage/* and chrome/browser/media_galleries/fileapi/*.
@hirono: PTAL at all the rest.

Thanks!

[commit-bot: I haz the power, 2015-11-12 05:56:48]

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

[commit-bot: I haz the power, 2015-11-12 05:56:49]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Lei Zhang, 2015-11-12 07:25:17]

c/b/media_galleries lgtm, and thanks for getting rid of that hack. The rest is
all nits:

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
File chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc
(right):

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc:1584: for
(size_t i = 0; i < mtp_entries.size(); ++i) {
for (const auto& mtp_entry : mtp_entries) {
   ...

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
File chrome/browser/media_galleries/linux/mtp_device_task_helper.cc (right):

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.cc:10: #include
"base/numerics/safe_conversions.h"
These 2 conversion headers can be deleted too.

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.cc:271: uint32
file_id = 0;
Get rid of the temporary and pass |entry.file_id| to GetEntryId() ?

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
File chrome/browser/media_galleries/linux/mtp_device_task_helper.h (right):

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.h:30: MTPEntry();
blank line after

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.h:31: uint32
file_id;
uint32_t in new code.

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.h:36: typedef
std::vector<MTPEntry> MTPEntries;
Can we use "using foo = bar" in new code?

[mtomasz, 2015-11-12 07:35:51]

The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-12 07:35:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1432403003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1432403003/60001

[mtomasz, 2015-11-12 07:48:25]

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
File chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc
(right):

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_delegate_impl_linux.cc:1584: for
(size_t i = 0; i < mtp_entries.size(); ++i) {
On 2015/11/12 07:25:16, Lei Zhang wrote:
> for (const auto& mtp_entry : mtp_entries) {
>    ...

Done.

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
File chrome/browser/media_galleries/linux/mtp_device_task_helper.cc (right):

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.cc:10: #include
"base/numerics/safe_conversions.h"
On 2015/11/12 07:25:16, Lei Zhang wrote:
> These 2 conversion headers can be deleted too.

Done.

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.cc:271: uint32
file_id = 0;
On 2015/11/12 07:25:17, Lei Zhang wrote:
> Get rid of the temporary and pass |entry.file_id| to GetEntryId() ?

Done.

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
File chrome/browser/media_galleries/linux/mtp_device_task_helper.h (right):

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.h:30: MTPEntry();
On 2015/11/12 07:25:17, Lei Zhang wrote:
> blank line after

Done.

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.h:31: uint32
file_id;
On 2015/11/12 07:25:17, Lei Zhang wrote:
> uint32_t in new code.

Oops. Done.

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.h:36: typedef
std::vector<MTPEntry> MTPEntries;
On 2015/11/12 07:25:17, Lei Zhang wrote:
> Can we use "using foo = bar" in new code?

I think we can, but it would be very inconsistent here.

[mtomasz, 2015-11-12 07:48:31]

The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-12 07:48:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1432403003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1432403003/80001

[hirono, 2015-11-12 07:54:45]

lgtm!

[Lei Zhang, 2015-11-12 07:56:48]

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
File chrome/browser/media_galleries/linux/mtp_device_task_helper.h (right):

https://codereview.chromium.org/1432403003/diff/40001/chrome/browser/media_ga...
chrome/browser/media_galleries/linux/mtp_device_task_helper.h:36: typedef
std::vector<MTPEntry> MTPEntries;
On 2015/11/12 07:48:24, mtomasz wrote:
> On 2015/11/12 07:25:17, Lei Zhang wrote:
> > Can we use "using foo = bar" in new code?
> 
> I think we can, but it would be very inconsistent here.

Ok, no worries.

[commit-bot: I haz the power, 2015-11-12 08:04:28]

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

[commit-bot: I haz the power, 2015-11-12 08:04:29]

Dry run: Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[mtomasz, 2015-11-12 08:17:54]

The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-12 08:18:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1432403003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1432403003/100001

[commit-bot: I haz the power, 2015-11-12 08:27:09]

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

[commit-bot: I haz the power, 2015-11-12 08:27:10]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[mtomasz, 2015-11-12 09:44:24]

The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-12 09:44:36]

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

[commit-bot: I haz the power, 2015-11-12 10:17:28]

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

[commit-bot: I haz the power, 2015-11-12 10:17:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mtomasz, 2015-11-13 01:04:30]

The CQ bit was checked by mtomasz@chromium.org to run a CQ dry run

[mtomasz, 2015-11-13 01:06:36]

mtomasz@chromium.org changed reviewers:
+ kinuko@google.com, nhiroki@google.com

[mtomasz, 2015-11-13 01:07:02]

mtomasz@chromium.org changed reviewers:
+ nhiroki@chromium.org
- kinuko@google.com, nhiroki@google.com

[mtomasz, 2015-11-13 01:08:16]

@nhiroki: Could you take a look at storage/* and
chrome/browser/media_galleries/fileapi/*.
@tzik seems to be OOO?

[commit-bot: I haz the power, 2015-11-13 01:11:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1432403003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1432403003/140001

[commit-bot: I haz the power, 2015-11-13 02:28:36]

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

[commit-bot: I haz the power, 2015-11-13 02:28:37]

Dry run: This issue passed the CQ dry run.

[tzik, 2015-11-15 04:38:51]

Sorry for delay. Yes, we are ooo for BlinkOn5 this week.

LGTM. Good point.
The stat is just for detecting the entry is a directory or file on local file
systems. So there's no reason to do it if the file type is known.

[mtomasz, 2015-11-15 07:04:19]

The CQ bit was checked by mtomasz@chromium.org

[mtomasz, 2015-11-15 07:04:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, hirono@chromium.org
Link to the patchset: https://codereview.chromium.org/1432403003/#ps140001
(title: "Fixed FSP tests.")

[commit-bot: I haz the power, 2015-11-15 07:04:36]

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

[commit-bot: I haz the power, 2015-11-15 12:11:31]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-11-15 12:12:26]

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

[mtomasz, 2015-11-19 03:28:25]

Description was changed from

==========
Do not call stat() when reading directories via File API.

It's very heavy, and on the JavaScript side the metadata is discarded
anyway, and another call is used to obtain it.

TEST=All existing tests should pass. Tested manually with MTP and
     chrome://quota-internals, then filesystem://.../isolated in the browser.
BUG=None

Committed: https://crrev.com/4c70fc9c006c7d5a3099cac3842cef8b411d2a49
Cr-Commit-Position: refs/heads/master@{#359768}
==========

to

==========
Do not call stat() when reading directories via File API.

It's very heavy, and on the JavaScript side the metadata is discarded
anyway, and another call is used to obtain it.

TEST=All existing tests should pass. Tested manually with MTP and
     chrome://quota-internals, then filesystem://.../isolated in the browser.
BUG=558191

Committed: https://crrev.com/4c70fc9c006c7d5a3099cac3842cef8b411d2a49
Cr-Commit-Position: refs/heads/master@{#359768}
==========