Fixed remaining CL comments from review of http://codereview.chromium.org/9561009/

BUG=chromium-os:26961
TEST=none
Review URL: https://chromiumcodereview.appspot.com/9580016

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

chrome/browser/chromeos/gdata/gdata_file_system.cc

@@ -472,6 +472,10 @@ void GDataFileSystem::CreateDirectoryInternal(
       // this function.
       break;
     }
+    default: {
+      NOTREACHED();
+      break;
+    }
   }
 
   // Do we have a parent directory here as well? We can't then create target
@@ -588,10 +592,11 @@ void GDataFileSystem::OnCreateDirectoryCompleted(
   // Not done yet with recursive directory creation?
   if (params.target_directory_path != params.created_directory_path &&
       params.is_recursive) {
-    CreateDirectory(params.target_directory_path,
+    CreateDirectoryInternal(params.target_directory_path,
                             params.is_exclusive,
                             params.is_recursive,
-                    params.callback);
+                            params.callback,
+                            params.proxy);
     return;
   }
 
@@ -755,7 +760,6 @@ base::PlatformFileError GDataFileSystem::UpdateDirectoryWithDocumentFeed(
   return base::PLATFORM_FILE_OK;
 }
 
-
 base::PlatformFileError GDataFileSystem::AddNewDirectory(
     const FilePath& directory_path, base::Value* entry_value) {
   if (!entry_value)
@@ -779,6 +783,9 @@ base::PlatformFileError GDataFileSystem::AddNewDirectory(
   if (!file)
     return base::PLATFORM_FILE_ERROR_FAILED;
 
+  // Check if parent is a directory since in theory since this is a callback
+  // something could in the meantime have nuked the parent dir and created a
+  // file with the exact same name.
   GDataDirectory* parent_dir = file->AsGDataDirectory();
   if (!parent_dir)
     return base::PLATFORM_FILE_ERROR_FAILED;

chrome/browser/chromeos/gdata/gdata_file_system.h

@@ -305,7 +305,7 @@ class GDataFileSystem : public ProfileKeyedService {
     FOUND_INVALID,
     // Found missing directory segment while searching for given directory.
     FOUND_MISSING,
-    // Found target directory, it's already exists.
+    // Found target directory, it already exists.
     DIRECTORY_ALREADY_PRESENT,
   };
 

chrome/browser/chromeos/gdata/gdata_file_system_proxy.cc

@@ -50,10 +50,7 @@ class FindFileDelegateReplyBase : public FindFileDelegate {
       : file_system_(file_system),
         search_file_path_(search_file_path),
         require_content_(require_content) {
-    BrowserThread::ID thread_id;
-    CHECK(BrowserThread::GetCurrentThreadIdentifier(&thread_id));
-    replay_message_proxy_ =
-        BrowserThread::GetMessageLoopProxyForThread(thread_id);
+    reply_message_proxy_ = MessageLoopProxy::current();
   }
 
   virtual ~FindFileDelegateReplyBase() {}
@@ -98,7 +95,7 @@ class FindFileDelegateReplyBase : public FindFileDelegate {
   FilePath search_file_path_;
   // True if the final directory content is required.
   bool require_content_;
-  scoped_refptr<MessageLoopProxy> replay_message_proxy_;
+  scoped_refptr<MessageLoopProxy> reply_message_proxy_;
 };
 
 // GetFileInfoDelegate is used to handle results of proxy's content search
@@ -138,7 +135,7 @@ class GetFileInfoDelegate : public FindFileDelegateReplyBase {
              const base::PlatformFileInfo& file_info,
              const FilePath& platform_path) {
     if (!callback_.is_null()) {
-      replay_message_proxy_->PostTask(FROM_HERE,
+      reply_message_proxy_->PostTask(FROM_HERE,
           Bind(&GetFileInfoDelegate::ReplyOnCallingThread,
                this,
                result,
@@ -218,7 +215,7 @@ class ReadDirectoryDelegate : public FindFileDelegateReplyBase {
              const std::vector<base::FileUtilProxy::Entry>& file_list,
              bool has_more) {
     if (!callback_.is_null()) {
-      replay_message_proxy_->PostTask(FROM_HERE,
+      reply_message_proxy_->PostTask(FROM_HERE,
           Bind(&ReadDirectoryDelegate::ReplyOnCallingThread,
                this,
                result,

chrome/browser/chromeos/gdata/gdata_file_system_unittest.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 <string>
 #include <vector>
 
@@ -45,7 +46,7 @@ class GDataFileSystemTest : public testing::Test {
 
   // Loads test json file as root ("/gdata") element.
   void LoadRootFeedDocument(const std::string& filename) {
-    LoadSubdirFeedDocument(FilePath("gdata"), filename);
+    LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata")), filename);
   }
 
   // Loads test json file as subdirectory content of |directory_path|.
@@ -183,10 +184,10 @@ TEST_F(GDataFileSystemTest, SearchRootDirectory) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnDirectoryFound(FilePath("gdata"), _))
+              OnDirectoryFound(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1);
 
-  file_system_->FindFileByPath(FilePath("gdata"),
+  file_system_->FindFileByPath(FilePath(FILE_PATH_LITERAL("gdata")),
                                mock_find_file_delegate);
 }
 
@@ -196,13 +197,13 @@ TEST_F(GDataFileSystemTest, SearchExistingFile) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath("gdata"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(), OnFileFound(_))
       .Times(1);
 
-  file_system_->FindFileByPath(FilePath("gdata/File 1.txt"),
+  file_system_->FindFileByPath(FilePath(FILE_PATH_LITERAL("gdata/File 1.txt")),
                                mock_find_file_delegate);
 }
 
@@ -212,13 +213,14 @@ TEST_F(GDataFileSystemTest, SearchExistingDocument) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath("gdata"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(), OnFileFound(_))
       .Times(1);
 
-  file_system_->FindFileByPath(FilePath("gdata/Document 1.gdocument"),
+  file_system_->FindFileByPath(
+      FilePath(FILE_PATH_LITERAL("gdata/Document 1.gdocument")),
       mock_find_file_delegate);
 }
 
@@ -228,23 +230,25 @@ TEST_F(GDataFileSystemTest, SearchDuplicateNames) {
   scoped_refptr<MockFindFileDelegate> mock_find_file_delegate =
       new MockFindFileDelegate();
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath("gdata"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(), OnFileFound(_))
       .Times(1);
-  file_system_->FindFileByPath(FilePath("gdata/Duplicate Name.txt"),
+  file_system_->FindFileByPath(
+      FilePath(FILE_PATH_LITERAL("gdata/Duplicate Name.txt")),
       mock_find_file_delegate);
 
   scoped_refptr<MockFindFileDelegate> mock_find_file_delegate2 =
       new MockFindFileDelegate();
   EXPECT_CALL(*mock_find_file_delegate2.get(),
-              OnEnterDirectory(FilePath("gdata"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate2.get(), OnFileFound(_))
       .Times(1);
-  file_system_->FindFileByPath(FilePath("gdata/Duplicate Name (2).txt"),
+  file_system_->FindFileByPath(
+      FilePath(FILE_PATH_LITERAL("gdata/Duplicate Name (2).txt")),
       mock_find_file_delegate2);
 }
 
@@ -254,13 +258,13 @@ TEST_F(GDataFileSystemTest, SearchExistingDirectory) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath("gdata"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(), OnDirectoryFound(_, _))
       .Times(1);
 
-  file_system_->FindFileByPath(FilePath("gdata/Directory 1"),
+  file_system_->FindFileByPath(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
                                mock_find_file_delegate);
 }
 
@@ -271,14 +275,15 @@ TEST_F(GDataFileSystemTest, SearchNonExistingFile) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath("gdata"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(),
               OnError(base::PLATFORM_FILE_ERROR_NOT_FOUND))
       .Times(1);
 
-  file_system_->FindFileByPath(FilePath("gdata/nonexisting.file"),
+  file_system_->FindFileByPath(
+      FilePath(FILE_PATH_LITERAL("gdata/nonexisting.file")),
       mock_find_file_delegate);
 }
 
@@ -289,28 +294,30 @@ TEST_F(GDataFileSystemTest, StopFileSearch) {
 
   // Stop on first directory entry.
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath("gdata"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_TERMINATES));
 
-  file_system_->FindFileByPath(FilePath("gdata/Directory 1"),
+  file_system_->FindFileByPath(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
                                mock_find_file_delegate);
 }
 
 TEST_F(GDataFileSystemTest, SearchInSubdir) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath("gdata/Directory 1"), "subdir_feed.json");
+  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+                         "subdir_feed.json");
 
   scoped_refptr<MockFindFileDelegate> mock_find_file_delegate =
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath("gdata"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath("gdata/Directory 1"), _))
+              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+                               _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
 
@@ -318,27 +325,31 @@ TEST_F(GDataFileSystemTest, SearchInSubdir) {
       .Times(1);
 
   file_system_->FindFileByPath(
-      FilePath("gdata/Directory 1/SubDirectory File 1.txt"),
+      FilePath(FILE_PATH_LITERAL("gdata/Directory 1/SubDirectory File 1.txt")),
       mock_find_file_delegate);
 }
 
 TEST_F(GDataFileSystemTest, FilePathTests) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath("gdata/Directory 1"), "subdir_feed.json");
+  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+                         "subdir_feed.json");
 
-  FindAndTestFilePath(FilePath("gdata/File 1.txt"));
-  FindAndTestFilePath(FilePath("gdata/Directory 1"));
-  FindAndTestFilePath(FilePath("gdata/Directory 1/SubDirectory File 1.txt"));
+  FindAndTestFilePath(FilePath(FILE_PATH_LITERAL("gdata/File 1.txt")));
+  FindAndTestFilePath(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")));
+  FindAndTestFilePath(
+      FilePath(FILE_PATH_LITERAL("gdata/Directory 1/SubDirectory File 1.txt")));
 }
 
 TEST_F(GDataFileSystemTest, RemoveFiles) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath("gdata/Directory 1"), "subdir_feed.json");
+  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+                         "subdir_feed.json");
 
-  FilePath nonexisting_file("gdata/Dummy file.txt");
-  FilePath file_in_root("gdata/File 1.txt");
-  FilePath dir_in_root("gdata/Directory 1");
-  FilePath file_in_subdir("gdata/Directory 1/SubDirectory File 1.txt");
+  FilePath nonexisting_file(FILE_PATH_LITERAL("gdata/Dummy file.txt"));
+  FilePath file_in_root(FILE_PATH_LITERAL("gdata/File 1.txt"));
+  FilePath dir_in_root(FILE_PATH_LITERAL("gdata/Directory 1"));
+  FilePath file_in_subdir(
+      FILE_PATH_LITERAL("gdata/Directory 1/SubDirectory File 1.txt"));
 
   EXPECT_TRUE(FindFile(file_in_root) != NULL);
   EXPECT_TRUE(FindFile(dir_in_root) != NULL);
@@ -363,22 +374,22 @@ TEST_F(GDataFileSystemTest, RemoveFiles) {
   EXPECT_FALSE(RemoveFile(nonexisting_file));
 
   // Try removing root file element.
-  EXPECT_FALSE(RemoveFile(FilePath("gdata")));
+  EXPECT_FALSE(RemoveFile(FilePath(FILE_PATH_LITERAL("gdata"))));
 }
 
-
 TEST_F(GDataFileSystemTest, CreateDirectory) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath("gdata/Directory 1"), "subdir_feed.json");
+  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+                         "subdir_feed.json");
 
   // Create directory in root.
-  FilePath dir_path("gdata/New Folder 1");
+  FilePath dir_path(FILE_PATH_LITERAL("gdata/New Folder 1"));
   EXPECT_TRUE(FindFile(dir_path) == NULL);
   AddDirectoryFromFile(dir_path, "directory_entry_atom.json");
   EXPECT_TRUE(FindFile(dir_path) != NULL);
 
   // Create directory in a sub dirrectory.
-  FilePath subdir_path("gdata/New Folder 1/New Folder 2");
+  FilePath subdir_path(FILE_PATH_LITERAL("gdata/New Folder 1/New Folder 2"));
   EXPECT_TRUE(FindFile(subdir_path) == NULL);
   AddDirectoryFromFile(subdir_path, "directory_entry_atom.json");
   EXPECT_TRUE(FindFile(subdir_path) != NULL);
@@ -386,56 +397,60 @@ TEST_F(GDataFileSystemTest, CreateDirectory) {
 
 TEST_F(GDataFileSystemTest, FindFirstMissingParentDirectory) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath("gdata/Directory 1"), "subdir_feed.json");
+  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+                         "subdir_feed.json");
 
   GURL last_dir_content_url;
   FilePath first_missing_parent_path;
 
   // Create directory in root.
-  FilePath dir_path("gdata/New Folder 1");
+  FilePath dir_path(FILE_PATH_LITERAL("gdata/New Folder 1"));
   EXPECT_EQ(
+      GDataFileSystem::FOUND_MISSING,
       file_system_->FindFirstMissingParentDirectory(dir_path,
           &last_dir_content_url,
-          &first_missing_parent_path),
-      GDataFileSystem::FOUND_MISSING);
-  EXPECT_EQ(dir_path, first_missing_parent_path);
+          &first_missing_parent_path));
+  EXPECT_EQ(FilePath(FILE_PATH_LITERAL("gdata/New Folder 1")),
+            first_missing_parent_path);
   EXPECT_TRUE(last_dir_content_url.is_empty());    // root directory.
 
   // Missing folders in subdir of an existing folder.
-  FilePath dir_path2("gdata/Directory 1/New Folder 2");
+  FilePath dir_path2(FILE_PATH_LITERAL("gdata/Directory 1/New Folder 2"));
   EXPECT_EQ(
+      GDataFileSystem::FOUND_MISSING,
       file_system_->FindFirstMissingParentDirectory(dir_path2,
           &last_dir_content_url,
-          &first_missing_parent_path),
-      GDataFileSystem::FOUND_MISSING);
-  EXPECT_EQ(dir_path2, first_missing_parent_path);
+          &first_missing_parent_path));
+  EXPECT_EQ(FilePath(FILE_PATH_LITERAL("gdata/Directory 1/New Folder 2")),
+            first_missing_parent_path);
   EXPECT_FALSE(last_dir_content_url.is_empty());    // non-root directory.
 
   // Missing two folders on the path.
-  FilePath dir_path3 = dir_path2.Append("Another Foder");
+  FilePath dir_path3 = dir_path2.Append(FILE_PATH_LITERAL("Another Foder"));
   EXPECT_EQ(
+      GDataFileSystem::FOUND_MISSING,
       file_system_->FindFirstMissingParentDirectory(dir_path3,
           &last_dir_content_url,
-          &first_missing_parent_path),
-      GDataFileSystem::FOUND_MISSING);
-  EXPECT_EQ(dir_path3.DirName(), first_missing_parent_path);
+          &first_missing_parent_path));
+  EXPECT_EQ(FilePath(FILE_PATH_LITERAL("gdata/Directory 1/New Folder 2")),
+            first_missing_parent_path);
   EXPECT_FALSE(last_dir_content_url.is_empty());    // non-root directory.
 
   // Folders on top of an existing file.
   EXPECT_EQ(
+      GDataFileSystem::FOUND_INVALID,
       file_system_->FindFirstMissingParentDirectory(
-          FilePath("gdata/File 1.txt/BadDir"),
+          FilePath(FILE_PATH_LITERAL("gdata/File 1.txt/BadDir")),
           &last_dir_content_url,
-          &first_missing_parent_path),
-      GDataFileSystem::FOUND_INVALID);
+          &first_missing_parent_path));
 
   // Existing folder.
   EXPECT_EQ(
+      GDataFileSystem::DIRECTORY_ALREADY_PRESENT,
       file_system_->FindFirstMissingParentDirectory(
-          FilePath("gdata/Directory 1"),
+          FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
           &last_dir_content_url,
-          &first_missing_parent_path),
-      GDataFileSystem::DIRECTORY_ALREADY_PRESENT);
+          &first_missing_parent_path));
 }
 
 }   // namespace gdata