Simplify threading in browser thread by making only ChromeThread deal with different thread lifetimes.The rest of the code doesn't get MessageLoop pointers since they're not thread-safe and instead just call PostTask on ChromeThread. If the target thread is not alive, then the task is simply deleted.In a followup change, I'll remove any remaining MessageLoop* caching. With this change, there's little to be gained by caching since no locks are involved if the target MessageLoop is guaranteed to outlive the current thread (inferred automatically by the order of the chrome_threads_ array).BUG=25354

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

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

3 files changed, 43 insertions, 52 deletions

diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc
index 6bb0866..48d7b31 100644
--- a/chrome/browser/download/download_file.cc
+++ b/chrome/browser/download/download_file.cc
@@ -254,8 +254,10 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
     // about this download so we have to clean up 'info'. We need to get back
     // to the IO thread to cancel the network request and CancelDownloadRequest
     // on the UI thread is the safe way to do that.
-    ui_loop_->PostTask(FROM_HERE,
-        NewRunnableFunction(&DownloadManager::CancelDownloadRequest,
+    ChromeThread::PostTask(
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
+                            resource_dispatcher_host_,
                             info->child_id,
                             info->request_id));
     delete info;
@@ -385,7 +387,12 @@ void DownloadFileManager::OnStartDownload(DownloadCreateInfo* info) {
   DownloadManager* manager = DownloadManagerFromRenderIds(info->child_id,
                                                           info->render_view_id);
   if (!manager) {
-    DownloadManager::CancelDownloadRequest(info->child_id, info->request_id);
+    ChromeThread::PostTask(
+        ChromeThread::IO, FROM_HERE,
+        NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
+                            resource_dispatcher_host_,
+                            info->child_id,
+                            info->request_id));
     delete info;
     return;
   }
@@ -607,8 +614,10 @@ void DownloadFileManager::OnFinalDownloadName(int id,
                             &DownloadManager::DownloadCancelled,
                             id));
     } else {
-      ui_loop_->PostTask(FROM_HERE,
-          NewRunnableFunction(&DownloadManager::CancelDownloadRequest,
+      ChromeThread::PostTask(
+          ChromeThread::IO, FROM_HERE,
+          NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
+                              resource_dispatcher_host_,
                               download->child_id(),
                               download->request_id()));
     }
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index f412e33..e2f998a 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -931,26 +931,10 @@ void DownloadManager::DangerousDownloadRenamed(int64 download_handle,
 }
 
 // static
-// We have to tell the ResourceDispatcherHost to cancel the download from this
-// thread, since we can't forward tasks from the file thread to the IO thread
-// reliably (crash on shutdown race condition).
-void DownloadManager::CancelDownloadRequest(int render_process_id,
-                                            int request_id) {
-  ResourceDispatcherHost* rdh = g_browser_process->resource_dispatcher_host();
-  base::Thread* io_thread = g_browser_process->io_thread();
-  if (!io_thread || !rdh)
-    return;
-  io_thread->message_loop()->PostTask(FROM_HERE,
-      NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
-                          rdh,
-                          render_process_id,
-                          request_id));
-}
-
-// 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);
 }
 
@@ -975,8 +959,13 @@ void DownloadManager::DownloadCancelled(int32 download_id) {
 void DownloadManager::DownloadCancelledInternal(int download_id,
                                                 int render_process_id,
                                                 int request_id) {
-  // Cancel the network request.
-  CancelDownloadRequest(render_process_id, request_id);
+  // Cancel the network request.  RDH is guaranteed to outlive the IO thread.
+  ChromeThread::PostTask(
+      ChromeThread::IO, FROM_HERE,
+      NewRunnableFunction(&DownloadManager::OnCancelDownloadRequest,
+                          g_browser_process->resource_dispatcher_host(),
+                          render_process_id,
+                          request_id));
 
   // Tell the file manager to cancel the download.
   file_manager_->RemoveDownload(download_id, this);  // On the UI thread
@@ -988,24 +977,21 @@ void DownloadManager::DownloadCancelledInternal(int download_id,
 
 void DownloadManager::PauseDownload(int32 download_id, bool pause) {
   DownloadMap::iterator it = in_progress_.find(download_id);
-  if (it != in_progress_.end()) {
-    DownloadItem* download = it->second;
-    if (pause == download->is_paused())
-      return;
-
-    // Inform the ResourceDispatcherHost of the new pause state.
-    base::Thread* io_thread = g_browser_process->io_thread();
-    ResourceDispatcherHost* rdh = g_browser_process->resource_dispatcher_host();
-    if (!io_thread || !rdh)
-      return;
-
-    io_thread->message_loop()->PostTask(FROM_HERE,
-        NewRunnableFunction(&DownloadManager::OnPauseDownloadRequest,
-                            rdh,
-                            download->render_process_id(),
-                            download->request_id(),
-                            pause));
-  }
+  if (it == in_progress_.end())
+    return;
+
+  DownloadItem* download = it->second;
+  if (pause == download->is_paused())
+    return;
+
+  // Inform the ResourceDispatcherHost of the new pause state.
+  ChromeThread::PostTask(
+      ChromeThread::IO, FROM_HERE,
+      NewRunnableFunction(&DownloadManager::OnPauseDownloadRequest,
+                          g_browser_process->resource_dispatcher_host(),
+                          download->render_process_id(),
+                          download->request_id(),
+                          pause));
 }
 
 // static
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index b2d6b3c..34a2e42 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -349,11 +349,7 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   void UpdateDownload(int32 download_id, int64 size);
   void DownloadFinished(int32 download_id, int64 size);
 
-  // Helper method for cancelling the network request associated with a
-  // download.
-  static void CancelDownloadRequest(int render_process_id, int request_id);
-
-  // Called from a view when a user clicks a UI button or link.
+ // Called from a view when a user clicks a UI button or link.
   void DownloadCancelled(int32 download_id);
   void PauseDownload(int32 download_id, bool pause);
   void RemoveDownload(int64 download_handle);
@@ -461,6 +457,11 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   static bool IsExtensionInstall(const DownloadItem* item);
   static bool IsExtensionInstall(const DownloadCreateInfo* info);
 
+  // Runs the network cancel.  Must be called on the IO thread.
+  static void OnCancelDownloadRequest(ResourceDispatcherHost* rdh,
+                                      int render_process_id,
+                                      int request_id);
+
  private:
   // Opens a download via the Windows shell.
   void OpenDownloadInShell(const DownloadItem* download,
@@ -510,11 +511,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
                                  int render_process_id,
                                  int request_id);
 
-  // Runs the network cancel on the IO thread.
-  static void OnCancelDownloadRequest(ResourceDispatcherHost* rdh,
-                                      int render_process_id,
-                                      int request_id);
-
   // Runs the pause on the IO thread.
   static void OnPauseDownloadRequest(ResourceDispatcherHost* rdh,
                                      int render_process_id,