Index: chrome/browser/download/download_history.h |
diff --git a/chrome/browser/download/download_history.h b/chrome/browser/download/download_history.h |
index b1f827b4c0f64bf7eb3bcf224b45800ae46bbc86..f038d28cd00eadbb9bb0955039f4f0ebc3a30667 100644 |
--- a/chrome/browser/download/download_history.h |
+++ b/chrome/browser/download/download_history.h |
@@ -6,37 +6,56 @@ |
#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_HISTORY_H_ |
#include <map> |
+#include <set> |
+#include <vector> |
-#include "base/basictypes.h" |
-#include "base/callback.h" |
+#include "base/memory/weak_ptr.h" |
#include "chrome/browser/cancelable_request.h" |
#include "chrome/browser/history/history.h" |
+#include "content/public/browser/download_item.h" |
+#include "content/public/browser/download_manager.h" |
-class Profile; |
+struct DownloadPersistentStoreInfo; |
-namespace base { |
-class Time; |
-} |
+// Wrap HistoryService for easy mocking and to hide CancelableRequestConsumer. |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
Document the lifetime guarantees that this class p
benjhayden
2012/08/17 18:20:45
Done.
|
+class HistoryServiceDownloadAdapter { |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
Why in .h file rather than a forward decl?
benjhayden
2012/08/17 18:20:45
The test needs to be able to override the methods.
|
+ public: |
+ explicit HistoryServiceDownloadAdapter(HistoryService* history_service); |
+ virtual ~HistoryServiceDownloadAdapter(); |
+ virtual void QueryDownloads(const HistoryService::DownloadQueryCallback&); |
+ virtual HistoryService::Handle GetVisibleVisitCountToHost( |
+ const GURL& referrer_url, |
+ const HistoryService::GetVisibleVisitCountToHostCallback& callback); |
+ virtual void CreateDownload( |
+ int32 id, |
+ const DownloadPersistentStoreInfo& info, |
+ const HistoryService::DownloadCreateCallback& callback); |
+ virtual void UpdateDownload(const DownloadPersistentStoreInfo&); |
+ virtual void RemoveDownloads(const std::set<int64>& handles); |
+ virtual void OnDownloadHistoryDestroyed(); |
+ |
+ private: |
+ CancelableRequestConsumer history_consumer_; |
+ HistoryService* history_service_; |
-namespace content { |
-class DownloadItem; |
-} |
+ DISALLOW_COPY_AND_ASSIGN(HistoryServiceDownloadAdapter); |
+}; |
-// Interacts with the HistoryService on behalf of the download subsystem. |
-class DownloadHistory { |
+// Observes a single DownloadManager and all its DownloadItems, keeping the |
+// DownloadDatabase up to date. |
+class DownloadHistory: public content::DownloadManager::Observer, |
+ public content::DownloadItem::Observer, |
+ public base::SupportsWeakPtr<DownloadHistory> { |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
We've talked about my concerns about SupportsWeakP
benjhayden
2012/08/17 18:20:45
Done.
|
public: |
typedef base::Callback<void(bool)> VisitedBeforeDoneCallback; |
+ typedef std::vector<DownloadPersistentStoreInfo> InfoVector; |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
InfoVector isn't used in the public portion of the
benjhayden
2012/08/17 18:20:45
Done.
|
+ typedef std::set<int64> HandleSet; |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
Ditto HandleSet.
benjhayden
2012/08/17 18:20:45
Done.
|
- explicit DownloadHistory(Profile* profile); |
- ~DownloadHistory(); |
- |
- // Retrieves the next_id counter from the sql meta_table. |
- // Should be much faster than Load so that we may delay downloads until after |
- // this call with minimal performance penalty. |
- void GetNextId(const HistoryService::DownloadNextIdCallback& callback); |
+ DownloadHistory( |
+ content::DownloadManager* manager, |
+ HistoryServiceDownloadAdapter* history); |
- // Retrieves DownloadCreateInfos saved in the history. |
- void Load(const HistoryService::DownloadQueryCallback& callback); |
+ virtual ~DownloadHistory(); |
// Checks whether |referrer_url| has been visited before today. This takes |
// ownership of |callback|. |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
Not this CL's problem, but: "Takes ownership of |c
benjhayden
2012/08/17 18:20:45
I think it meant that it copied the callback to an
Randy Smith (Not in Mondays)
2012/08/20 18:57:04
Thanks.
|
@@ -44,49 +63,56 @@ class DownloadHistory { |
const GURL& referrer_url, |
const VisitedBeforeDoneCallback& callback); |
- // Adds a new entry for a download to the history database. |
- void AddEntry(content::DownloadItem* download_item, |
- const HistoryService::DownloadCreateCallback& callback); |
- |
- // Updates the history entry for |download_item|. |
- void UpdateEntry(content::DownloadItem* download_item); |
- |
- // Updates the download path for |download_item| to |new_path|. |
- void UpdateDownloadPath(content::DownloadItem* download_item, |
- const FilePath& new_path); |
+ // content::DownloadManager::Observer |
+ virtual void OnDownloadCreated( |
+ content::DownloadManager* manager, |
+ content::DownloadItem* item) OVERRIDE; |
+ virtual void ManagerGoingDown(content::DownloadManager* manager) OVERRIDE; |
- // Removes |download_item| from the history database. |
- void RemoveEntry(content::DownloadItem* download_item); |
- |
- // Removes download-related history entries in the given time range. |
- void RemoveEntriesBetween(const base::Time remove_begin, |
- const base::Time remove_end); |
- |
- // Returns a new unique database handle which will not collide with real ones. |
- int64 GetNextFakeDbHandle(); |
+ // content::DownloadItem::Observer |
+ virtual void OnDownloadUpdated(content::DownloadItem* item) OVERRIDE; |
+ virtual void OnDownloadOpened(content::DownloadItem* item) OVERRIDE; |
+ virtual void OnDownloadRemoved(content::DownloadItem* item) OVERRIDE; |
+ virtual void OnDownloadDestroyed(content::DownloadItem* item) OVERRIDE; |
private: |
+ typedef std::set<int32> IdSet; |
+ typedef std::map<int32, DownloadPersistentStoreInfo> InfoMap; |
typedef std::map<HistoryService::Handle, VisitedBeforeDoneCallback> |
VisitedBeforeRequestsMap; |
+ void MaybeAddToHistory(content::DownloadItem* item); |
+ void ItemAdded(int32 id, int64 db_handle); |
+ void RemoveDownloadsBatch(); |
void OnGotVisitCountToHost(HistoryService::Handle handle, |
bool found_visits, |
int count, |
base::Time first_visit); |
+ void QueryCallback(InfoVector* infos); |
- Profile* profile_; |
+ content::DownloadManager* manager_; |
- // In case we don't have a valid db_handle, we use |fake_db_handle_| instead. |
- // This is useful for incognito mode or when the history database is offline. |
- // Downloads are expected to have unique handles, so we decrement the next |
- // fake handle value on every use. |
- int64 next_fake_db_handle_; |
- |
- CancelableRequestConsumer history_consumer_; |
+ HistoryServiceDownloadAdapter* history_; |
// The outstanding requests made by CheckVisitedReferrerBefore(). |
VisitedBeforeRequestsMap visited_before_requests_; |
+ // Whether we are in process of creating DownloadItems from the database. |
+ // OnDownloadCreated is called before we have a chance to attach a |
+ // DownloadHistoryData to the item, so this field is used to disable adding |
+ // new items to the database. |
+ bool loading_; |
+ |
+ // |db_handles| of items that are scheduled for removal from history. |
+ // Cannot be merged into DownloadHistoryData. |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
Document that the motivation for this is batching
benjhayden
2012/08/17 18:20:45
Done.
|
+ HandleSet removing_; |
+ |
+ // |GetId()|s of items that were removed while they were being added. |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
I wince because these comments aren't really under
benjhayden
2012/08/17 18:20:45
Done.
|
+ std::set<int32> removed_while_adding_; |
Randy Smith (Not in Mondays)
2012/08/15 20:47:44
Document that the motivation is that we can't remo
benjhayden
2012/08/17 18:20:45
Done.
|
+ |
+ // Count the number of items in the history for UMA. |
+ int64 history_size_; |
+ |
DISALLOW_COPY_AND_ASSIGN(DownloadHistory); |
}; |