Revert 58196 - 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.

TEST=unit_tests, browser_tests, ui_tests
BUG=48913

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

TBR=phajdan.jr@chromium.org
Review URL: http://codereview.chromium.org/3348010

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

10 files changed, 255 insertions, 129 deletions

diff --git a/chrome/browser/download/base_file.cc b/chrome/browser/download/base_file.cc
index 33b9372..0bf74eb 100644
--- a/chrome/browser/download/base_file.cc
+++ b/chrome/browser/download/base_file.cc
@@ -20,14 +20,13 @@
 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_(received_bytes),
+      bytes_so_far_(0),
       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 6c57165..fa3873f 100644
--- a/chrome/browser/download/base_file.h
+++ b/chrome/browser/download/base_file.h
@@ -22,7 +22,6 @@ 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 3c135aa..d1b93a9 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(), 0, file_stream_));
+    base_file_.reset(new BaseFile(FilePath(), GURL(), GURL(), file_stream_));
   }
 
   virtual void TearDown() {
diff --git a/chrome/browser/download/download_file.cc b/chrome/browser/download/download_file.cc
index ab56380..c877a6e 100644
--- a/chrome/browser/download/download_file.cc
+++ b/chrome/browser/download/download_file.cc
@@ -10,19 +10,15 @@
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_types.h"
 
-DownloadFile::DownloadFile(const DownloadCreateInfo* info,
-                           DownloadManager* download_manager)
+DownloadFile::DownloadFile(const DownloadCreateInfo* info)
     : 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),
-      download_manager_(download_manager) {
+      request_id_(info->request_id) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-
 }
 
 DownloadFile::~DownloadFile() {
@@ -44,13 +40,3 @@ 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 88c0715..ca188c9 100644
--- a/chrome/browser/download/download_file.h
+++ b/chrome/browser/download/download_file.h
@@ -7,12 +7,10 @@
 #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
@@ -21,8 +19,7 @@ class ResourceDispatcherHost;
 // cancelled, the DownloadFile is destroyed.
 class DownloadFile : public BaseFile {
  public:
-  DownloadFile(const DownloadCreateInfo* info,
-               DownloadManager* download_manager);
+  explicit DownloadFile(const DownloadCreateInfo* info);
   virtual ~DownloadFile();
 
   // Deletes its .crdownload intermediate file.
@@ -32,10 +29,7 @@ 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
@@ -48,9 +42,6 @@ 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 3c1edf9..2862742 100644
--- a/chrome/browser/download/download_file_manager.cc
+++ b/chrome/browser/download/download_file_manager.cc
@@ -35,21 +35,6 @@ 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)
@@ -58,29 +43,37 @@ 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_);
 }
 
-void DownloadFileManager::CreateDownloadFile(
-    DownloadCreateInfo* info, DownloadManager* download_manager) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+// Notifications sent from the download thread and run on the UI thread.
 
-  scoped_ptr<DownloadFile> download_file(
-      new DownloadFile(info, download_manager));
-  if (!download_file->Initialize()) {
+// 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) {
     ChromeThread::PostTask(
         ChromeThread::IO, FROM_HERE,
         NewRunnableFunction(&download_util::CancelDownloadRequest,
@@ -91,26 +84,58 @@ void DownloadFileManager::CreateDownloadFile(
     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;
-
   StartUpdateTimer();
 
-  ChromeThread::PostTask(
-      ChromeThread::UI, FROM_HERE,
-      NewRunnableMethod(download_manager,
-                        &DownloadManager::StartDownload, info));
+  // 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);
+}
+
+// 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);
 }
 
+// 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::FILE));
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   if (!update_timer_.IsRunning()) {
     update_timer_.Start(base::TimeDelta::FromMilliseconds(kUpdatePeriodMs),
                         this, &DownloadFileManager::UpdateInProgressDownloads);
@@ -118,22 +143,21 @@ void DownloadFileManager::StartUpdateTimer() {
 }
 
 void DownloadFileManager::StopUpdateTimer() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   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::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()));
-    }
+  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);
   }
 }
 
@@ -144,13 +168,21 @@ 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::UI));
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
   DCHECK(info);
 
-  DownloadManager* manager = DownloadManagerForRenderViewHost(
-      info->child_id, info->render_view_id);
-  if (!manager) {
+  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.
     ChromeThread::PostTask(
         ChromeThread::IO, FROM_HERE,
         NewRunnableFunction(&download_util::CancelDownloadRequest,
@@ -158,12 +190,22 @@ void DownloadFileManager::StartDownload(DownloadCreateInfo* info) {
                             info->child_id,
                             info->request_id));
     delete info;
+    delete download;
     return;
   }
 
-  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &DownloadFileManager::CreateDownloadFile,
-                        info, manager));
+  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));
 }
 
 // We don't forward an update to the UI thread here, since we want to throttle
@@ -179,14 +221,25 @@ 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)
-      download->AppendDataToFile(data->data(), data_len);
+    if (download) {
+      if (download->AppendDataToFile(data->data(), data_len))
+        progress_bytes += data_len;
+    }
     data->Release();
   }
