Add files saved using 'Save page as' to the download history....

chrome/browser/download/save_package.cc

Index: chrome/browser/download/save_package.cc
===================================================================
--- chrome/browser/download/save_package.cc	(revision 73953)
+++ chrome/browser/download/save_package.cc	(working copy)
@@ -20,6 +20,7 @@
 #include "base/threading/thread.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_thread.h"
+#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_manager.h"
@@ -347,18 +348,11 @@
 
   request_context_getter_ = profile->GetRequestContext();
 
-  // Create the fake DownloadItem and display the view.
-  DownloadManager* download_manager =
-      tab_contents_->profile()->GetDownloadManager();
-  download_ = new DownloadItem(download_manager,
-                               saved_main_file_path_,
-                               page_url_,
-                               profile->IsOffTheRecord());
+  // Create the fake DownloadItem and add it to the download history.
+  CreateDownloadItem(saved_main_file_path_,
+                     page_url_,
+                     profile->IsOffTheRecord());
 
-  // Transfer the ownership to the download manager. We need the DownloadItem
-  // to be alive as long as the Profile is alive.
-  download_manager->SavePageAsDownloadStarted(download_);
-
   tab_contents_->OnStartDownload(download_);
 
   // Check save type and process the save page job.
@@ -386,6 +380,39 @@
   return true;
 }
 
+void SavePackage::OnDownloadEntryAdded(DownloadCreateInfo info,
+                                       int64 db_handle) {
+  DownloadManager* download_manager =
+      tab_contents_->profile()->GetDownloadManager();
+
+  download_manager->AddDownloadItemToHistory(download_, db_handle);
+}
+
+void SavePackage::CreateDownloadItem(const FilePath& path,
+                                     const GURL& url,
+                                     bool is_otr) {
+  DownloadManager* download_manager =
+      tab_contents_->profile()->GetDownloadManager();
+
+  DownloadItem* item = new DownloadItem(download_manager, path, url, is_otr);

[Paweł Hajdan Jr., 2011/02/11 19:17:49]

nit: If it's going to be stored on |item_| anyway, why have the temporary
|item|?

[magnus, 2011/02/13 03:32:41]

Done.

+  download_ = item;
+  // Transfer the ownership to the download manager. We need the DownloadItem
+  // to be alive as long as the Profile is alive.
+  download_manager->SavePageAsDownloadStarted(item);
+  // Copy over the fields used by the history service
+  DownloadCreateInfo info(item->full_path(),
+                          item->url(),
+                          item->start_time(),
+                          0, 0,
+                          item->state(),
+                          item->id(),
+                          false);
+  DownloadHistory* download_history = download_manager->download_history();
+  // Add entry to the history service

[Paweł Hajdan Jr., 2011/02/11 19:17:49]

nit: Dot at the end. Also, please add a blank line above each comment, including
above.

[magnus, 2011/02/13 03:32:41]

Done.

+  download_history->AddEntry(info, download_,
+      NewCallback(this, &SavePackage::OnDownloadEntryAdded));
+}
+
 // Generate name for saving resource.
 bool SavePackage::GenerateFileName(const std::string& disposition,
                                    const GURL& url,
@@ -663,7 +690,11 @@
   wait_state_ = FAILED;
 
   // Inform the DownloadItem we have canceled whole save page job.
+  DownloadManager* download_manager =
+      tab_contents_->profile()->GetDownloadManager();
   download_->Cancel(false);
+  // Remove DownloadItem from history.
+  download_manager->download_history()->RemoveEntry(download_);

[Randy Smith (Not in Mondays), 2011/02/11 19:10:07]

I'm ok with the difference in behavior, but this is a difference in behavior
between downloads and save package activities.  downloads are left in the
history when cancelled (unless they're dangerous).  See for instance
DownloadManager::DownloadCancelled.

[magnus, 2011/02/13 03:32:41]

Changed it to keep the download in the history.

 }
 
 void SavePackage::CheckFinish() {
@@ -716,11 +747,16 @@
                         &SaveFileManager::RemoveSavedFileFromFileMap,
                         save_ids));
 
-  download_->OnAllDataSaved(all_save_items_count_);
-  download_->MarkAsComplete();
+  DownloadItem* item = download_;

[Randy Smith (Not in Mondays), 2011/02/11 19:10:07]

What's the value of the local variable rather than using download_?

[magnus, 2011/02/13 03:32:41]

Removed local variable.

+  item->OnAllDataSaved(all_save_items_count_);
+  item->MarkAsComplete();
   // Notify download observers that we are complete (the call
   // to OnReadyToFinish() set the state to complete but did not notify).
-  download_->UpdateObservers();
+  item->UpdateObservers();
+  // Update the download history

[Paweł Hajdan Jr., 2011/02/11 19:17:49]

nit: Dot at the end.

[magnus, 2011/02/13 03:32:41]

Done.

+  DownloadManager* download_manager =
+      tab_contents_->profile()->GetDownloadManager();
+  download_manager->download_history()->UpdateEntry(item);
 
   NotificationService::current()->Notify(
       NotificationType::SAVE_PACKAGE_SUCCESSFULLY_FINISHED,