Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1752)

Unified Diff: chrome/browser/download/download_history.cc

Issue 10915180: Make DownloadHistory observe manager, items (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: . Created 8 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/download/download_history.cc
diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc
index 9d701950dcb5375eba799c55229a3e71a0588c42..0dfd1ea597ed79e13c366eba0d2f91af16d9b68d 100644
--- a/chrome/browser/download/download_history.cc
+++ b/chrome/browser/download/download_history.cc
@@ -2,153 +2,434 @@
// 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_db_handle_| to disable adding these items to
+// the database as it matches them up with their db_handles. 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/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_database.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;
+using content::DownloadManager;
+
+namespace {
+
+// 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);
+ }
+
+ DownloadHistoryData(DownloadItem* item, int64 handle)
+ : is_adding_(false),
+ db_handle_(handle),
+ 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_ != history::DownloadDatabase::kUninitializedHandle;
+ }
+
+ int64 db_handle() const { return db_handle_; }
+ void set_db_handle(int64 h) { db_handle_ = h; }
+
+ // This allows DownloadHistory::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";
-DownloadHistory::DownloadHistory(Profile* profile)
- : profile_(profile),
- next_fake_db_handle_(DownloadItem::kUninitializedHandle - 1) {
- DCHECK(profile);
+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()
+ : history::DownloadDatabase::kUninitializedHandle),
+ item->GetOpened());
}
-DownloadHistory::~DownloadHistory() {}
+typedef std::vector<DownloadPersistentStoreInfo> InfoVector;
-void DownloadHistory::GetNextId(
- const HistoryService::DownloadNextIdCallback& callback) {
- HistoryService* hs = HistoryServiceFactory::GetForProfile(
- profile_, Profile::EXPLICIT_ACCESS);
- if (!hs)
+// The HistoryService would normally use CancelableRequestConsumer to ensure
+// that callbacks from the HistoryService are run on the same thread that issued
+// the request, but DownloadHistory has opted for less magic by relying on
+// WeakPtrs instead, so it uses Ensure*OnUI to explicitly bounce to the UI
+// thread.
+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());
+}
- hs->GetNextDownloadId(&history_consumer_, callback);
+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 DownloadHistory::Load(
- const HistoryService::DownloadQueryCallback& callback) {
- HistoryService* hs = HistoryServiceFactory::GetForProfile(
- profile_, Profile::EXPLICIT_ACCESS);
- if (!hs)
+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);
+}
- hs->QueryDownloads(&history_consumer_, callback);
+} // anonymous namespace
- // This is the initial load, so do a cleanup of corrupt in-progress entries.
- hs->CleanUpInProgressEntries();
+DownloadHistory::DownloadHistory(
+ DownloadManager* manager,
+ HistoryService* history)
+ : manager_(manager),
+ history_(history),
+ loading_db_handle_(history::DownloadDatabase::kUninitializedHandle),
+ history_size_(0),
+ ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(manager_);
+ manager_->AddObserver(this);
+ DownloadManager::DownloadVector items;
+ manager_->GetAllDownloads(&items);
+ for (DownloadManager::DownloadVector::const_iterator it = items.begin();
+ it != items.end(); ++it) {
+ OnDownloadCreated(manager_, *it);
+ }
+ history_->QueryDownloads(base::Bind(&EnsureQueryOnUI, base::Bind(
+ &DownloadHistory::QueryCallback, weak_ptr_factory_.GetWeakPtr())));
}
-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;
- }
+DownloadHistory::~DownloadHistory() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ for (ItemSet::const_iterator iter = observing_items_.begin();
+ iter != observing_items_.end(); ++iter) {
+ (*iter)->RemoveObserver(this);
}
- 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());
+ 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;
+ for (InfoVector::const_iterator it = infos->begin();
+ it != infos->end(); ++it) {
+ // OnDownloadCreated() is called inside DM::CreateDownloadItem(), so set
+ // loading_db_handle_ to match up the created item with its db_handle. All
+ // methods run on the UI thread and CreateDownloadItem() is synchronous.
+ loading_db_handle_ = it->db_handle;
+ 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);
+
+ // If this DCHECK fails, then you probably added an Observer that
+ // synchronously creates a DownloadItem in response to
+ // DownloadManager::OnDownloadCreated(), and your observer runs before
+ // DownloadHistory, and DownloadManager creates items synchronously. Just
+ // bounce your DownloadItem creation off the message loop to flush
+ // DownloadHistory::OnDownloadCreated.
+ DCHECK_EQ(it->db_handle, dhd->db_handle());
+ ++history_size_;
+ }
+ manager_->CheckForHistoryFilesRemoval();
+}
+
+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, loading_db_handle_);
+ loading_db_handle_ = history::DownloadDatabase::kUninitializedHandle;
+ if (item->GetState() == DownloadItem::IN_PROGRESS) {
+ dhd->set_info(GetPersistentStoreInfo(item));
}
+ MaybeAddToHistory(item);
+}
- int32 id = download_item->GetId();
- DownloadPersistentStoreInfo history_info =
- download_item->GetPersistentStoreInfo();
- hs->CreateDownload(id, history_info, &history_consumer_, callback);
+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 (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::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)
+void DownloadHistory::ItemAdded(int32 download_id, int64 db_handle) {
+ 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;
+ }
+
+ if (!manager_)
return;
- HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
- profile_, Profile::EXPLICIT_ACCESS);
- if (!hs)
+ DownloadItem* item = manager_->GetDownload(download_id);
+ if (!item) {
+ // This item will have called OnDownloadDestroyed(). If the item should
+ // have been removed from history, then it would have also called
+ // OnDownloadRemoved(), which would have put |download_id| in
+ // removed_while_adding_, handled above.
return;
- hs->UpdateDownload(download_item->GetPersistentStoreInfo());
+ }
+
+ 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_NE(db_handle, history::DownloadDatabase::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();
+ }
}
-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)
+// 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;
+ }
- HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
- profile_, Profile::EXPLICIT_ACCESS);
- if (hs)
- hs->UpdateDownloadPath(new_path, download_item->GetDbHandle());
+ DownloadPersistentStoreInfo current_info(GetPersistentStoreInfo(item));
+ history_->UpdateDownload(current_info);
+ if (item->GetState() == DownloadItem::IN_PROGRESS) {
+ dhd->set_info(current_info);
+ }
}
-void DownloadHistory::RemoveEntry(DownloadItem* download_item) {
- // No update necessary if the download was initiated while in incognito mode.
- if (download_item->GetDbHandle() <= DownloadItem::kUninitializedHandle)
+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;
+ }
- HistoryService* hs = HistoryServiceFactory::GetForProfileIfExists(
- profile_, Profile::EXPLICIT_ACCESS);
- if (hs)
- hs->RemoveDownload(download_item->GetDbHandle());
+ // 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(history::DownloadDatabase::kUninitializedHandle);
+ --history_size_;
}
-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);
+void DownloadHistory::RemoveDownloadsBatch() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ HandleSet remove_handles;
+ removing_.swap(remove_handles);
+ history_->RemoveDownloads(remove_handles);
}
-int64 DownloadHistory::GetNextFakeDbHandle() {
- return next_fake_db_handle_--;
+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) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ if (!referrer_url.is_valid()) {
+ callback.Run(false);
+ return;
+ }
+ history_->GetVisibleVisitCountToHostSimple(
+ referrer_url,
+ base::Bind(&EnsureVisitedOnUI, base::Bind(
+ &DownloadHistory::OnGotVisitCountToHost,
+ weak_ptr_factory_.GetWeakPtr(), callback)));
}
-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);
+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()));
}

Powered by Google App Engine
This is Rietveld 408576698