+
+  if (download) {
+    AutoLock lock(progress_lock_);
+    ui_progress_[download->id()] += progress_bytes;
+  }
 }
 
 void DownloadFileManager::DownloadFinished(int id, DownloadBuffer* buffer) {
@@ -197,15 +250,18 @@ void DownloadFileManager::DownloadFinished(int id, DownloadBuffer* buffer) {
     DownloadFile* download = it->second;
     download->Finish();
 
-    DownloadManager* download_manager = download->GetDownloadManager();
-    if (download_manager) {
-      ChromeThread::PostTask(
-          ChromeThread::UI, FROM_HERE,
-          NewRunnableMethod(
-              download_manager, &DownloadManager::DownloadFinished,
-              id, download->bytes_so_far()));
+    int64 download_size = -1;
+    {
+      AutoLock lock(progress_lock_);
+      download_size = ui_progress_[download->id()];
     }
 
+    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()) {
@@ -215,7 +271,9 @@ void DownloadFileManager::DownloadFinished(int id, DownloadBuffer* buffer) {
   }
 
   if (downloads_.empty())
-    StopUpdateTimer();
+    ChromeThread::PostTask(
+      ChromeThread::UI, FROM_HERE,
+      NewRunnableMethod(this, &DownloadFileManager::StopUpdateTimer));
 }
 
 // This method will be sent via a user action, or shutdown on the UI thread, and
@@ -228,26 +286,94 @@ 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())
-    StopUpdateTimer();
+  if (downloads_.empty()) {
+    ChromeThread::PostTask(
+      ChromeThread::UI, FROM_HERE,
+      NewRunnableMethod(this, &DownloadFileManager::StopUpdateTimer));
+  }
 }
 
-void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+// 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;
+}
+
+// Called by DownloadManagers in their destructor, and only on the UI thread.
+void DownloadFileManager::RemoveDownloadManager(DownloadManager* manager) {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
   DCHECK(manager);
+  RequestMap::iterator it = requests_.find(manager);
+  if (it == requests_.end())
+    return;
 
-  for (DownloadFileMap::iterator i = downloads_.begin();
-       i != downloads_.end(); ++i) {
-    DownloadFile* download_file = i->second;
-    if (download_file->GetDownloadManager() == manager)
-      download_file->OnDownloadManagerShutdown();
+  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);
+    }
   }
+
+  requests_.erase(it);
 }
 
 // Actions from the UI thread and run on the download thread
@@ -346,8 +472,11 @@ void DownloadFileManager::OnFinalDownloadName(int id,
     delete download;
   }
 
-  if (downloads_.empty())
-    StopUpdateTimer();
+  if (downloads_.empty()) {
+    ChromeThread::PostTask(
+      ChromeThread::UI, FROM_HERE,
+      NewRunnableMethod(this, &DownloadFileManager::StopUpdateTimer));
+  }
 }
 
 // Called only from OnFinalDownloadName or OnIntermediateDownloadName
@@ -359,14 +488,13 @@ void DownloadFileManager::CancelDownloadOnRename(int id) {
   if (!download)
     return;
 
-  DownloadManager* download_manager = download->GetDownloadManager();
-  if (!download_manager) {
+  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 {
     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 d51d35f..15e7d21 100644
--- a/chrome/browser/download/download_file_manager.h
+++ b/chrome/browser/download/download_file_manager.h
@@ -44,6 +44,7 @@
 
 #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"
@@ -74,17 +75,16 @@ 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 FILE thread by DownloadManager at the beginning of its shutdown.
-  void OnDownloadManagerShutdown(DownloadManager* manager);
+  // Called on the UI thread to remove a download item or manager.
+  void RemoveDownloadManager(DownloadManager* manager);
+  void RemoveDownload(int id, DownloadManager* manager);
 
 #if !defined(OS_MACOSX)
   // The open and show methods run on the file thread, which does not work on
@@ -125,14 +125,22 @@ class DownloadFileManager
   // Clean up helper that runs on the download thread.
   void OnShutdown();
 
-  // Creates DownloadFile on FILE thread and continues starting the download
-  // process.
-  void CreateDownloadFile(DownloadCreateInfo* info,
-                          DownloadManager* download_manager);
+  // 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);
+
+  // 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);
 
   // 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);
@@ -144,12 +152,28 @@ class DownloadFileManager
   typedef base::hash_map<int, DownloadFile*> DownloadFileMap;
   DownloadFileMap downloads_;
 
-  // Schedule periodic updates of the download progress. This timer
-  // is controlled from the FILE thread, and posts updates to the UI thread.
+  // Throttle 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 657f4d1..ad9b604 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -74,12 +74,9 @@ DownloadManager::~DownloadManager() {
 void DownloadManager::Shutdown() {
   DCHECK(shutdown_needed_) << "Shutdown called when not needed.";
 
-  if (file_manager_) {
-    ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
-        NewRunnableMethod(file_manager_,
-                          &DownloadFileManager::OnDownloadManagerShutdown,
-                          this));
-  }
+  // Stop receiving download updates
+  if (file_manager_)
+    file_manager_->RemoveDownloadManager(this);
 
   // 'in_progress_' may contain DownloadItems that have not finished the start
   // complete (from the history service) and thus aren't in downloads_.
@@ -665,6 +662,8 @@ 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_unittest.cc b/chrome/browser/download/download_manager_unittest.cc
index 428ae8d..d63edc0 100644
--- a/chrome/browser/download/download_manager_unittest.cc
+++ b/chrome/browser/download/download_manager_unittest.cc
@@ -179,7 +179,7 @@ const struct {
 class MockDownloadFile : public DownloadFile {
  public:
   explicit MockDownloadFile(DownloadCreateInfo* info)
-      : DownloadFile(info, NULL), renamed_count_(0) { }
+      : DownloadFile(info), 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 98667a3..b05f7c5 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(), 0, linked_ptr<net::FileStream>()),
+    : BaseFile(FilePath(), info->url, GURL(), linked_ptr<net::FileStream>()),
       info_(info) {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));