Most Visited: remove over-eager optimization that causes a lot of blank thumbnails

after storing thumbnails in the thread safe cache, we were removing those which are not currently in the top 8. This sucks because then when you blacklist a site, the thumbnail for the replacement is not in the cache, so we use a default image even though the thumbnail is available in the DB. The optimization hardly saves us anything, as the number of cached thumbnails is capped to begin with, and we are only trimming to 8, and it's ref counted data anyway so it's already resident somewhere else in memory either way.

BUG=none
TEST=blacklist a site. Most likely you will get an actual thumbnail for its replacement


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

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

4 files changed, 1 insertions, 30 deletions

diff --git a/chrome/browser/history/top_sites.cc b/chrome/browser/history/top_sites.cc
index f85d1d8..2bea2d3 100644
--- a/chrome/browser/history/top_sites.cc
+++ b/chrome/browser/history/top_sites.cc
@@ -948,7 +948,6 @@ void TopSites::ResetThreadSafeCache() {
 void TopSites::ResetThreadSafeImageCache() {
   base::AutoLock lock(lock_);
   thread_safe_cache_->SetThumbnails(cache_->images());
-  thread_safe_cache_->RemoveUnreferencedThumbnails();
 }
 
 void TopSites::NotifyTopSitesChanged() {
diff --git a/chrome/browser/history/top_sites_cache.cc b/chrome/browser/history/top_sites_cache.cc
index 5312c0a..10ae296 100644
--- a/chrome/browser/history/top_sites_cache.cc
+++ b/chrome/browser/history/top_sites_cache.cc
@@ -24,14 +24,6 @@ void TopSitesCache::SetThumbnails(const URLToImagesMap& images) {
   images_ = images;
 }
 
-void TopSitesCache::SetPageThumbnail(const GURL& url,
-                                     RefCountedBytes* thumbnail,
-                                     const ThumbnailScore& score) {
-  Images& img = images_[GetCanonicalURL(url)];
-  img.thumbnail = thumbnail;
-  img.thumbnail_score = score;
-}
-
 Images* TopSitesCache::GetImage(const GURL& url) {
   return &images_[GetCanonicalURL(url)];
 }
@@ -75,19 +67,6 @@ size_t TopSitesCache::GetURLIndex(const GURL& url) {
   return GetCanonicalURLsIterator(url)->second;
 }
 
-void TopSitesCache::RemoveUnreferencedThumbnails() {
-  for (URLToImagesMap::iterator i = images_.begin(); i != images_.end(); ) {
-    if (IsKnownURL(i->first)) {
-      ++i;
-    } else {
-      URLToImagesMap::iterator next_i = i;
-      ++next_i;
-      images_.erase(i);
-      i = next_i;
-    }
-  }
-}
-
 void TopSitesCache::GenerateCanonicalURLs() {
   canonical_urls_.clear();
   for (size_t i = 0; i < top_sites_.size(); i++)
diff --git a/chrome/browser/history/top_sites_cache.h b/chrome/browser/history/top_sites_cache.h
index a7cf8a7..08b3d35 100644
--- a/chrome/browser/history/top_sites_cache.h
+++ b/chrome/browser/history/top_sites_cache.h
@@ -31,11 +31,6 @@ class TopSitesCache {
   void SetThumbnails(const URLToImagesMap& images);
   const URLToImagesMap& images() const { return images_; }
 
-  // Set a thumbnail.
-  void SetPageThumbnail(const GURL& url,
-                        RefCountedBytes* thumbnail,
-                        const ThumbnailScore& score);
-
   // Returns the thumbnail as an Image for the specified url. This adds an entry
   // for |url| if one has not yet been added.
   Images* GetImage(const GURL& url);
@@ -59,9 +54,6 @@ class TopSitesCache {
   // Returns the index into |top_sites_| for |url|.
   size_t GetURLIndex(const GURL& url);
 
-  // Removes any thumbnails that are no longer referenced by the top sites.
-  void RemoveUnreferencedThumbnails();
-
  private:
   // The entries in CanonicalURLs, see CanonicalURLs for details. The second
   // argument gives the index of the URL into MostVisitedURLs redirects.
diff --git a/chrome/browser/history/top_sites_unittest.cc b/chrome/browser/history/top_sites_unittest.cc
index e82d406..58f65f4 100644
--- a/chrome/browser/history/top_sites_unittest.cc
+++ b/chrome/browser/history/top_sites_unittest.cc
@@ -547,6 +547,7 @@ TEST_F(TopSitesTest, ThumbnailRemoved) {
 
   // Reset the thumbnails and make sure we don't get it back.
   SetTopSites(MostVisitedURLList());
+  RefreshTopSitesAndRecreate();
   EXPECT_FALSE(top_sites()->GetPageThumbnail(url, &result));
 }