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..3cf17b8fa36ed5561e050d4fdeffbdb4341c1b95 100644
--- a/chrome/browser/chromeos/gdata/gdata_download_observer.cc
+++ b/chrome/browser/chromeos/gdata/gdata_download_observer.cc
@@ -30,6 +30,7 @@ const int64 kStreamingFileSize = 1 << 20;  // 1MB
 // Keys for DownloadItem::ExternalData.
 const char kUploadingKey[] = "Uploading";
 const char kGDataPathKey[] = "GDataPath";
+const char kCachePathKey[] = "CachePath";
 
 // External Data stored in DownloadItem for ongoing uploads.
 class UploadingExternalData : public DownloadCompletionBlocker {
@@ -62,6 +63,26 @@ class GDataExternalData : public DownloadItem::ExternalData {
   FilePath file_path_;
 };
 
+// External Data stored in DownloadItem for information required to move the
+// downloaded file to gdata cache.
+class GDataCacheExternalData : public DownloadItem::ExternalData {

[achuithb, 2012/07/10 23:04:35]

Why not add entry_ to UploadingExternalData? It's a bit hard to keep track of
the various ExternalData classes and when they get added and removed.

[hshi1, 2012/07/10 23:46:57]

Sounds good, done.
On 2012/07/10 23:04:35, achuith.bhandarkar wrote:

+ public:
+  explicit GDataCacheExternalData(const FilePath& virtual_dir_path,
+                                  scoped_ptr<DocumentEntry> entry)
+      : virtual_dir_path_(virtual_dir_path),
+        entry_(entry.release()) {
+  }
+  virtual ~GDataCacheExternalData() {}
+
+  const FilePath& virtual_dir_path() const { return virtual_dir_path_; }
+  DocumentEntry* entry() const { return entry_.get(); }

[achuithb, 2012/07/10 23:04:35]

Is this used?

[hshi1, 2012/07/10 23:46:57]

Yes it is used in a DCHECK in MoveFileToGDataCache. I see that it may be
redundant, so I've removed it.

+  scoped_ptr<DocumentEntry> entry_passed() { return entry_.Pass(); }
+
+ private:
+  FilePath virtual_dir_path_;
+  scoped_ptr<DocumentEntry> entry_;
+};
+
 // Extracts UploadingExternalData* from |download|.
 UploadingExternalData* GetUploadingExternalData(DownloadItem* download) {
   return static_cast<UploadingExternalData*>(
@@ -74,6 +95,12 @@ GDataExternalData* GetGDataExternalData(DownloadItem* download) {
       download->GetExternalData(&kGDataPathKey));
 }
 
+// Extracts GDataCacheExternalData* from |download|.
+GDataCacheExternalData* GetGDataCacheExternalData(DownloadItem* download) {
+  return static_cast<GDataCacheExternalData*>(
+      download->GetExternalData(&kCachePathKey));
+}
+
 void RunSubstituteGDataDownloadCallback(
     const GDataDownloadObserver::SubstituteGDataDownloadPathCallback& callback,
     const FilePath* file_path) {
@@ -169,8 +196,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 +216,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);
@@ -353,6 +380,7 @@ void GDataDownloadObserver::OnDownloadUpdated(DownloadItem* download) {
 
     case DownloadItem::COMPLETE:
       UploadDownloadItem(download);
+      MoveDownloadedFileToCache(download);
       RemovePendingDownload(download);
       break;
 
@@ -366,6 +394,7 @@ void GDataDownloadObserver::OnDownloadUpdated(DownloadItem* download) {
     default:
       NOTREACHED();
   }
+
   DVLOG(1) << "Number of pending downloads=" << pending_downloads_.size();
 }
 
@@ -396,6 +425,7 @@ void GDataDownloadObserver::RemovePendingDownload(DownloadItem* download) {
 void GDataDownloadObserver::DetachFromDownload(DownloadItem* download) {
   download->SetExternalData(&kUploadingKey, NULL);
   download->SetExternalData(&kGDataPathKey, NULL);
+  download->SetExternalData(&kCachePathKey, NULL);
   download->RemoveObserver(this);
 }
 
@@ -474,6 +504,10 @@ void GDataDownloadObserver::OnUploadComplete(
     base::PlatformFileError error,
     scoped_ptr<UploadFileInfo> upload_file_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(upload_file_info.get());
+  DCHECK(upload_file_info->entry.get());
+
+  // Look up the DownloadItem for the |download_id|.
   DownloadMap::iterator iter = pending_downloads_.find(download_id);
   if (iter == pending_downloads_.end()) {
     DVLOG(1) << "Pending download not found" << download_id;
@@ -481,9 +515,41 @@ void GDataDownloadObserver::OnUploadComplete(
   }
   DVLOG(1) << "Completing upload for download ID " << download_id;
   DownloadItem* download_item = iter->second;
+
+  // Fill GDataCacheExternalData struct for use by MoveDownloadedFileToCache().
+  download_item->SetExternalData(&kCachePathKey,
+                                 new GDataCacheExternalData(
+                                     upload_file_info->gdata_path.DirName(),
+                                     upload_file_info->entry.Pass()));
+
+  // Allow the download item to complete.
   UploadingExternalData* upload_data = GetUploadingExternalData(download_item);
   DCHECK(upload_data);
   upload_data->CompleteDownload();
 }
 
+void GDataDownloadObserver::MoveDownloadedFileToCache(DownloadItem* download) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  GDataCacheExternalData* data = GetGDataCacheExternalData(download);
+  if (!data || !data->entry()) {
+    NOTREACHED();
+    return;
+  }
+
+  // Take ownership of the DocumentEntry object. It will be released upon
+  // completion of the AddUploadedFile() call below.
+  DocumentEntry* entry = data->entry_passed().release();
+
+  // Move downloaded file to gdata cache. Note that |content_file_path| should
+  // use the final target path when the download item is in COMPLETE state.
+  file_system_->AddUploadedFile(UPLOAD_NEW_FILE,
+                                data->virtual_dir_path(),
+                                entry,
+                                download->GetTargetFilePath(),
+                                GDataCache::FILE_OPERATION_MOVE,
+                                base::Bind(&base::DeletePointer<DocumentEntry>,
+                                           entry));
+}
+
 }  // namespace gdata