Add an API to get total size of Drive cache including non-evictable ones.

Add an API to get total size of Drive cache including non-evictable ones.

The API is needed to show the total size of Drive cache in storage manager.
This CL also changes the type of cache size to int64_t, to be consistent with functions under base/files.

BUG=591958
TEST=manually

Committed: https://crrev.com/6380c07c5be816838aa3e8027d32b1cc2404c9c9
Cr-Commit-Position: refs/heads/master@{#398805}

[fukino, 2016-06-08 11:31:37]

Description was changed from

==========
Add an API to get total size of Drive cache including non-evictable ones.

The API is needed to show the total size of Drive cache in storage manager.

BUG=591958
TEST=manually
==========

to

==========
Add an API to get total size of Drive cache including non-evictable ones.

The API is needed to show the total size of Drive cache in storage manager.
This CL also changes the type of cache size to int64_t, to be consistent with
functions under base/files.

BUG=591958
TEST=manually
==========

[fukino, 2016-06-08 11:32:07]

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

[fukino, 2016-06-08 11:32:08]

Awano-san, could you take a look?

[yawano, 2016-06-08 11:45:46]

lgtm!

[yawano, 2016-06-08 11:48:20]

Sorry. not lgtm. I'll add a comment.

[yawano, 2016-06-08 11:53:45]

https://codereview.chromium.org/2048993002/diff/20001/components/drive/chrome...
File components/drive/chromeos/file_cache.cc (right):

https://codereview.chromium.org/2048993002/diff/20001/components/drive/chrome...
components/drive/chromeos/file_cache.cc:276: if
(base::GetFileSize(GetCacheFilePath(it->GetID()), &cache_size)) {
We should check the existence of cache by checking metadata before getting file
size by base::GetFileSize to avoid unnecessary IO.

[fukino, 2016-06-09 04:36:47]

Thank you!
Could you take another look?

https://codereview.chromium.org/2048993002/diff/20001/components/drive/chrome...
File components/drive/chromeos/file_cache.cc (right):

https://codereview.chromium.org/2048993002/diff/20001/components/drive/chrome...
components/drive/chromeos/file_cache.cc:276: if
(base::GetFileSize(GetCacheFilePath(it->GetID()), &cache_size)) {
On 2016/06/08 11:53:44, yawano wrote:
> We should check the existence of cache by checking metadata before getting
file
> size by base::GetFileSize to avoid unnecessary IO.

Nice catch! Done.

[yawano, 2016-06-09 05:19:19]

https://codereview.chromium.org/2048993002/diff/20001/components/drive/chrome...
File components/drive/chromeos/file_cache.cc (right):

https://codereview.chromium.org/2048993002/diff/20001/components/drive/chrome...
components/drive/chromeos/file_cache.cc:276: if
(base::GetFileSize(GetCacheFilePath(it->GetID()), &cache_size)) {
You also need to check is_present to know whether cache file presents or not. -
https://cs.chromium.org/chromium/src/out/Debug/gen/components/drive/drive.pb....

Also would it be better to check has_file_specific_info too since it's optional
field? -
https://cs.chromium.org/chromium/src/out/Debug/gen/components/drive/drive.pb....

[fukino, 2016-06-09 06:38:22]

Thanks!
Updated. Is this how you think to fix?

https://codereview.chromium.org/2048993002/diff/20001/components/drive/chrome...
File components/drive/chromeos/file_cache.cc (right):

https://codereview.chromium.org/2048993002/diff/20001/components/drive/chrome...
components/drive/chromeos/file_cache.cc:276: if
(base::GetFileSize(GetCacheFilePath(it->GetID()), &cache_size)) {
On 2016/06/09 05:19:19, yawano wrote:
> You also need to check is_present to know whether cache file presents or not.
-
>
https://cs.chromium.org/chromium/src/out/Debug/gen/components/drive/drive.pb....
> 
> Also would it be better to check has_file_specific_info too since it's
optional
> field? -
>
https://cs.chromium.org/chromium/src/out/Debug/gen/components/drive/drive.pb....

Done.

[yawano, 2016-06-09 06:45:31]

lgtm. Thank you!

[fukino, 2016-06-09 06:46:30]

On 2016/06/09 06:45:31, yawano wrote:
> lgtm. Thank you!

Thank you!

[fukino, 2016-06-09 06:46:45]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2016-06-09 06:46:57]

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

[commit-bot: I haz the power, 2016-06-09 07:28:42]

Description was changed from

==========
Add an API to get total size of Drive cache including non-evictable ones.

The API is needed to show the total size of Drive cache in storage manager.
This CL also changes the type of cache size to int64_t, to be consistent with
functions under base/files.

BUG=591958
TEST=manually
==========

to

==========
Add an API to get total size of Drive cache including non-evictable ones.

The API is needed to show the total size of Drive cache in storage manager.
This CL also changes the type of cache size to int64_t, to be consistent with
functions under base/files.

BUG=591958
TEST=manually
==========

[commit-bot: I haz the power, 2016-06-09 07:28:43]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-09 07:30:20]

Description was changed from

==========
Add an API to get total size of Drive cache including non-evictable ones.

The API is needed to show the total size of Drive cache in storage manager.
This CL also changes the type of cache size to int64_t, to be consistent with
functions under base/files.

BUG=591958
TEST=manually
==========

to

==========
Add an API to get total size of Drive cache including non-evictable ones.

The API is needed to show the total size of Drive cache in storage manager.
This CL also changes the type of cache size to int64_t, to be consistent with
functions under base/files.

BUG=591958
TEST=manually

Committed: https://crrev.com/6380c07c5be816838aa3e8027d32b1cc2404c9c9
Cr-Commit-Position: refs/heads/master@{#398805}
==========

[commit-bot: I haz the power, 2016-06-09 07:30:21]

Patchset 5 (id:??) landed as
https://crrev.com/6380c07c5be816838aa3e8027d32b1cc2404c9c9
Cr-Commit-Position: refs/heads/master@{#398805}