Kill DownloadManagerImpl::save_page_downloads_

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"
@@ skipping 33 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 SavePageExternalData : public DownloadItem::ExternalData {
+ public:
+  // A spoonful of syntactic sugar.
+  static bool Get(DownloadItem* item) {
+    return item->GetExternalData(kKey) != NULL;
+  }
+
+  explicit SavePageExternalData(DownloadItem* item) {
+    item->SetExternalData(kKey, this);
+  }
+
+  virtual ~SavePageExternalData() {}
+
+ private:
+  static const char kKey[];
+
+  DISALLOW_COPY_AND_ASSIGN(SavePageExternalData);
+};
+
+const char SavePageExternalData::kKey[] = "DownloadItem SavePageExternalData";
+
 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 235 matching lines @@
   // in DownloadItem destruction.
   DownloadMap downloads_to_delete;
   downloads_to_delete.swap(downloads_);
 
   active_downloads_.clear();
   STLDeleteValues(&downloads_to_delete);
 
   // We'll have nothing more to report to the observers after this point.
   observers_.Clear();
 
-  DCHECK(save_page_downloads_.empty());
-
   file_manager_ = NULL;
   if (delegate_)
     delegate_->Shutdown();
 }
 
 void DownloadManagerImpl::GetTemporaryDownloads(
     const FilePath& dir_path, DownloadVector* result) {
   DCHECK(result);
 
   for (DownloadMap::iterator it = downloads_.begin();
@@ skipping 212 matching lines @@
       page_url,
       is_otr,
       GetNextId(),
       mime_type,
       bound_net_log);
 
   download->AddObserver(observer);
 
   DCHECK(!ContainsKey(downloads_, download->GetId()));
   downloads_[download->GetId()] = download;
-  DCHECK(!ContainsKey(save_page_downloads_, download->GetId()));
-  save_page_downloads_[download->GetId()] = download;
+  DCHECK(!SavePageExternalData::Get(download));
+  new SavePageExternalData(download);
+  DCHECK(SavePageExternalData::Get(download));
 
   // Will notify the observer in the callback.
   if (delegate_)
     delegate_->AddItemToPersistentStore(download);
 
   return download;
 }
 
 // The target path for the download item is now valid. We proceed with the
 // determination of an intermediate path.
