Make DownloadHistory observe manager, items

chrome/browser/download/download_history.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.
 
+// DownloadHistory manages persisting DownloadItems to the history service by
+// observing a single DownloadManager and all its DownloadItems.
+// DownloadHistory decides whether and when to add items to, remove items from,
+// and update items in the database. DownloadHistory uses DownloadHistoryData to
+// store per-DownloadItem data such as its db_handle, whether the item is being
+// added and waiting for its db_handle, and the last DownloadPersistentStoreInfo
+// that was passed to the database.  When the DownloadManager and its delegate
+// (ChromeDownloadManagerDelegate) are initialized, DownloadHistory is created
+// and queries the HistoryService. When the HistoryService calls back from
+// QueryDownloads() to QueryCallback(), DownloadHistory uses
+// DownloadManager::CreateDownloadItem() to inform DownloadManager of these
+// persisted DownloadItems. CreateDownloadItem() internally calls
+// OnDownloadCreated(), which normally adds items to the database, so
+// QueryCallback() uses |loading_| to disable adding items to the database.  If
+// a download is removed via OnDownloadRemoved() while the item is still being
+// added to the database, DownloadHistory uses |removed_while_adding_| to
+// remember to remove the item when its ItemAdded() callback is called.  All
+// callbacks are bound with a weak pointer to DownloadHistory to prevent
+// use-after-free bugs.
+// ChromeDownloadManagerDelegate owns DownloadHistory, and deletes it in
+// Shutdown(), which is called by DownloadManagerImpl::Shutdown() after all
+// DownloadItems are destroyed.
+
 #include "chrome/browser/download/download_history.h"
 
-#include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "chrome/browser/cancelable_request.h"
 #include "chrome/browser/download/download_crx_util.h"
-#include "chrome/browser/history/history_marshaling.h"
-#include "chrome/browser/history/history_service_factory.h"
-#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/history/download_persistent_store_info.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/download_item.h"
-#include "content/public/browser/download_persistent_store_info.h"
-
+#include "content/public/browser/download_manager.h"
+
+using content::BrowserThread;
 using content::DownloadItem;
