Download code cleanup:

- choose better names for some helper methods
- move code to less random places

This change also adds bigger tuples support, up to Tuple8 in base/tuple.h.
The plan is to stop using such big number of parameters, but for now it's not
trivial.

This change also fixes some UI tests, not sure why it is so.

TEST=unit_tests, browser_tests, ui_tests
BUG=48913

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

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

8 files changed, 202 insertions, 107 deletions

diff --git a/base/task.h b/base/task.h
index 64b10f3..0cff589 100644
--- a/base/task.h
+++ b/base/task.h
@@ -490,4 +490,36 @@ inline CancelableTask* NewRunnableFunction(Function function,
                                                                           e));
 }
 
+template <class Function, class A, class B, class C, class D, class E,
+          class F>
+inline CancelableTask* NewRunnableFunction(Function function,
+                                           const A& a, const B& b,
+                                           const C& c, const D& d,
+                                           const E& e, const F& f) {
+  return new RunnableFunction<Function, Tuple6<A, B, C, D, E, F> >(function,
+      MakeTuple(a, b, c, d, e, f));
+}
+
+template <class Function, class A, class B, class C, class D, class E,
+          class F, class G>
+inline CancelableTask* NewRunnableFunction(Function function,
+                                           const A& a, const B& b,
+                                           const C& c, const D& d,
+                                           const E& e, const F& f,
+                                           const G& g) {
+  return new RunnableFunction<Function, Tuple7<A, B, C, D, E, F, G> >(function,
+      MakeTuple(a, b, c, d, e, f, g));
+}
+
+template <class Function, class A, class B, class C, class D, class E,
+          class F, class G, class H>
+inline CancelableTask* NewRunnableFunction(Function function,
+                                           const A& a, const B& b,
+                                           const C& c, const D& d,
+                                           const E& e, const F& f,
+                                           const G& g, const H& h) {
+  return new RunnableFunction<Function, Tuple8<A, B, C, D, E, F, G, H> >(
+      function, MakeTuple(a, b, c, d, e, f, g, h));
+}
+
 #endif  // BASE_TASK_H_
diff --git a/base/tuple.h b/base/tuple.h
index b3a7515..d17d9f5 100644
--- a/base/tuple.h
+++ b/base/tuple.h
@@ -307,6 +307,65 @@ struct Tuple7 {
   G g;
 };
 
+template <class A, class B, class C, class D, class E, class F, class G,
+          class H>
+struct Tuple8 {
+ public:
+  typedef A TypeA;
+  typedef B TypeB;
+  typedef C TypeC;
+  typedef D TypeD;
+  typedef E TypeE;
+  typedef F TypeF;
+  typedef G TypeG;
+  typedef H TypeH;
+  typedef Tuple8<typename TupleTraits<A>::ValueType,
+    typename TupleTraits<B>::ValueType,
+    typename TupleTraits<C>::ValueType,
+    typename TupleTraits<D>::ValueType,
+    typename TupleTraits<E>::ValueType,
+    typename TupleTraits<F>::ValueType,
+    typename TupleTraits<G>::ValueType,
+    typename TupleTraits<H>::ValueType> ValueTuple;
+  typedef Tuple8<typename TupleTraits<A>::RefType,
+    typename TupleTraits<B>::RefType,
+    typename TupleTraits<C>::RefType,
+    typename TupleTraits<D>::RefType,
+    typename TupleTraits<E>::RefType,
+    typename TupleTraits<F>::RefType,
+    typename TupleTraits<G>::RefType,
+    typename TupleTraits<H>::RefType> RefTuple;
+  typedef Tuple8<typename TupleTraits<A>::ParamType,
+    typename TupleTraits<B>::ParamType,
+    typename TupleTraits<C>::ParamType,
+    typename TupleTraits<D>::ParamType,
+    typename TupleTraits<E>::ParamType,
+    typename TupleTraits<F>::ParamType,
+    typename TupleTraits<G>::ParamType,
+    typename TupleTraits<H>::ParamType> ParamTuple;
+
+  Tuple8() {}
+  Tuple8(typename TupleTraits<A>::ParamType a,
+    typename TupleTraits<B>::ParamType b,
+    typename TupleTraits<C>::ParamType c,
+    typename TupleTraits<D>::ParamType d,
+    typename TupleTraits<E>::ParamType e,
+    typename TupleTraits<F>::ParamType f,
+    typename TupleTraits<G>::ParamType g,
+    typename TupleTraits<H>::ParamType h)
+    : a(a), b(b), c(c), d(d), e(e), f(f), g(g), h(h) {
+  }
+
+  A a;
+  B b;
+  C c;
+  D d;
+  E e;
+  F f;
+  G g;
+  H h;
+};
+
 // Tuple creators -------------------------------------------------------------
 //
 // Helper functions for constructing tuples while inferring the template
