GTTF: Clean up DownloadFileManager

This removes a lot of duplication, locking, and thread jumping.

Most of the operations run on the FILE thread, and we do not duplicate
so much information. Each DownloadFile keeps track of its DownloadManager
(each Profile has its own DownloadManager). This allows us to remove
many maps from DownloadFileManager that were duplicating that information.

There is still SaveFileManager, but hopefully I will be able
to merge those two in small steps.

Hopefully, this is http://codereview.chromium.org/3245005 done right.

TEST=unit_tests, browser_tests, ui_tests
BUG=48913

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

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

14 files changed, 173 insertions, 267 deletions

diff --git a/chrome/browser/download/base_file.cc b/chrome/browser/download/base_file.cc
index 0bf74eb..33b9372 100644
--- a/chrome/browser/download/base_file.cc
+++ b/chrome/browser/download/base_file.cc
@@ -20,13 +20,14 @@
 BaseFile::BaseFile(const FilePath& full_path,
                    const GURL& source_url,
                    const GURL& referrer_url,
+                   int64 received_bytes,
                    const linked_ptr<net::FileStream>& file_stream)
     : full_path_(full_path),
       path_renamed_(false),
       source_url_(source_url),
       referrer_url_(referrer_url),
       file_stream_(file_stream),
-      bytes_so_far_(0),
+      bytes_so_far_(received_bytes),
       power_save_blocker_(true) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 }
diff --git a/chrome/browser/download/base_file.h b/chrome/browser/download/base_file.h
index fa3873f..6c57165 100644
--- a/chrome/browser/download/base_file.h
+++ b/chrome/browser/download/base_file.h
@@ -22,6 +22,7 @@ class BaseFile {
   BaseFile(const FilePath& full_path,
            const GURL& source_url,
            const GURL& referrer_url,
+           int64 received_bytes,
            const linked_ptr<net::FileStream>& file_stream);
   ~BaseFile();
 
diff --git a/chrome/browser/download/base_file_unittest.cc b/chrome/browser/download/base_file_unittest.cc
index d1b93a9..3c135aa 100644
--- a/chrome/browser/download/base_file_unittest.cc
+++ b/chrome/browser/download/base_file_unittest.cc
@@ -23,7 +23,7 @@ class BaseFileTest : public testing::Test {
 
   virtual void SetUp() {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    base_file_.reset(new BaseFile(FilePath(), GURL(), GURL(), file_stream_));
+    base_file_.reset(new BaseFile(FilePath(), GURL(), GURL(), 0, file_stream_));
   }
 
   virtual void TearDown() {
diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc
index c877a6e..ab56380 100644
--- a/chrome/browser/download/download_file.cc
+++ b/chrome/browser/download/download_file.cc
@@ -10,15 +10,19 @@
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_types.h"
 
-DownloadFile::DownloadFile(const DownloadCreateInfo* info)
+DownloadFile::DownloadFile(const DownloadCreateInfo* info,
+                           DownloadManager* download_manager)
     : BaseFile(info->save_info.file_path,
                info->url,
                info->referrer_url,
+               info->received_bytes,
                info->save_info.file_stream),
       id_(info->download_id),
       child_id_(info->child_id),
-      request_id_(info->request_id) {
+      request_id_(info->request_id),
+      download_manager_(download_manager) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+
 }
 
 DownloadFile::~DownloadFile() {
@@ -40,3 +44,13 @@ void DownloadFile::CancelDownloadRequest(ResourceDispatcherHost* rdh) {
                           child_id_,
                           request_id_));
 }
+
+void DownloadFile::OnDownloadManagerShutdown() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  download_manager_ = NULL;
+}
+
+DownloadManager* DownloadFile::GetDownloadManager() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  return download_manager_.get();
+}
diff --git a/chrome/browser/download/download_file.h b/chrome/browser/download/download_file.h
index ca188c9..88c0715 100644
--- a/chrome/browser/download/download_file.h
+++ b/chrome/browser/download/download_file.h
@@ -7,10 +7,12 @@
 #pragma once
 
 #include "base/basictypes.h"