-using content::DownloadPersistentStoreInfo;
-
-DownloadHistory::DownloadHistory(Profile* profile)
-    : profile_(profile),
-      next_fake_db_handle_(DownloadItem::kUninitializedHandle - 1) {
-  DCHECK(profile);
-}
-
-DownloadHistory::~DownloadHistory() {}
-
-void DownloadHistory::GetNextId(
-    const HistoryService::DownloadNextIdCallback& callback) {
-  HistoryService* hs = HistoryServiceFactory::GetForProfile(
-      profile_, Profile::EXPLICIT_ACCESS);
-  if (!hs)
-    return;
-
-  hs->GetNextDownloadId(&history_consumer_, callback);
-}
-
-void DownloadHistory::Load(
+using content::DownloadManager;
+
+namespace {
+
+// The value of |db_handle| indicating that the associated DownloadItem is not
+// yet persisted.
+static const int64 kUninitializedHandle = -1;
+
+// Per-DownloadItem data. This information does not belong inside DownloadItem,
+// and keeping maps in DownloadHistory from DownloadItem to this information is
+// error-prone and complicated. Unfortunately, DownloadHistory::removing_ and
+// removed_while_adding_ cannot be moved into this class partly because
+// DownloadHistoryData is destroyed when DownloadItems are destroyed, and we
+// have no control over when DownloadItems are destroyed.
+class DownloadHistoryData : public base::SupportsUserData::Data {
+ public:
+  static DownloadHistoryData* Get(DownloadItem* item) {
+    base::SupportsUserData::Data* data = item->GetUserData(kKey);
+    return (data == NULL) ? NULL :
+      static_cast<DownloadHistoryData*>(data);
+  }
+
+  explicit DownloadHistoryData(DownloadItem* item)
+    : is_adding_(false),
+      db_handle_(kUninitializedHandle),
+      info_(NULL) {
+    item->SetUserData(kKey, this);
+  }
+
+  virtual ~DownloadHistoryData() {
+  }
+
+  // Whether this item is currently being added to the database.
+  bool is_adding() const { return is_adding_; }
+  void set_is_adding(bool a) { is_adding_ = a; }
+
+  // Whether this item is already persisted in the database.
+  bool is_persisted() const { return db_handle_ > kUninitializedHandle; }

[Randy Smith (Not in Mondays), 2012/08/27 18:25:11]

As noted elsewhere, I'd sorta like download_database.* to be the authoritative
source of db_handles (actually, I don't care where it is, I just want it to be
in one place, and there's a lot of tricky code about it in download_database.cc
already).  So the implicit knowledge of the namespace in this (that we allocate
values above kUninitializedHandle rather than below) seems wrong to have present
in this file.  Can we do != kUninitializedHandle?

[benjhayden, 2012/09/10 19:02:56]

Done.

+
+  int64 db_handle() const { return db_handle_; }
+  void set_db_handle(int64 h) { db_handle_ = h; }
+
+  // This allows OnDownloadUpdated() to see what changed in a DownloadItem if
+  // anything, in order to prevent writing to the database unnecessarily.  It is
+  // nullified when the item is no longer in progress in order to save memory.
+  DownloadPersistentStoreInfo* info() { return info_.get(); }
+  void set_info(const DownloadPersistentStoreInfo& i) {
+    info_.reset(new DownloadPersistentStoreInfo(i));
+  }
+  void clear_info() {
+    info_.reset();
+  }
+
+ private:
+  static const char kKey[];
+
+  bool is_adding_;
+  int64 db_handle_;
+  scoped_ptr<DownloadPersistentStoreInfo> info_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadHistoryData);
+};
+
+const char DownloadHistoryData::kKey[] =
+  "DownloadItem DownloadHistoryData";
+
+DownloadPersistentStoreInfo GetPersistentStoreInfo(DownloadItem* item) {
+  DownloadHistoryData* dhd = DownloadHistoryData::Get(item);
+  return DownloadPersistentStoreInfo(
+      item->GetFullPath(),
+      item->GetURL(),
+      item->GetReferrerUrl(),
+      item->GetStartTime(),
+      item->GetEndTime(),
+      item->GetReceivedBytes(),
+      item->GetTotalBytes(),
+      item->GetState(),
+      ((dhd != NULL) ? dhd->db_handle() : kUninitializedHandle),
+      item->GetOpened());
+}
+
+typedef std::vector<DownloadPersistentStoreInfo> InfoVector;
+
+// CancelableRequestConsumer would normally ensure that callbacks from the

[Randy Smith (Not in Mondays), 2012/08/27 18:25:11]

How about "The HistoryService normally uses CancelableRequestConsumer to ..."? 
I got confused because we aren't using CancelableRequestConsumer anymore so I
wasn't sure why we were refering to it.

[benjhayden, 2012/09/10 19:02:56]

Done.

+// HistoryService are run on the same thread that issued the request, but
+// DownloadHistory has opted for less magic, so it uses Ensure*OnUI to

[Randy Smith (Not in Mondays), 2012/08/27 18:25:11]

"... less magic by relying on WeakPtrs instead ..."?

[benjhayden, 2012/09/10 19:02:56]

Done.

+// explicitly bounce to the UI thread. These methods are static to avoid
+// creating WeakPtrs on the wrong thread.

[Randy Smith (Not in Mondays), 2012/08/27 18:25:11]

I don't understand the weak pointer reference in this context?

[benjhayden, 2012/09/10 19:02:56]

If these functions were methods on DownloadHistory, then they might need to
create WeakPtrs for the posted callback, but you can't create WeakPtrs on a
thread other than the thread that the WeakPtrFactory was created on. So these
functions need to be static.

+void EnsureQueryOnUI(
+    const base::Callback<void(scoped_ptr<InfoVector>)>& callback,
+    scoped_ptr<InfoVector> infos) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
+        callback, base::Passed(infos.Pass())));
+    return;
+  }
+  callback.Run(infos.Pass());
+}
+
+void EnsureItemAddedOnUI(const base::Callback<void(int64)>& callback,
+                         int64 handle) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
+        callback, handle));
+    return;
+  }
+  callback.Run(handle);
+}
+
+void EnsureVisitedOnUI(
+    const base::Callback<void(bool, int, base::Time)>& callback,
+    bool success, int count, base::Time first_visit) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
+        callback, success, count, first_visit));
+    return;
+  }
+  callback.Run(success, count, first_visit);
+}
+
+}  // anonymous namespace
+
+HistoryServiceDownloadAdapter::HistoryServiceDownloadAdapter(
+    HistoryService* history_service)
+  : history_service_(history_service) {
+}
+
+HistoryServiceDownloadAdapter::~HistoryServiceDownloadAdapter() {}
+
+void HistoryServiceDownloadAdapter::QueryDownloads(
     const HistoryService::DownloadQueryCallback& callback) {
-  HistoryService* hs = HistoryServiceFactory::GetForProfile(
-      profile_, Profile::EXPLICIT_ACCESS);
-  if (!hs)
-    return;
-
-  hs->QueryDownloads(&history_consumer_, callback);
-
-  // This is the initial load, so do a cleanup of corrupt in-progress entries.
-  hs->CleanUpInProgressEntries();
+  history_service_->QueryDownloads(callback);
+  history_service_->CleanUpInProgressEntries();
+}
+
+void HistoryServiceDownloadAdapter::GetVisibleVisitCountToHost(
+    const GURL& referrer_url,
+    const HistoryService::GetVisibleVisitCountToHostSimpleCallback& callback) {
+  history_service_->GetVisibleVisitCountToHostSimple(
+      referrer_url, callback);
+}
+
+void HistoryServiceDownloadAdapter::CreateDownload(
+    const DownloadPersistentStoreInfo& info,
+    const HistoryService::DownloadCreateCallback& callback) {
+  history_service_->CreateDownload(info, callback);
+}
+
+void HistoryServiceDownloadAdapter::UpdateDownload(
+    const DownloadPersistentStoreInfo& info) {
+  history_service_->UpdateDownload(info);
+}
+
+void HistoryServiceDownloadAdapter::RemoveDownloads(
+    const std::set<int64>& handles) {
+  history_service_->RemoveDownloads(handles);
+}
+
+DownloadHistory::DownloadHistory(
+    DownloadManager* manager,
+    scoped_ptr<HistoryServiceDownloadAdapter> history)
+  : manager_(manager),
+    history_(history.Pass()),
+    loading_(false),
+    history_size_(0),
+    ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(manager_);
+  manager_->AddObserver(this);
+  history_->QueryDownloads(base::Bind(&EnsureQueryOnUI, base::Bind(
+      &DownloadHistory::QueryCallback, weak_ptr_factory_.GetWeakPtr())));
+}
+
+DownloadHistory::~DownloadHistory() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  for (ItemSet::const_iterator iter = observing_items_.begin();
+       iter != observing_items_.end(); ++iter) {
+    (*iter)->RemoveObserver(this);
+  }
+  observing_items_.clear();
+  if (manager_)
+    manager_->RemoveObserver(this);
+}
+
+void DownloadHistory::QueryCallback(scoped_ptr<InfoVector> infos) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  // ManagerGoingDown() may have happened before the history loaded.
+  if (!manager_)
+    return;
+  // OnDownloadCreated() is called inside DownloadManager::CreateDownloadItem(),
+  // so temporarily disable adding new unpersisted items to the history.  All
+  // methods run on the UI thread and CreateDownloadItem() is synchronous, so it
+  // is impossible for an OnDownloadCreated() to come in for another
+  // DownloadItem while QueryCallback() is processing the database.

