child_files_ and child_directories_ have resource_ids instead of GDataFile* and GDataDirectory*.

* GDataFileCollection and GDataDirectoryCollection are replaced by GDataChildMap which has resource_id as value instead of GDataFile* and GDataDirectory*.
* FindChild returns a resource_id instead of GDataEntry*
* Get rid of AddChild as it's only being used in one place.
* TakeOverEntries return type is now void instead of bool.
* Add helper TakeOverEntry to avoid cut n paste.

BUG=137725
TEST=unit tests.
Review URL: https://chromiumcodereview.appspot.com/10855243

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@152415 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/gdata/gdata_directory_service.cc

@@ -300,10 +300,13 @@ GDataEntry* GDataDirectoryService::FindEntryByPathSync(
   GDataDirectory* current_dir = root_.get();
 
   for (size_t i = 1; i < components.size() && current_dir; ++i) {
-    GDataEntry* entry = current_dir->FindChild(components[i]);
-    if (!entry)
+    std::string resource_id = current_dir->FindChild(components[i]);
+    if (resource_id.empty())
       return NULL;
 
+    GDataEntry* entry = GetEntryByResourceId(resource_id);
+    DCHECK(entry);
+
     if (i == components.size() - 1)  // Last component.
       return entry;
     else
@@ -313,9 +316,9 @@ GDataEntry* GDataDirectoryService::FindEntryByPathSync(
 }
 
 GDataEntry* GDataDirectoryService::GetEntryByResourceId(
-    const std::string& resource) {
-  // GDataFileSystem has already locked.
-  ResourceMap::const_iterator iter = resource_map_.find(resource);
+    const std::string& resource_id) {
+  DCHECK(!resource_id.empty());
+  ResourceMap::const_iterator iter = resource_map_.find(resource_id);
   return iter == resource_map_.end() ? NULL : iter->second;
 }
 

chrome/browser/chromeos/gdata/gdata_files.cc

@@ -217,7 +217,7 @@ void GDataDirectory::AddEntry(GDataEntry* entry) {
   FilePath full_file_name(entry->base_name());
   const std::string extension = full_file_name.Extension();
   const std::string file_name = full_file_name.RemoveExtension().value();
-  while (FindChild(full_file_name.value())) {
+  while (!FindChild(full_file_name.value()).empty()) {
     if (!extension.empty()) {
       full_file_name = FilePath(base::StringPrintf("%s (%d)%s",
                                                    file_name.c_str(),
@@ -240,30 +240,36 @@ void GDataDirectory::AddEntry(GDataEntry* entry) {
   directory_service_->AddEntryToResourceMap(entry);
 
   // Setup child and parent links.
-  AddChild(entry);
+  if (entry->AsGDataFile())
+    child_files_.insert(std::make_pair(entry->base_name(),
+                                       entry->resource_id()));
+
+  if (entry->AsGDataDirectory())
+    child_directories_.insert(std::make_pair(entry->base_name(),
+                                             entry->resource_id()));
   entry->SetParent(this);
 }
 
-bool GDataDirectory::TakeOverEntries(GDataDirectory* dir) {
-  for (GDataFileCollection::const_iterator iter = dir->child_files_.begin();
+void GDataDirectory::TakeOverEntries(GDataDirectory* dir) {
+  for (GDataChildMap::const_iterator iter = dir->child_files_.begin();
        iter != dir->child_files_.end(); ++iter) {
-    GDataEntry* entry = iter->second;
-    directory_service_->RemoveEntryFromResourceMap(entry->resource_id());
-    entry->SetParent(NULL);
-    AddEntry(entry);
+    TakeOverEntry(iter->second);
   }
   dir->child_files_.clear();
 
-  for (GDataDirectoryCollection::iterator iter =
-      dir->child_directories_.begin();
+  for (GDataChildMap::iterator iter = dir->child_directories_.begin();
        iter != dir->child_directories_.end(); ++iter) {
-    GDataEntry* entry = iter->second;
-    directory_service_->RemoveEntryFromResourceMap(entry->resource_id());
-    entry->SetParent(NULL);
-    AddEntry(entry);
+    TakeOverEntry(iter->second);
   }
   dir->child_directories_.clear();
-  return true;
+}
+
+void GDataDirectory::TakeOverEntry(const std::string& resource_id) {
+  GDataEntry* entry = directory_service_->GetEntryByResourceId(resource_id);
+  DCHECK(entry);
+  directory_service_->RemoveEntryFromResourceMap(resource_id);
+  entry->SetParent(NULL);
+  AddEntry(entry);
 }
 
 void GDataDirectory::RemoveEntry(GDataEntry* entry) {
@@ -273,30 +279,17 @@ void GDataDirectory::RemoveEntry(GDataEntry* entry) {
   delete entry;
 }
 
-GDataEntry* GDataDirectory::FindChild(
+std::string GDataDirectory::FindChild(
     const FilePath::StringType& file_name) const {
-  GDataFileCollection::const_iterator iter = child_files_.find(file_name);
+  GDataChildMap::const_iterator iter = child_files_.find(file_name);
   if (iter != child_files_.end())
     return iter->second;
 
-  GDataDirectoryCollection::const_iterator itd =
-      child_directories_.find(file_name);
-  if (itd != child_directories_.end())
-    return itd->second;
-
-  return NULL;
-}
-
-void GDataDirectory::AddChild(GDataEntry* entry) {
-  DCHECK(entry);
-
-  if (entry->AsGDataFile())
-    child_files_.insert(std::make_pair(entry->base_name(),
-                                       entry->AsGDataFile()));
+  iter = child_directories_.find(file_name);
+  if (iter != child_directories_.end())
+    return iter->second;
 
-  if (entry->AsGDataDirectory())
-    child_directories_.insert(std::make_pair(entry->base_name(),
-                                             entry->AsGDataDirectory()));
+  return std::string();
 }
 
 void GDataDirectory::RemoveChild(GDataEntry* entry) {
@@ -304,7 +297,7 @@ void GDataDirectory::RemoveChild(GDataEntry* entry) {
 
   const std::string& base_name(entry->base_name());
   // entry must be present in this directory.
-  DCHECK_EQ(entry, FindChild(base_name));
+  DCHECK_EQ(entry->resource_id(), FindChild(base_name));
   // Remove entry from resource map first.
   directory_service_->RemoveEntryFromResourceMap(entry->resource_id());
 
@@ -322,32 +315,38 @@ void GDataDirectory::RemoveChildren() {
 
 void GDataDirectory::RemoveChildFiles() {
   DVLOG(1) << "RemoveChildFiles " << resource_id();
-  for (GDataFileCollection::const_iterator iter = child_files_.begin();
+  for (GDataChildMap::const_iterator iter = child_files_.begin();
        iter != child_files_.end(); ++iter) {
-    directory_service_->RemoveEntryFromResourceMap(iter->second->resource_id());
+    GDataEntry* child = directory_service_->GetEntryByResourceId(iter->second);
+    DCHECK(child);
+    directory_service_->RemoveEntryFromResourceMap(iter->second);
+    delete child;
   }
-  STLDeleteValues(&child_files_);
   child_files_.clear();
 }
 
 void GDataDirectory::RemoveChildDirectories() {
-  for (GDataDirectoryCollection::iterator iter = child_directories_.begin();
+  for (GDataChildMap::iterator iter = child_directories_.begin();
        iter != child_directories_.end(); ++iter) {
-    GDataDirectory* dir = iter->second;
+    GDataDirectory* dir = directory_service_->GetEntryByResourceId(
+        iter->second)->AsGDataDirectory();
+    DCHECK(dir);
     // Remove directories recursively.
     dir->RemoveChildren();
-    directory_service_->RemoveEntryFromResourceMap(dir->resource_id());
+    directory_service_->RemoveEntryFromResourceMap(iter->second);
+    delete dir;
   }
-  STLDeleteValues(&child_directories_);
   child_directories_.clear();
 }
 
 void GDataDirectory::GetChildDirectoryPaths(std::set<FilePath>* child_dirs) {
-  for (GDataDirectoryCollection::const_iterator it = child_directories_.begin();
-       it != child_directories_.end(); ++it) {
-    GDataDirectory* child_dir = it->second;
-    child_dirs->insert(child_dir->GetFilePath());
-    child_dir->GetChildDirectoryPaths(child_dirs);
+  for (GDataChildMap::const_iterator iter = child_directories_.begin();
+       iter != child_directories_.end(); ++iter) {
+    GDataDirectory* dir = directory_service_->GetEntryByResourceId(
+        iter->second)->AsGDataDirectory();
+    DCHECK(dir);
+    child_dirs->insert(dir->GetFilePath());
+    dir->GetChildDirectoryPaths(child_dirs);
   }
 }
 
@@ -470,33 +469,35 @@ void GDataDirectory::ToProto(GDataDirectoryProto* proto) const {
   GDataEntry::ToProto(proto->mutable_gdata_entry());
   DCHECK(proto->gdata_entry().file_info().is_directory());
 
-  for (GDataFileCollection::const_iterator iter = child_files_.begin();
+  for (GDataChildMap::const_iterator iter = child_files_.begin();
        iter != child_files_.end(); ++iter) {
-    GDataFile* file = iter->second;
+    GDataFile* file = directory_service_->GetEntryByResourceId(
+        iter->second)->AsGDataFile();
+    DCHECK(file);
     file->ToProto(proto->add_child_files());
   }
-  for (GDataDirectoryCollection::const_iterator iter =
-       child_directories_.begin();
+  for (GDataChildMap::const_iterator iter = child_directories_.begin();
        iter != child_directories_.end(); ++iter) {
-    GDataDirectory* dir = iter->second;
+    GDataDirectory* dir = directory_service_->GetEntryByResourceId(
+        iter->second)->AsGDataDirectory();
+    DCHECK(dir);
     dir->ToProto(proto->add_child_directories());
   }
 }
 
 scoped_ptr<GDataEntryProtoVector> GDataDirectory::ToProtoVector() const {
   scoped_ptr<GDataEntryProtoVector> entries(new GDataEntryProtoVector);
-  for (GDataFileCollection::const_iterator iter = child_files_.begin();
+  // Use ToProtoFull, as we don't want to include children in |proto|.
+  for (GDataChildMap::const_iterator iter = child_files_.begin();
        iter != child_files_.end(); ++iter) {
     GDataEntryProto proto;
-    iter->second->ToProto(&proto);
+    directory_service_->GetEntryByResourceId(iter->second)->ToProtoFull(&proto);
     entries->push_back(proto);
   }
-  for (GDataDirectoryCollection::const_iterator iter =
-           child_directories_.begin();
+  for (GDataChildMap::const_iterator iter = child_directories_.begin();
        iter != child_directories_.end(); ++iter) {
     GDataEntryProto proto;
-    // Convert to GDataEntry, as we don't want to include children in |proto|.
-    static_cast<const GDataEntry*>(iter->second)->ToProtoFull(&proto);
+    directory_service_->GetEntryByResourceId(iter->second)->ToProtoFull(&proto);
     entries->push_back(proto);
   }
 

chrome/browser/chromeos/gdata/gdata_files.h

@@ -252,14 +252,15 @@ class GDataDirectory : public GDataEntry {
 
   // Takes over all entries from |dir|.
   // TODO(satorux): Remove this. crbug.com/139649
-  bool TakeOverEntries(GDataDirectory* dir);
+  void TakeOverEntries(GDataDirectory* dir);
 
-  // Find a child by its name.
-  // TODO(satorux): Remove this. crbug.com/139649
-  GDataEntry* FindChild(const FilePath::StringType& file_name) const;
+  // Takes over entry represented by |resource_id|. Helper function for
+  // TakeOverEntries. TODO(satorux): Remove this. crbug.com/139649
+  void TakeOverEntry(const std::string& resource_id);
 
-  // Add |entry| to children.
-  void AddChild(GDataEntry* entry);
+  // Find a child's resource_id by its name. Returns the empty string if not
+  // found. TODO(satorux): Remove this. crbug.com/139649
+  std::string FindChild(const FilePath::StringType& file_name) const;
 
   // Removes the entry from its children without destroying the
   // entry instance.
@@ -273,13 +274,11 @@ class GDataDirectory : public GDataEntry {
   // Recursively extracts the paths set of all sub-directories.
   void GetChildDirectoryPaths(std::set<FilePath>* child_dirs);
 
-  // Maps between base_name and resource_id of files and directories.
-  typedef std::map<FilePath::StringType, GDataFile*> GDataFileCollection;
-  typedef std::map<FilePath::StringType, GDataDirectory*>
-      GDataDirectoryCollection;
+  // Map between base_name and resource_id of files and directories.
+  typedef std::map<FilePath::StringType, std::string> GDataChildMap;
   // Collection of children.
-  GDataFileCollection child_files_;
-  GDataDirectoryCollection child_directories_;
+  GDataChildMap child_files_;
+  GDataChildMap child_directories_;
 
   DISALLOW_COPY_AND_ASSIGN(GDataDirectory);
 };