+#include "base/ref_counted.h"
 #include "chrome/browser/download/base_file.h"
 #include "chrome/browser/download/download_types.h"
 
 struct DownloadCreateInfo;
+class DownloadManager;
 class ResourceDispatcherHost;
 
 // These objects live exclusively on the download thread and handle the writing
@@ -19,7 +21,8 @@ class ResourceDispatcherHost;
 // cancelled, the DownloadFile is destroyed.
 class DownloadFile : public BaseFile {
  public:
-  explicit DownloadFile(const DownloadCreateInfo* info);
+  DownloadFile(const DownloadCreateInfo* info,
+               DownloadManager* download_manager);
   virtual ~DownloadFile();
 
   // Deletes its .crdownload intermediate file.
@@ -29,7 +32,10 @@ class DownloadFile : public BaseFile {
   // Cancels the download request associated with this file.
   void CancelDownloadRequest(ResourceDispatcherHost* rdh);
 
+  void OnDownloadManagerShutdown();
+
   int id() const { return id_; }
+  DownloadManager* GetDownloadManager();
 
  private:
   // The unique identifier for this download, assigned at creation by
@@ -42,6 +48,9 @@ class DownloadFile : public BaseFile {
   // Handle for informing the ResourceDispatcherHost of a UI based cancel.
   int request_id_;
 
+  // DownloadManager this download belongs to.
+  scoped_refptr<DownloadManager> download_manager_;
+
   DISALLOW_COPY_AND_ASSIGN(DownloadFile);
 };
 
diff --git a/chrome/browser/download/download_file_manager.cc b/chrome/browser/download/download_file_manager.cc
index 2862742..df28bbf 100644
--- a/chrome/browser/download/download_file_manager.cc
+++ b/chrome/browser/download/download_file_manager.cc
@@ -35,6 +35,21 @@ namespace {
 // cause it to become unresponsive (in milliseconds).
 const int kUpdatePeriodMs = 500;
 
+DownloadManager* DownloadManagerForRenderViewHost(int render_process_id,
+                                                  int render_view_id) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::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)
@@ -43,37 +58,29 @@ DownloadFileManager::DownloadFileManager(ResourceDispatcherHost* rdh)
 }
 
 DownloadFileManager::~DownloadFileManager() {
-  // Check for clean shutdown.
   DCHECK(downloads_.empty());
 }
 
-// Called during the browser shutdown process to clean up any state (open files,
-// timers) that live on the download_thread_.
 void DownloadFileManager::Shutdown() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  StopUpdateTimer();
   ChromeThread::PostTask(
       ChromeThread::FILE, FROM_HERE,
       NewRunnableMethod(this, &DownloadFileManager::OnShutdown));
 }
 
-// Cease download thread operations.
 void DownloadFileManager::OnShutdown() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  StopUpdateTimer();
   STLDeleteValues(&downloads_);
 }
 
