[Backport] TopSites: Clear thumbnails from the cache when their URLs get removed

We already cleared the thumbnails from persistent storage, but they
remained in the in-memory cache, so they remained accessible (until the
next Chrome restart) even after all browsing data was cleared.

Bug: 758169
Reviewed-on: https://chromium-review.googlesource.com/758640
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514861}
(CVE-2018-6053)

Change-Id: I4d35f2935f836237ed5d1fd7cf10264046bf8775
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

5 files changed, 61 insertions, 0 deletions

diff --git a/chromium/components/history/core/browser/top_sites_cache.cc b/chromium/components/history/core/browser/top_sites_cache.cc
index c436e68cb0e..b4465d3320f 100644
--- a/chromium/components/history/core/browser/top_sites_cache.cc
+++ b/chromium/components/history/core/browser/top_sites_cache.cc
@@ -39,6 +39,16 @@ void TopSitesCache::SetThumbnails(const URLToImagesMap& images) {
   images_ = images;
 }
 
+void TopSitesCache::ClearUnreferencedThumbnails() {
+  URLToImagesMap images_to_keep;
+  for (const std::pair<GURL, Images>& entry : images_) {
+    if (IsKnownURL(entry.first)) {
+      images_to_keep.insert(entry);
+    }
+  }
+  images_ = std::move(images_to_keep);
+}
+
 Images* TopSitesCache::GetImage(const GURL& url) {
   return &images_[GetCanonicalURL(url)];
 }
diff --git a/chromium/components/history/core/browser/top_sites_cache.h b/chromium/components/history/core/browser/top_sites_cache.h
index 37c94826cbb..65f861e2124 100644
--- a/chromium/components/history/core/browser/top_sites_cache.h
+++ b/chromium/components/history/core/browser/top_sites_cache.h
@@ -51,6 +51,8 @@ class TopSitesCache {
   void SetThumbnails(const URLToImagesMap& images);
   const URLToImagesMap& images() const { return images_; }
 
+  void ClearUnreferencedThumbnails();
+
   // 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);
diff --git a/chromium/components/history/core/browser/top_sites_cache_unittest.cc b/chromium/components/history/core/browser/top_sites_cache_unittest.cc
index 5ca85700192..c317f3cab7d 100644
--- a/chromium/components/history/core/browser/top_sites_cache_unittest.cc
+++ b/chromium/components/history/core/browser/top_sites_cache_unittest.cc
@@ -7,9 +7,12 @@
 #include <stddef.h>
 
 #include <set>
+#include <string>
+#include <vector>
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted_memory.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -35,6 +38,11 @@ class TopSitesCacheTest : public testing::Test {
   // Initializes |top_sites_| and |cache_| based on |spec|.
   void InitTopSiteCache(const char** spec, size_t size);
 
+  bool HasPageThumbnail(const GURL& url) {
+    scoped_refptr<base::RefCountedMemory> memory;
+    return cache_.GetPageThumbnail(url, &memory);
+  }
+
   MostVisitedURLList top_sites_;
   TopSitesCache cache_;
 
@@ -255,6 +263,44 @@ TEST_F(TopSitesCacheTest, CacheForcedURLs) {
   EXPECT_EQ(2u, cache_.GetNumNonForcedURLs());
 }
 
+TEST_F(TopSitesCacheTest, ClearUnreferencedThumbnails) {
+  InitTopSiteCache(kTopSitesSpecBasic, arraysize(kTopSitesSpecBasic));
+
+  // A "primary" URL.
+  const GURL url1("http://www.google.com");
+  ASSERT_TRUE(cache_.IsKnownURL(url1));
+  // A URL that's part of a redirect chain.
+  const GURL url2("https://www.gogle.com");
+  ASSERT_TRUE(cache_.IsKnownURL(url2));
+
+  // Add thumbnails for these two URLs.
+  Images thumbnail1;
+  thumbnail1.thumbnail =
+      new base::RefCountedBytes(std::vector<unsigned char>());
+  Images thumbnail2;
+  thumbnail2.thumbnail =
+      new base::RefCountedBytes(std::vector<unsigned char>());
+  URLToImagesMap images;
+  images[cache_.GetCanonicalURL(url1)] = thumbnail1;
+  images[cache_.GetCanonicalURL(url2)] = thumbnail2;
+  cache_.SetThumbnails(images);
+
+  ASSERT_TRUE(HasPageThumbnail(url1));
+  ASSERT_TRUE(HasPageThumbnail(url2));
+
+  // Since both URLs are known, ClearUnreferencedThumbnails should do nothing.
+  cache_.ClearUnreferencedThumbnails();
+  EXPECT_TRUE(HasPageThumbnail(url1));
+  EXPECT_TRUE(HasPageThumbnail(url2));
+
+  // After the top sites themselves are cleared, ClearUnreferencedThumbnails
+  // should clear the corresponding thumbnails.
+  cache_.SetTopSites(MostVisitedURLList());
+  cache_.ClearUnreferencedThumbnails();
+  EXPECT_FALSE(HasPageThumbnail(url1));
+  EXPECT_FALSE(HasPageThumbnail(url2));
+}
+
 }  // namespace
 
 }  // namespace history
diff --git a/chromium/components/history/core/browser/top_sites_impl.cc b/chromium/components/history/core/browser/top_sites_impl.cc
index aa758327087..dc429ed7a2a 100644
--- a/chromium/components/history/core/browser/top_sites_impl.cc
+++ b/chromium/components/history/core/browser/top_sites_impl.cc
@@ -756,6 +756,7 @@ void TopSitesImpl::SetTopSites(const MostVisitedURLList& new_top_sites,
   // thread safe cache ...) as this method is invoked during startup at which
   // point the caches haven't been updated yet.
   cache_->SetTopSites(top_sites);
+  cache_->ClearUnreferencedThumbnails();
 
   // See if we have any temp thumbnails for the new sites, and promote them to
   // proper thumbnails.
diff --git a/chromium/components/history/core/browser/top_sites_impl_unittest.cc b/chromium/components/history/core/browser/top_sites_impl_unittest.cc
index 00d1e10793b..70a1d0457ad 100644
--- a/chromium/components/history/core/browser/top_sites_impl_unittest.cc
+++ b/chromium/components/history/core/browser/top_sites_impl_unittest.cc
@@ -560,6 +560,8 @@ TEST_F(TopSitesImplTest, ThumbnailRemoved) {
 
   // Reset the thumbnails and make sure we don't get it back.
   SetTopSites(MostVisitedURLList());
+  EXPECT_FALSE(top_sites()->GetPageThumbnail(url, false, &result));
+  // Recreating the TopSites object should also not bring it back.
   RefreshTopSitesAndRecreate();
   EXPECT_FALSE(top_sites()->GetPageThumbnail(url, false, &result));
 }