Modify ThumbnailStore to make one call to the HistoryBackend using QueryTopURLsAndRedirects instead of a seperate call for each URL. Also clean up some of the code and fix bug 14644.

BUG=14644
TEST=none

Review URL: http://codereview.chromium.org/149126

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19879 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 35 insertions, 66 deletions

diff --git a/chrome/browser/thumbnail_store.h b/chrome/browser/thumbnail_store.h
index 6369224..cf29e43 100644
--- a/chrome/browser/thumbnail_store.h
+++ b/chrome/browser/thumbnail_store.h
@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_THUMBNAIL_STORE_H_
 
 #include <map>
-#include <set>
 #include <string>
 #include <vector>
 
@@ -23,7 +22,6 @@
 class DictionaryValue;
 class GURL;
 class HistoryService;
-class PageUsageData;
 class Pickle;
 class Profile;
 class SkBitmap;
@@ -36,9 +34,6 @@ class Time;
 // by the new_tab_ui.
 class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
  public:
-  typedef Callback2<int, scoped_refptr<RefCountedBytes> >::Type
-      ThumbnailDataCallback;
-
   ThumbnailStore();
   ~ThumbnailStore();
 
@@ -56,49 +51,45 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
                         bool write_to_disk);
 
   // Sets *data to point to the thumbnail for the given url.
-  // A return value of ASYNC means you should call GetPageThumbnailAsync.
-  // On a return value of SUCCESS, the refcount of the out parameter data
-  // is incremented for the caller who takes ownership of that reference.
-  enum GetStatus { SUCCESS, FAIL, ASYNC };
-  ThumbnailStore::GetStatus GetPageThumbnail(const GURL& url,
-                                             RefCountedBytes** data);
-
-  // Retrieves the redirects list for the given url asynchronously.
-  // Calls the callback with the request_id and thumbnail data if found.
-  void GetPageThumbnailAsync(const GURL& url,
-                             int request_id,
-                             ThumbnailStore::ThumbnailDataCallback* cb);
-
-  // Cancels the given set of request_id's which were issued from
-  // GetPageThumbnailAsync.
-  // This method is called from ~DOMUIThumbnailSource.  If a
-  // DOMUIThumbnailSource requests a thumbnail but is destroyed before the
-  // data is sent back, this method will cancel the request and delete the
-  // callback.
-  void CancelPendingRequests(const std::set<int>& pending_requests);
+  // Returns false if no thumbnail available.
+  bool GetPageThumbnail(const GURL& url, RefCountedBytes** data);
 
  private:
   FRIEND_TEST(ThumbnailStoreTest, RetrieveFromCache);
   FRIEND_TEST(ThumbnailStoreTest, RetrieveFromDisk);
   FRIEND_TEST(ThumbnailStoreTest, UpdateThumbnail);
   FRIEND_TEST(ThumbnailStoreTest, FollowRedirects);
+  friend class ThumbnailStoreTest;
 
   // Data structure used to store thumbnail data in memory.
   typedef std::map<GURL, std::pair<scoped_refptr<RefCountedBytes>,
       ThumbnailScore> > Cache;
 
-  // Data structure used to cache the redirect lists for urls.
-  typedef std::map<GURL, scoped_refptr<RefCountedVector<GURL> > > RedirectMap;
+  // Most visited URLs and their redirect lists -------------------------------
+
+  // Query the HistoryService for the most visited URLs and the most recent
+  // redirect lists for those URLs.  This happens in the background and the
+  // callback is OnURLDataAvailable.
+  void UpdateURLData();
+
+  // The callback for UpdateURLData. The ThumbnailStore takes ownership of
+  // the most visited urls list and redirect lists passed in.
+  void OnURLDataAvailable(std::vector<GURL>* urls,
+                          history::RedirectMap* redirects);
 
-  // Data structure used to store request_id's and callbacks for
-  // GetPageThumbnailAsync.
-  typedef std::map<int, std::pair<ThumbnailStore::ThumbnailDataCallback*,
-                                  HistoryService::Handle> > RequestMap;
+  // Remove stale data --------------------------------------------------------
+
+  // Remove entries from the in memory thumbnail cache and redirect lists
+  // cache that have been blacklisted or are not in the top kMaxCacheSize
+  // visited sites.
+  void CleanCacheData();
 
   // Deletes thumbnail data from disk for the given list of urls.
   void DeleteThumbnails(
       scoped_refptr<RefCountedVector<GURL> > thumbnail_urls) const;
 
