DownloadManager intereface refactoring to allow cleaner DownloadItem unit tests.

content/browser/download/download_manager_impl.cc

 // Copyright (c) 2011 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"
@@ skipping 83 matching lines @@
 DownloadManagerImpl::~DownloadManagerImpl() {
   DCHECK(!shutdown_needed_);
   if (status_updater_.get() != NULL)
     status_updater_->RemoveDelegate(this);
 }
 
 DownloadId DownloadManagerImpl::GetNextId() {
   return id_factory_->GetNextId();
 }
 
+bool DownloadManagerImpl::ShouldOpenDownload(DownloadItem* item) {
+  return delegate_->ShouldOpenDownload(item);
+}
+
+bool DownloadManagerImpl::ShouldOpenFileBasedOnExtension(const FilePath& path) {
+  return delegate_->ShouldOpenFileBasedOnExtension(path);
+}
+
 void DownloadManagerImpl::Shutdown() {
   VLOG(20) << __FUNCTION__ << "()"
            << " shutdown_needed_ = " << shutdown_needed_;
   if (!shutdown_needed_)
     return;
   shutdown_needed_ = false;
 
   FOR_EACH_OBSERVER(Observer, observers_, ManagerGoingDown());
   // TODO(benjhayden): Consider clearing observers_.
 
@@ skipping 13 matching lines @@
     // Save iterator from potential erases in this set done by called code.
     // Iterators after an erasure point are still valid for lists and
     // associative containers such as sets.
     it++;
 
     if (download->GetSafetyState() == DownloadItem::DANGEROUS &&
         download->IsPartialDownload()) {
       // The user hasn't accepted it, so we need to remove it
       // from the disk.  This may or may not result in it being
       // removed from the DownloadManager queues and deleted
-      // (specifically, DownloadManager::RemoveDownload only
+      // (specifically, DownloadManager::DownloadRemoved only
       // removes and deletes it if it's known to the history service)
       // so the only thing we know after calling this function is that
       // the download was deleted if-and-only-if it was removed
       // from all queues.
       download->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
     } else if (download->IsPartialDownload()) {
       download->Cancel(false);
       delegate_->UpdateItemInPersistentStore(download);
     }
   }
@@ skipping 162 matching lines @@
         contents, suggested_path, reinterpret_cast<void*>(id_ptr));
 
     FOR_EACH_OBSERVER(Observer, observers_,
                       SelectFileDialogDisplayed(download_id));
   } else {
     // No prompting for download, just continue with the suggested name.
     ContinueDownloadWithPath(download, suggested_path);
   }
 }
 
-content::BrowserContext* DownloadManagerImpl::BrowserContext() {
+content::BrowserContext* DownloadManagerImpl::BrowserContext() const {
   return browser_context_;
 }
 
 FilePath DownloadManagerImpl::LastDownloadPath() {
   return last_download_path_;
 }
 
 void DownloadManagerImpl::CreateDownloadItem(
     DownloadCreateInfo* info, const DownloadRequestHandle& request_handle) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadItem* download = new DownloadItemImpl(
       this, *info, new DownloadRequestHandle(request_handle),
       browser_context_->IsOffTheRecord());
   int32 download_id = info->download_id.local();
   DCHECK(!ContainsKey(in_progress_, download_id));
 
   // TODO(rdsmith): Remove after http://crbug.com/85408 resolved.
   CHECK(!ContainsKey(active_downloads_, download_id));
   downloads_.insert(download);
   active_downloads_[download_id] = download;
 }
 
+DownloadItem* DownloadManagerImpl::CreateSavePackageDownloadItem(
+    const FilePath& main_file_path,
+    const GURL& page_url,
+    bool is_otr,
+    DownloadItem::Observer* observer) {
+  DownloadItem* download = new DownloadItemImpl(
+      this, main_file_path, page_url, is_otr, GetNextId());
+
+  download->AddObserver(observer);
+
+  DCHECK(!ContainsKey(save_page_downloads_, download->GetId()));
+  downloads_.insert(download);
+  save_page_downloads_[download->GetId()] = download;
+
+  // Will notify the observer in the callback.
+  delegate_->AddItemToPersistentStore(download);
+
+  return download;
+}
+
 void DownloadManagerImpl::ContinueDownloadWithPath(
     DownloadItem* download, const FilePath& chosen_file) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(download);
 
   int32 download_id = download->GetId();
 
   // NOTE(ahendrickson) Eventually |active_downloads_| will replace
   // |in_progress_|, but we don't want to change the semantics yet.
   DCHECK(!ContainsKey(in_progress_, download_id));
