This CL adds prompting for dangerous types of files (executable) when they are automatically downloaded.

The file is saved with a temporary name (dangerous_download_xxxx.download) in the download directory and the user is presented (in the download shelf and the download tab if opened) with a warning message and buttons to save/discard the download.
If discarded the download is removed (and its file deleted).
If saved, download goes as usual.
Dangerous downloads not confirmed by the user are deleted on shutdown.

TEST=Download a small exe file, try using the save/discard button from the download shelf and from the download tab (the intent is that the file has been entirely downloaded by the time you take action). Try again with a slow/big download (that time the download is expected not to be finished when approved/discarded).


Review URL: http://codereview.chromium.org/6043

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@3228 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 304 insertions, 45 deletions

diff --git a/chrome/browser/download/download_exe.cc b/chrome/browser/download/download_exe.cc
index c9fdc4d..ce33781 100644
--- a/chrome/browser/download/download_exe.cc
+++ b/chrome/browser/download/download_exe.cc
@@ -59,6 +59,7 @@ static const wchar_t* const g_executables[] = {
   L"app",
   L"application",
   L"asp",
+  L"asx",
   L"bas",
   L"bat",
   L"chm",
@@ -66,6 +67,7 @@ static const wchar_t* const g_executables[] = {
   L"com",
   L"cpl",
   L"crt",
+  L"dll",
   L"exe",
   L"fxp",
   L"hlp",
@@ -73,6 +75,7 @@ static const wchar_t* const g_executables[] = {
   L"inf",
   L"ins",
   L"isp",
+  L"jar",
   L"js",
   L"jse",
   L"lnk",
diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc
index 4fa3637..6b0fba4 100644
--- a/chrome/browser/download/download_file.cc
+++ b/chrome/browser/download/download_file.cc
@@ -578,3 +578,9 @@ void DownloadFileManager::CreateDirectory(const std::wstring& directory) {
   if (!file_util::PathExists(directory))
     file_util::CreateDirectory(directory);
 }
+
+void DownloadFileManager::DeleteFile(const std::wstring& path) {
+  // Make sure we only delete files.
+  if (file_util::PathExists(path) && !file_util::DirectoryExists(path))
+    file_util::Delete(path, false);
+}
diff --git a/chrome/browser/download/download_file.h b/chrome/browser/download/download_file.h
index 5dc5d63..cce6398 100644
--- a/chrome/browser/download/download_file.h
+++ b/chrome/browser/download/download_file.h
@@ -210,6 +210,9 @@ class DownloadFileManager
   // download directory exists.
   void CreateDirectory(const std::wstring& directory);
 
+  // Called by the donwload manager to delete non validated dangerous downloads.
+  void DeleteFile(const std::wstring& path);
+
  private:
   // Timer helpers for updating the UI about the current progress of a download.
   void StartUpdateTimer();
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index d83ddc2..cff6935 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -15,6 +15,7 @@
 #include "base/task.h"
 #include "base/thread.h"
 #include "base/timer.h"
+#include "base/rand_util.h"
 #include "base/win_util.h"
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/browser_process.h"
@@ -97,6 +98,7 @@ static bool DownloadPathIsDangerous(const std::wstring& download_path) {
 DownloadItem::DownloadItem(const DownloadCreateInfo& info)
     : id_(-1),
       full_path_(info.path),
+      original_name_(info.original_name),
       url_(info.url),
       total_bytes_(info.total_bytes),
       received_bytes_(info.received_bytes),
@@ -105,6 +107,7 @@ DownloadItem::DownloadItem(const DownloadCreateInfo& info)
       start_time_(info.start_time),
       db_handle_(info.db_handle),
       manager_(NULL),
+      safety_state_(SAFE),
       is_paused_(false),
       open_when_complete_(false),
       render_process_id_(-1),
@@ -118,13 +121,16 @@ DownloadItem::DownloadItem(const DownloadCreateInfo& info)
 DownloadItem::DownloadItem(int32 download_id,
                            const std::wstring& path,
                            const std::wstring& url,
+                           const std::wstring& original_name,
                            const Time start_time,
                            int64 download_size,
                            int render_process_id,
-                           int request_id)
+                           int request_id,
+                           bool is_dangerous)
     : id_(download_id),
       full_path_(path),
       url_(url),
+      original_name_(original_name),
       total_bytes_(download_size),
       received_bytes_(0),
       start_tick_(GetTickCount()),
@@ -132,6 +138,7 @@ DownloadItem::DownloadItem(int32 download_id,
       start_time_(start_time),
       db_handle_(kUninitializedHandle),
       manager_(NULL),
+      safety_state_(is_dangerous ? DANGEROUS : SAFE),
       is_paused_(false),
       open_when_complete_(false),
       render_process_id_(render_process_id),
@@ -198,14 +205,16 @@ void DownloadItem::Cancel(bool update_history) {
 void DownloadItem::Finished(int64 size) {
   state_ = COMPLETE;
   UpdateSize(size);
-  UpdateObservers();
   StopProgressTimer();
 }
 
-void DownloadItem::Remove() {
+void DownloadItem::Remove(bool delete_on_disk) {
   Cancel(true);
   state_ = REMOVING;
+  if (delete_on_disk)
+    manager_->DeleteDownload(full_path_);
   manager_->RemoveDownload(db_handle_);
+  // We are now deleted.
 }
 
 void DownloadItem::StartProgressTimer() {
@@ -255,6 +264,12 @@ void DownloadItem::TogglePause() {
   UpdateObservers();
 }
 
+std::wstring DownloadItem::GetFileName() const {
+  if (safety_state_ == DownloadItem::SAFE)
+    return file_name_;
+  return original_name_;
+}
+
 // DownloadManager implementation ----------------------------------------------
 
 // static
@@ -313,12 +328,19 @@ void DownloadManager::Shutdown() {
   // 'in_progress_' may contain DownloadItems that have not finished the start
   // complete (from the history service) and thus aren't in downloads_.
   DownloadMap::iterator it = in_progress_.begin();
+  std::set<DownloadItem*> to_remove;
   for (; it != in_progress_.end(); ++it) {
     DownloadItem* download = it->second;
-    if (download->state() == DownloadItem::IN_PROGRESS) {
-      download->Cancel(false);
-      UpdateHistoryForDownload(download);
+    if (download->safety_state() == DownloadItem::DANGEROUS) {
+      // Forget about any download that the user did not approve.
+      // Note that we cannot call download->Remove() this would invalidate our
+      // iterator.
+      to_remove.insert(download);
+      continue;
     }
+    DCHECK_EQ(DownloadItem::IN_PROGRESS, download->state());
+    download->Cancel(false);
+    UpdateHistoryForDownload(download);
     if (download->db_handle() == kUninitializedHandle) {
       // An invalid handle means that 'download' does not yet exist in
       // 'downloads_', so we have to delete it here.
@@ -326,7 +348,25 @@ void DownloadManager::Shutdown() {
     }
   }
 
+  // 'dangerous_finished_' contains all complete downloads that have not been
+  // approved.  They should be removed.
+  it = dangerous_finished_.begin();
+  for (; it != dangerous_finished_.end(); ++it)
+    to_remove.insert(it->second);
+
+  // Remove the dangerous download that are not approved.
+  for (std::set<DownloadItem*>::const_iterator rm_it = to_remove.begin();
+       rm_it != to_remove.end(); ++rm_it) {
+    DownloadItem* download = *rm_it;
+    download->Remove(true);
+    // Same as above, delete the download if it is not in 'downloads_'.
+    if (download->db_handle() == kUninitializedHandle)
+      delete download;
+  }
+  to_remove.clear();
+
   in_progress_.clear();
+  dangerous_finished_.clear();
   STLDeleteValues(&downloads_);
 
   file_manager_ = NULL;
@@ -486,17 +526,36 @@ void DownloadManager::CheckIfSuggestedPathExists(DownloadCreateInfo* info) {
 
   // Check writability of the suggested path. If we can't write to it, default
   // to the user's "My Documents" directory. We'll prompt them in this case.
-  std::wstring path = file_util::GetDirectoryFromPath(info->suggested_path);
-  if (!file_util::PathIsWritable(path)) {
+  std::wstring dir = file_util::GetDirectoryFromPath(info->suggested_path);
+  const std::wstring filename =
+      file_util::GetFilenameFromPath(info->suggested_path);
+  if (!file_util::PathIsWritable(dir)) {
     info->save_as = true;
-    const std::wstring filename =
-        file_util::GetFilenameFromPath(info->suggested_path);
     PathService::Get(chrome::DIR_USER_DOCUMENTS, &info->suggested_path);
     file_util::AppendToPath(&info->suggested_path, filename);
   }
 
   info->suggested_path_exists = !UniquifyPath(&info->suggested_path);
 
+  // If the download is deemmed dangerous, we'll use a temporary name for it.
+  if (!info->save_as && IsDangerous(filename)) {
+    info->original_name = file_util::GetFilenameFromPath(info->suggested_path);
+    // Create a temporary file to hold the file until the user approves its
+    // download.
+    std::wstring file_name;
+    std::wstring path;
+    while (path.empty()) {
+      SStringPrintf(&file_name, L"dangerous_download_%d.download",
+                    base::RandInt(0, 100000));
+       path = dir;
+       file_util::AppendToPath(&path, file_name);
+       if (file_util::PathExists(path))
+         path.clear();
+    }
+    info->suggested_path = path;
+    info->is_dangerous = true;
+  }
+
   // Now we return to the UI thread.
   ui_loop_->PostTask(FROM_HERE,
       NewRunnableMethod(this,
@@ -537,10 +596,12 @@ void DownloadManager::ContinueStartDownload(DownloadCreateInfo* info,
     download = new DownloadItem(info->download_id,
                                 info->path,
                                 info->url,
+                                info->original_name,
                                 info->start_time,
                                 info->total_bytes,
                                 info->render_process_id,
-                                info->request_id);
+                                info->request_id,
+                                info->is_dangerous);
     download->set_manager(this);
     in_progress_[info->download_id] = download;
   } else {
@@ -628,30 +689,7 @@ void DownloadManager::UpdateDownload(int32 download_id, int64 size) {
 
 void DownloadManager::DownloadFinished(int32 download_id, int64 size) {
   DownloadMap::iterator it = in_progress_.find(download_id);
-  if (it != in_progress_.end()) {
-    // Remove the id from the list of pending ids.
-    PendingFinishedMap::iterator erase_it =
-        pending_finished_downloads_.find(download_id);
-    if (erase_it != pending_finished_downloads_.end())
-      pending_finished_downloads_.erase(erase_it);
-
-    DownloadItem* download = it->second;
-    download->Finished(size);
-
-    // Open the download if the user or user prefs indicate it should be.
-    const std::wstring extension =
-        file_util::GetFileExtensionFromPath(download->full_path());
-    if (download->open_when_complete() || ShouldOpenFileExtension(extension))
-      OpenDownloadInShell(download, NULL);
-
-    // Clean up will happen when the history system create callback runs if we
-    // don't have a valid db_handle yet.
-    if (download->db_handle() != kUninitializedHandle) {
-      in_progress_.erase(it);
-      NotifyAboutDownloadStop();
-      UpdateHistoryForDownload(download);
-    }
-  } else {
+  if (it == in_progress_.end()) {
     // The download is done, but the user hasn't selected a final location for
     // it yet (the Save As dialog box is probably still showing), so just keep
     // track of the fact that this download id is complete, when the
@@ -660,7 +698,102 @@ void DownloadManager::DownloadFinished(int32 download_id, int64 size) {
         pending_finished_downloads_.find(download_id);
     DCHECK(erase_it == pending_finished_downloads_.end());
     pending_finished_downloads_[download_id] = size;
+    return;
+  }
+
+  // Remove the id from the list of pending ids.
+  PendingFinishedMap::iterator erase_it =
+      pending_finished_downloads_.find(download_id);
+  if (erase_it != pending_finished_downloads_.end())
+    pending_finished_downloads_.erase(erase_it);
+
+  DownloadItem* download = it->second;
+  download->Finished(size);
+
+  // Clean up will happen when the history system create callback runs if we
+  // don't have a valid db_handle yet.
+  if (download->db_handle() != kUninitializedHandle) {
+    in_progress_.erase(it);
+    NotifyAboutDownloadStop();
+    UpdateHistoryForDownload(download);
+  }
+
+  // If this a dangerous download not yet validated by the user, don't do
+  // anything. When the user notifies us, it will trigger a call to
+  // ProceedWithFinishedDangerousDownload.
+  if (download->safety_state() == DownloadItem::DANGEROUS) {
+    dangerous_finished_[download_id] = download;
+    return;
+  }
+
+  if (download->safety_state() == DownloadItem::DANGEROUS_BUT_VALIDATED) {
+    // We first need to rename the donwloaded file from its temporary name to
+    // its final name before we can continue.
+    file_loop_->PostTask(FROM_HERE,
+        NewRunnableMethod(
+            this, &DownloadManager::ProceedWithFinishedDangerousDownload,
+            download->db_handle(),
+            download->full_path(), download->original_name()));
+    return;
   }
+  ContinueDownloadFinished(download);
+}
+
+void DownloadManager::ContinueDownloadFinished(DownloadItem* download) {
+  // If this was a dangerous download, it has now been approved and must be
+  // removed from dangerous_finished_ so it does not get deleted on shutdown.
+  DownloadMap::iterator it = dangerous_finished_.find(download->id());
+  if (it != dangerous_finished_.end())
+    dangerous_finished_.erase(it);
+
+  // Notify our observers that we are complete (the call to Finished() set the
+  // state to complete but did not notify).
+  download->UpdateObservers();
+
+  // Open the download if the user or user prefs indicate it should be.
+  const std::wstring extension =
+      file_util::GetFileExtensionFromPath(download->full_path());
+  if (download->open_when_complete() || ShouldOpenFileExtension(extension))
+    OpenDownloadInShell(download, NULL);
+}
+
+// Called on the file thread.  Renames the downloaded file to its original name.
+void DownloadManager::ProceedWithFinishedDangerousDownload(
+    int64 download_handle,
+    const std::wstring& path,
+    const std::wstring& original_name) {
+  bool success = false;
+  std::wstring new_path = path;
+  if (file_util::PathExists(path)) {
+    new_path = file_util::GetDirectoryFromPath(new_path);
+    file_util::AppendToPath(&new_path, original_name);
+    success = file_util::Move(path, new_path);
+  } else {
+    NOTREACHED();
+  }
+  
+  ui_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &DownloadManager::DangerousDownloadRenamed,
+                        download_handle, success, new_path));
+}
+
+// Call from the file thread when the finished dangerous download was renamed.
+void DownloadManager::DangerousDownloadRenamed(int64 download_handle,
+                                               bool success,
+                                               const std::wstring& new_path) {
+  DownloadMap::iterator it = downloads_.find(download_handle);
+  if (it == downloads_.end()) {
+    NOTREACHED();
+    return;
+  }
+
+  DownloadItem* download = it->second;
+  // If we failed to rename the file, we'll just keep the name as is.
+  if (success)
+    RenameDownload(download, new_path);
+
+  // Continue the download finished sequence.
+  ContinueDownloadFinished(download);
 }
 
 // static
@@ -741,6 +874,27 @@ void DownloadManager::OnPauseDownloadRequest(ResourceDispatcherHost* rdh,
   rdh->PauseRequest(render_process_id, request_id, pause);
 }
 
+bool DownloadManager::IsDangerous(const std::wstring& file_name) {
+  // TODO(jcampan): Improve me.
+  return IsExecutable(file_util::GetFileExtensionFromPath(file_name));
+}
+
+void DownloadManager::RenameDownload(DownloadItem* download,
+                                     const std::wstring& new_path) {
+  download->Rename(new_path);
+
+  // Update the history.
+
+  // No update necessary if the download was initiated while in incognito mode.
+  if (download->db_handle() <= kUninitializedHandle)
+    return;
+
+  // FIXME(acw|paulg) see bug 958058. EXPLICIT_ACCESS below is wrong.
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (hs)
+    hs->UpdateDownloadPath(new_path, download->db_handle());
+}
+
 void DownloadManager::RemoveDownload(int64 download_handle) {
   DownloadMap::iterator it = downloads_.find(download_handle);
   if (it == downloads_.end())
@@ -752,6 +906,9 @@ void DownloadManager::RemoveDownload(int64 download_handle) {
 
   // Remove from our tables and delete.
   downloads_.erase(it);
+  it = dangerous_finished_.find(download->id());
+  if (it != dangerous_finished_.end())
+    dangerous_finished_.erase(it);
   delete download;
 
   // Tell observers to refresh their views.
@@ -1014,6 +1171,29 @@ void DownloadManager::FileSelectionCanceled(void* params) {
                         info->download_id));
 }
 
+void DownloadManager::DeleteDownload(const std::wstring& path) {
+  file_loop_->PostTask(FROM_HERE, NewRunnableMethod(
+      file_manager_, &DownloadFileManager::DeleteFile, path));
+}
+
+
+void DownloadManager::DangerousDownloadValidated(DownloadItem* download) {
+  DCHECK_EQ(DownloadItem::DANGEROUS, download->safety_state());
+  download->set_safety_state(DownloadItem::DANGEROUS_BUT_VALIDATED);
+  download->UpdateObservers();
+
+  // If the download is not complete, nothing to do.  The required
+  // post-processing will be performed when it does complete.
+  if (download->state() != DownloadItem::COMPLETE)
+    return;
+
+  file_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this,
+                        &DownloadManager::ProceedWithFinishedDangerousDownload,
+                        download->db_handle(), download->full_path(),
+                        download->original_name()));
+}
+
 // Operations posted to us from the history service ----------------------------
 
 // The history service has retrieved all download entries. 'entries' contains
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index 0ab3dc1..6e78fb6 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -80,6 +80,12 @@ class DownloadItem {
     REMOVING
   };
 
+  enum SafetyState {
+    SAFE = 0,
+    DANGEROUS,
+    DANGEROUS_BUT_VALIDATED  // Dangerous but the user confirmed the download.
+  };
+
   // Interface that observers of a particular download must implement in order
   // to receive updates to the download's status.
   class Observer {
@@ -94,10 +100,12 @@ class DownloadItem {
   DownloadItem(int32 download_id,
                const std::wstring& path,
                const std::wstring& url,
+               const std::wstring& original_name,
                const Time start_time,
                int64 download_size,
                int render_process_id,
-               int request_id);
+               int request_id,
+               bool is_dangerous);
 
   ~DownloadItem();
 
@@ -125,12 +133,12 @@ class DownloadItem {
   // when resuming a download (assuming the server supports byte ranges).
   void Cancel(bool update_history);
 
-  // Download operation completed
+  // Download operation completed.
   void Finished(int64 size);
 
-  // The user wants to remove the download from the views and history. This
-  // operation does not delete the file on the disk.
-  void Remove();
+  // The user wants to remove the download from the views and history. If
+  // |delete_file| is true, the file is deleted on the disk.
+  void Remove(bool delete_file);
 
   // Start/stop sending periodic updates to our observers
   void StartProgressTimer();
@@ -158,8 +166,10 @@ class DownloadItem {
 
   // Accessors
   DownloadState state() const { return state_; }
-  std::wstring full_path() const { return full_path_; }
   std::wstring file_name() const { return file_name_; }
+  void set_file_name(const std::wstring& name) { file_name_ = name; }
+  std::wstring full_path() const { return full_path_; }
+  void set_full_path(const std::wstring& path) { full_path_ = path; }
   std::wstring url() const { return url_; }
   int64 total_bytes() const { return total_bytes_; }
   void set_total_bytes(int64 total_bytes) { total_bytes_ = total_bytes; }
@@ -176,6 +186,16 @@ class DownloadItem {
   void set_open_when_complete(bool open) { open_when_complete_ = open; }
   int render_process_id() const { return render_process_id_; }
   int request_id() const { return request_id_; }
+  SafetyState safety_state() const { return safety_state_; }
+  void set_safety_state(SafetyState safety_state) {
+    safety_state_ = safety_state;
+  }
+  std::wstring original_name() const { return original_name_; }
+  void set_original_name(const std::wstring& name) { original_name_ = name; }
+
+  // Returns the file-name that should be reported to the user, which is
+  // file_name_ for safe downloads and original_name_ for dangerous ones.
+  std::wstring GetFileName() const;
 
  private:
   // Internal helper for maintaining consistent received and total sizes.
@@ -226,6 +246,14 @@ class DownloadItem {
   // A flag for indicating if the download should be opened at completion.
   bool open_when_complete_;
 
+  // Whether the download is considered potentially safe or dangerous
+  // (executable files are typically considered dangerous).
+  SafetyState safety_state_;
+
+  // Dangerous download are given temporary names until the user approves them.
+  // This stores their original name.
+  std::wstring original_name_;
+
   // For canceling or pausing requests.
   int render_process_id_;
   int request_id_;
@@ -354,6 +382,12 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   virtual void FileSelected(const std::wstring& path, void* params);
   virtual void FileSelectionCanceled(void* params);
 
+  // Deletes the specified path on the file thread.
+  void DeleteDownload(const std::wstring& path);
+
+  // Called when the user has validated the donwload of a dangerous file.
+  void DangerousDownloadValidated(DownloadItem* download);
+
  private:
   // Shutdown the download manager.  This call is needed only after Init.
   void Shutdown();
@@ -405,6 +439,32 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
                                      int request_id,
                                      bool pause);
 
+  // Performs the last steps required when a download has been completed.
+  // It is necessary to break down the flow when a download is finished as
+  // dangerous downloads are downloaded to temporary files that need to be
+  // renamed on the file thread first.
+  // Invoked on the UI thread.
+  void ContinueDownloadFinished(DownloadItem* download);
+
+  // Renames a finished dangerous download from its temporary file name to its
+  // real file name.
+  // Invoked on the file thread.
+  void ProceedWithFinishedDangerousDownload(int64 download_handle, 
+                                            const std::wstring& path,
+                                            const std::wstring& original_name);
+
+  // Invoked on the UI thread when a dangerous downloaded file has been renamed.
+  void DangerousDownloadRenamed(int64 download_handle,
+                                bool success,
+                                const std::wstring& new_path);
+
+  // Checks whether a file represents a risk if downloaded.
+  bool IsDangerous(const std::wstring& file_name);
+
+  // Changes the paths and file name of the specified |download|, propagating
+  // the change to the history system.
+  void RenameDownload(DownloadItem* download, const std::wstring& new_path);
+
   // 'downloads_' is map of all downloads in this profile. The key is the handle
   // returned by the history system, which is unique across sessions. This map
   // owns all the DownloadItems once they have been created in the history
@@ -415,6 +475,12 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   // ResourceDispatcherHost, which is unique for the current session. This map
   // does not own the DownloadItems.
   //
+  // 'dangerous_finished_' is a map of dangerous download that have finished
+  // but were not yet approved by the user.  Similarly to in_progress_, the key
+  // is the ID assigned by the ResourceDispatcherHost and the map does not own
+  // the DownloadItems.  It is used on shutdown to delete completed downloads
+  // that have not been approved.
+  // 
   // When a download is created through a user action, the corresponding
   // DownloadItem* is placed in 'in_progress_' and remains there until it has
   // received a valid handle from the history system. Once it has a valid
@@ -426,6 +492,7 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   typedef base::hash_map<int64, DownloadItem*> DownloadMap;
   DownloadMap downloads_;
   DownloadMap in_progress_;
+  DownloadMap dangerous_finished_;
 
   // True if the download manager has been initialized and requires a shutdown.
   bool shutdown_needed_;
diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc
index 530eade..96de46c 100644
--- a/chrome/browser/download/download_util.cc
+++ b/chrome/browser/download/download_util.cc
@@ -133,7 +133,7 @@ void BaseContextMenu::ExecuteCommand(int id) {
       break;
     }
     case REMOVE_ITEM:
-      download_->Remove();
+      download_->Remove(false);
       break;
     case CANCEL:
       download_->Cancel(true);
diff --git a/chrome/browser/download/save_package.cc b/chrome/browser/download/save_package.cc
index 127fa53..ff56be2 100644
--- a/chrome/browser/download/save_package.cc
+++ b/chrome/browser/download/save_package.cc
@@ -126,7 +126,7 @@ SavePackage::~SavePackage() {
     // We call this to remove the view from the shelf. It will invoke
     // DownloadManager::RemoveDownload, but since the fake DownloadItem is not
     // owned by DownloadManager, it will do nothing to our fake item.
-    download_->Remove();
+    download_->Remove(false);
     delete download_;
     download_ = NULL;
   }
@@ -174,8 +174,8 @@ bool SavePackage::Init() {
   }
 
   // Create the fake DownloadItem and display the view.
-  download_ = new DownloadItem(1, saved_main_file_path_,
-                               page_url_, Time::Now(), 0, -1, -1);
+  download_ = new DownloadItem(1, saved_main_file_path_, page_url_,
+                               std::wstring(), Time::Now(), 0, -1, -1, false);
   download_->set_manager(web_contents_->profile()->GetDownloadManager());
   DownloadShelfView* shelf = web_contents_->GetDownloadShelfView();
   shelf->AddDownloadView(new DownloadItemView(