gdata: Move GDataFileSystem::AddUploadedFile() call out of uploader and into GDataDownloadObserver.

chrome/browser/chromeos/gdata/gdata_download_observer.cc

Index: chrome/browser/chromeos/gdata/gdata_download_observer.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_download_observer.cc b/chrome/browser/chromeos/gdata/gdata_download_observer.cc
index d3989fdf569d4a26dbebf7a144cbd4790f55cafb..f4eb2f8c1e70ad43b30fcf8eebfb4b68883a798b 100644
--- a/chrome/browser/chromeos/gdata/gdata_download_observer.cc
+++ b/chrome/browser/chromeos/gdata/gdata_download_observer.cc
@@ -169,8 +169,11 @@ void OnAuthenticate(Profile* profile,
 
 }  // namespace
 
-GDataDownloadObserver::GDataDownloadObserver()
-    : gdata_uploader_(NULL),
+GDataDownloadObserver::GDataDownloadObserver(
+    GDataUploader* uploader,
+    GDataFileSystem* file_system)
+    : gdata_uploader_(uploader),
+      file_system_(file_system),
       download_manager_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
 }
@@ -186,12 +189,9 @@ GDataDownloadObserver::~GDataDownloadObserver() {
 }
 
 void GDataDownloadObserver::Initialize(
-    GDataUploader* gdata_uploader,
     DownloadManager* download_manager,
     const FilePath& gdata_tmp_download_path) {
-  DCHECK(gdata_uploader);
   DCHECK(!gdata_tmp_download_path.empty());
-  gdata_uploader_ = gdata_uploader;
   download_manager_ = download_manager;
   if (download_manager_)
     download_manager_->AddObserver(this);
@@ -474,6 +474,8 @@ void GDataDownloadObserver::OnUploadComplete(
     base::PlatformFileError error,
     scoped_ptr<UploadFileInfo> upload_file_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(upload_file_info.get());
+
   DownloadMap::iterator iter = pending_downloads_.find(download_id);
   if (iter == pending_downloads_.end()) {
     DVLOG(1) << "Pending download not found" << download_id;
@@ -484,6 +486,25 @@ void GDataDownloadObserver::OnUploadComplete(
   UploadingExternalData* upload_data = GetUploadingExternalData(download_item);
   DCHECK(upload_data);
   upload_data->CompleteDownload();
+
+  // After the download item is complete, move the file to gdata cache.
+  if (error == base::PLATFORM_FILE_OK && upload_file_info->entry.get()) {
+    // Save a local copy of the UploadFileInfo pointer. Depending on order of
+    // argument evaluation, scoped_ptr<T>::release() may invalidate the scoped
+    // pointer before it can be dereferenced to access its members.
+    const UploadFileInfo* upload_file_info_ptr = upload_file_info.get();
+    // Note that the CompleteDownload() call may rename the downloaded file so
+    // call GetTargetFilePath() here to obtain the final path.
+    const FilePath& file_content_path = download_item->GetTargetFilePath();

[achuithb, 2012/07/10 16:58:44]

Are we sure that the file has been renamed at this point? Otherwise
AddUploadedFile will fail, right? The file rename actually happens on the FILE
thread so this can actually be racy.

[hshi1, 2012/07/10 22:43:36]

You're right, the renaming is asynchronous. Observer must wait for COMPLETE
notification then call AddUploadedFile.

On 2012/07/10 16:58:44, achuith.bhandarkar wrote:

+    file_system_->AddUploadedFile(
+        UPLOAD_NEW_FILE,
+        upload_file_info_ptr->gdata_path.DirName(),
+        upload_file_info_ptr->entry.get(),
+        file_content_path,
+        GDataCache::FILE_OPERATION_MOVE,
+        base::Bind(&base::DeletePointer<UploadFileInfo>,
+                   upload_file_info.release()));
+  }
 }
 
 }  // namespace gdata