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

chrome/browser/chromeos/gdata/gdata_download_observer.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 "chrome/browser/chromeos/gdata/gdata_download_observer.h"
 
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "chrome/browser/chromeos/gdata/gdata.pb.h"
 #include "chrome/browser/chromeos/gdata/gdata_documents_service.h"
@@ skipping 151 matching lines @@
         base::Bind(&OnEntryFound, profile, gdata_dir_path,
                    substitute_callback));
   } else {
     // TODO(achuith): Handle this.
     NOTREACHED();
   }
 }
 
 }  // 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)) {
 }
 
 GDataDownloadObserver::~GDataDownloadObserver() {
   if (download_manager_)
     download_manager_->RemoveObserver(this);
 
   for (DownloadMap::iterator iter = pending_downloads_.begin();
        iter != pending_downloads_.end(); ++iter) {
     DetachFromDownload(iter->second);
   }
 }
 
 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);
   gdata_tmp_download_path_ = gdata_tmp_download_path;
 }
 
 // static
 void GDataDownloadObserver::SubstituteGDataDownloadPath(Profile* profile,
     const FilePath& gdata_path, content::DownloadItem* download,
     const SubstituteGDataDownloadPathCallback& callback) {
@@ skipping 262 matching lines @@
                  download->GetId());
 
   return upload_file_info.Pass();
 }
 
 void GDataDownloadObserver::OnUploadComplete(
     int32 download_id,
     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;
     return;
   }
   DVLOG(1) << "Completing upload for download ID " << download_id;
   DownloadItem* download_item = iter->second;
   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