@@ -356,6 +415,15 @@ inline Tuple7<A, B, C, D, E, F, G> MakeTuple(const A& a, const B& b, const C& c,
   return Tuple7<A, B, C, D, E, F, G>(a, b, c, d, e, f, g);
 }
 
+template <class A, class B, class C, class D, class E, class F, class G,
+          class H>
+inline Tuple8<A, B, C, D, E, F, G, H> MakeTuple(const A& a, const B& b,
+                                                const C& c, const D& d,
+                                                const E& e, const F& f,
+                                                const G& g, const H& h) {
+  return Tuple8<A, B, C, D, E, F, G, H>(a, b, c, d, e, f, g, h);
+}
+
 // The following set of helpers make what Boost refers to as "Tiers" - a tuple
 // of references.
 
@@ -396,6 +464,14 @@ inline Tuple7<A&, B&, C&, D&, E&, F&, G&> MakeRefTuple(A& a, B& b, C& c, D& d,
   return Tuple7<A&, B&, C&, D&, E&, F&, G&>(a, b, c, d, e, f, g);
 }
 
+template <class A, class B, class C, class D, class E, class F, class G,
+          class H>
+inline Tuple8<A&, B&, C&, D&, E&, F&, G&, H&> MakeRefTuple(A& a, B& b, C& c,
+                                                           D& d, E& e, F& f,
+                                                           G& g, H& h) {
+  return Tuple8<A&, B&, C&, D&, E&, F&, G&, H&>(a, b, c, d, e, f, g, h);
+}
+
 // Dispatchers ----------------------------------------------------------------
 //
 // Helper functions that call the given method on an object, with the unpacked
@@ -506,6 +582,20 @@ inline void DispatchToFunction(Function function,
   (*function)(arg.a, arg.b, arg.c, arg.d, arg.e, arg.f);
 }
 
+template<class Function, class A, class B, class C, class D, class E, class F,
+         class G>
+inline void DispatchToFunction(Function function,
+                               const Tuple7<A, B, C, D, E, F, G>& arg) {
+  (*function)(arg.a, arg.b, arg.c, arg.d, arg.e, arg.f, arg.g);
+}
+
+template<class Function, class A, class B, class C, class D, class E, class F,
+         class G, class H>
+inline void DispatchToFunction(Function function,
+                               const Tuple8<A, B, C, D, E, F, G, H>& arg) {
+  (*function)(arg.a, arg.b, arg.c, arg.d, arg.e, arg.f, arg.g, arg.h);
+}
+
 // Dispatchers with 0 out param (as a Tuple0).
 
 template <class ObjT, class Method>
diff --git a/chrome/browser/download/download_file_manager.cc b/chrome/browser/download/download_file_manager.cc
index 04336cf..68c5e9c 100644
--- a/chrome/browser/download/download_file_manager.cc
+++ b/chrome/browser/download/download_file_manager.cc
@@ -10,6 +10,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/download/download_manager.h"
+#include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_types.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/platform_util.h"
@@ -42,7 +43,6 @@ DownloadFileManager::DownloadFileManager(ResourceDispatcherHost* rdh)
 DownloadFileManager::~DownloadFileManager() {
   // Check for clean shutdown.
   DCHECK(downloads_.empty());
-  ui_progress_.clear();
 }
 
 // Called during the browser shutdown process to clean up any state (open files,
@@ -69,32 +69,6 @@ void DownloadFileManager::OnShutdown() {
   downloads_.clear();
 }
 
