Allowing multiple SaveItems to have same URLs.

content/browser/download/save_file_manager.cc

Index: content/browser/download/save_file_manager.cc
diff --git a/content/browser/download/save_file_manager.cc b/content/browser/download/save_file_manager.cc
index 3b6cd22715f673ce6ffff738caa5642dc748a21a..01874d0f6895684c63b8f167c479d5d1dfc5957d 100644
--- a/content/browser/download/save_file_manager.cc
+++ b/content/browser/download/save_file_manager.cc
@@ -25,9 +25,7 @@
 
 namespace content {
 
-SaveFileManager::SaveFileManager()
-    : next_id_(0) {
-}
+SaveFileManager::SaveFileManager() {}
 
 SaveFileManager::~SaveFileManager() {
   // Check for clean shutdown.
@@ -48,72 +46,23 @@ void SaveFileManager::OnShutdown() {
   STLDeleteValues(&save_file_map_);
 }
 
-SaveFile* SaveFileManager::LookupSaveFile(int save_id) {
-  SaveFileMap::iterator it = save_file_map_.find(save_id);
+SaveFile* SaveFileManager::LookupSaveFile(int save_item_id) {
+  SaveFileMap::iterator it = save_file_map_.find(save_item_id);
   return it == save_file_map_.end() ? NULL : it->second;
 }
 
-// Called on the IO thread when
-// a) The ResourceDispatcherHostImpl has decided that a request is savable.
-// b) The resource does not come from the network, but we still need a
-// save ID for for managing the status of the saving operation. So we
-// file a request from the file thread to the IO thread to generate a
-// unique save ID.
-int SaveFileManager::GetNextId() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  return next_id_++;
-}
-
-void SaveFileManager::RegisterStartingRequest(const GURL& save_url,
-                                              SavePackage* save_package) {
-  // Make sure it runs in the UI thread.
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  int save_package_id = save_package->id();
-
-  // Register this starting request.
-  StartingRequestsMap& starting_requests =
-      contents_starting_requests_[save_package_id];
-  bool never_present = starting_requests.insert(
-      StartingRequestsMap::value_type(save_url.spec(), save_package)).second;
-  DCHECK(never_present);
-}
-
-SavePackage* SaveFileManager::UnregisterStartingRequest(const GURL& save_url,
-                                                        int save_package_id) {
-  // Make sure it runs in UI thread.
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  ContentsToStartingRequestsMap::iterator it =
-      contents_starting_requests_.find(save_package_id);
-  if (it != contents_starting_requests_.end()) {
-    StartingRequestsMap& requests = it->second;
-    StartingRequestsMap::iterator sit = requests.find(save_url.spec());
-    if (sit == requests.end())
-      return NULL;
-
-    // Found, erase it from starting list and return SavePackage.
-    SavePackage* save_package = sit->second;
-    requests.erase(sit);
-    // If there is no element in requests, remove it
-    if (requests.empty())
-      contents_starting_requests_.erase(it);
-    return save_package;
-  }
-
-  return NULL;
-}
-
 // Look up a SavePackage according to a save id.
-SavePackage* SaveFileManager::LookupPackage(int save_id) {
+SavePackage* SaveFileManager::LookupPackage(int save_item_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  SavePackageMap::iterator it = packages_.find(save_id);
+  SavePackageMap::iterator it = packages_.find(save_item_id);
   if (it != packages_.end())
     return it->second;
   return NULL;
 }
 
 // Call from SavePackage for starting a saving job
-void SaveFileManager::SaveURL(const GURL& url,
+void SaveFileManager::SaveURL(int save_item_id,
+                              const GURL& url,
                               const Referrer& referrer,
                               int render_process_host_id,
                               int render_view_routing_id,
@@ -125,56 +74,40 @@ void SaveFileManager::SaveURL(const GURL& url,
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // Register a saving job.
-  RegisterStartingRequest(url, save_package);
   if (save_source == SaveFileCreateInfo::SAVE_FILE_FROM_NET) {
     DCHECK(url.is_valid());
 
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&SaveFileManager::OnSaveURL, this, url, referrer,
-                   save_package->id(), render_process_host_id,
+                   save_item_id, save_package->id(), render_process_host_id,
                    render_view_routing_id, render_frame_routing_id, context));
   } else {
     // We manually start the save job.
-    SaveFileCreateInfo* info = new SaveFileCreateInfo(file_full_path,
-         url,
-         save_source,
-         -1);
-    info->render_process_id = render_process_host_id;
-    info->render_frame_routing_id = render_frame_routing_id;
-    info->save_package_id = save_package->id();
+    SaveFileCreateInfo* info = new SaveFileCreateInfo(
+        file_full_path, url, save_item_id, save_package->id(),
+        render_process_host_id, render_frame_routing_id, save_source);
 
     // Since the data will come from render process, so we need to start
     // this kind of save job by ourself.
     BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        base::Bind(&SaveFileManager::OnRequireSaveJobFromOtherSource,
-            this, info));
+        BrowserThread::FILE, FROM_HERE,
+        base::Bind(&SaveFileManager::StartSave, this, info));
   }
 }
 
 // Utility function for look up table maintenance, called on the UI thread.
 // A manager may have multiple save page job (SavePackage) in progress,
 // so we just look up the save id and remove it from the tracking table.
-// If the save id is -1, it means we just send a request to save, but the
-// saving action has still not happened, need to call UnregisterStartingRequest
-// to remove it from the tracking map.
-void SaveFileManager::RemoveSaveFile(int save_id,
-                                     const GURL& save_url,
+void SaveFileManager::RemoveSaveFile(int save_item_id,
                                      SavePackage* save_package) {
   DCHECK(save_package);
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // A save page job (SavePackage) can only have one manager,
   // so remove it if it exists.
-  if (save_id == -1) {
-    SavePackage* old_package =
-        UnregisterStartingRequest(save_url, save_package->id());
-    DCHECK_EQ(old_package, save_package);
-  } else {
-    SavePackageMap::iterator it = packages_.find(save_id);
-    if (it != packages_.end())
-      packages_.erase(it);
-  }
+  SavePackageMap::iterator it = packages_.find(save_item_id);
+  if (it != packages_.end())
+    packages_.erase(it);
 }
 
 // Static
@@ -204,12 +137,12 @@ void SaveFileManager::DeleteDirectoryOrFile(const base::FilePath& full_path,
           this, full_path, is_dir));
 }
 
