Replace GDataDirectory::TakeEntry with GDataDirectoryService::AddEntryToDirectory.

chrome/browser/chromeos/gdata/gdata_files.h

 // 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.
 
 #ifndef CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILES_H_
 #define CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILES_H_
 
 #include <map>
 #include <string>
 
@@ skipping 56 matching lines @@
 const char kGDataRootDirectoryResourceId[] = "folder:root";
 
 // This should be incremented when incompatibility change is made in
 // gdata.proto.
 const int32 kProtoVersion = 1;
 
 // Used for file operations like removing files.
 typedef base::Callback<void(GDataFileError error)>
     FileOperationCallback;
 
+// Callback similar to FileOperationCallback but with a given |file_path|.
+typedef base::Callback<void(GDataFileError error,
+                            const FilePath& file_path)>
+    FilePathUpdateCallback;
+
 // Base class for representing files and directories in gdata virtual file
 // system.
 class GDataEntry {
  public:
   GDataEntry(GDataDirectory* parent, GDataDirectoryService* directory_service);
   virtual ~GDataEntry();
 
   virtual GDataFile* AsGDataFile();
   virtual GDataDirectory* AsGDataDirectory();
 
@@ skipping 187 matching lines @@
   // Converts DocumentEntry into GDataEntry.
   static GDataEntry* FromDocumentEntry(
       GDataDirectory* parent,
       DocumentEntry* doc,
       GDataDirectoryService* directory_service);
 
   // Converts to/from proto.
   bool FromProto(const GDataDirectoryProto& proto) WARN_UNUSED_RESULT;
   void ToProto(GDataDirectoryProto* proto) const;
 
-  // Removes child elements.
-  void RemoveChildren();
-  void RemoveChildFiles();
-  void RemoveChildDirectories();
-
   // Collection of children files/directories.
   const GDataFileCollection& child_files() const { return child_files_; }
   const GDataDirectoryCollection& child_directories() const {
     return child_directories_;
   }
 
  private:
   // TODO(satorux): Remove the friend statements. crbug.com/139649
   friend class GDataDirectoryService;
   friend class GDataFileSystem;
   friend class GDataWapiFeedProcessor;
 
   // Adds child file to the directory and takes over the ownership of |file|
   // object. The method will also do name de-duplication to ensure that the
   // exposed presentation path does not have naming conflicts. Two files with
   // the same name "Foo" will be renames to "Foo (1)" and "Foo (2)".
   // TODO(satorux): Remove this. crbug.com/139649
   void AddEntry(GDataEntry* entry);
 
   // Removes the entry from its children list and destroys the entry instance.
   // TODO(satorux): Remove this. crbug.com/139649
   void RemoveEntry(GDataEntry* entry);
 
-  // Takes the ownership of |entry| from its current parent. If this directory
-  // is already the current parent of |file|, this method effectively goes
-  // through the name de-duplication for |file| based on the current state of
-  // the file system.
-  // TODO(satorux): Remove this. crbug.com/139649
-  bool TakeEntry(GDataEntry* entry);
-
   // Takes over all entries from |dir|.
   // TODO(satorux): Remove this. crbug.com/139649
   bool TakeOverEntries(GDataDirectory* dir);
 
   // Find a child by its name.
   // TODO(satorux): Remove this. crbug.com/139649
   GDataEntry* FindChild(const FilePath::StringType& file_name) const;
 
   // Add |entry| to children.
   void AddChild(GDataEntry* entry);
 
   // Removes the entry from its children without destroying the
   // entry instance.
   void RemoveChild(GDataEntry* entry);
 
+  // Removes child elements.
+  void RemoveChildren();
+  void RemoveChildFiles();
+  void RemoveChildDirectories();
+
   // Collection of children GDataEntry items.
   GDataFileCollection child_files_;
   GDataDirectoryCollection child_directories_;
 
   DISALLOW_COPY_AND_ASSIGN(GDataDirectory);
 };
 
 // TODO(achuith,hashimoto,satorux): Move this to a separate file.
 // crbug.com/140317.
 // Class to handle GDataEntry* lookups, add/remove GDataEntry*.
@@ skipping 20 matching lines @@
   // Largest change timestamp that was the source of content for the current
   // state of the root directory.
   const int largest_changestamp() const { return largest_changestamp_; }
   void set_largest_changestamp(int value) { largest_changestamp_ = value; }
 
   // The root directory content origin.
   const ContentOrigin origin() const { return origin_; }
   void set_origin(ContentOrigin value) { origin_ = value; }
 
   // Adds |entry| to |directory_path| asynchronously.
   // Must be called on UI thread. |callback| is called on the UI thread.

[satorux1, 2012/08/08 14:09:06]

The semantics of the function seems to be changed. Could you update the comment?

[achuithb, 2012/08/08 22:08:41]

I can update the comment to state that the entry is removed from the previous
parent. The prior implementation was buggy/incorrect - not removing from the
previous parent means the entry will eventually get double deleted. We weren't
running crashing before because this was only used in test code without a valid
parent. 

   void AddEntryToDirectory(const FilePath& directory_path,
                            GDataEntry* entry,
-                           const FileOperationCallback& callback);
+                           const FilePathUpdateCallback& callback);
 
   // Adds the entry to resource map.
   void AddEntryToResourceMap(GDataEntry* entry);
 
   // Removes the entry from resource map.
   void RemoveEntryFromResourceMap(GDataEntry* entry);
 
   // Searches for |file_path| synchronously.
   // TODO(satorux): Replace this with an async version crbug.com/137160
   GDataEntry* FindEntryByPathSync(const FilePath& file_path);
@@ skipping 62 matching lines @@
   // This should remain the last member so it'll be destroyed first and
   // invalidate its weak pointers before other members are destroyed.
   base::WeakPtrFactory<GDataDirectoryService> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(GDataDirectoryService);
 };
 
 }  // namespace gdata
 
 #endif  // CHROME_BROWSER_CHROMEOS_GDATA_GDATA_FILES_H_