-// Notifications sent from the download thread and run on the UI thread.
+void DownloadFileManager::CreateDownloadFile(
+    DownloadCreateInfo* info, DownloadManager* download_manager) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
-// Lookup the DownloadManager for this TabContents' profile and inform it of
-// a new download.
-// TODO(paulg): When implementing download restart via the Downloads tab,
-//              there will be no 'render_process_id' or 'render_view_id'.
-void DownloadFileManager::OnStartDownload(DownloadCreateInfo* info) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  DownloadManager* manager = DownloadManagerFromRenderIds(info->child_id,
-                                                          info->render_view_id);
-  if (!manager) {
+  scoped_ptr<DownloadFile> download_file(
+      new DownloadFile(info, download_manager));
+  if (!download_file->Initialize()) {
     ChromeThread::PostTask(
         ChromeThread::IO, FROM_HERE,
         NewRunnableFunction(&download_util::CancelDownloadRequest,
@@ -84,58 +91,39 @@ void DownloadFileManager::OnStartDownload(DownloadCreateInfo* info) {
     return;
   }
 
+  DCHECK(GetDownloadFile(info->download_id) == NULL);
+  downloads_[info->download_id] = download_file.release();
+  // TODO(phajdan.jr): fix the duplication of path info below.
+  info->path = info->save_info.file_path;
+
+  // The file is now ready, we can un-pause the request and start saving data.
+  ChromeThread::PostTask(
+      ChromeThread::IO, FROM_HERE,
+      NewRunnableMethod(this, &DownloadFileManager::ResumeDownloadRequest,
+                        info->child_id, info->request_id));
+
   StartUpdateTimer();
 
-  // Add the download manager to our request maps for future updates. We want to
-  // be able to cancel all in progress downloads when a DownloadManager is
-  // deleted, such as when a profile is closed. We also want to be able to look
-  // up the DownloadManager associated with a given request without having to
-  // rely on using tab information, since a tab may be closed while a download
-  // initiated from that tab is still in progress.
-  DownloadRequests& downloads = requests_[manager];
-  downloads.insert(info->download_id);
-
-  // TODO(paulg): The manager will exist when restarts are implemented.
-  DownloadManagerMap::iterator dit = managers_.find(info->download_id);
-  if (dit == managers_.end())
-    managers_[info->download_id] = manager;
-  else
-    NOTREACHED();
-
-  // StartDownload will clean up |info|.
-  manager->StartDownload(info);
+  ChromeThread::PostTask(
+      ChromeThread::UI, FROM_HERE,
+      NewRunnableMethod(download_manager,
+                        &DownloadManager::StartDownload, info));
 }
 
-// Update the Download Manager with the finish state, and remove the request
-// tracking entries.
-void DownloadFileManager::OnDownloadFinished(int id,
-                                             int64 bytes_so_far) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  DownloadManager* manager = GetDownloadManager(id);
-  if (manager)
-    manager->DownloadFinished(id, bytes_so_far);
-  RemoveDownload(id, manager);
-  RemoveDownloadFromUIProgress(id);
+void DownloadFileManager::ResumeDownloadRequest(int child_id, int request_id) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO));
+
+  // This balances the pause in DownloadResourceHandler::OnResponseStarted.
+  resource_dispatcher_host_->PauseRequest(child_id, request_id, false);
 }
 
-// Lookup one in-progress download.
 DownloadFile* DownloadFileManager::GetDownloadFile(int id) {
   DownloadFileMap::iterator it = downloads_.find(id);
   return it == downloads_.end() ? NULL : it->second;
 }
 
-// The UI progress is updated on the file thread and removed on the UI thread.
-void DownloadFileManager::RemoveDownloadFromUIProgress(int id) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  AutoLock lock(progress_lock_);
-  if (ui_progress_.find(id) != ui_progress_.end())
-    ui_progress_.erase(id);
-}
-
-// Throttle updates to the UI thread by only posting update notifications at a
-// regularly controlled interval.
 void DownloadFileManager::StartUpdateTimer() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   if (!update_timer_.IsRunning()) {
     update_timer_.Start(base::TimeDelta::FromMilliseconds(kUpdatePeriodMs),
                         this, &DownloadFileManager::UpdateInProgressDownloads);
@@ -143,21 +131,22 @@ void DownloadFileManager::StartUpdateTimer() {
 }
 
 void DownloadFileManager::StopUpdateTimer() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   update_timer_.Stop();
 }
 
-// Our periodic timer has fired so send the UI thread updates on all in progress
-// downloads.
 void DownloadFileManager::UpdateInProgressDownloads() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  AutoLock lock(progress_lock_);
