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

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 "base/platform_file.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
@@ skipping 199 matching lines @@
   // We need to first restore the file name based on the title before going
   // through name de-duplication again when it is added to another directory.
   entry->SetBaseNameFromTitle();
 
   // Do file name de-duplication - find files with the same name and
   // append a name modifier to the name.
   int max_modifier = 1;
   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(),
                                                    ++max_modifier,
                                                    extension.c_str()));
     } else {
       full_file_name = FilePath(base::StringPrintf("%s (%d)",
                                                    file_name.c_str(),
                                                    ++max_modifier));
     }
   }
   entry->set_base_name(full_file_name.value());
 
   DVLOG(1) << "AddEntry: dir = " << GetFilePath().value()
            << ", file = " + entry->base_name()
            << ", parent resource = " << entry->parent_resource_id()
            << ", resource = " + entry->resource_id();
 
   // Add entry to resource map.
   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) {

[satorux1, 2012/08/20 16:40:08]

maybe add:

  const std::string& resource_id = iter->second;

to make it clear iter->second is a resource id.

[achuithb, 2012/08/20 21:14:57]

I've added a method TakeOverEntry instead to avoid a chunk of cut 'n paste code.

-    GDataEntry* entry = iter->second;
-    directory_service_->RemoveEntryFromResourceMap(entry->resource_id());
+    GDataEntry* entry = directory_service_->GetEntryByResourceId(iter->second);
+    DCHECK(entry);
+    directory_service_->RemoveEntryFromResourceMap(iter->second);
     entry->SetParent(NULL);
     AddEntry(entry);
   }
   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());
+    GDataEntry* entry = directory_service_->GetEntryByResourceId(iter->second);
+    DCHECK(entry);
+    directory_service_->RemoveEntryFromResourceMap(iter->second);
     entry->SetParent(NULL);
     AddEntry(entry);
   }
   dir->child_directories_.clear();
-  return true;
 }
 
 void GDataDirectory::RemoveEntry(GDataEntry* entry) {
   DCHECK(entry);
 
   RemoveChild(entry);
   delete entry;
 }
 
-GDataEntry* GDataDirectory::FindChild(
+const std::string& GDataDirectory::FindChild(

[satorux1, 2012/08/20 16:40:08]

I'd just return const std::string rather than a reference. Less efficient, but
less tricky.

[achuithb, 2012/08/20 21:14:57]

Done.

     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;
+  iter = child_directories_.find(file_name);
+  if (iter != child_directories_.end())
+    return iter->second;
 
-  return NULL;
-}
-
-void GDataDirectory::AddChild(GDataEntry* entry) {
-  DCHECK(entry);
-
-  if (entry->AsGDataFile())
-    child_files_.insert(std::make_pair(entry->base_name(),
-                                       entry->AsGDataFile()));
-
-  if (entry->AsGDataDirectory())
-    child_directories_.insert(std::make_pair(entry->base_name(),
-                                             entry->AsGDataDirectory()));
+  return EmptyString();
 }
 
 void GDataDirectory::RemoveChild(GDataEntry* entry) {
   DCHECK(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());
 
   // Then delete it from tree.
   child_files_.erase(base_name);
   child_directories_.erase(base_name);
 
   entry->SetParent(NULL);
 }
 
 void GDataDirectory::RemoveChildren() {
   RemoveChildFiles();
   RemoveChildDirectories();
 }
 
 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);

[satorux1, 2012/08/20 16:40:08]

DCHECK(child); ?

[achuithb, 2012/08/20 21:14:57]

Done.

+    directory_service_->RemoveEntryFromResourceMap(iter->second);
+    delete child;
   }
-  STLDeleteValues(&child_files_);
   child_files_.clear();
 }
 
 void GDataDirectory::RemoveChildDirectories() {
-  for (GDataDirectoryCollection::iterator iter = child_directories_.begin();
+  std::set<GDataDirectory*> child_directories;
+  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);
   }
 }
 
 // Convert to/from proto.
 
 // static
 void GDataEntry::ConvertProtoToPlatformFileInfo(
     const PlatformFileInfoProto& proto,
     base::PlatformFileInfo* file_info) {
   file_info->size = proto.size();
@@ skipping 102 matching lines @@
 
   // The states of the directory should be updated after children are
   // handled successfully, so that incomplete states are not left.
   GDataEntry::FromProto(proto.gdata_entry());
 }
 
 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);
   }
 
   return entries.Pass();
 }
 
 void GDataEntry::SerializeToString(std::string* serialized_proto) const {
   const GDataFile* file = AsGDataFileConst();
   const GDataDirectory* dir = AsGDataDirectoryConst();
 
   if (file) {
     GDataEntryProto entry_proto;
     file->ToProto(&entry_proto);
     const bool ok = entry_proto.SerializeToString(serialized_proto);
     DCHECK(ok);
   } else if (dir) {
     GDataDirectoryProto dir_proto;
     dir->ToProto(&dir_proto);
     const bool ok = dir_proto.SerializeToString(serialized_proto);
     DCHECK(ok);
   }
 }
 
 }  // namespace gdata