Revert 153563 - Remove DownloadFileManager in favor of direct ownership of DownloadFiles.

content/browser/download/download_manager_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/download/download_manager_impl.h"
 
 #include <iterator>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/debug/alias.h"
 #include "base/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
 #include "base/supports_user_data.h"
 #include "base/synchronization/lock.h"
 #include "base/sys_string_conversions.h"
 #include "build/build_config.h"
 #include "content/browser/download/byte_stream.h"
 #include "content/browser/download/download_create_info.h"
-#include "content/browser/download/download_file_factory.h"
-#include "content/browser/download/download_item_factory.h"
+#include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_item_impl.h"
 #include "content/browser/download/download_stats.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/resource_dispatcher_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_manager_delegate.h"
@@ skipping 10 matching lines @@
 
 using content::BrowserThread;
 using content::DownloadId;
 using content::DownloadItem;
 using content::DownloadPersistentStoreInfo;
 using content::ResourceDispatcherHostImpl;
 using content::WebContents;
 
 namespace {
 
+// This is just used to remember which DownloadItems come from SavePage.
+class SavePageData : public base::SupportsUserData::Data {
+ public:
+  // A spoonful of syntactic sugar.
+  static bool Get(DownloadItem* item) {
+    return item->GetUserData(kKey) != NULL;
+  }
+
+  explicit SavePageData(DownloadItem* item) {
+    item->SetUserData(kKey, this);
+  }
+
+  virtual ~SavePageData() {}
+
+ private:
+  static const char kKey[];
+
+  DISALLOW_COPY_AND_ASSIGN(SavePageData);
+};
+
+const char SavePageData::kKey[] = "DownloadItem SavePageData";
+
 void BeginDownload(content::DownloadUrlParameters* params) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   // ResourceDispatcherHost{Base} is-not-a URLRequest::Delegate, and
   // DownloadUrlParameters can-not include resource_dispatcher_host_impl.h, so
   // we must down cast. RDHI is the only subclass of RDH as of 2012 May 4.
   scoped_ptr<net::URLRequest> request(new net::URLRequest(
       params->url(),
       NULL,
       params->resource_context()->GetRequestContext()));
   request->set_referrer(params->referrer().url.spec());
@@ skipping 50 matching lines @@
 
   bool operator!=(const MapValueIteratorAdapter& that) const {
     return iter_ != that.iter_;
   }
 
  private:
   base::hash_map<int64, DownloadItem*>::const_iterator iter_;
   // Allow copy and assign.
 };
 
-void EnsureNoPendingDownloadJobsOnFile(bool* result) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  *result = (content::DownloadFile::GetNumberOfDownloadFiles() == 0);
+void EnsureNoPendingDownloadsOnFile(scoped_refptr<DownloadFileManager> dfm,
+                                    bool* result) {
+  if (dfm->NumberOfActiveDownloads())
+    *result = false;
   BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE, MessageLoop::QuitClosure());
 }
 
+void EnsureNoPendingDownloadJobsOnIO(bool* result) {
+  scoped_refptr<DownloadFileManager> download_file_manager =
+      ResourceDispatcherHostImpl::Get()->download_file_manager();
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&EnsureNoPendingDownloadsOnFile,
+                 download_file_manager, result));
+}
+
 class DownloadItemFactoryImpl : public content::DownloadItemFactory {
  public:
     DownloadItemFactoryImpl() {}
     virtual ~DownloadItemFactoryImpl() {}
 
     virtual DownloadItemImpl* CreatePersistedItem(
         DownloadItemImplDelegate* delegate,
         content::DownloadId download_id,
         const content::DownloadPersistentStoreInfo& info,
         const net::BoundNetLog& bound_net_log) OVERRIDE {
@@ skipping 22 matching lines @@
 };
 
 }  // namespace
 
 namespace content {
 
 bool DownloadManager::EnsureNoPendingDownloadsForTesting() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   bool result = true;
   BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&EnsureNoPendingDownloadJobsOnFile, &result));
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&EnsureNoPendingDownloadJobsOnIO, &result));
   MessageLoop::current()->Run();
   return result;
 }
 
 }  // namespace content
 
 DownloadManagerImpl::DownloadManagerImpl(
-    scoped_ptr<content::DownloadItemFactory> item_factory,
-    scoped_ptr<content::DownloadFileFactory> file_factory,
+    DownloadFileManager* file_manager,
+    scoped_ptr<content::DownloadItemFactory> factory,
     net::NetLog* net_log)
