Replace NewRunnable with Callback in DownloadManager.

content/browser/download/download_manager.cc

Index: content/browser/download/download_manager.cc
===================================================================
--- content/browser/download/download_manager.cc	(revision 107442)
+++ content/browser/download/download_manager.cc	(working copy)
@@ -15,7 +15,7 @@
 #include "base/stringprintf.h"
 #include "base/synchronization/lock.h"
 #include "base/sys_string_conversions.h"
-#include "base/task.h"
+#include "base/tuple.h"
 #include "build/build_config.h"
 #include "content/browser/browser_context.h"
 #include "content/browser/browser_thread.h"
@@ -37,24 +37,20 @@
 
 namespace {
 
-void BeginDownload(
-    const GURL& url,
-    const GURL& referrer,
-    const DownloadSaveInfo& save_info,
-    ResourceDispatcherHost* resource_dispatcher_host,
-    int render_process_id,
-    int render_view_id,
-    const content::ResourceContext* context) {
-  net::URLRequest* request = new net::URLRequest(url, resource_dispatcher_host);
-  request->set_referrer(referrer.spec());
-  resource_dispatcher_host->BeginDownload(
-      request,
-      save_info,
-      true,
+typedef Tuple7<GURL,                      // url
+               GURL,                      // referrer
+               DownloadSaveInfo,
+               ResourceDispatcherHost*,
+               int,                       // render_process_id
+               int,                       // render_view_id
+               const content::ResourceContext*> BeginDownloadParams;

[Randy Smith (Not in Mondays), 2011/10/28 17:37:53]

This is very sledgehammery, and the fact that we're accessing it with
params.a/b/c/d/e/f/g tends to reinforce that.  Could you possibly instead create
new structures to get us down below the limit that have some limited conceptual
integrity?  E.g. Combine the url and referred, and combine the render_process_id
and render_view_id?

[achuithb, 2011/10/28 19:51:05]

Done.

+
+void BeginDownload(const BeginDownloadParams& params) {
+  net::URLRequest* request = new net::URLRequest(params.a, params.d);
+  request->set_referrer(params.b.spec());
+  params.d->BeginDownload(request, params.c, true,
       DownloadResourceHandler::OnStartedCallback(),
-      render_process_id,
-      render_view_id,
-      *context);
+      params.e, params.f, *params.g);
 }
 
 }  // namespace
@@ -94,9 +90,8 @@
 
   if (file_manager_) {
     BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-        NewRunnableMethod(file_manager_,
-                          &DownloadFileManager::OnDownloadManagerShutdown,
-                          make_scoped_refptr(this)));
+        base::Bind(&DownloadFileManager::OnDownloadManagerShutdown,
+                   file_manager_, make_scoped_refptr(this)));
   }
 
   AssertContainersConsistent();
@@ -263,10 +258,9 @@
       !download_item->file_externally_removed()) {
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
-        NewRunnableMethod(this,
-                          &DownloadManager::CheckForFileRemovalOnFileThread,
-                          download_item->db_handle(),
-                          download_item->GetTargetFilePath()));
+        base::Bind(&DownloadManager::CheckForFileRemovalOnFileThread,
+                   this, download_item->db_handle(),
+                   download_item->GetTargetFilePath()));
   }
 }
 
@@ -276,9 +270,7 @@
   if (!file_util::PathExists(path)) {
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(this,
-                          &DownloadManager::OnFileRemovalDetected,
-                          db_handle));
+        base::Bind(&DownloadManager::OnFileRemovalDetected, this, db_handle));
   }
 }
 
@@ -371,9 +363,8 @@
 
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          file_manager_, &DownloadFileManager::RenameInProgressDownloadFile,
-          download->global_id(), download_path));
+      base::Bind(&DownloadFileManager::RenameInProgressDownloadFile,
+                 file_manager_, download->global_id(), download_path));
 
   download->Rename(download_path);
 
@@ -534,10 +525,10 @@
     DCHECK_EQ(0, uniquifier) << "We should not uniquify SAFE downloads twice";
   }
 
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, NewRunnableMethod(
-      file_manager_,
-      &DownloadFileManager::CompleteDownload,
-      item->global_id()));
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DownloadFileManager::CompleteDownload,
+                 file_manager_, item->global_id()));
 
   if (uniquifier)
     item->set_path_uniquifier(uniquifier);
@@ -718,16 +709,14 @@
       content::GetContentClient()->browser()->GetResourceDispatcherHost();
   // We send a pointer to content::ResourceContext, instead of the usual
   // reference, so that a copy of the object isn't made.
-  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-      NewRunnableFunction(&BeginDownload,
-                          url,
-                          referrer,
-                          save_info,
-                          resource_dispatcher_host,
-                          tab_contents->GetRenderProcessHost()->id(),
-                          tab_contents->render_view_host()->routing_id(),
-                          &tab_contents->browser_context()->
-                              GetResourceContext()));
+  // We use Tuple7 because base::Bind can't handle 7 arguments.
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&BeginDownload, MakeTuple(
+          url, referrer, save_info, resource_dispatcher_host,
+          tab_contents->GetRenderProcessHost()->id(),
+          tab_contents->render_view_host()->routing_id(),
+          &tab_contents->browser_context()->GetResourceContext())));
 }
 
 void DownloadManager::AddObserver(Observer* observer) {