Added DownloadProcessHandle class.

chrome/browser/download/download_file_manager.cc

Index: chrome/browser/download/download_file_manager.cc
diff --git a/chrome/browser/download/download_file_manager.cc b/chrome/browser/download/download_file_manager.cc
index 2522a9804c7ea7b1a737e287a4be5f0582b7871b..41efb5e1eecd4606050e09b827f3176cac16106b 100644
--- a/chrome/browser/download/download_file_manager.cc
+++ b/chrome/browser/download/download_file_manager.cc
@@ -11,6 +11,7 @@
 #include "base/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/download/download_manager.h"
+#include "chrome/browser/download/download_process_handle.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_create_info.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
@@ -30,21 +31,6 @@ namespace {
 // cause it to become unresponsive (in milliseconds).
 const int kUpdatePeriodMs = 500;
 
-DownloadManager* DownloadManagerForRenderViewHost(int render_process_id,
-                                                  int render_view_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  TabContents* contents = tab_util::GetTabContentsByID(render_process_id,
-                                                       render_view_id);
-  if (contents) {
-    Profile* profile = contents->profile();
-    if (profile)
-      return profile->GetDownloadManager();
-  }
-
-  return NULL;
-}
-
 }  // namespace
 
 DownloadFileManager::DownloadFileManager(ResourceDispatcherHost* rdh)
@@ -70,6 +56,7 @@ void DownloadFileManager::OnShutdown() {
 }
 
 void DownloadFileManager::CreateDownloadFile(DownloadCreateInfo* info,
+                                             DownloadProcessHandle process,
                                              DownloadManager* download_manager,
                                              bool get_hash) {
   VLOG(20) << __FUNCTION__ << "()" << " info = " << info->DebugString();
@@ -78,13 +65,7 @@ void DownloadFileManager::CreateDownloadFile(DownloadCreateInfo* info,
   scoped_ptr<DownloadFile>
       download_file(new DownloadFile(info, download_manager));
   if (!download_file->Initialize(get_hash)) {
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        NewRunnableFunction(&download_util::CancelDownloadRequest,
-                            resource_dispatcher_host_,
-                            info->child_id,
-                            info->request_id));
-    delete info;
+    download_util::CancelDownloadRequest(resource_dispatcher_host_, process);
     return;

[Paweł Hajdan Jr., 2011/05/06 18:04:42]

This used to delete |info|, now that code is gone. Was that intentional? If so,
doesn't that introduce a leak?

[ahendrickson, 2011/05/09 15:57:31]

Ah, you're right.  This is left over from splitting the CL, where |info| was
deleted automatically in this routine.

Fixed.

   }
 
@@ -97,7 +78,7 @@ void DownloadFileManager::CreateDownloadFile(DownloadCreateInfo* info,
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       NewRunnableMethod(this, &DownloadFileManager::ResumeDownloadRequest,
-                        info->child_id, info->request_id));
+                        process));
 
   StartUpdateTimer();
 
@@ -107,11 +88,14 @@ void DownloadFileManager::CreateDownloadFile(DownloadCreateInfo* info,
                         &DownloadManager::StartDownload, info));
 }
 
-void DownloadFileManager::ResumeDownloadRequest(int child_id, int request_id) {
+void DownloadFileManager::ResumeDownloadRequest(
+    DownloadProcessHandle process) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   // This balances the pause in DownloadResourceHandler::OnResponseStarted.
-  resource_dispatcher_host_->PauseRequest(child_id, request_id, false);
+  resource_dispatcher_host_->PauseRequest(process.child_id(),
+                                          process.request_id(),
+                                          false);
 }
 
 DownloadFile* DownloadFileManager::GetDownloadFile(int id) {
@@ -158,15 +142,10 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(info);
 
-  DownloadManager* manager = DownloadManagerForRenderViewHost(
-      info->child_id, info->render_view_id);
+  DownloadManager* manager = info->process_handle.GetDownloadManager();
   if (!manager) {
-    BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        NewRunnableFunction(&download_util::CancelDownloadRequest,
-                            resource_dispatcher_host_,
-                            info->child_id,
-                            info->request_id));
+    download_util::CancelDownloadRequest(resource_dispatcher_host_,
+                                         info->process_handle);
     delete info;
     return;
   }
@@ -178,7 +157,8 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
 
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
       NewRunnableMethod(this, &DownloadFileManager::CreateDownloadFile,
-                        info, make_scoped_refptr(manager), hash_needed));
+                        info, info->process_handle,

[Paweł Hajdan Jr., 2011/05/06 18:04:42]

If we pass info, why do we also need info->process_handle?

Also, just wondering - if we can extract a DownloadManager from the process
handle, do we need to pass |manager| as a parameter here?

[ahendrickson, 2011/05/09 15:57:31]

We don't need to pass in process_handle.  Removed the parameter.

I believe that the intent was to make sure that download_manager stayed valid
until after |CreateDownloadFile| was called.

+                        make_scoped_refptr(manager), hash_needed));
 }
 
 // We don't forward an update to the UI thread here, since we want to throttle