gdata: Add GDataDirectoryService::GetEntryInfoPairByPaths()

gdata: Add GDataDirectoryService::GetEntryInfoPairByPaths()

This function will be used to replace two calls of FindEntryByPathSync()
placed in one function (ex. GDataFileSystem::CopyOnUIThread()).
With this function we expect to be able to eliminate 8 callers of
FindEntryByPathSync().

BUG=137694
TEST=added unit tests

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

[satorux1, 2012-08-08 20:04:11]

A weapon to shave more sync callers.

[achuithb, 2012-08-08 20:50:32]

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

http://codereview.chromium.org/10831212/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.cc:1188: return;
Why is this necessary? Why not look for the second one?

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

http://codereview.chromium.org/10831212/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:97: FilePath first_path;
Have you considered creating another struct like
struct EntryInfoResult {
  EntryInfoResult(const FilePath& path, GDataFileError error,
scoped_ptr<GDataEntryProto> proto);
  FilePath path; 
  GDataFileError error;
  scoped_ptr<GDataEntryProto> proto;
}

and then
struct EntryInfoPairResult {
  EntryInfoResult first;
  EntryInfoResult second;
};

I think this will make the code cleaner (especially having the ctor for
EntryInfoResult).

[satorux1, 2012-08-08 21:21:21]

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

http://codereview.chromium.org/10831212/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.cc:1188: return;
On 2012/08/08 20:50:32, achuith.bhandarkar wrote:
> Why is this necessary? Why not look for the second one?

Looking at the code in gdata_file_system.cc, we don't need to look for the
second one if the first one is not found. We can revisit this at a later time,
if there is an exception.

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

http://codereview.chromium.org/10831212/diff/1/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.h:97: FilePath first_path;
On 2012/08/08 20:50:32, achuith.bhandarkar wrote:
> Have you considered creating another struct like
> struct EntryInfoResult {
>   EntryInfoResult(const FilePath& path, GDataFileError error,
> scoped_ptr<GDataEntryProto> proto);
>   FilePath path; 
>   GDataFileError error;
>   scoped_ptr<GDataEntryProto> proto;
> }
> 
> and then
> struct EntryInfoPairResult {
>   EntryInfoResult first;
>   EntryInfoResult second;
> };
> 
> I think this will make the code cleaner (especially having the ctor for
> EntryInfoResult).

Considered but I was too lazy to rewrite. :) Reconsidered and changed per your
suggestion.

[achuithb, 2012-08-08 22:02:47]

lgtm

http://codereview.chromium.org/10831212/diff/7/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_files.cc (right):

http://codereview.chromium.org/10831212/diff/7/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.cc:1186: result->first.path =
first_path;
You could optionally add a member like
void EntryInfoPair::Init(const FilePath&, GDataFileError,
scoped_ptr<GDataEntryProto>); to do this in one line. Up to you.

http://codereview.chromium.org/10831212/diff/7/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.cc:1191: callback.Run(result.Pass());
Let's also add a comment here saying that if the first one fails, we don't
continue.

[satorux1, 2012-08-08 22:13:34]

http://codereview.chromium.org/10831212/diff/7/chrome/browser/chromeos/gdata/...
File chrome/browser/chromeos/gdata/gdata_files.cc (right):

http://codereview.chromium.org/10831212/diff/7/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.cc:1186: result->first.path =
first_path;
On 2012/08/08 22:02:47, achuith.bhandarkar wrote:
> You could optionally add a member like
> void EntryInfoPair::Init(const FilePath&, GDataFileError,
> scoped_ptr<GDataEntryProto>); to do this in one line. Up to you.

Thank you for the suggestion, but I'd keep this as is.

http://codereview.chromium.org/10831212/diff/7/chrome/browser/chromeos/gdata/...
chrome/browser/chromeos/gdata/gdata_files.cc:1191: callback.Run(result.Pass());
On 2012/08/08 22:02:47, achuith.bhandarkar wrote:
> Let's also add a comment here saying that if the first one fails, we don't
> continue.

Done.