[Randy Smith (Not in Mondays), 2012/08/27 18:25:11]

This is actually not strictly speaking true, or at least it won't be in the
future, though I'm not sure what to do about it.  If there's some other observer
on the DownloadManager that hooks OnDownloadCreated, and it calls the
programmatic download interface from OnDownloadCreated, and we change that
interface to create the download item right away and link it up with the request
when the request comes in a la download resumption (this is the "in the future"
part), we could have an extraneous OnDownloadCreated() showing up.    

I suppose we could tell the difference by whether the download item is being in
created in a terminal state (which indicates that it's coming from the history)
or IN_PROGRESS (which is pretty much all other downloads).  Or we could just
take a sledgehammer to it, and make loading_ store the list of downloads created
off to the side, and then do a set-difference against the ones we create here. 
Or we could ignore it for now.  I'm not sure what the right thing is.  

I think what I'll say is: If you want to ignore this for now, put a note next to
"OnDownloadCreated" saying that you're not allowed to call the programmatic
download initiation or CreateDownloadItem from that notification to allow
embedders to reason about the relationship between creating a download item and
receiving the callback.  Maybe with a "TODO()" to fix embedders so they don't
make that assumption :-} :-J.

[benjhayden, 2012/09/10 19:02:56]

Done.

+  loading_ = true;
+  for (InfoVector::const_iterator it = infos->begin();
+       it != infos->end(); ++it) {
+    DownloadItem* download_item = manager_->CreateDownloadItem(
+        it->path,
+        it->url,
+        it->referrer_url,
+        it->start_time,
+        it->end_time,
+        it->received_bytes,
+        it->total_bytes,
+        it->state,
+        it->opened);
+    DownloadHistoryData* dhd = DownloadHistoryData::Get(download_item);
+    DCHECK(it->db_handle > kUninitializedHandle);
+    dhd->set_db_handle(it->db_handle);
+    ++history_size_;
+  }
+  manager_->CheckForHistoryFilesRemoval();
+  loading_ = false;
+}
+
+void DownloadHistory::OnDownloadCreated(
+    DownloadManager* manager, DownloadItem* item) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // Observe even temporary downloads in case they are marked not temporary.
+  item->AddObserver(this);
+  observing_items_.insert(item);
+  // All downloads should pass through OnDownloadCreated exactly once.
+  CHECK(!DownloadHistoryData::Get(item));
+  DownloadHistoryData* dhd = new DownloadHistoryData(item);
+  if (item->GetState() == DownloadItem::IN_PROGRESS) {
+    dhd->set_info(GetPersistentStoreInfo(item));
+  }
+  MaybeAddToHistory(item);
+}
+
+void DownloadHistory::MaybeAddToHistory(DownloadItem* item) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  int32 download_id = item->GetId();
+  DownloadHistoryData* dhd = DownloadHistoryData::Get(item);
+  bool removing = (removing_.find(dhd->db_handle()) != removing_.end());
+  // TODO(benjhayden): Remove IsTemporary().
+  if (loading_ ||
+      download_crx_util::IsExtensionDownload(*item) ||
+      dhd->is_persisted() ||
+      item->IsTemporary() ||
+      removing ||
+      dhd->is_adding())
+    return;
+  dhd->set_is_adding(true);
+  if (dhd->info() == NULL) {
+    // Keep the info here regardless of whether the item is in progress so
+    // that, when ItemAdded() calls OnDownloadUpdated(), it can choose more
+    // intelligently whether to Update the db and/or discard the info.
+    dhd->set_info(GetPersistentStoreInfo(item));
+  }
+  history_->CreateDownload(*dhd->info(), base::Bind(
+      &EnsureItemAddedOnUI, base::Bind(
+          &DownloadHistory::ItemAdded, weak_ptr_factory_.GetWeakPtr(),
+          download_id)));
+}
+
+void DownloadHistory::ItemAdded(int32 download_id, int64 db_handle) {
+  if (!manager_)
+    return;
+
+  if (removed_while_adding_.find(download_id) !=
+      removed_while_adding_.end()) {
+    removed_while_adding_.erase(download_id);
+    if (removing_.empty()) {
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+          base::Bind(&DownloadHistory::RemoveDownloadsBatch,
+                     weak_ptr_factory_.GetWeakPtr()));
+    }
+    removing_.insert(db_handle);
+    return;
+  }
+
+  DownloadItem* item = manager_->GetDownload(download_id);
+  if (!item) {
+    // This item will have called OnDownloadDestroyed().  If the item should
+    // have been removed from history, then t would have also called

[Randy Smith (Not in Mondays), 2012/08/27 18:25:11]

nit: t -> it

[benjhayden, 2012/09/10 19:02:56]

Done.

+    // OnDownloadRemoved(), which would have put |download_id| in
+    // removed_while_adding_, handled above.
+    return;
+  }
+
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Download.HistorySize2",
+                              history_size_,
+                              0/*min*/,
+                              (1 << 23)/*max*/,
+                              (1 << 7)/*num_buckets*/);
+  ++history_size_;
+
+  DownloadHistoryData* dhd = DownloadHistoryData::Get(item);
+  dhd->set_is_adding(false);
+  DCHECK(db_handle > kUninitializedHandle);
+  dhd->set_db_handle(db_handle);
+
+  // In case the item changed or became temporary while it was being added.
+  // Don't just update all of the item's observers because we're the only
+  // observer that can also see db_handle, which is the only thing that
+  // ItemAdded changed.
+  OnDownloadUpdated(item);
+}
+
+void DownloadHistory::OnDownloadUpdated(DownloadItem* item) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  DownloadHistoryData* dhd = DownloadHistoryData::Get(item);
+  if (!dhd->is_persisted()) {
+    MaybeAddToHistory(item);
+    return;
+  }
+  if (item->IsTemporary()) {
+    OnDownloadRemoved(item);
+    return;
+  }
+
+  // TODO(asanka): Persist GetTargetFilePath() as well.
+  DownloadPersistentStoreInfo current_info(GetPersistentStoreInfo(item));
+  DownloadPersistentStoreInfo* previous_info = dhd->info();
+  bool do_update = (
+      (previous_info == NULL) ||
+      (previous_info->path != current_info.path) ||
+      (previous_info->end_time != current_info.end_time) ||
+      (previous_info->received_bytes != current_info.received_bytes) ||
+      (previous_info->total_bytes != current_info.total_bytes) ||
+      (previous_info->state != current_info.state) ||
+      (previous_info->opened != current_info.opened));
+  UMA_HISTOGRAM_ENUMERATION("Download.HistoryPropagatedUpdate", do_update, 2);
+  if (do_update) {
+    history_->UpdateDownload(current_info);
+  }
+  if (item->GetState() == DownloadItem::IN_PROGRESS) {
+    dhd->set_info(current_info);
+  } else {
+    dhd->clear_info();
+  }
+}
+
+// Downloads may be opened after they are completed.
+void DownloadHistory::OnDownloadOpened(DownloadItem* item) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadHistoryData* dhd = DownloadHistoryData::Get(item);
+  if (!dhd->is_persisted()) {
+    MaybeAddToHistory(item);
+    return;
+  }
+  if (item->IsTemporary()) {
+    OnDownloadRemoved(item);
+    return;
+  }
+
+  DownloadPersistentStoreInfo current_info(GetPersistentStoreInfo(item));
+  history_->UpdateDownload(current_info);
+  if (item->GetState() == DownloadItem::IN_PROGRESS) {
+    dhd->set_info(current_info);
+  }
+}
+
+void DownloadHistory::OnDownloadRemoved(DownloadItem* item) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  DownloadHistoryData* dhd = DownloadHistoryData::Get(item);
+  if (!dhd->is_persisted()) {
+    if (dhd->is_adding()) {
+      removed_while_adding_.insert(item->GetId());
+    }
+    return;
+  }
+
+  // For database efficiency, batch removals together if they happen all at
+  // once.
+  if (removing_.empty()) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+        base::Bind(&DownloadHistory::RemoveDownloadsBatch,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+  removing_.insert(dhd->db_handle());
+  dhd->set_db_handle(kUninitializedHandle);
+  --history_size_;
+}
+
+void DownloadHistory::RemoveDownloadsBatch() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  HandleSet remove_handles;
+  removing_.swap(remove_handles);
+  history_->RemoveDownloads(remove_handles);
+}
+
+void DownloadHistory::ManagerGoingDown(DownloadManager* manager) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(manager_, manager);
+  manager_->RemoveObserver(this);
+  manager_ = NULL;
+}
+
+void DownloadHistory::OnDownloadDestroyed(DownloadItem* item) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  item->RemoveObserver(this);
+  observing_items_.erase(item);
 }
 
 void DownloadHistory::CheckVisitedReferrerBefore(
     int32 download_id,
     const GURL& referrer_url,
     const VisitedBeforeDoneCallback& callback) {
-  if (referrer_url.is_valid()) {
-    HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
-        profile_, Profile::EXPLICIT_ACCESS);
-    if (hs) {
-      HistoryService::Handle handle =
-          hs->GetVisibleVisitCountToHost(referrer_url, &history_consumer_,
-              base::Bind(&DownloadHistory::OnGotVisitCountToHost,
-                         base::Unretained(this)));
-      visited_before_requests_[handle] = callback;
-      return;
-    }
-  }
-  callback.Run(false);
-}
-
-void DownloadHistory::AddEntry(
-    DownloadItem* download_item,
-    const HistoryService::DownloadCreateCallback& callback) {
-  DCHECK(download_item);
-  // Do not store the download in the history database for a few special cases:
-  // - incognito mode (that is the point of this mode)
-  // - extensions (users don't think of extension installation as 'downloading')
-  // - temporary download, like in drag-and-drop
-  // - history service is not available (e.g. in tests)
-  // We have to make sure that these handles don't collide with normal db
-  // handles, so we use a negative value. Eventually, they could overlap, but
-  // you'd have to do enough downloading that your ISP would likely stab you in
-  // the neck first. YMMV.
-  HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
-      profile_, Profile::EXPLICIT_ACCESS);
-  if (download_crx_util::IsExtensionDownload(*download_item) ||
-      download_item->IsTemporary() || !hs) {
-    callback.Run(download_item->GetId(), GetNextFakeDbHandle());
-    return;
-  }
-
-  int32 id = download_item->GetId();
-  DownloadPersistentStoreInfo history_info =
-      download_item->GetPersistentStoreInfo();
-  hs->CreateDownload(id, history_info, &history_consumer_, callback);
-}
-
-void DownloadHistory::UpdateEntry(DownloadItem* download_item) {
-  // Don't store info in the database if the download was initiated while in
-  // incognito mode or if it hasn't been initialized in our database table.
-  if (download_item->GetDbHandle() <= DownloadItem::kUninitializedHandle)
-    return;
-
-  HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
-      profile_, Profile::EXPLICIT_ACCESS);
-  if (!hs)
-    return;
-  hs->UpdateDownload(download_item->GetPersistentStoreInfo());
-}
-
-void DownloadHistory::UpdateDownloadPath(DownloadItem* download_item,
-                                         const FilePath& new_path) {
-  // No update necessary if the download was initiated while in incognito mode.
-  if (download_item->GetDbHandle() <= DownloadItem::kUninitializedHandle)
-    return;
-
-  HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
-      profile_, Profile::EXPLICIT_ACCESS);
-  if (hs)
-    hs->UpdateDownloadPath(new_path, download_item->GetDbHandle());
-}
-
-void DownloadHistory::RemoveEntry(DownloadItem* download_item) {
-  // No update necessary if the download was initiated while in incognito mode.
-  if (download_item->GetDbHandle() <= DownloadItem::kUninitializedHandle)
-    return;
-
-  HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
-      profile_, Profile::EXPLICIT_ACCESS);
-  if (hs)
-    hs->RemoveDownload(download_item->GetDbHandle());
-}
-
-void DownloadHistory::RemoveEntriesBetween(const base::Time remove_begin,
-                                           const base::Time remove_end) {
-  HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
-      profile_, Profile::EXPLICIT_ACCESS);
-  if (hs)
-    hs->RemoveDownloadsBetween(remove_begin, remove_end);
-}
-
-int64 DownloadHistory::GetNextFakeDbHandle() {
-  return next_fake_db_handle_--;
-}
-
-void DownloadHistory::OnGotVisitCountToHost(HistoryService::Handle handle,
-                                            bool found_visits,
-                                            int count,
-                                            base::Time first_visit) {
-  VisitedBeforeRequestsMap::iterator request =
-      visited_before_requests_.find(handle);
-  DCHECK(request != visited_before_requests_.end());
-  VisitedBeforeDoneCallback callback = request->second;
-  visited_before_requests_.erase(request);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!referrer_url.is_valid()) {
+    callback.Run(false);
+    return;
+  }
+  history_->GetVisibleVisitCountToHost(
+      referrer_url,
+      base::Bind(&EnsureVisitedOnUI, base::Bind(
+          &DownloadHistory::OnGotVisitCountToHost,
+          weak_ptr_factory_.GetWeakPtr(), callback)));
+}
+
+void DownloadHistory::OnGotVisitCountToHost(
+    const VisitedBeforeDoneCallback& callback,
+    bool found_visits,
+    int count,
+    base::Time first_visit) {
   callback.Run(found_visits && count &&
       (first_visit.LocalMidnight() < base::Time::Now().LocalMidnight()));
 }