Simplify implementation of FindEntryByPathSync

chrome/browser/chromeos/gdata/gdata_files.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/gdata/gdata_files.h"
 
 #include <leveldb/db.h>
 #include <vector>
 
 #include "base/message_loop_proxy.h"
@@ skipping 46 matching lines @@
   // the proto data if the older format is detected.
   if (entry_proto.resource_id() != kGDataRootDirectoryResourceId) {
     LOG(ERROR) << "Incompatible proto detected (bad resource ID): "
                << entry_proto.resource_id();
     return false;
   }
 
   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.
 
 GDataEntry::GDataEntry(GDataDirectory* parent,
                        GDataDirectoryService* directory_service)
     : directory_service_(directory_service),
       deleted_(false) {
   SetParent(parent);
 }
@@ skipping 266 matching lines @@
 
   for (GDataDirectoryCollection::iterator iter =
       dir->child_directories_.begin();
        iter != dir->child_directories_.end(); ++iter) {
     AddEntry(iter->second);
   }
   dir->child_directories_.clear();
   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(
     const FilePath::StringType& file_name) const {
   GDataFileCollection::const_iterator it = child_files_.find(file_name);
   if (it != child_files_.end())
     return it->second;
 
   GDataDirectoryCollection::const_iterator itd =
       child_directories_.find(file_name);
   if (itd != child_directories_.end())
     return itd->second;
 
   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);
 
   GDataFile* file = entry->AsGDataFile();
   if (file)
     child_files_.insert(std::make_pair(entry->base_name(), file));
 
   GDataDirectory* directory = entry->AsGDataDirectory();
   if (directory)
     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);
 
   // Then delete it from tree.
   child_files_.erase(file_name);
   child_directories_.erase(file_name);
-
-  return true;
 }
 
 void GDataDirectory::RemoveChildren() {
   RemoveChildFiles();
   RemoveChildDirectories();
 }
 
 void GDataDirectory::RemoveChildFiles() {
   for (GDataFileCollection::const_iterator iter = child_files_.begin();
        iter != child_files_.end(); ++iter) {
@@ skipping 194 matching lines @@
   resource_map_.insert(std::make_pair(entry->resource_id(), entry));
 }
 
 void GDataDirectoryService::RemoveEntryFromResourceMap(GDataEntry* entry) {
   // GDataFileSystem has already locked.
   resource_map_.erase(entry->resource_id());
 }
 
 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();
 
-  GDataDirectory* current_dir = root_.get();
-  FilePath directory_path;
+  for (size_t i = 1; i < components.size() && current_dir; ++i) {
+    GDataEntry* entry = current_dir->FindChild(components[i]);
+    if (!entry)
+      return NULL;
 
-  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) {
-      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;
 }
 
 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(
     const std::string& resource) {
   // GDataFileSystem has already locked.
   ResourceMap::const_iterator iter = resource_map_.find(resource);
   return iter == resource_map_.end() ? NULL : iter->second;
 }
 
 void GDataDirectoryService::GetEntryByResourceIdAsync(
@@ skipping 423 matching lines @@
     if (file->FromProto(entry_proto)) {
       entry.reset(file.release());
     } else {
       NOTREACHED() << "FromProto (file) failed";
     }
   }
   return entry.Pass();
 }
 
 }  // namespace gdata