drive: Move FileSystem::LoadDirectoryIfNeeded to ChangeListLoader

drive: Move FileSystem::LoadDirectoryIfNeeded() to ChangeListLoader

ChangeListLoader::LoadIfNeeded()'s only user in the production code is FileSystem::LoadDiretoryIfNeeded().
It means that only test related code is passing empty DirectoryFetchInfo for LoadIfNeeded().

Move FileSystem::LoadDirectoryIfNeeded() to ChangeListLoader.
Merge LoadIfNeeded() to LoadDirectoryIfNeeded().
Add LoadForTesting() for the sake of test code.

BUG=260526
TEST=unit_tests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242081

[kinaba, 2013-12-20 04:54:14]

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/change_list_loader.cc (right):

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/change_list_loader.cc:422:
directory_path.DirName(),
This recursive call implicitly assume that it eventually escapes
FILE_ERROR_NOT_FOUND, in other words, the path is always an ancestor of /drive.

I think it always holds so there's no problem, but to be explicit, could you add
a check or a comment on that here?

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/change_list_loader.cc:448: if (loaded_ &&
!IsRefreshing()) {
Can we move this check to the very beginning of LoadDirectoryIfNeeded?
I think you put it here for not to change the existing code, but if after all it
returns without updating the resource metadata, the recursive access to DB
doesn't look quite meaningful to me.

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/file_system.cc (right):

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/file_system.cc:245: callback);
drive-internals has a button labeled "Clear local data and reload". If you do
this change (I think it's fine), you might want to change the text there by
dropping the words "and reload".

[hashimoto, 2013-12-20 06:29:28]

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/change_list_loader.cc (right):

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/change_list_loader.cc:422:
directory_path.DirName(),
On 2013/12/20 04:54:14, kinaba wrote:
> This recursive call implicitly assume that it eventually escapes
> FILE_ERROR_NOT_FOUND, in other words, the path is always an ancestor of
/drive.
> 
> I think it always holds so there's no problem, but to be explicit, could you
add
> a check or a comment on that here?

Moved a check from FileSystem::GetResourceEntry.

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/change_list_loader.cc:448: if (loaded_ &&
!IsRefreshing()) {
On 2013/12/20 04:54:14, kinaba wrote:
> Can we move this check to the very beginning of LoadDirectoryIfNeeded?
> I think you put it here for not to change the existing code, but if after all
it
> returns without updating the resource metadata, the recursive access to DB
> doesn't look quite meaningful to me.

Sounds good, but this results in LoadDirectoryIfNeeded returns FILE_ERROR_OK
even when the directory does not exist in the local metadata.
Changed user code to ignore result.

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/drive/file_system.cc (right):

https://codereview.chromium.org/118993002/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/drive/file_system.cc:245: callback);
On 2013/12/20 04:54:14, kinaba wrote:
> drive-internals has a button labeled "Clear local data and reload". If you do
> this change (I think it's fine), you might want to change the text there by
> dropping the words "and reload".

Changed the text and renamed Reload() to Reset().

[kinaba, 2013-12-20 06:53:40]

lgtm

[commit-bot: I haz the power, 2013-12-20 07:22:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/118993002/160001

[commit-bot: I haz the power, 2013-12-20 12:53:11]

Change committed as 242081