-  ProgressMap::iterator it = ui_progress_.begin();
-  for (; it != ui_progress_.end(); ++it) {
-    const int id = it->first;
-    DownloadManager* manager = GetDownloadManager(id);
-    if (manager)
-      manager->UpdateDownload(id, it->second);
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  for (DownloadFileMap::iterator i = downloads_.begin();
+       i != downloads_.end(); ++i) {
+    int id = i->first;
+    DownloadFile* download_file = i->second;
+    DownloadManager* manager = download_file->GetDownloadManager();
+    if (manager) {
+      ChromeThread::PostTask(ChromeThread::UI, FROM_HERE,
+          NewRunnableMethod(manager, &DownloadManager::UpdateDownload,
+                            id, download_file->bytes_so_far()));
+    }
   }
 }
 
@@ -168,21 +157,13 @@ int DownloadFileManager::GetNextId() {
   return next_id_++;
 }
 
-// Notifications sent from the IO thread and run on the download thread:
-
-// The IO thread created 'info', but the download thread (this method) uses it
-// to create a DownloadFile, then passes 'info' to the UI thread where it is
-// finally consumed and deleted.
 void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   DCHECK(info);
 
-  DownloadFile* download = new DownloadFile(info);
-  if (!download->Initialize()) {
-    // Couldn't open, cancel the operation. The UI thread does not yet know
-    // 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.
+  DownloadManager* manager = DownloadManagerForRenderViewHost(
+      info->child_id, info->render_view_id);
+  if (!manager) {
     ChromeThread::PostTask(
         ChromeThread::IO, FROM_HERE,
         NewRunnableFunction(&download_util::CancelDownloadRequest,
@@ -190,22 +171,12 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
                             info->child_id,
                             info->request_id));
     delete info;
-    delete download;
     return;
   }
 
-  DCHECK(GetDownloadFile(info->download_id) == NULL);
-  downloads_[info->download_id] = download;
-  // TODO(phajdan.jr): fix the duplication of path info below.
-  info->path = info->save_info.file_path;
-  {
-    AutoLock lock(progress_lock_);
-    ui_progress_[info->download_id] = info->received_bytes;
-  }
-
-  ChromeThread::PostTask(
-      ChromeThread::UI, FROM_HERE,
-      NewRunnableMethod(this, &DownloadFileManager::OnStartDownload, info));
+  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
+      NewRunnableMethod(this, &DownloadFileManager::CreateDownloadFile,
+                        info, manager));
 }
 
 // We don't forward an update to the UI thread here, since we want to throttle
@@ -221,25 +192,14 @@ void DownloadFileManager::UpdateDownload(int id, DownloadBuffer* buffer) {
     contents.swap(buffer->contents);
   }
 
-  // Keep track of how many bytes we have successfully saved to update
-  // our progress status in the UI.
-  int64 progress_bytes = 0;
-
   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;
-    if (download) {
-      if (download->AppendDataToFile(data->data(), data_len))
-        progress_bytes += data_len;
-    }
+    if (download)
+      download->AppendDataToFile(data->data(), data_len);
     data->Release();
   }