@@ skipping 242 matching lines @@
     const DownloadVector& pending_deletes) {
   if (pending_deletes.empty())
     return 0;
 
   // Delete from internal maps.
   for (DownloadVector::const_iterator it = pending_deletes.begin();
       it != pending_deletes.end();
       ++it) {
     DownloadItem* download = *it;
     DCHECK(download);
-    save_page_downloads_.erase(download->GetId());
     downloads_.erase(download->GetId());
   }
 
   // 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;
@@ skipping 150 matching lines @@
   // This includes buttons to save or cancel, for a dangerous download.
   ShowDownloadInBrowser(download);
 
   // Inform interested objects about the new download.
   NotifyModelChanged();
 }
 
 
 void DownloadManagerImpl::OnItemAddedToPersistentStore(int32 download_id,
                                                        int64 db_handle) {
-  if (save_page_downloads_.count(download_id)) {
-    OnSavePageItemAddedToPersistentStore(download_id, db_handle);
-  } else if (active_downloads_.count(download_id)) {
-    OnDownloadItemAddedToPersistentStore(download_id, db_handle);
+  DownloadItem* item = GetDownload(download_id);
+  // It's valid that we don't find a matching item, i.e. on shutdown.
+  if (!item)
+    return;
+  AddDownloadItemToHistory(item, db_handle);
+  if (SavePageExternalData::Get(item)) {
+    OnSavePageItemAddedToPersistentStore(item);
+  } else {
+    OnDownloadItemAddedToPersistentStore(item);
   }
-  // It's valid that we don't find a matching item, i.e. on shutdown.
 }
 
 // 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
 // item when it's created so they can observe it directly. Are there any
 // clients that actually need to know when the item is added to the history?).
 void DownloadManagerImpl::OnDownloadItemAddedToPersistentStore(
-    int32 download_id, int64 db_handle) {
+    DownloadItem* item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download)
-    return;
 
-  VLOG(20) << __FUNCTION__ << "()" << " db_handle = " << db_handle
-           << " download_id = " << download_id
-           << " download = " << download->DebugString(true);
-
-  AddDownloadItemToHistory(download, db_handle);
+  VLOG(20) << __FUNCTION__ << "()" << " db_handle = " << item->GetDbHandle()
+           << " download_id = " << item->GetId()
+           << " download = " << item->DebugString(true);
 
   // If the download is still in progress, try to complete it.
   //
   // Otherwise, download has been cancelled or interrupted before we've
   // received the DB handle.  We post one final message to the history
   // service so that it can be properly in sync with the DownloadItem's
   // completion status, and also inform any observers so that they get
   // more than just the start notification.
-  if (download->IsInProgress()) {
-    MaybeCompleteDownload(download);
+  if (item->IsInProgress()) {
+    MaybeCompleteDownload(item);
   } else {
-    DCHECK(download->IsCancelled());
-    active_downloads_.erase(download_id);
+    DCHECK(item->IsCancelled());
+    active_downloads_.erase(item->GetId());
     if (delegate_)
-      delegate_->UpdateItemInPersistentStore(download);
-    download->UpdateObservers();
+      delegate_->UpdateItemInPersistentStore(item);
+    item->UpdateObservers();
   }
 }
 
 void DownloadManagerImpl::ShowDownloadInBrowser(DownloadItem* download) {
   // The 'contents' may no longer exist if the user closed the contents before
   // we get this start completion event.
   WebContents* content = download->GetWebContents();
 
   // If the contents no longer exists, we ask the embedder to suggest another
   // contents.
@@ skipping 40 matching lines @@
   if (ContainsKey(active_downloads_, download_id))
     return active_downloads_[download_id];
   return NULL;
 }
 
 // Confirm that everything in all maps is also in |downloads_|, and that
 // everything in |downloads_| is also in some other map.
 void DownloadManagerImpl::AssertContainersConsistent() const {
 #if !defined(NDEBUG)
   // Turn everything into sets.
-  const DownloadMap* input_maps[] = {&active_downloads_,
-                                     &save_page_downloads_};
-  DownloadSet active_set, save_page_set;
-  DownloadSet* all_sets[] = {&active_set, &save_page_set};
+  const DownloadMap* input_maps[] = {&active_downloads_};
+  DownloadSet active_set;
+  DownloadSet* all_sets[] = {&active_set};
   DCHECK_EQ(ARRAYSIZE_UNSAFE(input_maps), ARRAYSIZE_UNSAFE(all_sets));
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(input_maps); i++) {
     for (DownloadMap::const_iterator it = input_maps[i]->begin();
          it != input_maps[i]->end(); ++it) {
       all_sets[i]->insert(&*it->second);
     }
   }
 
   DownloadSet all_downloads;
   for (DownloadMap::const_iterator it = downloads_.begin();
@@ skipping 21 matching lines @@
 // 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
 // history entry being wrong on a reload of chrome (specifically in the case of
 // Initiation -> History Callback -> Removal -> Completion), but there's no way
 // to solve that without canceling on Remove (which would then update the DB).
 
 void DownloadManagerImpl::OnSavePageItemAddedToPersistentStore(
-    int32 download_id, int64 db_handle) {
+    DownloadItem* item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  DownloadMap::const_iterator it = save_page_downloads_.find(download_id);
-  // This can happen if the download manager is shutting down and all maps
-  // have been cleared.
-  if (it == save_page_downloads_.end())
-    return;
-
-  DownloadItem* download = it->second;
-  if (!download) {
-    NOTREACHED();
-    return;
-  }
-
-  AddDownloadItemToHistory(download, db_handle);
-
   // Finalize this download if it finished before the history callback.
-  if (!download->IsInProgress())
-    SavePageDownloadFinished(download);
+  if (!item->IsInProgress())
+    SavePageDownloadFinished(item);
 }
 
 void DownloadManagerImpl::SavePageDownloadFinished(DownloadItem* download) {
   if (download->IsPersisted()) {
     if (delegate_)
       delegate_->UpdateItemInPersistentStore(download);
-    DCHECK(ContainsKey(save_page_downloads_, download->GetId()));
-    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::DownloadOpened(DownloadItem* download) {
   if (delegate_)
@@ skipping 21 matching lines @@
 void DownloadManagerImpl::DownloadRenamedToFinalName(
     DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // If the rename failed, we receive an OnDownloadInterrupted() call before we
   // receive the DownloadRenamedToFinalName() call.
   if (delegate_) {
     delegate_->UpdatePathForItemInPersistentStore(
         download, download->GetFullPath());
   }
 }