gdata: Add GetEntryInfoByPath() and ReadDirectoryByPath() to GDataDirectoryService

chrome/browser/chromeos/gdata/gdata_files.cc

Index: chrome/browser/chromeos/gdata/gdata_files.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_files.cc b/chrome/browser/chromeos/gdata/gdata_files.cc
index d49b5f41af3e3c00001e0731dc018dbf78549cc1..c63acd67544d7f54769a36832ffdf33f1b856fa1 100644
--- a/chrome/browser/chromeos/gdata/gdata_files.cc
+++ b/chrome/browser/chromeos/gdata/gdata_files.cc
@@ -677,6 +677,70 @@ void GDataDirectoryService::GetEntryByResourceIdAsync(
   callback.Run(entry);
 }
 
+void GDataDirectoryService::GetEntryInfoByPath(
+    const FilePath& path,
+    const GetEntryInfoCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  scoped_ptr<GDataEntryProto> entry_proto;
+  GDataFileError error = GDATA_FILE_ERROR_FAILED;
+
+  GDataEntry* entry = FindEntryByPathSync(path);
+  if (entry) {
+    entry_proto.reset(new GDataEntryProto);
+    entry->ToProtoFull(entry_proto.get());
+    error = GDATA_FILE_OK;
+  } else {
+    error = GDATA_FILE_ERROR_NOT_FOUND;
+  }
+
+  if (!callback.is_null()) {

[achuithb, 2012/08/07 20:46:07]

Should this be a DCHECK? You should probably bail at the top of the function if
callback is null.

[satorux1, 2012/08/07 21:13:20]

Made it a DCHECK. There is no point of calling this function without a callback.

+    base::MessageLoopProxy::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback, error, base::Passed(&entry_proto)));
+  }
+}
+
+void GDataDirectoryService::ReadDirectoryByPath(
+    const FilePath& path,
+    const ReadDirectoryCallback& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  scoped_ptr<GDataEntryProtoVector> entries;
+  GDataFileError error = GDATA_FILE_ERROR_FAILED;
+
+  GDataEntry* entry = FindEntryByPathSync(path);
+  if (entry && entry->AsGDataDirectory()) {

[achuithb, 2012/08/07 20:46:07]

I feel like this would be easier to read if you used nested if statements.
if (entry) {
  GDataDirectory* directory = entry->AsGDataDirectory();
  if (directory) {
    ...
  } else {
    error = GDATA_FILE_ERROR_NOT_A_DIRECTORY;
  }
}

But I guess it's fine as it is.

[satorux1, 2012/08/07 21:13:20]

Wanted to save one indentation level, so keep it as is.

+    GDataDirectory* directory = entry->AsGDataDirectory();
+    entries.reset(new GDataEntryProtoVector);
+    for (GDataFileCollection::const_iterator iter =
+             directory->child_files().begin();
+         iter != directory->child_files().end(); ++iter) {
+      GDataEntryProto proto;
+      static_cast<const GDataEntry*>(iter->second)->ToProtoFull(&proto);

[achuithb, 2012/08/07 20:46:07]

Please add a comment as to why this static_cast is necessary.

[satorux1, 2012/08/07 21:13:20]

Done. Turned out static_cast was not needed here. To be a good citizen, made it
a function, and removed the duplicate code elsewhere :)

+      entries->push_back(proto);
+    }
+    for (GDataDirectoryCollection::const_iterator iter =
+             directory->child_directories().begin();
+         iter != directory->child_directories().end(); ++iter) {
+      GDataEntryProto proto;
+      static_cast<const GDataEntry*>(iter->second)->ToProtoFull(&proto);
+      entries->push_back(proto);
+    }
+    error = GDATA_FILE_OK;
+  } else if (entry && !entry->AsGDataDirectory()) {
+    error = GDATA_FILE_ERROR_NOT_A_DIRECTORY;
+  } else {
+    error = GDATA_FILE_ERROR_NOT_FOUND;

[achuithb, 2012/08/07 20:46:07]

Don't really need this but I suppose if you want to be explicit.

[satorux1, 2012/08/07 21:13:20]

Yes

+  }
+
+  if (!callback.is_null()) {

[achuithb, 2012/08/07 20:46:07]

Same as above. Don't think there's a point to this function if callback is null.

[satorux1, 2012/08/07 21:13:20]

Done.

+    base::MessageLoopProxy::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback, error, base::Passed(&entries)));
+  }
+}
+
 void GDataDirectoryService::RefreshFile(scoped_ptr<GDataFile> fresh_file) {
   DCHECK(fresh_file.get());