-
-  if (download) {
-    AutoLock lock(progress_lock_);
-    ui_progress_[download->id()] += progress_bytes;
-  }
 }
 
 void DownloadFileManager::DownloadFinished(int id, DownloadBuffer* buffer) {
@@ -250,18 +210,15 @@ void DownloadFileManager::DownloadFinished(int id, DownloadBuffer* buffer) {
     DownloadFile* download = it->second;
     download->Finish();
 
-    int64 download_size = -1;
-    {
-      AutoLock lock(progress_lock_);
-      download_size = ui_progress_[download->id()];
+    DownloadManager* download_manager = download->GetDownloadManager();
+    if (download_manager) {
+      ChromeThread::PostTask(
+          ChromeThread::UI, FROM_HERE,
+          NewRunnableMethod(
+              download_manager, &DownloadManager::DownloadFinished,
+              id, download->bytes_so_far()));
     }
 
-    ChromeThread::PostTask(
-        ChromeThread::UI, FROM_HERE,
-        NewRunnableMethod(
-            this, &DownloadFileManager::OnDownloadFinished,
-            id, download_size));
-
     // We need to keep the download around until the UI thread has finalized
     // the name.
     if (download->path_renamed()) {
@@ -271,9 +228,7 @@ void DownloadFileManager::DownloadFinished(int id, DownloadBuffer* buffer) {
   }
 
   if (downloads_.empty())
-    ChromeThread::PostTask(
-      ChromeThread::UI, FROM_HERE,
-      NewRunnableMethod(this, &DownloadFileManager::StopUpdateTimer));
+    StopUpdateTimer();
 }
 
 // This method will be sent via a user action, or shutdown on the UI thread, and
@@ -286,94 +241,26 @@ void DownloadFileManager::CancelDownload(int id) {
     DownloadFile* download = it->second;
     download->Cancel();
 
-    ChromeThread::PostTask(
-        ChromeThread::UI, FROM_HERE,
-        NewRunnableMethod(
-            this, &DownloadFileManager::RemoveDownloadFromUIProgress,
-            download->id()));
-
     if (download->path_renamed()) {
       downloads_.erase(it);
       delete download;
     }
   }
 
-  if (downloads_.empty()) {
-    ChromeThread::PostTask(
-      ChromeThread::UI, FROM_HERE,
-      NewRunnableMethod(this, &DownloadFileManager::StopUpdateTimer));
-  }
-}
-
-// Relate a download ID to its owning DownloadManager.
-DownloadManager* DownloadFileManager::GetDownloadManager(int download_id) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  DownloadManagerMap::iterator it = managers_.find(download_id);
-  if (it != managers_.end())
-    return it->second;
-  return NULL;
-}
-
-// Utility function for look up table maintenance, called on the UI thread.
-// A manager may have multiple downloads in progress, so we just look up the
-// one download (id) and remove it from the set, and remove the set if it
-// becomes empty.
-void DownloadFileManager::RemoveDownload(int id, DownloadManager* manager) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
-  if (manager) {
-    RequestMap::iterator it = requests_.find(manager);
-    if (it != requests_.end()) {
-      DownloadRequests& downloads = it->second;
-      DownloadRequests::iterator rit = downloads.find(id);
-      if (rit != downloads.end())
-        downloads.erase(rit);
-      if (downloads.empty())
-        requests_.erase(it);
-    }
-  }
-
-  // A download can only have one manager, so remove it if it exists.
-  DownloadManagerMap::iterator dit = managers_.find(id);
-  if (dit != managers_.end())
-    managers_.erase(dit);
-}
-
-// Utility function for converting request IDs to a TabContents. Must be called
-// only on the UI thread since Profile operations may create UI objects, such as
-// the first call to profile->GetDownloadManager().
-// static
-DownloadManager* DownloadFileManager::DownloadManagerFromRenderIds(
-    int render_process_id, int render_view_id) {
-  TabContents* contents = tab_util::GetTabContentsByID(render_process_id,
-                                                       render_view_id);
-  if (contents) {
-    Profile* profile = contents->profile();
-    if (profile)
-      return profile->GetDownloadManager();
-  }
-
-  return NULL;
+  if (downloads_.empty())
+    StopUpdateTimer();
 }
 
-// Called by DownloadManagers in their destructor, and only on the UI thread.
-void DownloadFileManager::RemoveDownloadManager(DownloadManager* manager) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   DCHECK(manager);
-  RequestMap::iterator it = requests_.find(manager);
-  if (it == requests_.end())
-    return;
 
-  const DownloadRequests& requests = it->second;
-  DownloadRequests::const_iterator i = requests.begin();
-  for (; i != requests.end(); ++i) {
-    DownloadManagerMap::iterator dit = managers_.find(*i);
-    if (dit != managers_.end()) {
-      DCHECK(dit->second == manager);
-      managers_.erase(dit);
-    }
+  for (DownloadFileMap::iterator i = downloads_.begin();
+       i != downloads_.end(); ++i) {
+    DownloadFile* download_file = i->second;
+    if (download_file->GetDownloadManager() == manager)
+      download_file->OnDownloadManagerShutdown();
   }
-
-  requests_.erase(it);
 }
 
 // Actions from the UI thread and run on the download thread
