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 12 matching lines @@
 
 namespace gdata {
 namespace {
 
 // Threshold file size after which we stream the file.
 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 {
  public:
   UploadingExternalData(GDataUploader* uploader, int upload_id)
       : uploader_(uploader),
         upload_id_(upload_id) {
   }
   virtual ~UploadingExternalData() {}
 
@@ skipping 12 matching lines @@
  public:
   explicit GDataExternalData(const FilePath& path) : file_path_(path) {}
   virtual ~GDataExternalData() {}
 
   const FilePath& file_path() const { return file_path_; }
 
  private:
   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*>(
       download->GetExternalData(&kUploadingKey));
 }
 
 // Extracts GDataExternalData* from |download|.
 GDataExternalData* GetGDataExternalData(DownloadItem* download) {
   return static_cast<GDataExternalData*>(
       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) {
   callback.Run(*file_path);
 }
 
 gdata::GDataSystemService* GetSystemService(Profile* profile) {
   gdata::GDataSystemService* system_service =
       gdata::GDataSystemServiceFactory::GetForProfile(
         profile ? profile : ProfileManager::GetDefaultProfile());
@@ skipping 75 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 141 matching lines @@
 
   const DownloadItem::DownloadState state = download->GetState();
   switch (state) {
     case DownloadItem::IN_PROGRESS:
       AddPendingDownload(download);
       UploadDownloadItem(download);
       break;
 
     case DownloadItem::COMPLETE:
       UploadDownloadItem(download);
+      MoveDownloadedFileToCache(download);
       RemovePendingDownload(download);
       break;
 
     // TODO(achuith): Stop the pending upload and delete the file.
     case DownloadItem::CANCELLED:
     case DownloadItem::REMOVING:
     case DownloadItem::INTERRUPTED:
       RemovePendingDownload(download);
       break;
 
     default:
       NOTREACHED();
   }
+
   DVLOG(1) << "Number of pending downloads=" << pending_downloads_.size();
 }
 
 void GDataDownloadObserver::AddPendingDownload(DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Add ourself as an observer of this download if we've never seen it before.
   if (pending_downloads_.count(download->GetId()) == 0) {
     pending_downloads_[download->GetId()] = download;
     download->AddObserver(this);
@@ skipping 10 matching lines @@
   DownloadMap::iterator it = pending_downloads_.find(download->GetId());
   if (it != pending_downloads_.end()) {
     DetachFromDownload(download);
     pending_downloads_.erase(it);
   }
 }
 
 void GDataDownloadObserver::DetachFromDownload(DownloadItem* download) {
   download->SetExternalData(&kUploadingKey, NULL);
   download->SetExternalData(&kGDataPathKey, NULL);
+  download->SetExternalData(&kCachePathKey, NULL);
   download->RemoveObserver(this);
 }
 
 void GDataDownloadObserver::UploadDownloadItem(DownloadItem* download) {
   // Update metadata of ongoing upload.
   UpdateUpload(download);
 
   if (!ShouldUpload(download))
     return;
 
@@ skipping 58 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());
+  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;
     return;
   }
   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