-    : item_factory_(item_factory.Pass()),
-      file_factory_(file_factory.Pass()),
+    : factory_(factory.Pass()),
       history_size_(0),
       shutdown_needed_(false),
       browser_context_(NULL),
+      file_manager_(file_manager),
       delegate_(NULL),
       net_log_(net_log) {
-  if (!item_factory_.get())
-    item_factory_.reset(new DownloadItemFactoryImpl());
-  if (!file_factory_.get())
-    file_factory_.reset(new content::DownloadFileFactory());
+  DCHECK(file_manager);
+  if (!factory_.get())
+    factory_.reset(new DownloadItemFactoryImpl());
 }
 
 DownloadManagerImpl::~DownloadManagerImpl() {
   DCHECK(!shutdown_needed_);
 }
 
 DownloadId DownloadManagerImpl::GetNextId() {
   DownloadId id;
   if (delegate_)
    id = delegate_->GetNextId();
   if (!id.IsValid()) {
     static int next_id;
     id = DownloadId(browser_context_, ++next_id);
   }
 
   return id;
 }
 
-void DownloadManagerImpl::DelegateStart(DownloadItemImpl* item) {
-  content::DownloadTargetCallback callback =
-      base::Bind(&DownloadManagerImpl::OnDownloadTargetDetermined,
-                 this, item->GetId());
-  if (!delegate_ || !delegate_->DetermineDownloadTarget(item, callback)) {
-    FilePath target_path = item->GetForcedFilePath();
-    // TODO(asanka): Determine a useful path if |target_path| is empty.
-    callback.Run(target_path,
-                 DownloadItem::TARGET_DISPOSITION_OVERWRITE,
-                 content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
-                 target_path);
-  }
+DownloadFileManager* DownloadManagerImpl::GetDownloadFileManager() {
+  return file_manager_;
 }
 
 bool DownloadManagerImpl::ShouldOpenDownload(DownloadItemImpl* item) {
   if (!delegate_)
     return true;
 
   return delegate_->ShouldOpenDownload(item);
 }
 
 bool DownloadManagerImpl::ShouldOpenFileBasedOnExtension(const FilePath& path) {
@@ skipping 15 matching lines @@
 void DownloadManagerImpl::Shutdown() {
   VLOG(20) << __FUNCTION__ << "()"
            << " shutdown_needed_ = " << shutdown_needed_;
   if (!shutdown_needed_)
     return;
   shutdown_needed_ = false;
 
   FOR_EACH_OBSERVER(Observer, observers_, ManagerGoingDown(this));
   // TODO(benjhayden): Consider clearing observers_.
 
-  // The DownloadFiles will be canceled and deleted by their DownloadItems.
+  DCHECK(file_manager_);
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DownloadFileManager::OnDownloadManagerShutdown,
+                 file_manager_, make_scoped_refptr(this)));
 
   AssertContainersConsistent();
 
   // Go through all downloads in downloads_.  Dangerous ones we need to
   // remove on disk, and in progress ones we need to cancel.
   for (DownloadMap::iterator it = downloads_.begin(); it != downloads_.end();) {
     DownloadItemImpl* download = it->second;
 
     // Save iterator from potential erases in this set done by called code.
     // Iterators after an erasure point are still valid for lists and
@@ skipping 22 matching lines @@
   // and all in progress downloads have been cancelled.  We can now delete
   // anything left.
 
   active_downloads_.clear();
   STLDeleteValues(&downloads_);
   downloads_.clear();
 
   // We'll have nothing more to report to the observers after this point.
   observers_.Clear();
 
+  file_manager_ = NULL;
   if (delegate_)
     delegate_->Shutdown();
   delegate_ = NULL;
 }
 
 void DownloadManagerImpl::GetTemporaryDownloads(
     const FilePath& dir_path, DownloadVector* result) {
   DCHECK(result);
 
   for (DownloadMap::iterator it = downloads_.begin();
@@ skipping 43 matching lines @@
 bool DownloadManagerImpl::Init(content::BrowserContext* browser_context) {
   DCHECK(browser_context);
   DCHECK(!shutdown_needed_)  << "DownloadManager already initialized.";
   shutdown_needed_ = true;
 
   browser_context_ = browser_context;
 
   return true;
 }
 
+// We have received a message from DownloadFileManager about a new download.
 content::DownloadId DownloadManagerImpl::StartDownload(
     scoped_ptr<DownloadCreateInfo> info,
     scoped_ptr<content::ByteStreamReader> stream) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  net::BoundNetLog bound_net_log =
-      net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
+  // |bound_net_log| will be used for logging both the download item's and
+  // the download file's events.
+  net::BoundNetLog bound_net_log = CreateDownloadItem(info.get());
 
-  // We create the DownloadItem before the DownloadFile because the
-  // DownloadItem already needs to handle a state in which there is
-  // no associated DownloadFile (history downloads, !IN_PROGRESS downloads)
-  DownloadItemImpl* download =
-      CreateDownloadItem(info.get(), bound_net_log);
-  scoped_ptr<content::DownloadFile> download_file(
-      file_factory_->CreateFile(
-          info->save_info, info->url(), info->referrer_url,
-          info->received_bytes, GenerateFileHash(),
-          stream.Pass(), bound_net_log,
-          download->DestinationObserverAsWeakPtr()));
-  download->Start(download_file.Pass());
+  // If info->download_id was unknown on entry to this function, it was
+  // assigned in CreateDownloadItem.
+  DownloadId download_id = info->download_id;
 
-  return download->GetGlobalId();
+  DownloadFileManager::CreateDownloadFileCallback callback(
+      base::Bind(&DownloadManagerImpl::OnDownloadFileCreated,
+                 this, download_id.local()));
+
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&DownloadFileManager::CreateDownloadFile,
+                 file_manager_, base::Passed(info.Pass()),
+                 base::Passed(stream.Pass()),
+                 make_scoped_refptr(this),
+                 GenerateFileHash(), bound_net_log,
+                 callback));
+
+  return download_id;
+}
+
+void DownloadManagerImpl::OnDownloadFileCreated(
+    int32 download_id, content::DownloadInterruptReason reason) {
+  if (reason != content::DOWNLOAD_INTERRUPT_REASON_NONE) {
+    OnDownloadInterrupted(download_id, reason);
+    // TODO(rdsmith): It makes no sense to continue along the
+    // regular download path after we've gotten an error.  But it's
+    // the way the code has historically worked, and this allows us
+    // to get the download persisted and observers of the download manager
+    // notified, so tests work.  When we execute all side effects of cancel
+    // (including queue removal) immedately rather than waiting for
+    // persistence we should replace this comment with a "return;".
+  }
+
+  DownloadMap::iterator download_iter = active_downloads_.find(download_id);
+  if (download_iter == active_downloads_.end())
+    return;
+
+  DownloadItemImpl* download = download_iter->second;
+  content::DownloadTargetCallback callback =
+      base::Bind(&DownloadManagerImpl::OnDownloadTargetDetermined,
+                 this, download_id);
+  if (!delegate_ || !delegate_->DetermineDownloadTarget(download, callback)) {
+    FilePath target_path = download->GetForcedFilePath();
+    // TODO(asanka): Determine a useful path if |target_path| is empty.
+    callback.Run(target_path,
+                 DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                 content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+                 target_path);
+  }
 }
 
 void DownloadManagerImpl::OnDownloadTargetDetermined(
     int32 download_id,
     const FilePath& target_path,
     DownloadItem::TargetDisposition disposition,
     content::DownloadDangerType danger_type,
     const FilePath& intermediate_path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator download_iter = active_downloads_.find(download_id);
@@ skipping 43 matching lines @@
 void DownloadManagerImpl::OnFileRemovalDetected(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (ContainsKey(downloads_, download_id))
     downloads_[download_id]->OnDownloadedFileRemoved();
 }
 
 content::BrowserContext* DownloadManagerImpl::GetBrowserContext() const {
   return browser_context_;
 }
 
-DownloadItemImpl* DownloadManagerImpl::CreateDownloadItem(
-    DownloadCreateInfo* info, const net::BoundNetLog& bound_net_log) {
+net::BoundNetLog DownloadManagerImpl::CreateDownloadItem(
+    DownloadCreateInfo* info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  net::BoundNetLog bound_net_log =
+      net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
   if (!info->download_id.IsValid())
     info->download_id = GetNextId();
-  DownloadItemImpl* download = item_factory_->CreateActiveItem(
+  DownloadItemImpl* download = factory_->CreateActiveItem(
       this, *info,
       scoped_ptr<DownloadRequestHandleInterface>(
           new DownloadRequestHandle(info->request_handle)).Pass(),
       bound_net_log);
 
   DCHECK(!ContainsKey(downloads_, download->GetId()));
   downloads_[download->GetId()] = download;
   DCHECK(!ContainsKey(active_downloads_, download->GetId()));
   active_downloads_[download->GetId()] = download;
   FOR_EACH_OBSERVER(Observer, observers_, OnDownloadCreated(this, download));
 
-  return download;
+  return bound_net_log;
 }
 
 DownloadItemImpl* DownloadManagerImpl::CreateSavePackageDownloadItem(
     const FilePath& main_file_path,
     const GURL& page_url,
     const std::string& mime_type,
     DownloadItem::Observer* observer) {
   net::BoundNetLog bound_net_log =
       net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
-  DownloadItemImpl* download = item_factory_->CreateSavePageItem(
+  DownloadItemImpl* download = factory_->CreateSavePageItem(
       this,
       main_file_path,
       page_url,
       GetNextId(),
       mime_type,
       bound_net_log);
 
   download->AddObserver(observer);
 
   DCHECK(!ContainsKey(downloads_, download->GetId()));
   downloads_[download->GetId()] = download;
+  DCHECK(!SavePageData::Get(download));
+  new SavePageData(download);
+  DCHECK(SavePageData::Get(download));
 
   FOR_EACH_OBSERVER(Observer, observers_, OnDownloadCreated(this, download));
 
   // Will notify the observer in the callback.
   if (delegate_)
     delegate_->AddItemToPersistentStore(download);
 
   return download;
 }
 
+void DownloadManagerImpl::UpdateDownload(int32 download_id,
+                                         int64 bytes_so_far,
+                                         int64 bytes_per_sec,
+                                         const std::string& hash_state) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadMap::iterator it = active_downloads_.find(download_id);
+  if (it != active_downloads_.end()) {
+    DownloadItemImpl* download = it->second;
+    if (download->IsInProgress()) {
+      download->UpdateProgress(bytes_so_far, bytes_per_sec, hash_state);
+      if (delegate_)
+        delegate_->UpdateItemInPersistentStore(download);
+    }
+  }
+}
+
+void DownloadManagerImpl::OnResponseCompleted(int32 download_id,
+                                              int64 size,
+                                              const std::string& hash) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  VLOG(20) << __FUNCTION__ << "()" << " download_id = " << download_id
+           << " size = " << size;
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // If it's not in active_downloads_, that means it was cancelled; just
+  // ignore the notification.
+  if (active_downloads_.count(download_id) == 0)
+    return;
+
+  DownloadItemImpl* download = active_downloads_[download_id];
+  download->OnAllDataSaved(size, hash);
+  MaybeCompleteDownload(download);
+}
+
 void DownloadManagerImpl::AssertStateConsistent(
     DownloadItemImpl* download) const {
   CHECK(ContainsKey(downloads_, download->GetId()));
 
   int64 state = download->GetState();
   base::debug::Alias(&state);
   if (ContainsKey(active_downloads_, download->GetId())) {
     if (download->IsPersisted())
       CHECK_EQ(DownloadItem::IN_PROGRESS, download->GetState());
     if (DownloadItem::IN_PROGRESS != download->GetState())
@@ skipping 101 matching lines @@
 void DownloadManagerImpl::DownloadStopped(DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   VLOG(20) << __FUNCTION__ << "()"
            << " download = " << download->DebugString(true);
 
   RemoveFromActiveList(download);
   // This function is called from the DownloadItem, so DI state
   // should already have been updated.
   AssertStateConsistent(download);
+
+  DCHECK(file_manager_);
+  download->OffThreadCancel();
+}
+
+void DownloadManagerImpl::OnDownloadInterrupted(
+    int32 download_id,
+    content::DownloadInterruptReason reason) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (!ContainsKey(active_downloads_, download_id))
+    return;
+  active_downloads_[download_id]->Interrupt(reason);
 }
 
 void DownloadManagerImpl::RemoveFromActiveList(DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(download);
 
   // Clean up will happen when the history system create callback runs if we
   // don't have a valid db_handle yet.
   if (download->IsPersisted()) {
     active_downloads_.erase(download->GetId());
     if (delegate_)
       delegate_->UpdateItemInPersistentStore(download);
   }
 }
 
 bool DownloadManagerImpl::GenerateFileHash() {
   return delegate_ && delegate_->GenerateFileHash();
 }
 
-void DownloadManagerImpl::SetDownloadFileFactoryForTesting(
-    scoped_ptr<content::DownloadFileFactory> file_factory) {
-  file_factory_ = file_factory.Pass();
-}
-
-content::DownloadFileFactory*
-DownloadManagerImpl::GetDownloadFileFactoryForTesting() {
-  return file_factory_.get();
-}
-
 int DownloadManagerImpl::RemoveDownloadItems(
     const DownloadItemImplVector& pending_deletes) {
   if (pending_deletes.empty())
     return 0;
 
   // Delete from internal maps.
   for (DownloadItemImplVector::const_iterator it = pending_deletes.begin();
        it != pending_deletes.end();
        ++it) {
     DownloadItemImpl* download = *it;
@@ skipping 87 matching lines @@
 // 'DownloadPersistentStoreInfo's in sorted order (by ascending start_time).
 void DownloadManagerImpl::OnPersistentStoreQueryComplete(
     std::vector<DownloadPersistentStoreInfo>* entries) {
   history_size_ = entries->size();
   for (size_t i = 0; i < entries->size(); ++i) {
     int64 db_handle = entries->at(i).db_handle;
     base::debug::Alias(&db_handle);
 
     net::BoundNetLog bound_net_log =
         net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
-    DownloadItemImpl* download = item_factory_->CreatePersistedItem(
+    DownloadItemImpl* download = factory_->CreatePersistedItem(
         this, GetNextId(), entries->at(i), bound_net_log);
     DCHECK(!ContainsKey(downloads_, download->GetId()));
     downloads_[download->GetId()] = download;
     FOR_EACH_OBSERVER(Observer, observers_, OnDownloadCreated(this, download));
     VLOG(20) << __FUNCTION__ << "()" << i << ">"
              << " download = " << download->DebugString(true);
   }
   NotifyModelChanged();
   CheckForHistoryFilesRemoval();
 }
@@ skipping 20 matching lines @@
 
 
 void DownloadManagerImpl::OnItemAddedToPersistentStore(int32 download_id,
                                                        int64 db_handle) {
   // It's valid that we don't find a matching item, i.e. on shutdown.
   if (!ContainsKey(downloads_, download_id))
     return;
 
   DownloadItemImpl* item = downloads_[download_id];
   AddDownloadItemToHistory(item, db_handle);
-  if (item->IsSavePackageDownload()) {
+  if (SavePageData::Get(item)) {
     OnSavePageItemAddedToPersistentStore(item);
   } else {
     OnDownloadItemAddedToPersistentStore(item);
   }
 }
 
 // Once the new DownloadItem has been committed to the persistent store,
 // associate it with its db_handle (TODO(benjhayden) merge db_handle with id),
 // show it in the browser (TODO(benjhayden) the ui should observe us instead),
 // and notify observers (TODO(benjhayden) observers should be able to see the
@@ skipping 126 matching lines @@
   // Finalize this download if it finished before the history callback.
   if (!item->IsInProgress())
     SavePageDownloadFinished(item);
 }
 
 void DownloadManagerImpl::SavePageDownloadFinished(
     content::DownloadItem* download) {
   if (download->IsPersisted()) {
     if (delegate_)
       delegate_->UpdateItemInPersistentStore(download);
+    if (download->IsComplete())
+      content::NotificationService::current()->Notify(
+          content::NOTIFICATION_SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
+          content::Source<DownloadManager>(this),
+          content::Details<DownloadItem>(download));
   }
 }
 
 void DownloadManagerImpl::DownloadOpened(DownloadItemImpl* download) {
   if (delegate_)
     delegate_->UpdateItemInPersistentStore(download);
   int num_unopened = 0;
   for (DownloadMap::iterator it = downloads_.begin();
        it != downloads_.end(); ++it) {
     DownloadItemImpl* item = it->second;
     if (item->IsComplete() &&
         !item->GetOpened())
       ++num_unopened;
   }
   download_stats::RecordOpensOutstanding(num_unopened);
 }
 
 void DownloadManagerImpl::DownloadRenamedToIntermediateName(
     DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // download->GetFullPath() is only expected to be meaningful after this
   // callback is received. Therefore we can now add the download to a persistent
-  // store. If the rename failed, we processed an interrupt
+  // store. If the rename failed, we receive an OnDownloadInterrupted() call
   // before we receive the DownloadRenamedToIntermediateName() call.
   if (delegate_) {
     delegate_->AddItemToPersistentStore(download);
   } else {
     OnItemAddedToPersistentStore(download->GetId(),
                                  DownloadItem::kUninitializedHandle);
   }
 }
 
 void DownloadManagerImpl::DownloadRenamedToFinalName(
     DownloadItemImpl* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  // If the rename failed, we processed an interrupt before we get here.
+  // If the rename failed, we receive an OnDownloadInterrupted() call before we
+  // receive the DownloadRenamedToFinalName() call.
   if (delegate_) {
     delegate_->UpdatePathForItemInPersistentStore(
         download, download->GetFullPath());
   }
 }