-void SaveFileManager::SendCancelRequest(int save_id) {
+void SaveFileManager::SendCancelRequest(int save_item_id) {
   // Cancel the request which has specific save id.
-  DCHECK_GT(save_id, -1);
+  DCHECK_GT(save_item_id, -1);
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      base::Bind(&SaveFileManager::CancelSave, this, save_id));
+      base::Bind(&SaveFileManager::CancelSave, this, save_item_id));
 }
 
 // Notifications sent from the IO thread and run on the file thread:
@@ -226,8 +159,8 @@ void SaveFileManager::StartSave(SaveFileCreateInfo* info) {
   // TODO(phajdan.jr): We should check the return value and handle errors here.
   save_file->Initialize();
 
-  DCHECK(!LookupSaveFile(info->save_id));
-  save_file_map_[info->save_id] = save_file;
+  DCHECK(!LookupSaveFile(info->save_item_id));
+  save_file_map_[info->save_item_id] = save_file;
   info->path = save_file->FullPath();
 
   BrowserThread::PostTask(
@@ -239,11 +172,11 @@ void SaveFileManager::StartSave(SaveFileCreateInfo* info) {
 // update the UI. If the user has canceled the saving action (in the UI
 // thread). We may receive a few more updates before the IO thread gets the
 // cancel message. We just delete the data since the SaveFile has been deleted.
-void SaveFileManager::UpdateSaveProgress(int save_id,
+void SaveFileManager::UpdateSaveProgress(int save_item_id,
                                          net::IOBuffer* data,
                                          int data_len) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  SaveFile* save_file = LookupSaveFile(save_id);
+  SaveFile* save_file = LookupSaveFile(save_item_id);
   if (save_file) {
     DCHECK(save_file->InProgress());
 
@@ -251,54 +184,36 @@ void SaveFileManager::UpdateSaveProgress(int save_id,
         save_file->AppendDataToFile(data->data(), data_len);
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        base::Bind(&SaveFileManager::OnUpdateSaveProgress,
-                   this,
-                   save_file->save_id(),
-                   save_file->BytesSoFar(),
+        base::Bind(&SaveFileManager::OnUpdateSaveProgress, this,
+                   save_file->save_item_id(), save_file->BytesSoFar(),
                    reason == DOWNLOAD_INTERRUPT_REASON_NONE));
   }
 }
 
 // The IO thread will call this when saving is completed or it got error when
-// fetching data. In the former case, we forward the message to OnSaveFinished
-// in UI thread. In the latter case, the save ID will be -1, which means the
-// saving action did not even start, so we need to call OnErrorFinished in UI
-// thread, which will use the save URL to find corresponding request record and
-// delete it.
-void SaveFileManager::SaveFinished(int save_id,
-                                   const GURL& save_url,
+// fetching data. We forward the message to OnSaveFinished in UI thread.
+void SaveFileManager::SaveFinished(int save_item_id,
                                    int save_package_id,
                                    bool is_success) {
   DVLOG(20) << " " << __FUNCTION__ << "()"
-            << " save_id = " << save_id << " save_url = \"" << save_url.spec()
-            << "\""
+            << " save_item_id = " << save_item_id
             << " save_package_id = " << save_package_id
             << " is_success = " << is_success;
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  SaveFileMap::iterator it = save_file_map_.find(save_id);
+  SaveFileMap::iterator it = save_file_map_.find(save_item_id);
   if (it != save_file_map_.end()) {
     SaveFile* save_file = it->second;
-    // This routine may be called twice from the same SavePackage - once for the
-    // file itself, and once when all frames have been serialized.
-    // So we can't assert that the file is InProgress() here.
-    // TODO(rdsmith): Fix this logic and put the DCHECK below back in.
-    // DCHECK(save_file->InProgress());
+    DCHECK(save_file->InProgress());
 
     DVLOG(20) << " " << __FUNCTION__ << "()"
               << " save_file = " << save_file->DebugString();
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        base::Bind(&SaveFileManager::OnSaveFinished, this, save_id,
-            save_file->BytesSoFar(), is_success));
+        base::Bind(&SaveFileManager::OnSaveFinished, this, save_item_id,
+                   save_file->BytesSoFar(), is_success));
 
     save_file->Finish();
     save_file->Detach();
-  } else if (save_id == -1) {
-    // Before saving started, we got error. We still call finish process.
-    DCHECK(!save_url.is_empty());
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                            base::Bind(&SaveFileManager::OnErrorFinished, this,
-                                       save_url, save_package_id));
   }
 }
 
@@ -310,64 +225,44 @@ void SaveFileManager::OnStartSave(const SaveFileCreateInfo* info) {
       info->render_process_id, info->render_frame_routing_id);
   if (!save_package) {
     // Cancel this request.
-    SendCancelRequest(info->save_id);
+    SendCancelRequest(info->save_item_id);
     return;
   }
 
   // Insert started saving job to tracking list.
-  SavePackageMap::iterator sit = packages_.find(info->save_id);
-  if (sit == packages_.end()) {
-    // Find the registered request. If we can not find, it means we have
-    // canceled the job before.
-    SavePackage* old_save_package =
-        UnregisterStartingRequest(info->url, info->save_package_id);
-    if (!old_save_package) {
-      // Cancel this request.
-      SendCancelRequest(info->save_id);
-      return;
-    }
-    DCHECK_EQ(old_save_package, save_package);
-    packages_[info->save_id] = save_package;
-  } else {
-    NOTREACHED();
-  }
+  SavePackageMap::iterator sit = packages_.find(info->save_item_id);
+  DCHECK(sit == packages_.end());
+  packages_[info->save_item_id] = save_package;
 
   // Forward this message to SavePackage.
   save_package->StartSave(info);
 }
 
-void SaveFileManager::OnUpdateSaveProgress(int save_id, int64 bytes_so_far,
+void SaveFileManager::OnUpdateSaveProgress(int save_item_id,
+                                           int64 bytes_so_far,
                                            bool write_success) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  SavePackage* package = LookupPackage(save_id);
+  SavePackage* package = LookupPackage(save_item_id);
   if (package)
-    package->UpdateSaveProgress(save_id, bytes_so_far, write_success);
+    package->UpdateSaveProgress(save_item_id, bytes_so_far, write_success);
   else
-    SendCancelRequest(save_id);
+    SendCancelRequest(save_item_id);
 }
 
-void SaveFileManager::OnSaveFinished(int save_id,
+void SaveFileManager::OnSaveFinished(int save_item_id,
                                      int64 bytes_so_far,
                                      bool is_success) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  SavePackage* package = LookupPackage(save_id);
+  SavePackage* package = LookupPackage(save_item_id);
   if (package)
-    package->SaveFinished(save_id, bytes_so_far, is_success);
-}
-
-void SaveFileManager::OnErrorFinished(const GURL& save_url,
-                                      int save_package_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  SavePackage* save_package =
-      UnregisterStartingRequest(save_url, save_package_id);
-  if (save_package)
-    save_package->SaveFailed(save_url);
+    package->SaveFinished(save_item_id, bytes_so_far, is_success);
 }
 
 // Notifications sent from the UI thread and run on the IO thread.
 
 void SaveFileManager::OnSaveURL(const GURL& url,
                                 const Referrer& referrer,
+                                int save_item_id,
                                 int save_package_id,
                                 int render_process_host_id,
                                 int render_view_routing_id,
@@ -375,22 +270,10 @@ void SaveFileManager::OnSaveURL(const GURL& url,
                                 ResourceContext* context) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   ResourceDispatcherHostImpl::Get()->BeginSaveFile(
-      url, referrer, save_package_id, render_process_host_id,
+      url, referrer, save_item_id, save_package_id, render_process_host_id,
       render_view_routing_id, render_frame_routing_id, context);
 }
 
-void SaveFileManager::OnRequireSaveJobFromOtherSource(
-    SaveFileCreateInfo* info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  DCHECK_EQ(info->save_id, -1);
-  // Generate a unique save id.
-  info->save_id = GetNextId();
-  // Start real saving action.
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&SaveFileManager::StartSave, this, info));
-}
-
 void SaveFileManager::ExecuteCancelSaveRequest(int render_process_id,
                                                int request_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -405,9 +288,9 @@ void SaveFileManager::ExecuteCancelSaveRequest(int render_process_id,
 // but we do forward the cancel to the IO thread. Since this message has been
 // sent from the UI thread, the saving job may have already completed and
 // won't exist in our map.
-void SaveFileManager::CancelSave(int save_id) {
+void SaveFileManager::CancelSave(int save_item_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  SaveFileMap::iterator it = save_file_map_.find(save_id);
+  SaveFileMap::iterator it = save_file_map_.find(save_item_id);
   if (it != save_file_map_.end()) {
     SaveFile* save_file = it->second;
 
@@ -435,14 +318,15 @@ void SaveFileManager::CancelSave(int save_id) {
   }
 }
 
-// It is possible that SaveItem which has specified save_id has been canceled
+// It is possible that SaveItem which has specified save_item_id has been
+// canceled
 // before this function runs. So if we can not find corresponding SaveFile by
-// using specified save_id, just return.
+// using specified save_item_id, just return.
 void SaveFileManager::SaveLocalFile(const GURL& original_file_url,
-                                    int save_id,
+                                    int save_item_id,
                                     int save_package_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  SaveFile* save_file = LookupSaveFile(save_id);
+  SaveFile* save_file = LookupSaveFile(save_item_id);
   if (!save_file)
     return;
   // If it has finished, just return.
@@ -459,14 +343,14 @@ void SaveFileManager::SaveLocalFile(const GURL& original_file_url,
   // If we can not get valid file path from original URL, treat it as
   // disk error.
   if (file_path.empty())
-    SaveFinished(save_id, original_file_url, save_package_id, false);
+    SaveFinished(save_item_id, save_package_id, false);
 
   // Copy the local file to the temporary file. It will be renamed to its
   // final name later.
   bool success = base::CopyFile(file_path, save_file->FullPath());
   if (!success)
     base::DeleteFile(save_file->FullPath(), false);
-  SaveFinished(save_id, original_file_url, save_package_id, success);
+  SaveFinished(save_item_id, save_package_id, success);
 }
 
 void SaveFileManager::OnDeleteDirectoryOrFile(const base::FilePath& full_path,
@@ -477,7 +361,7 @@ void SaveFileManager::OnDeleteDirectoryOrFile(const base::FilePath& full_path,
   base::DeleteFile(full_path, is_dir);
 }
 
-void SaveFileManager::RenameAllFiles(const FinalNameList& final_names,
+void SaveFileManager::RenameAllFiles(const FinalNamesMap& final_names,
                                      const base::FilePath& resource_dir,
                                      int render_process_id,
                                      int render_frame_routing_id,
@@ -487,13 +371,15 @@ void SaveFileManager::RenameAllFiles(const FinalNameList& final_names,
   if (!resource_dir.empty() && !base::PathExists(resource_dir))
     base::CreateDirectory(resource_dir);
 
-  for (FinalNameList::const_iterator i = final_names.begin();
-      i != final_names.end(); ++i) {
-    SaveFileMap::iterator it = save_file_map_.find(i->first);
+  for (const auto& i : final_names) {
+    int save_item_id = i.first;
+    const base::FilePath& final_name = i.second;
+
+    SaveFileMap::iterator it = save_file_map_.find(save_item_id);
     if (it != save_file_map_.end()) {
       SaveFile* save_file = it->second;
       DCHECK(!save_file->InProgress());
-      save_file->Rename(i->second);
+      save_file->Rename(final_name);
       delete save_file;
       save_file_map_.erase(it);
     }
@@ -518,12 +404,11 @@ void SaveFileManager::OnFinishSavePageJob(int render_process_id,
 }
 
 void SaveFileManager::RemoveSavedFileFromFileMap(
-    const SaveIDList& save_ids) {
+    const std::vector<int>& save_item_ids) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-  for (SaveIDList::const_iterator i = save_ids.begin();
-      i != save_ids.end(); ++i) {
-    SaveFileMap::iterator it = save_file_map_.find(*i);
+  for (const int save_item_id : save_item_ids) {
+    SaveFileMap::iterator it = save_file_map_.find(save_item_id);
     if (it != save_file_map_.end()) {
       SaveFile* save_file = it->second;
       DCHECK(!save_file->InProgress());