@@ -472,11 +359,8 @@ void DownloadFileManager::OnFinalDownloadName(int id,
     delete download;
   }
 
-  if (downloads_.empty()) {
-    ChromeThread::PostTask(
-      ChromeThread::UI, FROM_HERE,
-      NewRunnableMethod(this, &DownloadFileManager::StopUpdateTimer));
-  }
+  if (downloads_.empty())
+    StopUpdateTimer();
 }
 
 // Called only from OnFinalDownloadName or OnIntermediateDownloadName
@@ -488,13 +372,14 @@ void DownloadFileManager::CancelDownloadOnRename(int id) {
   if (!download)
     return;
 
-  DownloadManagerMap::iterator dmit = managers_.find(download->id());
-  if (dmit != managers_.end()) {
-    DownloadManager* dlm = dmit->second;
-    ChromeThread::PostTask(
-        ChromeThread::UI, FROM_HERE,
-        NewRunnableMethod(dlm, &DownloadManager::DownloadCancelled, id));
-  } else {
+  DownloadManager* download_manager = download->GetDownloadManager();
+  if (!download_manager) {
     download->CancelDownloadRequest(resource_dispatcher_host_);
+    return;
   }
+
+  ChromeThread::PostTask(
+      ChromeThread::UI, FROM_HERE,
+      NewRunnableMethod(download_manager,
+                        &DownloadManager::DownloadCancelled, id));
 }
diff --git a/chrome/browser/download/download_file_manager.h b/chrome/browser/download/download_file_manager.h
index 15e7d21..9b909d9 100644
--- a/chrome/browser/download/download_file_manager.h
+++ b/chrome/browser/download/download_file_manager.h
@@ -44,7 +44,6 @@
 
 #include "base/basictypes.h"
 #include "base/hash_tables.h"
-#include "base/lock.h"
 #include "base/ref_counted.h"
 #include "base/timer.h"
 #include "gfx/native_widget_types.h"
@@ -75,16 +74,17 @@ class DownloadFileManager
   // Called on the IO thread
   int GetNextId();
 
+  // Called on UI thread to make DownloadFileManager start the download.
+  void StartDownload(DownloadCreateInfo* info);
+
   // Handlers for notifications sent from the IO thread and run on the
   // download thread.
-  void StartDownload(DownloadCreateInfo* info);
   void UpdateDownload(int id, DownloadBuffer* buffer);
   void CancelDownload(int id);
   void DownloadFinished(int id, DownloadBuffer* buffer);
 
-  // Called on the UI thread to remove a download item or manager.
-  void RemoveDownloadManager(DownloadManager* manager);
-  void RemoveDownload(int id, DownloadManager* manager);
+  // Called on FILE thread by DownloadManager at the beginning of its shutdown.
+  void OnDownloadManagerShutdown(DownloadManager* manager);
 
 #if !defined(OS_MACOSX)
   // The open and show methods run on the file thread, which does not work on
@@ -125,22 +125,18 @@ class DownloadFileManager
   // Clean up helper that runs on the download thread.
   void OnShutdown();
 
-  // Handlers for notifications sent from the download thread and run on
-  // the UI thread.
-  void OnStartDownload(DownloadCreateInfo* info);
-  void OnDownloadFinished(int id, int64 bytes_so_far);
+  // Creates DownloadFile on FILE thread and continues starting the download
+  // process.
+  void CreateDownloadFile(DownloadCreateInfo* info,
+                          DownloadManager* download_manager);
 
-  // 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* GetDownloadManager(int download_id);
+  // Tells the ResourceDispatcherHost to resume a download request
+  // that was paused to wait for the on-disk file to be created.
+  void ResumeDownloadRequest(int child_id, int request_id);
 
   // Called only on the download thread.
   DownloadFile* GetDownloadFile(int id);
 
-  // Called on the UI thread to remove a download from the UI progress table.
-  void RemoveDownloadFromUIProgress(int id);
-
   // Called only from OnFinalDownloadName or OnIntermediateDownloadName
   // on the FILE thread.
   void CancelDownloadOnRename(int id);
@@ -152,28 +148,12 @@ class DownloadFileManager
   typedef base::hash_map<int, DownloadFile*> DownloadFileMap;
   DownloadFileMap downloads_;
 
-  // Throttle updates to the UI thread.
+  // Schedule periodic updates of the download progress. This timer
+  // is controlled from the FILE thread, and posts updates to the UI thread.
   base::RepeatingTimer<DownloadFileManager> update_timer_;
 
   ResourceDispatcherHost* resource_dispatcher_host_;
 
-  // Tracking which DownloadManager to send data to, called only on UI thread.
-  // DownloadManagerMap maps download IDs to their DownloadManager.
-  typedef base::hash_map<int, DownloadManager*> DownloadManagerMap;
-  DownloadManagerMap managers_;
-
-  // RequestMap maps a DownloadManager to all in-progress download IDs.
-  // Called only on the UI thread.
-  typedef base::hash_set<int> DownloadRequests;
-  typedef std::map<DownloadManager*, DownloadRequests> RequestMap;
-  RequestMap requests_;
-
-  // Used for progress updates on the UI thread, mapping download->id() to bytes
-  // received so far. Written to by the file thread and read by the UI thread.
-  typedef base::hash_map<int, int64> ProgressMap;
-  ProgressMap ui_progress_;
-  Lock progress_lock_;
-
   DISALLOW_COPY_AND_ASSIGN(DownloadFileManager);
 };
 
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index ad9b604..5d503dd 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -65,18 +65,22 @@ DownloadManager::DownloadManager()
 }
 
 DownloadManager::~DownloadManager() {
-  FOR_EACH_OBSERVER(Observer, observers_, ManagerGoingDown());
-
-  if (shutdown_needed_)
-    Shutdown();
+  DCHECK(!shutdown_needed_);
 }
 
 void DownloadManager::Shutdown() {
-  DCHECK(shutdown_needed_) << "Shutdown called when not needed.";
+  if (!shutdown_needed_)
+    return;
+  shutdown_needed_ = false;
 
-  // Stop receiving download updates
-  if (file_manager_)
-    file_manager_->RemoveDownloadManager(this);
+  FOR_EACH_OBSERVER(Observer, observers_, ManagerGoingDown());
+
+  if (file_manager_) {
+    ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
+        NewRunnableMethod(file_manager_,
+                          &DownloadFileManager::OnDownloadManagerShutdown,
+                          this));
+  }
 
   // 'in_progress_' may contain DownloadItems that have not finished the start
   // complete (from the history service) and thus aren't in downloads_.