+  // Disk operations ----------------------------------------------------------
+
   // Read all thumbnail data from the specified FilePath into a Cache object.
   // Done on the file_thread and returns to OnDiskDataAvailable on the thread
   // owning the specified MessageLoop.
@@ -114,13 +105,14 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
   // Once thumbnail data from the disk is available from the file_thread,
   // this function is invoked on the main thread.  It takes ownership of the
   // Cache* passed in and retains this Cache* for the lifetime of the object.
-  void OnDiskDataAvailable(ThumbnailStore::Cache* cache);
+  void OnDiskDataAvailable(Cache* cache);
 
   // Write thumbnail data to disk for a given url.
   bool WriteThumbnailToDisk(const GURL& url,
                             scoped_refptr<RefCountedBytes> data,
                             const ThumbnailScore& score) const;
 
+
   // Pack the given ThumbnailScore into the given Pickle.
   void PackScore(const ThumbnailScore& score, Pickle* packed) const;
 
@@ -130,6 +122,8 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
                    const Pickle& packed,
                    void*& iter) const;
 
+  // Decide whether to store data ---------------------------------------------
+
   bool ShouldStoreThumbnailForURL(const GURL& url) const;
 
   bool IsURLBlacklisted(const GURL& url) const;
@@ -139,39 +133,24 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
   // Returns true if url is in most_visited_urls_.
   bool IsPopular(const GURL& url) const;
 
-  // The callback for GetPageThumbnailAsync.  Caches the redirect list
-  // and calls the callback for the request asssociated with the url.
-  void OnRedirectQueryComplete(HistoryService::Handle request_handle,
-                               GURL url,
-                               bool success,
-                               history::RedirectList* redirects);
 
-  // Called on a timer initiated in Init().  Calls the HistoryService to
-  // update the list of most visited URLs.  The callback is
-  // OnMostVisitedURLsAvailable.
-  void UpdateMostVisited();
 
-  // Updates the list of most visited URLs.  Then calls CleanCacheData.
-  void OnMostVisitedURLsAvailable(CancelableRequestProvider::Handle handle,
-                                  std::vector<PageUsageData*>* data);
-
-  // Remove entries from the in memory thumbnail cache and redirect lists
-  // cache that have been blacklisted or are not in the top kMaxCacheSize
-  // visited sites.
-  void CleanCacheData();
+  // Member variables ---------------------------------------------------------
 
   // The Cache maintained by the object.
-  scoped_ptr<ThumbnailStore::Cache> cache_;
+  scoped_ptr<Cache> cache_;
   bool cache_initialized_;
 
   // The location of the thumbnail store.
   FilePath file_path_;
 
+  // We hold a reference to the history service to query for most visited URLs
+  // and redirect information.
   scoped_refptr<HistoryService> hs_;
 
   // A list of the most_visited_urls_ refreshed every 30mins from the
   // HistoryService.
-  std::vector<GURL> most_visited_urls_;
+  scoped_ptr<std::vector<GURL> > most_visited_urls_;
 
   // A pointer to the persistent URL blacklist for this profile.
   const DictionaryValue* url_blacklist_;
@@ -179,24 +158,14 @@ class ThumbnailStore : public base::RefCountedThreadSafe<ThumbnailStore> {
   // A map pairing the URL that a user typed to a list of URLs it was
   // redirected to. Ex:
   // google.com => { http://www.google.com/ }
-  ThumbnailStore::RedirectMap redirect_urls_;
+  scoped_ptr<history::RedirectMap> redirect_urls_;
 
-  // When GetPageThumbnailAsync is called, this map records the request_id
-  // and callback associated with the request.  When the thumbnail becomes
-  // available, the callback is taken from this map and the thumbnail data
-  // is returned to it.
-  ThumbnailStore::RequestMap redirect_requests_;
-
-  // Timer on which UpdateMostVisited runs.
+  // Timer on which UpdateURLData runs.
   base::RepeatingTimer<ThumbnailStore> timer_;
 
-  // Consumer for getting redirect lists from the history service.
-  CancelableRequestConsumerT<int, -1> hs_consumer_;
-
-  // Consumer for getting the list of most visited urls.
-  CancelableRequestConsumerTSimple<PageUsageData*> cancelable_consumer_;
+  // Consumer for queries to the HistoryService.
+  CancelableRequestConsumer consumer_;
 
-  static const int kMostVisitedScope = 90;
   static const unsigned int kMaxCacheSize = 45;
 
   DISALLOW_COPY_AND_ASSIGN(ThumbnailStore);