Simplify implementation of FindEntryByPathSync

chrome/browser/chromeos/gdata/gdata_files.cc

Index: chrome/browser/chromeos/gdata/gdata_files.cc
===================================================================
--- chrome/browser/chromeos/gdata/gdata_files.cc	(revision 150246)
+++ chrome/browser/chromeos/gdata/gdata_files.cc	(working copy)
@@ -64,6 +64,27 @@
   return true;
 }
 
+void FindNextComponent(std::vector<FilePath::StringType> components,

[satorux1, 2012/08/07 10:43:46]

function comment is missing.

+                       const FindEntryCallback& callback,
+                       GDataFileError error,
+                       GDataEntry* entry) {
+  if (components.size() == 0 || !entry) {
+    callback.Run(error, entry);
+    return;
+  }
+
+  GDataDirectory* current_dir = entry->AsGDataDirectory();
+  if (!current_dir) {
+    callback.Run(GDATA_FILE_ERROR_NOT_FOUND, NULL);
+    return;
+  }
+
+  FilePath::StringType file_name = components.back();
+  components.pop_back();
+  current_dir->FindChildAsync(file_name,
+      base::Bind(&FindNextComponent, components, callback));
+}
+
 }  // namespace
 
 // GDataEntry class.
@@ -350,14 +371,11 @@
   return true;
 }
 
-bool GDataDirectory::RemoveEntry(GDataEntry* entry) {
+void GDataDirectory::RemoveEntry(GDataEntry* entry) {
   DCHECK(entry);
 
-  if (!RemoveChild(entry))
-    return false;
-
+  RemoveChild(entry);
   delete entry;
-  return true;
 }
 
 GDataEntry* GDataDirectory::FindChild(
@@ -374,6 +392,13 @@
   return NULL;
 }
 
+void GDataDirectory::FindChildAsync(
+    const FilePath::StringType& file_name,
+    const FindEntryCallback& callback) const {
+  GDataEntry* entry = FindChild(file_name);
+  callback.Run(entry ? GDATA_FILE_OK : GDATA_FILE_ERROR_NOT_FOUND, entry);

[satorux1, 2012/08/07 10:43:46]

To make it more asynchronous, can you post the callback to the current message
loop, rather than calling it directly here?

+}
+
 void GDataDirectory::AddChild(GDataEntry* entry) {
   DCHECK(entry);
 
@@ -386,16 +411,10 @@
     child_directories_.insert(std::make_pair(entry->base_name(), directory));
 }
 
-bool GDataDirectory::RemoveChild(GDataEntry* entry) {
+void GDataDirectory::RemoveChild(GDataEntry* entry) {
   DCHECK(entry);
 
-  const std::string file_name(entry->base_name());
-  GDataEntry* found_entry = FindChild(file_name);
-  if (!found_entry)
-    return false;
-
-  DCHECK_EQ(entry, found_entry);
-
+  const std::string& file_name(entry->base_name());
   // Remove entry from resource map first.
   if (directory_service_)
     directory_service_->RemoveEntryFromResourceMap(entry);
@@ -403,8 +422,6 @@
   // Then delete it from tree.
   child_files_.erase(file_name);
   child_directories_.erase(file_name);
-
-  return true;
 }
 
 void GDataDirectory::RemoveChildren() {
@@ -619,39 +636,22 @@
 
 GDataEntry* GDataDirectoryService::FindEntryByPathSync(
     const FilePath& file_path) {
+  if (file_path == root_->GetFilePath())
+    return root_.get();
+
   std::vector<FilePath::StringType> components;
   file_path.GetComponents(&components);
-
   GDataDirectory* current_dir = root_.get();
-  FilePath directory_path;
 
-  for (size_t i = 0; i < components.size() && current_dir; i++) {
-    directory_path = directory_path.Append(current_dir->base_name());
-
-    // Last element must match, if not last then it must be a directory.
-    if (i == components.size() - 1) {
-      if (current_dir->base_name() == components[i])
-        return current_dir;
-      else
-        return NULL;
-    }
-
-    // Not the last part of the path, search for the next segment.
-    GDataEntry* entry = current_dir->FindChild(components[i + 1]);
-    if (!entry) {
+  for (size_t i = 1; i < components.size() && current_dir; ++i) {
+    GDataEntry* entry = current_dir->FindChild(components[i]);
+    if (!entry)
       return NULL;
-    }
 
-    // Found file, must be the last segment.
-    if (entry->file_info().is_directory) {
-      // Found directory, continue traversal.
+    if (i == components.size() - 1)  // Last component.
+      return entry;
+    else

[satorux1, 2012/08/07 10:43:46]

I'm rather confused. Simplifying the FindEntryByPathSync() is a good thing, but
it's logically a separate change. I'd suggest to do this in a separate patch.

       current_dir = entry->AsGDataDirectory();
-    } else {
-      if ((i + 1) == (components.size() - 1))
-        return entry;
-      else
-        return NULL;
-    }
   }
   return NULL;
 }
@@ -659,8 +659,16 @@
 void GDataDirectoryService::FindEntryByPathAndRunSync(
     const FilePath& search_file_path,
     const FindEntryCallback& callback) {
-  GDataEntry* entry = FindEntryByPathSync(search_file_path);
-  callback.Run(entry ? GDATA_FILE_OK : GDATA_FILE_ERROR_NOT_FOUND, entry);
+  if (search_file_path == root_->GetFilePath()) {
+    callback.Run(GDATA_FILE_OK, root_.get());
+    return;
+  }
+
+  std::vector<FilePath::StringType> components;
+  search_file_path.GetComponents(&components);
+  std::reverse(components.begin(), components.end());
+  components.pop_back();  // Pop root.
+  FindNextComponent(components, callback, GDATA_FILE_OK, root_.get());
 }
 
 GDataEntry* GDataDirectoryService::GetEntryByResourceId(