Remove parent* arg from GDataEntry ctor.

* Remove static FromDocumentEntry from GDataEntry, GDataFile, GDataDirectory. Replace with InitFromDocumentEntry.
* Move common code from GDataFile::InitFromDocumentEntry and GDataDirectory::InitFromDocumentEntry to GDataEntry::InitFromDocumentEntry.
* Add GDataDirectoryService::FromDocumentEntry and use this everywhere.
* Make ctors of GDataFile, GDataDirectory private, so these must be created by GDataDirectoryService's CreateGDataFile and
CreateGDataDirectory. Make GDataEntry ctor protected.

BUG=141494
TEST=unit tests.
Review URL: https://chromiumcodereview.appspot.com/10854083

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

chrome/browser/chromeos/gdata/gdata_file_system.cc

@@ -1745,9 +1745,7 @@ void GDataFileSystem::OnGetDocumentEntry(const FilePath& cache_file_path,
   if (error == GDATA_FILE_OK) {
     scoped_ptr<DocumentEntry> doc_entry(DocumentEntry::ExtractAndParse(*data));
     if (doc_entry.get()) {
-      fresh_entry.reset(
-          GDataEntry::FromDocumentEntry(NULL, doc_entry.get(),
-                                        directory_service_.get()));
+      fresh_entry.reset(directory_service_->FromDocumentEntry(doc_entry.get()));
     }
     if (!fresh_entry.get() || !fresh_entry->AsGDataFile()) {
       LOG(ERROR) << "Got invalid entry from server for " << params.resource_id;
@@ -2340,8 +2338,7 @@ void GDataFileSystem::OnSearch(const SearchCallback& callback,
   // result directory.
   for (size_t i = 0; i < feed->entries().size(); ++i) {
     DocumentEntry* doc = const_cast<DocumentEntry*>(feed->entries()[i]);
-    scoped_ptr<GDataEntry> entry(
-        GDataEntry::FromDocumentEntry(NULL, doc, directory_service_.get()));
+    scoped_ptr<GDataEntry> entry(directory_service_->FromDocumentEntry(doc));
 
     if (!entry.get())
       continue;
@@ -2480,8 +2477,7 @@ void GDataFileSystem::OnCopyDocumentCompleted(
     return;
   }
 
-  GDataEntry* entry = GDataEntry::FromDocumentEntry(
-      NULL, doc_entry.get(), directory_service_.get());
+  GDataEntry* entry = directory_service_->FromDocumentEntry(doc_entry.get());
   if (!entry) {
     callback.Run(GDATA_FILE_ERROR_FAILED);
     return;
@@ -2827,8 +2823,8 @@ GDataFileError GDataFileSystem::AddNewDirectory(
   if (!parent_dir)
     return GDATA_FILE_ERROR_FAILED;
 
-  GDataEntry* new_entry = GDataEntry::FromDocumentEntry(
-      NULL, doc_entry.get(), directory_service_.get());
+  GDataEntry* new_entry =
+      directory_service_->FromDocumentEntry(doc_entry.get());
   if (!new_entry)
     return GDATA_FILE_ERROR_FAILED;
 
@@ -2950,8 +2946,7 @@ void GDataFileSystem::AddUploadedFileOnUIThread(
     return;
 
   scoped_ptr<GDataEntry> new_entry(
-      GDataEntry::FromDocumentEntry(
-          NULL, entry.get(), directory_service_.get()));
+      directory_service_->FromDocumentEntry(entry.get()));
   if (!new_entry.get())
     return;
 

chrome/browser/chromeos/gdata/gdata_files.h

@@ -122,23 +122,18 @@ typedef base::Callback<void(scoped_ptr<EntryInfoPairResult> pair_result)>
 // system.
 class GDataEntry {
  public:
-  // TODO(achuith): Remove |parent| from ctor. crbug.com/141494
-  GDataEntry(GDataDirectory* parent, GDataDirectoryService* directory_service);
   virtual ~GDataEntry();
 
   virtual GDataFile* AsGDataFile();
   virtual GDataDirectory* AsGDataDirectory();
 
+  // Initializes from DocumentEntry.
+  virtual void InitFromDocumentEntry(DocumentEntry* doc);
+
   // const versions of AsGDataFile and AsGDataDirectory.
   const GDataFile* AsGDataFileConst() const;
   const GDataDirectory* AsGDataDirectoryConst() const;
 
-  // Converts DocumentEntry into GDataEntry.
-  static GDataEntry* FromDocumentEntry(
-      GDataDirectory* parent,
-      DocumentEntry* doc,
-      GDataDirectoryService* directory_service);
-
   // Serialize/Parse to/from string via proto classes.
   void SerializeToString(std::string* serialized_proto) const;
 
@@ -221,6 +216,8 @@ class GDataEntry {
   // For access to SetParent from AddEntry.
   friend class GDataDirectory;
 
+  explicit GDataEntry(GDataDirectoryService* directory_service);
+
   // Sets the parent directory of this file system entry.
   // It is intended to be used by GDataDirectory::AddEntry() only.
   void SetParent(GDataDirectory* parent);
@@ -265,17 +262,7 @@ typedef std::map<FilePath::StringType, GDataDirectory*>
 // this could be either a regular file or a server side document.
 class GDataFile : public GDataEntry {
  public:
-  // TODO(achuith): Remove |parent| from ctor. crbug.com/141494
-  explicit GDataFile(GDataDirectory* parent,
-                     GDataDirectoryService* directory_service);
   virtual ~GDataFile();
-  virtual GDataFile* AsGDataFile() OVERRIDE;
-
-  // Converts DocumentEntry into GDataEntry.
-  static GDataEntry* FromDocumentEntry(
-      GDataDirectory* parent,
-      DocumentEntry* doc,
-      GDataDirectoryService* directory_service);
 
   // Converts to/from proto.
   bool FromProto(const GDataEntryProto& proto) WARN_UNUSED_RESULT;
@@ -297,6 +284,14 @@ class GDataFile : public GDataEntry {
   virtual void SetBaseNameFromTitle() OVERRIDE;
 
  private:
+  friend class GDataDirectoryService;  // For access to ctor.
+
+  explicit GDataFile(GDataDirectoryService* directory_service);
+  // Initializes from DocumentEntry.
+  virtual void InitFromDocumentEntry(DocumentEntry* doc) OVERRIDE;
+
+  virtual GDataFile* AsGDataFile() OVERRIDE;
+
   DocumentEntry::EntryKind kind_;  // Not saved in proto.
   GURL thumbnail_url_;
   GURL alternate_url_;
@@ -312,17 +307,7 @@ class GDataFile : public GDataEntry {
 // collection element.
 class GDataDirectory : public GDataEntry {
  public:
-  // TODO(achuith): Remove |parent| from ctor. crbug.com/141494
-  GDataDirectory(GDataDirectory* parent,
-                 GDataDirectoryService* directory_service);
   virtual ~GDataDirectory();
-  virtual GDataDirectory* AsGDataDirectory() OVERRIDE;
-
-  // 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;
@@ -343,6 +328,13 @@ class GDataDirectory : public GDataEntry {
   friend class GDataFileSystem;
   friend class GDataWapiFeedProcessor;
 
+  explicit GDataDirectory(GDataDirectoryService* directory_service);
+
+  // Initializes from DocumentEntry.
+  virtual void InitFromDocumentEntry(DocumentEntry* doc) OVERRIDE;
+
+  virtual GDataDirectory* AsGDataDirectory() OVERRIDE;
+
   // 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
@@ -413,6 +405,15 @@ class GDataDirectoryService {
   const ContentOrigin origin() const { return origin_; }
   void set_origin(ContentOrigin value) { origin_ = value; }
 
+  // Creates a GDataEntry from a DocumentEntry.
+  GDataEntry* FromDocumentEntry(DocumentEntry* doc);
+
+  // Creates a GDataFile instance.
+  GDataFile* CreateGDataFile();
+
+  // Creates a GDataDirectory instance.
+  GDataDirectory* CreateGDataDirectory();
+
   // Sets root directory resource id and initialize the root entry.
   void InitializeRootEntry(const std::string& root_id);
 

chrome/browser/chromeos/gdata/gdata_files_unittest.cc

@@ -32,7 +32,7 @@ const char kResumableCreateMediaUrl[] = "http://resumable-create-media/";
 GDataDirectory* AddDirectory(GDataDirectory* parent,
                              GDataDirectoryService* directory_service,
                              int sequence_id) {
-  GDataDirectory* dir = new GDataDirectory(NULL, directory_service);
+  GDataDirectory* dir = directory_service->CreateGDataDirectory();
   const std::string dir_name = "dir" + base::IntToString(sequence_id);
   const std::string resource_id = std::string("dir_resource_id:") +
                                   dir_name;
@@ -57,7 +57,7 @@ GDataDirectory* AddDirectory(GDataDirectory* parent,
 GDataFile* AddFile(GDataDirectory* parent,
                    GDataDirectoryService* directory_service,
                    int sequence_id) {
-  GDataFile* file = new GDataFile(NULL, directory_service);
+  GDataFile* file = directory_service->CreateGDataFile();
   const std::string title = "file" + base::IntToString(sequence_id);
   const std::string resource_id = std::string("file_resource_id:") +
                                   title;
@@ -195,16 +195,18 @@ TEST(GDataEntryTest, FromProto_DetectBadUploadUrl) {
   GDataEntryProto proto;
   proto.set_title("test.txt");
 
-  GDataEntry entry(NULL, NULL);
+  GDataDirectoryService directory_service;
+
+  scoped_ptr<GDataEntry> entry(directory_service.CreateGDataFile());
   // This should fail as the upload URL is empty.
-  ASSERT_FALSE(entry.FromProto(proto));
+  ASSERT_FALSE(entry->FromProto(proto));
 
   // Set a upload URL.
   proto.set_upload_url(kResumableEditMediaUrl);
 
   // This should succeed as the upload URL is set.
-  ASSERT_TRUE(entry.FromProto(proto));
-  EXPECT_EQ(kResumableEditMediaUrl, entry.upload_url().spec());
+  ASSERT_TRUE(entry->FromProto(proto));
+  EXPECT_EQ(kResumableEditMediaUrl, entry->upload_url().spec());
 }
 
 TEST(GDataDirectoryServiceTest, VersionCheck) {
@@ -387,8 +389,7 @@ TEST(GDataDirectoryServiceTest, RefreshFile) {
 
   GDataDirectoryService directory_service;
   // Add a directory to the file system.
-  GDataDirectory* directory_entry = new GDataDirectory(NULL,
-                                                       &directory_service);
+  GDataDirectory* directory_entry = directory_service.CreateGDataDirectory();
   directory_entry->set_resource_id("folder:directory_resource_id");
   directory_entry->set_title("directory");
   directory_entry->SetBaseNameFromTitle();
@@ -407,7 +408,7 @@ TEST(GDataDirectoryServiceTest, RefreshFile) {
             moved_file_path);
 
   // Add a new file to the directory.
-  GDataFile* initial_file_entry = new GDataFile(NULL, &directory_service);
+  GDataFile* initial_file_entry = directory_service.CreateGDataFile();
   initial_file_entry->set_resource_id("file:file_resource_id");
   initial_file_entry->set_title("file");
   initial_file_entry->SetBaseNameFromTitle();
@@ -429,7 +430,7 @@ TEST(GDataDirectoryServiceTest, RefreshFile) {
   // Initial file system state set, let's try refreshing entries.
 
   // New value for the entry with resource id "file:file_resource_id".
-  GDataFile* new_file_entry = new GDataFile(NULL, &directory_service);
+  GDataFile* new_file_entry = directory_service.CreateGDataFile();
   new_file_entry->set_resource_id("file:file_resource_id");
   directory_service.RefreshFile(scoped_ptr<GDataFile>(new_file_entry).Pass());
   // Root should have |new_file_entry|, not |initial_file_entry|.
@@ -442,7 +443,7 @@ TEST(GDataDirectoryServiceTest, RefreshFile) {
   EXPECT_EQ(directory_entry, new_file_entry->parent());
 
   // Let's try refreshing file that didn't prviously exist.
-  GDataFile* non_existent_entry = new GDataFile(NULL, &directory_service);
+  GDataFile* non_existent_entry = directory_service.CreateGDataFile();
   non_existent_entry->set_resource_id("file:does_not_exist");
   directory_service.RefreshFile(
       scoped_ptr<GDataFile>(non_existent_entry).Pass());

chrome/browser/chromeos/gdata/gdata_wapi_feed_processor.cc

@@ -285,8 +285,7 @@ GDataFileError GDataWapiFeedProcessor::FeedToFileResourceMap(
              feed->entries().begin();
          iter != feed->entries().end(); ++iter) {
       DocumentEntry* doc = *iter;
-      GDataEntry* entry = GDataEntry::FromDocumentEntry(
-          NULL, doc, directory_service_);
+      GDataEntry* entry = directory_service_->FromDocumentEntry(doc);
       // Some document entries don't map into files (i.e. sites).
       if (!entry)
         continue;