Correct cache-size calculation of MetadataCache.

Correct cache-size calculation of MetadataCache.

- Make sure that the DirectoryContent.dispose() is called when necessary.
- Defer dispose() temporarily when we need to replace directory contents.

BUG=408893
TEST=watched the cache size manually, and ran browser_tests

Committed: https://crrev.com/bc9ca87d6261fc87939aeac01879301e4486ec22
Cr-Commit-Position: refs/heads/master@{#292593}

[fukino, 2014-08-29 05:39:14]

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

[fukino, 2014-08-29 05:39:14]

yoshiki@, PTAL the CL? Thanks!

[yoshiki, 2014-08-29 05:51:32]

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

https://codereview.chromium.org/520553002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_contents.js:569:
this.lastSpaceInMetadataCache_ = 0;
Our of curiosity, we shouldn't use the object after calling its dispose method.
Does your change means that some code still violate this rule? Or it's just in
case?

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

https://codereview.chromium.org/520553002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_model.js:614: var
previousDirContents = this.currentDirContents_;
Can't we cover this block with "if (this.currentDirContents_ !== dirContents)"?
It's simpler, I think.

[fukino, 2014-08-29 06:59:33]

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

https://codereview.chromium.org/520553002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_contents.js:569:
this.lastSpaceInMetadataCache_ = 0;
On 2014/08/29 05:51:32, yoshiki wrote:
> Our of curiosity, we shouldn't use the object after calling its dispose
method.
> Does your change means that some code still violate this rule? Or it's just in
> case?

It is just for clarification that the lastSpaceInMetadataCash_ must be
synchronized with the requested cache size. Is it better to remove it?

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

https://codereview.chromium.org/520553002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_model.js:614: var
previousDirContents = this.currentDirContents_;
On 2014/08/29 05:51:32, yoshiki wrote:
> Can't we cover this block with "if (this.currentDirContents_ !==
dirContents)"?
> It's simpler, I think.

As per the current implementation, this.currentDirContents_ and the given
dirContents of the function must be different instance.
So I simply removed if statement (and added comment/assertion for function
parameter.)

[yoshiki, 2014-08-29 07:46:52]

lgtm with a nit

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

https://codereview.chromium.org/520553002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_contents.js:569:
this.lastSpaceInMetadataCache_ = 0;
On 2014/08/29 06:59:33, fukino wrote:
> On 2014/08/29 05:51:32, yoshiki wrote:
> > Our of curiosity, we shouldn't use the object after calling its dispose
> method.
> > Does your change means that some code still violate this rule? Or it's just
in
> > case?
> 
> It is just for clarification that the lastSpaceInMetadataCash_ must be
> synchronized with the requested cache size. Is it better to remove it?

Got it. Please add a brief comment. Thanks

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

https://codereview.chromium.org/520553002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_model.js:614: var
previousDirContents = this.currentDirContents_;
On 2014/08/29 06:59:33, fukino wrote:
> On 2014/08/29 05:51:32, yoshiki wrote:
> > Can't we cover this block with "if (this.currentDirContents_ !==
> dirContents)"?
> > It's simpler, I think.
> 
> As per the current implementation, this.currentDirContents_ and the given
> dirContents of the function must be different instance.
> So I simply removed if statement (and added comment/assertion for function
> parameter.)

sgtm

[fukino, 2014-08-29 07:56:46]

Thanks!

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

https://codereview.chromium.org/520553002/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/directory_contents.js:569:
this.lastSpaceInMetadataCache_ = 0;
On 2014/08/29 07:46:52, yoshiki wrote:
> On 2014/08/29 06:59:33, fukino wrote:
> > On 2014/08/29 05:51:32, yoshiki wrote:
> > > Our of curiosity, we shouldn't use the object after calling its dispose
> > method.
> > > Does your change means that some code still violate this rule? Or it's
just
> in
> > > case?
> > 
> > It is just for clarification that the lastSpaceInMetadataCash_ must be
> > synchronized with the requested cache size. Is it better to remove it?
> 
> Got it. Please add a brief comment. Thanks

Done.

[fukino, 2014-08-29 07:57:09]

The CQ bit was checked by fukino@chromium.org

[commit-bot: I haz the power, 2014-08-29 07:57:27]

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

[commit-bot: I haz the power, 2014-08-29 08:53:29]

Committed patchset #4 (id:60001) as 1a6301b10065a2e303e11acee14486955dc9c828

[commit-bot: I haz the power, 2014-09-10 03:07:14]

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