gdata: Convert GDataFileSystem::AddUploadFile() to asynchronous.

chrome/browser/chromeos/gdata/gdata_file_system.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system.cc b/chrome/browser/chromeos/gdata/gdata_file_system.cc
index fdfabaf0bd4eb70e78ef2f72c6d4ec5b8f880204..442f3c001bb5c9993b63e3388f2ddbb6a14baf04 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system.cc
@@ -1177,8 +1177,23 @@ void GDataFileSystem::OnTransferCompleted(
     AddUploadedFile(upload_file_info->gdata_path.DirName(),
                     upload_file_info->entry.get(),
                     upload_file_info->file_path,
-                    GDataCache::FILE_OPERATION_COPY);
+                    GDataCache::FILE_OPERATION_COPY,
+                    base::Bind(&GDataFileSystem::OnAddUploadFileCompleted,
+                               ui_weak_ptr_,
+                               callback,
+                               error,
+                               upload_file_info));
+  } else {

[satorux1, 2012/06/14 20:30:06]

maybe:

else if (upload_file_info->entry.get()) {

[hshi1, 2012/06/14 20:47:27]

In the original code there is no such check; even if
upload_file_info->entry.get() == NULL the code would still execute the callback.

On 2012/06/14 20:30:06, satorux1 wrote:

+    OnAddUploadFileCompleted(callback, error, upload_file_info);
   }
+}
+
+void GDataFileSystem::OnAddUploadFileCompleted(
+    const FileOperationCallback& callback,
+    base::PlatformFileError error,
+    UploadFileInfo* upload_file_info) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   if (!callback.is_null())
     callback.Run(error);
 
@@ -1186,11 +1201,7 @@ void GDataFileSystem::OnTransferCompleted(
   if (error != base::PLATFORM_FILE_OK)
     return;
 
-  // TODO(achuith): GDataFileSystem should not have to call DeleteUpload.
-  GDataSystemService* service =
-      GDataSystemServiceFactory::GetForProfile(profile_);
-  if (service)
-    service->uploader()->DeleteUpload(upload_file_info);
+  delete upload_file_info;
 }
 
 void GDataFileSystem::Copy(const FilePath& src_file_path,
@@ -3530,11 +3541,35 @@ void GDataFileSystem::AddUploadedFile(
     const FilePath& virtual_dir_path,
     DocumentEntry* entry,
     const FilePath& file_content_path,
-    GDataCache::FileOperationType cache_operation) {
+    GDataCache::FileOperationType cache_operation,
+    const base::Closure& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // Post a task to the same thread, rather than calling it here, as
+  // AddUploadedFile() is asynchronous.
+  base::MessageLoopProxy::current()->PostTask(
+      FROM_HERE,
+      base::Bind(&GDataFileSystem::AddUploadedFileOnUIThread,
+                 ui_weak_ptr_,
+                 virtual_dir_path,
+                 entry,
+                 file_content_path,
+                 cache_operation,
+                 callback));
+}
+
+void GDataFileSystem::AddUploadedFileOnUIThread(
+    const FilePath& virtual_dir_path,
+    DocumentEntry* entry,
+    const FilePath& file_content_path,
+    GDataCache::FileOperationType cache_operation,
+    const base::Closure& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
 
   if (!entry) {
     NOTREACHED();
+    callback.Run();
     return;
   }
 
@@ -3543,17 +3578,23 @@ void GDataFileSystem::AddUploadedFile(
   {
     base::AutoLock lock(lock_);
     GDataEntry* dir_entry = GetGDataEntryByPath(virtual_dir_path);
-    if (!dir_entry)
+    if (!dir_entry) {
+      callback.Run();
       return;
+    }
 
     GDataDirectory* parent_dir  = dir_entry->AsGDataDirectory();
-    if (!parent_dir)
+    if (!parent_dir) {
+      callback.Run();
       return;
+    }
 
     scoped_ptr<GDataEntry> new_entry(
         GDataEntry::FromDocumentEntry(parent_dir, entry, root_.get()));
-    if (!new_entry.get())
+    if (!new_entry.get()) {
+      callback.Run();
       return;
+    }
 
     GDataFile* file = new_entry->AsGDataFile();
     DCHECK(file);
@@ -3565,6 +3606,7 @@ void GDataFileSystem::AddUploadedFile(
 
   StoreToCache(resource_id, md5, file_content_path, cache_operation,
                CacheOperationCallback());
+  callback.Run();

[satorux1, 2012/06/14 20:30:06]

Should we run the callback here? StoreToCache() is asynchronous hence returns
immediately. Shouldn't we run |callback| upon the completion of StoreToCache()?

[hshi1, 2012/06/14 20:47:27]

You're right, does the new patch set work?

On 2012/06/14 20:30:06, satorux1 wrote:

 }
 
 void GDataFileSystem::Observe(int type,