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 {
+ public:
+  explicit GDataCacheExternalData(const FilePath& virtual_dir_path,
+                                  const FilePath& file_content_path,
+                                  scoped_ptr<DocumentEntry> entry)
+      : virtual_dir_path_(virtual_dir_path),
+        file_content_path_(file_content_path),
+        entry_(entry.release()),
+        move_started_(false) {
+  }
+  virtual ~GDataCacheExternalData() {}
+
+  const FilePath& virtual_dir_path() const { return virtual_dir_path_; }
+  const FilePath& file_content_path() const { return file_content_path_; }
+  DocumentEntry* entry() const { return entry_.get(); }
+  bool move_started() const { return move_started_; }
+  void set_move_started(bool move_started) { move_started_ = move_started; }
+
+ private:
+  FilePath virtual_dir_path_;
+  FilePath file_content_path_;
+  scoped_ptr<DocumentEntry> entry_;
+  bool move_started_;
+};
+
 // 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);
-      RemovePendingDownload(download);
+      MoveDownloadedFileToCache(
+          download,
+          base::Bind(&GDataDownloadObserver::RemovePendingDownload,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     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);
     DVLOG(1) << "new download total bytes=" << download->GetTotalBytes()
              << ", full path=" << download->GetFullPath().value()
              << ", mime type=" << download->GetMimeType();
   }
+
+  DVLOG(1) << "Number of pending downloads=" << pending_downloads_.size();
 }
 
 void GDataDownloadObserver::RemovePendingDownload(DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!download->IsInProgress());
 
   DownloadMap::iterator it = pending_downloads_.find(download->GetId());
   if (it != pending_downloads_.end()) {
     DetachFromDownload(download);
     pending_downloads_.erase(it);
   }
+
+  DVLOG(1) << "Number of pending downloads=" << pending_downloads_.size();
 }
 
 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());
+
   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();
+
+  // Fill GDataCacheExternalData struct for use by MoveDownloadedFileToCache().
+  // Note that |content_file_path| should use the final target path.
+  download_item->SetExternalData(&kCachePathKey,
+                                 new GDataCacheExternalData(
+                                     upload_file_info->gdata_path.DirName(),
+                                     download_item->GetTargetFilePath(),

[asanka, 2012/07/10 21:11:59]

This doesn't affect GData uploads currently, but it is possible for the target
path to change during the completion stage (i.e. if there's a name collision for
some types of downloads). So if you want to read the downloaded file, you want
to look at GetTargetFilePath() when the download item is in the COMPLETE state.

In other words, you could just call GetTargetFilePath() directly in
MoveDownloadedFileToCache().

[hshi1, 2012/07/10 21:32:55]

I see, I've moved the GetTargetFilePath() call to inside
MoveDownloadedFileToCache().

On 2012/07/10 21:11:59, asanka wrote:

+                                     upload_file_info->entry.Pass()));
+}
+
+void GDataDownloadObserver::MoveDownloadedFileToCache(
+    DownloadItem* download,
+    const base::Closure& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!callback.is_null());
+
+  GDataCacheExternalData* data = GetGDataCacheExternalData(download);
+  DCHECK(data);
+  if (data) {
+    // The observer may receive multiple COMPLETE notifications, so only

[asanka, 2012/07/10 21:11:59]

COMPLETE is a state. So you may receive multiple OnDownloadUpdated()
notifications while it is in this state. 

If possible, why not call RemovePendingDownload() after calling
AddUploadedFile()? That way you don't have to worry about multiple
notifications.

[hshi1, 2012/07/10 21:32:55]

Good point! Done.

On 2012/07/10 21:11:59, asanka wrote:

+    // initiate the move if it is not already started.
+    if (!data->move_started()) {
+      // Move downloaded file to gdata cache. Remove the pending download upon
+      // comletion of the move.
+      data->set_move_started(true);
+      file_system_->AddUploadedFile(UPLOAD_NEW_FILE,
+                                    data->virtual_dir_path(),
+                                    data->entry(),
+                                    data->file_content_path(),
+                                    GDataCache::FILE_OPERATION_MOVE,
+                                    callback);
+    }
+  } else {
+    callback.Run();
+  }
 }
 
 }  // namespace gdata