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

chrome/browser/chromeos/gdata/gdata_uploader.cc

Index: chrome/browser/chromeos/gdata/gdata_uploader.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_uploader.cc b/chrome/browser/chromeos/gdata/gdata_uploader.cc
index f9dc6604b6655da8362e6fcd4d25d025e615318c..672c679f6b39377ce005e0e20cc188a330e36ba9 100644
--- a/chrome/browser/chromeos/gdata/gdata_uploader.cc
+++ b/chrome/browser/chromeos/gdata/gdata_uploader.cc
@@ -52,6 +52,8 @@ int GDataUploader::UploadFile(scoped_ptr<UploadFileInfo> upload_file_info) {
   DCHECK(!upload_file_info->title.empty());
   DCHECK(!upload_file_info->content_type.empty());
 
+  base::AutoLock lock(lock_);
+
   const int upload_id = next_upload_id_++;
   upload_file_info->upload_id = upload_id;
   // Add upload_file_info to our internal map and take ownership.
@@ -73,6 +75,8 @@ int GDataUploader::UploadFile(scoped_ptr<UploadFileInfo> upload_file_info) {
 
 void GDataUploader::UpdateUpload(int upload_id,
                                  content::DownloadItem* download) {

[satorux1, 2012/06/14 21:32:34]

on what thread is this function called? In other places, we are mostly on UI
thread, but we don't have a DCHECK here

could you add DCHECK(BrowserThread::CurrentlyOn(...))?

[hshi1, 2012/06/14 21:40:17]

Done.

[satorux1, 2012/06/14 21:47:25]

Hmm, then seems to me that all functions run on UI thread. If so, we don't need
a lock?

+  base::AutoLock lock(lock_);
+
   UploadFileInfo* upload_file_info = GetUploadFileInfo(upload_id);
   if (!upload_file_info)
     return;
@@ -134,6 +138,7 @@ int64 GDataUploader::GetUploadedBytes(int upload_id) const {
 
 UploadFileInfo* GDataUploader::GetUploadFileInfo(int upload_id) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  lock_.AssertAcquired();
 
   UploadFileInfoMap::const_iterator it = pending_uploads_.find(upload_id);
   DVLOG_IF(1, it == pending_uploads_.end()) << "No upload found for id "
@@ -156,6 +161,7 @@ void GDataUploader::OpenFile(UploadFileInfo* upload_file_info) {
 
 void GDataUploader::OpenCompletionCallback(int upload_id, int result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  base::AutoLock lock(lock_);
 
   UploadFileInfo* upload_file_info = GetUploadFileInfo(upload_id);
   if (!upload_file_info)
@@ -208,6 +214,7 @@ void GDataUploader::OnUploadLocationReceived(
     GDataErrorCode code,
     const GURL& upload_location) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  base::AutoLock lock(lock_);
 
   UploadFileInfo* upload_file_info = GetUploadFileInfo(upload_id);
   if (!upload_file_info)
@@ -230,6 +237,8 @@ void GDataUploader::OnUploadLocationReceived(
 
 void GDataUploader::UploadNextChunk(UploadFileInfo* upload_file_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  lock_.AssertAcquired();
+
   // Check that |upload_file_info| is in pending_uploads_.
   DCHECK(upload_file_info == GetUploadFileInfo(upload_file_info->upload_id));
   DVLOG(1) << "Number of pending uploads=" << pending_uploads_.size();
@@ -270,6 +279,8 @@ void GDataUploader::ReadCompletionCallback(
   // The Read is asynchronously executed on BrowserThread::UI, where
   // Read() was called.
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  base::AutoLock lock(lock_);
+
   DVLOG(1) << "ReadCompletionCallback bytes read=" << bytes_read;
 
   UploadFileInfo* upload_file_info = GetUploadFileInfo(upload_id);
@@ -304,6 +315,7 @@ void GDataUploader::OnResumeUploadResponseReceived(
     const ResumeUploadResponse& response,
     scoped_ptr<DocumentEntry> entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  base::AutoLock lock(lock_);
 
   UploadFileInfo* upload_file_info = GetUploadFileInfo(upload_id);
   if (!upload_file_info)
@@ -319,8 +331,10 @@ void GDataUploader::OnResumeUploadResponseReceived(
       upload_file_info->completion_callback.Run(base::PLATFORM_FILE_OK,
                                                 upload_file_info);
     }
-    // TODO(achuith): DeleteUpload() here and let clients call
-    // GDataFileSystem::AddUploadedFile.
+
+    // Remove |upload_file_info| from the UploadFileInfoMap. The UploadFileInfo
+    // object will be deleted later upon transfer completion.
+    RemoveUpload(upload_file_info);
     return;
   }
 
@@ -354,6 +368,8 @@ void GDataUploader::OnResumeUploadResponseReceived(
 
 void GDataUploader::MoveFileToCache(UploadFileInfo* upload_file_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  lock_.AssertAcquired();
+
   if (upload_file_info->entry == NULL)
     return;
 
@@ -362,30 +378,42 @@ void GDataUploader::MoveFileToCache(UploadFileInfo* upload_file_info) {
       upload_file_info->gdata_path.DirName(),
       upload_file_info->entry.get(),
       upload_file_info->file_path,
-      GDataCache::FILE_OPERATION_MOVE);
-  DeleteUpload(upload_file_info);
+      GDataCache::FILE_OPERATION_MOVE,
+      base::Bind(&GDataUploader::OnAddUploadFileComplete,
+                 uploader_factory_.GetWeakPtr(),
+                 upload_file_info));
+
+  // Remove |upload_file_info| from the UploadFileInfoMap. The UploadFileInfo
+  // object will be deleted in OnAddUploadFileComplete.
+  RemoveUpload(upload_file_info);
+}
+
+void GDataUploader::OnAddUploadFileComplete(UploadFileInfo* upload_file_info) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  delete upload_file_info;
 }
 
 void GDataUploader::UploadFailed(UploadFileInfo* upload_file_info,
                                  base::PlatformFileError error) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  lock_.AssertAcquired();
+
   LOG(ERROR) << "Upload failed " << upload_file_info->DebugString();
   if (!upload_file_info->completion_callback.is_null()) {
     upload_file_info->completion_callback.Run(error,
                                               upload_file_info);
   }
-  DeleteUpload(upload_file_info);
+
+  RemoveUpload(upload_file_info);
+  delete upload_file_info;
 }
 
-void GDataUploader::DeleteUpload(UploadFileInfo* upload_file_info) {
+void GDataUploader::RemoveUpload(UploadFileInfo* upload_file_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  lock_.AssertAcquired();
 
-  DVLOG(1) << "Deleting upload " << upload_file_info->gdata_path.value();
+  DVLOG(1) << "Removing upload " << upload_file_info->gdata_path.value();
   pending_uploads_.erase(upload_file_info->upload_id);
-
-  // The file stream is closed by the destructor asynchronously.
-  delete upload_file_info->file_stream;
-  delete upload_file_info;
 }
 
 }  // namespace gdata