-// Initiate a request for URL to be downloaded. Called from UI thread,
-// runs on IO thread.
-void DownloadFileManager::OnDownloadUrl(
-    const GURL& url,
-    const GURL& referrer,
-    const std::string& referrer_charset,
-    const DownloadSaveInfo& save_info,
-    int render_process_host_id,
-    int render_view_id,
-    URLRequestContextGetter* request_context_getter) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-
-  URLRequestContext* context = request_context_getter->GetURLRequestContext();
-  context->set_referrer_charset(referrer_charset);
-
-  // Show "Save As" UI.
-  bool prompt_for_save_location = true;
-  resource_dispatcher_host_->BeginDownload(url,
-                                           referrer,
-                                           save_info,
-                                           prompt_for_save_location,
-                                           render_process_host_id,
-                                           render_view_id,
-                                           context);
-}
-
 // Notifications sent from the download thread and run on the UI thread.
 
 // Lookup the DownloadManager for this TabContents' profile and inform it of
@@ -108,7 +82,7 @@ void DownloadFileManager::OnStartDownload(DownloadCreateInfo* info) {
   if (!manager) {
     ChromeThread::PostTask(
         ChromeThread::IO, FROM_HERE,
-        NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
+        NewRunnableFunction(&download_util::CancelDownloadRequest,
                             resource_dispatcher_host_,
                             info->child_id,
                             info->request_id));
@@ -143,7 +117,7 @@ void DownloadFileManager::OnStartDownload(DownloadCreateInfo* info) {
 void DownloadFileManager::OnDownloadFinished(int id,
                                              int64 bytes_so_far) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  DownloadManager* manager = LookupManager(id);
+  DownloadManager* manager = GetDownloadManager(id);
   if (manager)
     manager->DownloadFinished(id, bytes_so_far);
   RemoveDownload(id, manager);
@@ -151,7 +125,7 @@ void DownloadFileManager::OnDownloadFinished(int id,
 }
 
 // Lookup one in-progress download.
-DownloadFile* DownloadFileManager::LookupDownload(int id) {
+DownloadFile* DownloadFileManager::GetDownloadFile(int id) {
   DownloadFileMap::iterator it = downloads_.find(id);
   return it == downloads_.end() ? NULL : it->second;
 }
@@ -187,7 +161,7 @@ void DownloadFileManager::UpdateInProgressDownloads() {
   ProgressMap::iterator it = ui_progress_.begin();
   for (; it != ui_progress_.end(); ++it) {
     const int id = it->first;
-    DownloadManager* manager = LookupManager(id);
+    DownloadManager* manager = GetDownloadManager(id);
     if (manager)
       manager->UpdateDownload(id, it->second);
   }
@@ -217,7 +191,7 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
     // on the UI thread is the safe way to do that.
     ChromeThread::PostTask(
         ChromeThread::IO, FROM_HERE,
-        NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
+        NewRunnableFunction(&download_util::CancelDownloadRequest,
                             resource_dispatcher_host_,
                             info->child_id,
                             info->request_id));
@@ -226,7 +200,7 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
     return;
   }
 
-  DCHECK(LookupDownload(info->download_id) == NULL);
+  DCHECK(GetDownloadFile(info->download_id) == NULL);
   downloads_[info->download_id] = download;
   info->path = download->full_path();
   {
@@ -252,7 +226,7 @@ void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) {
     contents.swap(buffer->contents);
   }
 
-  DownloadFile* download = LookupDownload(id);
+  DownloadFile* download = GetDownloadFile(id);
   for (size_t i = 0; i < contents.size(); ++i) {
     net::IOBuffer* data = contents[i].first;
     const int data_len = contents[i].second;
@@ -326,30 +300,8 @@ void DownloadFileManager::CancelDownload(int id) {
   }
 }
 
-void DownloadFileManager::DownloadUrl(
-    const GURL& url,
-    const GURL& referrer,
-    const std::string& referrer_charset,
-    const DownloadSaveInfo& save_info,
-    int render_process_host_id,
-    int render_view_id,
-    URLRequestContextGetter* request_context_getter) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  ChromeThread::PostTask(
-        ChromeThread::IO, FROM_HERE,
-        NewRunnableMethod(this,
-                          &DownloadFileManager::OnDownloadUrl,
-                          url,
-                          referrer,
-                          referrer_charset,
-                          save_info,
-                          render_process_host_id,
-                          render_view_id,
-                          request_context_getter));
-}
-
 // Relate a download ID to its owning DownloadManager.
-DownloadManager* DownloadFileManager::LookupManager(int download_id) {
+DownloadManager* DownloadFileManager::GetDownloadManager(int download_id) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   DownloadManagerMap::iterator it = managers_.find(download_id);
   if (it != managers_.end())
@@ -487,7 +439,7 @@ void DownloadFileManager::OnFinalDownloadName(int id,
     } else {
       ChromeThread::PostTask(
           ChromeThread::IO, FROM_HERE,
-          NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
+          NewRunnableFunction(&download_util::CancelDownloadRequest,
                               resource_dispatcher_host_,
                               download->child_id(),
                               download->request_id()));
diff --git a/chrome/browser/download/download_file_manager.h b/chrome/browser/download/download_file_manager.h
index f4c1a5e..bfccbfd 100644
--- a/chrome/browser/download/download_file_manager.h
+++ b/chrome/browser/download/download_file_manager.h
@@ -78,16 +78,6 @@ class DownloadFileManager
   void CancelDownload(int id);
   void DownloadFinished(int id, DownloadBuffer* buffer);
 
-  // Download the URL. Called on the UI thread and forwarded to the
-  // ResourceDispatcherHost on the IO thread.
-  void DownloadUrl(const GURL& url,
-                   const GURL& referrer,
-                   const std::string& referrer_charset,
-                   const DownloadSaveInfo& save_info,
-                   int render_process_host_id,
-                   int render_view_id,
-                   URLRequestContextGetter* request_context_getter);
-
   // Called on the UI thread to remove a download item or manager.
   void RemoveDownloadManager(DownloadManager* manager);
   void RemoveDownload(int id, DownloadManager* manager);
@@ -123,15 +113,6 @@ class DownloadFileManager
   // Clean up helper that runs on the download thread.
   void OnShutdown();
 
-  // Run on the IO thread to initiate the download of a URL.
-  void OnDownloadUrl(const GURL& url,
-                     const GURL& referrer,
-                     const std::string& referrer_charset,
-                     const DownloadSaveInfo& save_info,
-                     int render_process_host_id,
-                     int render_view_id,
-                     URLRequestContextGetter* request_context_getter);
-
   // Handlers for notifications sent from the download thread and run on
   // the UI thread.
   void OnStartDownload(DownloadCreateInfo* info);
@@ -140,10 +121,10 @@ class DownloadFileManager
   // Called only on UI thread to get the DownloadManager for a tab's profile.
   static DownloadManager* DownloadManagerFromRenderIds(int render_process_id,
                                                        int review_view_id);
-  DownloadManager* LookupManager(int download_id);
+  DownloadManager* GetDownloadManager(int download_id);
 
   // Called only on the download thread.
-  DownloadFile* LookupDownload(int id);
+  DownloadFile* GetDownloadFile(int id);
 
   // Called on the UI thread to remove a download from the UI progress table.
   void RemoveDownloadFromUIProgress(int id);
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 622d108..97dc7ac 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -841,14 +841,6 @@ void DownloadManager::DangerousDownloadRenamed(int64 download_handle,
   ContinueDownloadFinished(download);
 }
 
-// static
-void DownloadManager::OnCancelDownloadRequest(ResourceDispatcherHost* rdh,
-                                              int render_process_id,
-                                              int request_id) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
-  rdh->CancelRequest(render_process_id, request_id, false);
-}
-
 void DownloadManager::DownloadCancelled(int32 download_id) {
   DownloadMap::iterator it = in_progress_.find(download_id);
   if (it == in_progress_.end())
@@ -874,7 +866,7 @@ void DownloadManager::DownloadCancelledInternal(int download_id,
   // Cancel the network request.  RDH is guaranteed to outlive the IO thread.
   ChromeThread::PostTask(
       ChromeThread::IO, FROM_HERE,
-      NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
+      NewRunnableFunction(&download_util::CancelDownloadRequest,
                           g_browser_process->resource_dispatcher_host(),
                           render_process_id,
                           request_id));
@@ -1049,13 +1041,8 @@ void DownloadManager::DownloadUrl(const GURL& url,
                                   const GURL& referrer,
                                   const std::string& referrer_charset,
                                   TabContents* tab_contents) {
-  file_manager_->DownloadUrl(url,
-                             referrer,
-                             referrer_charset,
-                             DownloadSaveInfo(),
-                             tab_contents->GetRenderProcessHost()->id(),
-                             tab_contents->render_view_host()->routing_id(),
-                             request_context_getter_);
+  DownloadUrlToFile(url, referrer, referrer_charset, DownloadSaveInfo(),
+                    tab_contents);
 }
 
 void DownloadManager::DownloadUrlToFile(const GURL& url,
@@ -1064,13 +1051,16 @@ void DownloadManager::DownloadUrlToFile(const GURL& url,
                                         const DownloadSaveInfo& save_info,
                                         TabContents* tab_contents) {
   DCHECK(tab_contents);
-  file_manager_->DownloadUrl(url,
-                             referrer,
-                             referrer_charset,
-                             save_info,
-                             tab_contents->GetRenderProcessHost()->id(),
-                             tab_contents->render_view_host()->routing_id(),
-                             request_context_getter_);
+  ChromeThread::PostTask(ChromeThread::IO, FROM_HERE,
+      NewRunnableFunction(&download_util::DownloadUrl,
+                          url,
+                          referrer,
+                          referrer_charset,
+                          save_info,
+                          g_browser_process->resource_dispatcher_host(),
+                          tab_contents->GetRenderProcessHost()->id(),
+                          tab_contents->render_view_host()->routing_id(),
+                          request_context_getter_));
 }
 
 void DownloadManager::GenerateExtension(
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index 704012d..55bc522 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -245,11 +245,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   static void GenerateSafeFileName(const std::string& mime_type,
                                    FilePath* file_name);
 
-  // Runs the network cancel.  Must be called on the IO thread.
-  static void OnCancelDownloadRequest(ResourceDispatcherHost* rdh,
-                                      int render_process_id,
-                                      int request_id);
-
   // Create a file name based on the response from the server.
   static void GenerateFileName(const GURL& url,
                                const std::string& content_disposition,
diff --git a/chrome/browser/download/download_util.cc b/chrome/browser/download/download_util.cc
index a3e064f..a532327 100644
--- a/chrome/browser/download/download_util.cc
+++ b/chrome/browser/download/download_util.cc
@@ -22,9 +22,12 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_manager.h"
+#include "chrome/browser/net/chrome_url_request_context.h"
+#include "chrome/browser/renderer_host/resource_dispatcher_host.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/time_format.h"
@@ -515,4 +518,34 @@ int GetUniquePathNumber(const FilePath& path) {
   return -1;
 }
 
+void DownloadUrl(
+    const GURL& url,
+    const GURL& referrer,
+    const std::string& referrer_charset,
+    const DownloadSaveInfo& save_info,
+    ResourceDispatcherHost* rdh,
+    int render_process_host_id,
+    int render_view_id,
+    URLRequestContextGetter* request_context_getter) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+
+  URLRequestContext* context = request_context_getter->GetURLRequestContext();
+  context->set_referrer_charset(referrer_charset);
+
+  rdh->BeginDownload(url,
+                     referrer,
+                     save_info,
+                     true,  // Show "Save as" UI.
+                     render_process_host_id,
+                     render_view_id,
+                     context);
+}
+
+void CancelDownloadRequest(ResourceDispatcherHost* rdh,
+                           int render_process_id,
+                           int request_id) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+  rdh->CancelRequest(render_process_id, request_id, false);
+}
+
 }  // namespace download_util
diff --git a/chrome/browser/download/download_util.h b/chrome/browser/download/download_util.h
index 6ee8f89..72af5d5 100644
--- a/chrome/browser/download/download_util.h
+++ b/chrome/browser/download/download_util.h
@@ -25,7 +25,12 @@ class BaseDownloadItemModel;
 class DictionaryValue;
 class DownloadItem;
 class FilePath;
+class GURL;
+class ResourceDispatcherHost;
 class SkBitmap;
+class URLRequestContextGetter;
+
+struct DownloadSaveInfo;
 
 namespace download_util {
 
@@ -157,6 +162,23 @@ void AppendNumberToPath(FilePath* path, int number);
 // a number, -1 is returned.
 int GetUniquePathNumber(const FilePath& path);
 
+// Download the URL. Must be called on the IO thread.
+void DownloadUrl(const GURL& url,
+                 const GURL& referrer,
+                 const std::string& referrer_charset,
+                 const DownloadSaveInfo& save_info,
+                 ResourceDispatcherHost* rdh,
+                 int render_process_host_id,
+                 int render_view_id,
+                 URLRequestContextGetter* request_context_getter);
+
+// Tells the resource dispatcher host to cancel a download request.
+// Must be called on the IO thread.
+void CancelDownloadRequest(ResourceDispatcherHost* rdh,
+                           int render_process_id,
+                           int request_id);
+
+
 }  // namespace download_util
 
 #endif  // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_UTIL_H_