@@ skipping 10 matching lines @@
   UpdateDownloadProgress();  // Reflect entry into in_progress_.
 
   // Rename to intermediate name.
   FilePath download_path;
   if (!delegate_->OverrideIntermediatePath(download, &download_path))
     download_path = download->GetFullPath();
 
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
       base::Bind(&DownloadFileManager::RenameInProgressDownloadFile,
-                 file_manager_, download->GetGlobalId(), download_path));
+                 file_manager_, download->GetGlobalId(),
+                 download_path));
 
   download->Rename(download_path);
 
   delegate_->AddItemToPersistentStore(download);
 }
 
 void DownloadManagerImpl::UpdateDownload(int32 download_id, int64 size) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator it = active_downloads_.find(download_id);
   if (it != active_downloads_.end()) {
@@ skipping 19 matching lines @@
   if (active_downloads_.count(download_id) == 0)
     return;
 
   DownloadItem* download = active_downloads_[download_id];
   download->OnAllDataSaved(size, hash);
   delegate_->OnResponseCompleted(download);
 
   MaybeCompleteDownload(download);
 }
 
-void DownloadManagerImpl::AssertQueueStateConsistent(DownloadItem* download) {
+void DownloadManagerImpl::AssertStateConsistent(DownloadItem* download) const {
   // TODO(rdsmith): Change to DCHECK after http://crbug.com/85408 resolved.
   if (download->GetState() == DownloadItem::REMOVING) {
     CHECK(!ContainsKey(downloads_, download));
     CHECK(!ContainsKey(active_downloads_, download->GetId()));
     CHECK(!ContainsKey(in_progress_, download->GetId()));
     CHECK(!ContainsKey(history_downloads_, download->GetDbHandle()));
     return;
   }
 
   // Should be in downloads_ if we're not REMOVING.
   CHECK(ContainsKey(downloads_, download));
 
   // Check history_downloads_ consistency.
   if (download->GetDbHandle() != DownloadItem::kUninitializedHandle) {
     CHECK(ContainsKey(history_downloads_, download->GetDbHandle()));
   } else {
     // TODO(rdsmith): Somewhat painful; make sure to disable in
     // release builds after resolution of http://crbug.com/85408.
-    for (DownloadMap::iterator it = history_downloads_.begin();
+    for (DownloadMap::const_iterator it = history_downloads_.begin();
          it != history_downloads_.end(); ++it) {
       CHECK(it->second != download);
     }
   }
 
   int64 state = download->GetState();
   base::debug::Alias(&state);
   if (ContainsKey(active_downloads_, download->GetId())) {
     if (download->GetDbHandle() != DownloadItem::kUninitializedHandle)
       CHECK_EQ(DownloadItem::IN_PROGRESS, download->GetState());
@@ skipping 59 matching lines @@
   // Remove the id from in_progress
   in_progress_.erase(download->GetId());
   UpdateDownloadProgress();  // Reflect removal from in_progress_.
 
   delegate_->UpdateItemInPersistentStore(download);
 
   // Finish the download.
   download->OnDownloadCompleting(file_manager_);
 }
 
-void DownloadManagerImpl::DownloadCompleted(int32 download_id) {
+void DownloadManagerImpl::DownloadCompleted(DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DownloadItem* download = GetDownloadItem(download_id);
   DCHECK(download);
   delegate_->UpdateItemInPersistentStore(download);
-  active_downloads_.erase(download_id);
-  AssertQueueStateConsistent(download);
+  active_downloads_.erase(download->GetId());
+  AssertStateConsistent(download);
 }
 
 void DownloadManagerImpl::OnDownloadRenamedToFinalName(
     int download_id,
     const FilePath& full_path,
     int uniquifier) {
   VLOG(20) << __FUNCTION__ << "()" << " download_id = " << download_id
            << " full_path = \"" << full_path.value() << "\""
            << " uniquifier = " << uniquifier;
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ skipping 21 matching lines @@
 void DownloadManagerImpl::CancelDownload(int32 download_id) {
   DownloadItem* download = GetActiveDownload(download_id);
   // A cancel at the right time could remove the download from the
   // |active_downloads_| map before we get here.
   if (!download)
     return;
 
   download->Cancel(true);
 }
 
-void DownloadManagerImpl::DownloadCancelledInternal(DownloadItem* download) {
+void DownloadManagerImpl::DownloadCancelled(DownloadItem* 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.
-  AssertQueueStateConsistent(download);
+  AssertStateConsistent(download);
 
   if (file_manager_)
     download->OffThreadCancel(file_manager_);
 }
 
 void DownloadManagerImpl::OnDownloadInterrupted(int32 download_id,
                                                 int64 size,
                                                 InterruptReason reason) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ skipping 71 matching lines @@
 
   // Tell observers to refresh their views.
   NotifyModelChanged();
 
   // Delete the download items themselves.
   const int num_deleted = static_cast<int>(pending_deletes.size());
   STLDeleteContainerPointers(pending_deletes.begin(), pending_deletes.end());
   return num_deleted;
 }
 
-void DownloadManagerImpl::RemoveDownload(int64 download_handle) {
-  DownloadMap::iterator it = history_downloads_.find(download_handle);
-  if (it == history_downloads_.end())
+void DownloadManagerImpl::DownloadRemoved(DownloadItem* download) {
+  if (history_downloads_.find(download->GetDbHandle()) ==
+      history_downloads_.end())
     return;
 
   // Make history update.
-  DownloadItem* download = it->second;
   delegate_->RemoveItemFromPersistentStore(download);
 
   // Remove from our tables and delete.
   int downloads_count = RemoveDownloadItems(DownloadVector(1, download));
   DCHECK_EQ(1, downloads_count);
 }
 
 int DownloadManagerImpl::RemoveDownloadsBetween(const base::Time remove_begin,
                                                 const base::Time remove_end) {
   delegate_->RemoveItemsFromPersistentStoreBetween(remove_begin, remove_end);
 
   // All downloads visible to the user will be in the history,
   // so scan that map.
   DownloadVector pending_deletes;
   for (DownloadMap::const_iterator it = history_downloads_.begin();
       it != history_downloads_.end();
       ++it) {
     DownloadItem* download = it->second;
     if (download->GetStartTime() >= remove_begin &&
         (remove_end.is_null() || download->GetStartTime() < remove_end) &&
         (download->IsComplete() || download->IsCancelled())) {
-      AssertQueueStateConsistent(download);
+      AssertStateConsistent(download);
       pending_deletes.push_back(download);
     }
   }
   return RemoveDownloadItems(pending_deletes);
 }
 
 int DownloadManagerImpl::RemoveDownloads(const base::Time remove_begin) {
   return RemoveDownloadsBetween(remove_begin, base::Time());
 }
 
@@ skipping 126 matching lines @@
 
 // The history service has retrieved all download entries. 'entries' contains
 // 'DownloadPersistentStoreInfo's in sorted order (by ascending start_time).
 void DownloadManagerImpl::OnPersistentStoreQueryComplete(
     std::vector<DownloadPersistentStoreInfo>* entries) {
   // TODO(rdsmith): Remove this and related logic when
   // http://crbug.com/85408 is fixed.
   largest_db_handle_in_history_ = 0;
 
   for (size_t i = 0; i < entries->size(); ++i) {
-    DownloadItem* download = new DownloadItemImpl(this, entries->at(i));
+    DownloadItem* download = new DownloadItemImpl(
+        this, GetNextId(), entries->at(i));
     // TODO(rdsmith): Remove after http://crbug.com/85408 resolved.
     CHECK(!ContainsKey(history_downloads_, download->GetDbHandle()));
     downloads_.insert(download);
     history_downloads_[download->GetDbHandle()] = download;
     VLOG(20) << __FUNCTION__ << "()" << i << ">"
              << " download = " << download->DebugString(true);
 
     if (download->GetDbHandle() > largest_db_handle_in_history_)
       largest_db_handle_in_history_ = download->GetDbHandle();
   }
@@ skipping 169 matching lines @@
   DownloadSet remainder;
   std::insert_iterator<DownloadSet>
       insert_remainder(remainder, remainder.begin());
   std::set_difference(downloads_.begin(), downloads_.end(),
                       downloads_union.begin(), downloads_union.end(),
                       insert_remainder);
   DCHECK(remainder.empty());
 #endif
 }
 
-void DownloadManagerImpl::SavePageDownloadStarted(DownloadItem* download) {
-  DCHECK(!ContainsKey(save_page_downloads_, download->GetId()));
-  downloads_.insert(download);
-  save_page_downloads_[download->GetId()] = download;
-
-  // Add this entry to the history service.
-  // Additionally, the UI is notified in the callback.
-  delegate_->AddItemToPersistentStore(download);
-}
-
 // SavePackage will call SavePageDownloadFinished upon completion/cancellation.
 // The history callback will call OnSavePageItemAddedToPersistentStore.
 // If the download finishes before the history callback,
 // OnSavePageItemAddedToPersistentStore calls SavePageDownloadFinished, ensuring
 // that the history event is update regardless of the order in which these two
 // events complete.
 // If something removes the download item from the download manager (Remove,
 // Shutdown) the result will be that the SavePage system will not be able to
 // properly update the download item (which no longer exists) or the download
 // history, but the action will complete properly anyway.  This may lead to the
@@ skipping 36 matching lines @@
     save_page_downloads_.erase(download->GetId());
 
     if (download->IsComplete())
       content::NotificationService::current()->Notify(
           content::NOTIFICATION_SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
           content::Source<DownloadManager>(this),
           content::Details<DownloadItem>(download));
   }
 }
 
-void DownloadManagerImpl::MarkDownloadOpened(DownloadItem* download) {
+void DownloadManagerImpl::DownloadOpened(DownloadItem* download) {
   delegate_->UpdateItemInPersistentStore(download);
   int num_unopened = 0;
   for (DownloadMap::iterator it = history_downloads_.begin();
        it != history_downloads_.end(); ++it) {
     if (it->second->IsComplete() && !it->second->GetOpened())
       ++num_unopened;
   }
   download_stats::RecordOpensOutstanding(num_unopened);
 }
 
 void DownloadManagerImpl::SetFileManager(DownloadFileManager* file_manager) {
   file_manager_ = file_manager;
 }