gdata: Add GDataDirectoryService::AddEntryToDirectory()

gdata: Add GDataDirectoryService::AddEntryToDirectory()

Replace uses of GDataDirectory::AddEntry() with the new function in tests.
Along the way, make GDataEntry::AddEntry() private.

BUG=137725
TEST=out/Release/unit_tests --gtest_filter=GData*  && out/Release/browser_tests --gtest_filter=GData*

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

[satorux1, 2012-07-30 23:34:47]

I haven't reviewed the patch myself yet so there may be rough edges (i need to
catch a bus soon), but it should be meaty enough for having your look.

[achuithb, 2012-07-31 00:06:52]

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_file_system.h (right):

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_file_system.h:545: static void
AddEntryToDirectoryAndCollectChangedDirectories(
Why are these moved here from anon namespace?

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_files.h (right):

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:362: // ...
I'm assuming comment is coming?

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:363: void AddEntryAt(const FilePath&
path, GDataEntry* entry,
Can we rename |path| to something more descriptive like parent_directory?

Why not just take GDataDirectory* instead?

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:390: static void
RefreshFileInternal(scoped_ptr<GDataFile> fresh_file,
Why is this moved from anon namespace?

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_test_util.h (right):

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_test_util.h:44: void
CopyResultFromFileOperationCallback(
This name is a bit misleading. CopyErrorCodeFromFileOperationCallback?

[satorux1, 2012-07-31 00:28:30]

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_file_system.h (right):

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_file_system.h:545: static void
AddEntryToDirectoryAndCollectChangedDirectories(
On 2012/07/31 00:06:52, achuith.bhandarkar wrote:
> Why are these moved here from anon namespace?

so that GDataDirectory::AddEntry() can be called. It's changed to private.

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_files.h (right):

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:362: // ...
On 2012/07/31 00:06:52, achuith.bhandarkar wrote:
> I'm assuming comment is coming?

yes. sorry!

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:363: void AddEntryAt(const FilePath&
path, GDataEntry* entry,
On 2012/07/31 00:06:52, achuith.bhandarkar wrote:
> Can we rename |path| to something more descriptive like parent_directory?

Will change it to directory_path

> 
> Why not just take GDataDirectory* instead?

GDataEntry/GDataFile/GDataDirectory* will be gone, so I'm minimizing the use of
these for API. GDataEntry* here was not avoidable here for now.

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:390: static void
RefreshFileInternal(scoped_ptr<GDataFile> fresh_file,
On 2012/07/31 00:06:52, achuith.bhandarkar wrote:
> Why is this moved from anon namespace?

so that GDataDirectory::AddEntry() can be called.

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_test_util.h (right):

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_test_util.h:44: void
CopyResultFromFileOperationCallback(
On 2012/07/31 00:06:52, achuith.bhandarkar wrote:
> This name is a bit misleading. CopyErrorCodeFromFileOperationCallback?

Sounds good. Will rename it accordingly.

[satorux1, 2012-07-31 07:13:22]

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_files.h (right):

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:362: // ...
On 2012/07/31 00:28:30, satorux1 wrote:
> On 2012/07/31 00:06:52, achuith.bhandarkar wrote:
> > I'm assuming comment is coming?
> 
> yes. sorry!

Done.

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:363: void AddEntryAt(const FilePath&
path, GDataEntry* entry,
On 2012/07/31 00:28:30, satorux1 wrote:
> On 2012/07/31 00:06:52, achuith.bhandarkar wrote:
> > Can we rename |path| to something more descriptive like parent_directory?
> 
> Will change it to directory_path
> 
> > 
> > Why not just take GDataDirectory* instead?
> 
> GDataEntry/GDataFile/GDataDirectory* will be gone, so I'm minimizing the use
of
> these for API. GDataEntry* here was not avoidable here for now. 

Done. Renamed it to AddEntryToDirectory() as well

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_test_util.h (right):

http://codereview.chromium.org/10828083/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_test_util.h:44: void
CopyResultFromFileOperationCallback(
On 2012/07/31 00:28:30, satorux1 wrote:
> On 2012/07/31 00:06:52, achuith.bhandarkar wrote:
> > This name is a bit misleading. CopyErrorCodeFromFileOperationCallback?
> 
> Sounds good. Will rename it accordingly.

Done.

[achuithb, 2012-07-31 09:22:07]

lgtm

[achuithb, 2012-07-31 09:22:54]

On 2012/07/31 09:22:07, achuith.bhandarkar wrote:
> lgtm

Please also update the subject line of this CL.