@@ -662,8 +666,6 @@ void DownloadManager::DownloadCancelledInternal(int download_id,
                           render_process_id,
                           request_id));
 
-  // Tell the file manager to cancel the download.
-  file_manager_->RemoveDownload(download_id, this);  // On the UI thread
   ChromeThread::PostTask(
       ChromeThread::FILE, FROM_HERE,
       NewRunnableMethod(
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index bd0f83e..bff3822 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -66,6 +66,9 @@ class DownloadManager
  public:
   DownloadManager();
 
+  // Shutdown the download manager. Must be called before destruction.
+  void Shutdown();
+
   // Interface to implement for observers that wish to be informed of changes
   // to the DownloadManager's collection of downloads.
   class Observer {
@@ -226,9 +229,6 @@ class DownloadManager
   void OpenDownloadInShell(DownloadItem* download,
                            gfx::NativeView parent_window);
 
-  // Shutdown the download manager.  This call is needed only after Init.
-  void Shutdown();
-
   // Called on the download thread to check whether the suggested file path
   // exists.  We don't check if the file exists on the UI thread to avoid UI
   // stalls from interacting with the file system.
diff --git a/chrome/browser/download/download_manager_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index d63edc0..726f491 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -38,6 +38,7 @@ class DownloadManagerTest : public testing::Test {
   }
 
   ~DownloadManagerTest() {
+    download_manager_->Shutdown();
     // profile_ must outlive download_manager_, so we explicitly delete
     // download_manager_ first.
     download_manager_ = NULL;
@@ -179,7 +180,7 @@ const struct {
 class MockDownloadFile : public DownloadFile {
  public:
   explicit MockDownloadFile(DownloadCreateInfo* info)
-      : DownloadFile(info), renamed_count_(0) { }
+      : DownloadFile(info, NULL), renamed_count_(0) { }
   virtual ~MockDownloadFile() { Destructed(); }
   MOCK_METHOD2(Rename, bool(const FilePath&, bool));
   MOCK_METHOD0(DeleteCrDownload, void());
diff --git a/chrome/browser/download/save_file.cc b/chrome/browser/download/save_file.cc
index b05f7c5..98667a3 100644
--- a/chrome/browser/download/save_file.cc
+++ b/chrome/browser/download/save_file.cc
@@ -9,7 +9,7 @@
 #include "net/base/file_stream.h"
 
 SaveFile::SaveFile(const SaveFileCreateInfo* info)
-    : BaseFile(FilePath(), info->url, GURL(), linked_ptr<net::FileStream>()),
+    : BaseFile(FilePath(), info->url, GURL(), 0, linked_ptr<net::FileStream>()),
       info_(info) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index b2d7eba..a843377 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -519,9 +519,13 @@ class OffTheRecordProfileImpl : public Profile,
 #endif  // defined(OS_CHROMEOS)
 
   virtual void ExitedOffTheRecordMode() {
-    // Drop our download manager so we forget about all the downloads made
-    // in off-the-record mode.
-    download_manager_ = NULL;
+    // DownloadManager is lazily created, so check before accessing it.
+    if (download_manager_.get()) {
+      // Drop our download manager so we forget about all the downloads made
+      // in off-the-record mode.
+      download_manager_->Shutdown();
+      download_manager_ = NULL;
+    }
   }
 
   virtual void Observe(NotificationType type,
diff --git a/chrome/browser/profile_impl.cc b/chrome/browser/profile_impl.cc
index 9d35e6e..f972716 100644
--- a/chrome/browser/profile_impl.cc
+++ b/chrome/browser/profile_impl.cc
@@ -461,9 +461,13 @@ ProfileImpl::~ProfileImpl() {
   // shut down the database.
   template_url_model_.reset();
 
-  // The download manager queries the history system and should be deleted
-  // before the history is shutdown so it can properly cancel all requests.
-  download_manager_ = NULL;
+  // DownloadManager is lazily created, so check before accessing it.
+  if (download_manager_.get()) {
+    // The download manager queries the history system and should be shutdown
+    // before the history is shutdown so it can properly cancel all requests.
+    download_manager_->Shutdown();
+    download_manager_ = NULL;
+  }
 
   // The theme provider provides bitmaps to whoever wants them.
   theme_provider_.reset();
diff --git a/chrome/browser/renderer_host/download_resource_handler.cc b/chrome/browser/renderer_host/download_resource_handler.cc
index 911edf8..3e5f3c0 100644
--- a/chrome/browser/renderer_host/download_resource_handler.cc
+++ b/chrome/browser/renderer_host/download_resource_handler.cc
@@ -92,9 +92,14 @@ bool DownloadResourceHandler::OnResponseStarted(int request_id,
   info->referrer_charset = request_->context()->referrer_charset();
   info->save_info = save_info_;
   ChromeThread::PostTask(
-      ChromeThread::FILE, FROM_HERE,
+      ChromeThread::UI, FROM_HERE,
       NewRunnableMethod(
           download_file_manager_, &DownloadFileManager::StartDownload, info));
+
+  // We can't start saving the data before we create the file on disk.
+  // The request will be un-paused in DownloadFileManager::CreateDownloadFile.
+  rdh_->PauseRequest(global_id_.child_id, global_id_.request_id, true);
+
   return true;
 }