Implementing URL prefix match for history thumbnail cache.

We wish to make MostLikely URL recommendations use local thumbnail. To increase the likelihood of a hit, this CL enables a thumbnail request to perform URL prefix match. For example, if thumbnail is stored for
==> http://www.chromium.org/Home
but not
==>  http://www.chromium.org/
then previously the request
==>  chrome://thumb/http://www.chromium.org/
would fail. This CL aims to create a fallback, by adding "thumb2" that matches prefix, i.e.:
==>  chrome://thumb2/http://www.chromium.org/
would match "http://www.chromium.org/Home" and display the corresponding thumbnail. Details:

- We consider "URL prefix" match, which is not same as "string prefix" match. Specifics:
--- Need identical protocol/scheme, host, and port (GURL is smart about this).
--- Query strings are ignored.
--- For path, we require entire path components to match. E.g., "base/test" is a prefix of "base/test/sub", but is NOT a prefix of "base/testing" or "best/test-case".

- If multiple matches exist, the first URL-lexicographical match is taken. This allows the "nearest" children to be matched, but favors siblings that are alphabetically closer. For example, "base" prefers "base/test" over "base/test/sub", but also prefers "base/deep/sub/dir/" over "base/test" (since "deep" < "test").

- To implement the above match, a new comparator is used in the thumbnail cache.

- Adding new test TopSitesCacheTest, which also tests existing code.

- Adding url_utils.cc in chrome\browser\history, along with unit tests.

- Adding the "chrome://thumb2" path, which causes most of the 1-2-line changes in files.

BUG=284634
TEST=TopSitesCacheTest.*

Review URL: https://chromiumcodereview.appspot.com/23477033

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

chrome/browser/android/dev_tools_server.cc

@@ -75,7 +75,7 @@ class DevToolsServerDelegate : public content::DevToolsHttpHandlerDelegate {
     history::TopSites* top_sites = profile->GetTopSites();
     if (top_sites) {
       scoped_refptr<base::RefCountedMemory> data;
-      if (top_sites->GetPageThumbnail(url, &data))
+      if (top_sites->GetPageThumbnail(url, false, &data))
         return std::string(reinterpret_cast<const char*>(data->front()),
                            data->size());
     }

chrome/browser/android/most_visited_sites.cc

@@ -112,7 +112,7 @@ void GetUrlThumbnailTask(
   GURL gurl(url_string);
 
   scoped_refptr<base::RefCountedMemory> data;
-  if (top_sites->GetPageThumbnail(gurl, &data)) {
+  if (top_sites->GetPageThumbnail(gurl, false, &data)) {
     SkBitmap thumbnail_bitmap = ExtractThumbnail(*data.get());
     if (!thumbnail_bitmap.empty()) {
       j_bitmap_ref->Reset(

chrome/browser/android/provider/chrome_browser_provider.cc

@@ -1563,7 +1563,7 @@ ScopedJavaLocalRef<jbyteArray> ChromeBrowserProvider::GetThumbnail(
   // GetPageThumbnail is synchronous and can be called from any thread.
   scoped_refptr<base::RefCountedMemory> thumbnail;
   if (top_sites_)
-    top_sites_->GetPageThumbnail(url, &thumbnail);
+    top_sites_->GetPageThumbnail(url, false, &thumbnail);
 
   if (!thumbnail.get() || !thumbnail->front()) {
     return ScopedJavaLocalRef<jbyteArray>();

chrome/browser/devtools/browser_list_tabcontents_provider.cc

@@ -78,7 +78,7 @@ std::string BrowserListTabContentsProvider::GetPageThumbnailData(
     if (!top_sites)
       continue;
     scoped_refptr<base::RefCountedMemory> data;
-    if (top_sites->GetPageThumbnail(url, &data))
+    if (top_sites->GetPageThumbnail(url, false, &data))
       return std::string(
           reinterpret_cast<const char*>(data->front()), data->size());
   }

chrome/browser/extensions/extension_service.cc

@@ -1166,7 +1166,7 @@ void ExtensionService::NotifyExtensionLoaded(const Extension* extension) {
   // Same for chrome://thumb/ resources.
   if (extensions::PermissionsData::HasHostPermission(
           extension, GURL(chrome::kChromeUIThumbnailURL))) {
-    ThumbnailSource* thumbnail_source = new ThumbnailSource(profile_);
+    ThumbnailSource* thumbnail_source = new ThumbnailSource(profile_, false);
     content::URLDataSource::Add(profile_, thumbnail_source);
   }
 }

chrome/browser/history/expire_history_backend_unittest.cc

@@ -311,7 +311,7 @@ bool ExpireHistoryTest::HasThumbnail(URLID url_id) {
     return false;
   GURL url = info.url();
   scoped_refptr<base::RefCountedMemory> data;
-  return top_sites_->GetPageThumbnail(url, &data);
+  return top_sites_->GetPageThumbnail(url, false, &data);
 }
 
 void ExpireHistoryTest::EnsureURLInfoGone(const URLRow& row) {

chrome/browser/history/top_sites.h

@@ -73,10 +73,15 @@ class TopSites
 
   // Get a thumbnail for a given page. Returns true iff we have the thumbnail.
   // This may be invoked on any thread.
+  // If an exact thumbnail URL match fails, |prefix_match| specifies whether or
+  // not to try harder by matching the query thumbnail URL as URL prefix (as
+  // defined by UrlIsPrefix()).
   // As this method may be invoked on any thread the ref count needs to be
   // incremented before this method returns, so this takes a scoped_refptr*.
   virtual bool GetPageThumbnail(
-      const GURL& url, scoped_refptr<base::RefCountedMemory>* bytes) = 0;
+      const GURL& url,
+      bool prefix_match,
+      scoped_refptr<base::RefCountedMemory>* bytes) = 0;
 
   // Get a thumbnail score for a given page. Returns true iff we have the
   // thumbnail score.  This may be invoked on any thread. The score will

chrome/browser/history/top_sites_cache.cc

@@ -55,7 +55,12 @@ bool TopSitesCache::GetPageThumbnailScore(const GURL& url,
 }
 
 const GURL& TopSitesCache::GetCanonicalURL(const GURL& url) {
-  CanonicalURLs::iterator i = TopSitesCache::GetCanonicalURLsIterator(url);
+  CanonicalURLs::iterator i = GetCanonicalURLsIterator(url);
+  return i == canonical_urls_.end() ? url : i->first.first->url;
+}
+
+const GURL& TopSitesCache::GetCanonicalURLForPrefix(const GURL& url) {
+  CanonicalURLs::iterator i = GetCanonicalURLsIteratorForPrefix(url);
   return i == canonical_urls_.end() ? url : i->first.first->url;
 }
 
@@ -76,7 +81,7 @@ void TopSitesCache::GenerateCanonicalURLs() {
 
 void TopSitesCache::StoreRedirectChain(const RedirectList& redirects,
                                        size_t destination) {
-  // redirects is empty if the user pinned a site and there are not enough top
+  // |redirects| is empty if the user pinned a site and there are not enough top
   // sites before the pinned site.
 
   // Map all the redirected URLs to the destination.
@@ -101,4 +106,24 @@ TopSitesCache::CanonicalURLs::iterator TopSitesCache::GetCanonicalURLsIterator(
   return canonical_urls_.find(entry);
 }
 
+TopSitesCache::CanonicalURLs::iterator
+    TopSitesCache::GetCanonicalURLsIteratorForPrefix(const GURL& prefix_url) {
+  MostVisitedURL most_visited_url;
+  most_visited_url.redirects.push_back(prefix_url);
+  CanonicalURLEntry entry;
+  entry.first = &most_visited_url;
+  entry.second = 0u;
+
+  // Perform effective binary search for URL prefix search.
+  TopSitesCache::CanonicalURLs::iterator it =
+      canonical_urls_.lower_bound(entry);
+  // Perform prefix match.
+  if (it != canonical_urls_.end()) {
+    const GURL& comp_url = it->first.first->redirects[it->first.second];
+    if (!UrlIsPrefix(prefix_url, comp_url))
+      it = canonical_urls_.end();
+  }
+  return it;
+}
+
 }  // namespace history

chrome/browser/history/top_sites_cache.h

@@ -10,9 +10,27 @@
 
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/history/history_types.h"
+#include "chrome/browser/history/url_utils.h"
 
 namespace history {
 
+// TopSiteCache caches thumbnails for visited pages. Retrieving thumbnails from
+// a given input URL is a two-stage process:
+//
+//   input URL --(map 1)--> canonical URL --(map 2)--> image.
+//
+// (map 1) searches input URL in |canonical_urls_|. canonical URL is
+// assigned to the resulting value if found; else input URL.
+//
+// (map 2) simply looks up canonical URL in |images_|.
+//
+// TopSiteCache also provides GetCanonicalURLsIteratorForPrefix(), which is an
+// alternative implementation of (map 1) that does the following:
+// - if canonical URL is a key in |canonical_urls_|, return the value.
+// - else if canonical URL is a "URL prefix" (see comment in url_utils.h) of
+//   some key in |canonical_urls_|, return the value corresponding to the key.
+// - else return input URL.
+
 // TopSitesCache caches the top sites and thumbnails for TopSites.
 class TopSitesCache {
  public:
@@ -44,6 +62,11 @@ class TopSitesCache {
   // Returns the canonical URL for |url|.
   const GURL& GetCanonicalURL(const GURL& url);
 
+  // Returns the canonical URL for |url_prefix| that matches by prefix.
+  // Multiple matches exst, returns the canonical URL for the first
+  // matching entry under lexicographical order.
+  const GURL& GetCanonicalURLForPrefix(const GURL& url_prefix);
+
   // Returns true if |url| is known.
   bool IsKnownURL(const GURL& url);
 
@@ -60,7 +83,8 @@ class TopSitesCache {
    public:
     bool operator()(const CanonicalURLEntry& e1,
                     const CanonicalURLEntry& e2) const {
-      return e1.first->redirects[e1.second] < e2.first->redirects[e2.second];
+      return CanonicalURLStringCompare(e1.first->redirects[e1.second].spec(),
+                                       e2.first->redirects[e2.second].spec());
     }
   };
 
@@ -80,9 +104,14 @@ class TopSitesCache {
   // Stores a set of redirects. This is used by GenerateCanonicalURLs.
   void StoreRedirectChain(const RedirectList& redirects, size_t destination);
 
-  // Returns the iterator into canconical_urls_ for the specified url.
+  // Returns the iterator into |canonical_urls_| for the |url|.
   CanonicalURLs::iterator GetCanonicalURLsIterator(const GURL& url);
 
+  // Returns the first iterator into |canonical_urls_| for which |prefix_url|
+  // is a URL prefix. Returns |canonical_urls_.end()| if no match is found.
+  CanonicalURLs::iterator GetCanonicalURLsIteratorForPrefix(
+      const GURL& prefix_url);
+
   // The top sites.
   MostVisitedURLList top_sites_;
 

chrome/browser/history/top_sites_cache_unittest.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/history/top_sites_cache.h"
+
+#include <set>
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "base/strings/string16.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/utf_string_conversions.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace history {
+
+namespace {
+
+class TopSitesCacheTest : public testing::Test {
+ public:
+  TopSitesCacheTest() {
+  }
+
+ protected:
+  // Initializes |top_sites_| and |cache_| based on |spec|, which is a list of
+  // URL strings with optional indents: indentated URLs redirect to the last
+  // non-indented URL. Titles are assigned as "Title 1", "Title 2", etc., in the
+  // order of appearance. See |kTopSitesSpecBasic| for an example.
+  void InitTopSiteCache(const char** spec, int size);
+
+  MostVisitedURLList top_sites_;
+  TopSitesCache cache_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TopSitesCacheTest);
+};
+
+void TopSitesCacheTest::InitTopSiteCache(const char** spec, int size) {
+  std::set<std::string> urls_seen;
+  for (int i = 0; i < size; ++i) {
+    const char* spec_item = spec[i];
+    while (*spec_item && *spec_item == ' ')  // Eat indent.
+      ++spec_item;
+    if (urls_seen.find(spec_item) != urls_seen.end())
+      NOTREACHED() << "Duplicate URL found: " << spec_item;
+    urls_seen.insert(spec_item);
+    if (spec_item == spec[i]) {  // No indent: add new MostVisitedURL.
+      string16 title(ASCIIToUTF16("Title ") +
+                     base::Uint64ToString16(top_sites_.size() + 1));
+      top_sites_.push_back(MostVisitedURL(GURL(spec_item), title));
+    }
+    ASSERT_TRUE(!top_sites_.empty());
+    // Set up redirect to canonical URL. Canonical URL redirects to itself, too.
+    top_sites_.back().redirects.push_back(GURL(spec_item));
+  }
+  cache_.SetTopSites(top_sites_);
+}
+
+const char* kTopSitesSpecBasic[] = {
+  "http://www.google.com",
+  "  http://www.gogle.com",  // Redirects.
+  "  http://www.gooogle.com",  // Redirects.
+  "http://www.youtube.com/a/b",
+  "  http://www.youtube.com/a/b?test=1",  // Redirects.
+  "https://www.google.com/",
+  "  https://www.gogle.com",  // Redirects.
+  "http://www.example.com:3141/",
+};
+
+TEST_F(TopSitesCacheTest, GetCanonicalURL) {
+  InitTopSiteCache(kTopSitesSpecBasic, arraysize(kTopSitesSpecBasic));
+  struct {
+    const char* expected;
+    const char* query;
+  } test_cases[] = {
+    // Already is canonical: redirects.
+    {"http://www.google.com/", "http://www.google.com"},
+    // Exact match with stored URL: redirects.
+    {"http://www.google.com/", "http://www.gooogle.com"},
+    // Recognizes despite trailing "/": redirects
+    {"http://www.google.com/", "http://www.gooogle.com/"},
+    // Exact match with URL with query: redirects.
+    {"http://www.youtube.com/a/b", "http://www.youtube.com/a/b?test=1"},
+    // No match with URL with query: as-is.
+    {"http://www.youtube.com/a/b?test", "http://www.youtube.com/a/b?test"},
+    // Never-seen-before URL: as-is.
+    {"http://maps.google.com/", "http://maps.google.com/"},
+    // Changing port number, does not match: as-is.
+    {"http://www.example.com:1234/", "http://www.example.com:1234"},
+    // Smart enough to know that port 80 is HTTP: redirects.
+    {"http://www.google.com/", "http://www.gooogle.com:80"},
+    // Prefix should not work: as-is.
+    {"http://www.youtube.com/a", "http://www.youtube.com/a"},
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    std::string expected(test_cases[i].expected);
+    std::string query(test_cases[i].query);
+    EXPECT_EQ(expected, cache_.GetCanonicalURL(GURL(query)).spec())
+      << " for test_case[" << i << "]";
+  }
+}
+
+TEST_F(TopSitesCacheTest, IsKnownUrl) {
+  InitTopSiteCache(kTopSitesSpecBasic, arraysize(kTopSitesSpecBasic));
+  // Matches.
+  EXPECT_TRUE(cache_.IsKnownURL(GURL("http://www.google.com")));
+  EXPECT_TRUE(cache_.IsKnownURL(GURL("http://www.gooogle.com")));
+  EXPECT_TRUE(cache_.IsKnownURL(GURL("http://www.google.com/")));
+
+  // Non-matches.
+  EXPECT_FALSE(cache_.IsKnownURL(GURL("http://www.google.com?")));
+  EXPECT_FALSE(cache_.IsKnownURL(GURL("http://www.google.net")));
+  EXPECT_FALSE(cache_.IsKnownURL(GURL("http://www.google.com/stuff")));
+  EXPECT_FALSE(cache_.IsKnownURL(GURL("https://www.gooogle.com")));
+  EXPECT_FALSE(cache_.IsKnownURL(GURL("http://www.youtube.com/a")));
+}
+
+const char* kTopSitesSpecPrefix[] = {
+  "http://www.google.com/",
+  "  http://www.google.com/test?q=3",  // Redirects.
+  "  http://www.google.com/test/y?b",  // Redirects.
+  "http://www.google.com/2",
+  "  http://www.google.com/test/q",  // Redirects.
+  "  http://www.google.com/test/y?a",  // Redirects.
+  "http://www.google.com/3",
+  "  http://www.google.com/testing",  // Redirects.
+  "http://www.google.com/test-hyphen",
+  "http://www.google.com/sh",
+  "  http://www.google.com/sh/1/2",  // Redirects.
+  "http://www.google.com/sh/1",
+};
+
+TEST_F(TopSitesCacheTest, GetCanonicalURLForPrefix) {
+  InitTopSiteCache(kTopSitesSpecPrefix, arraysize(kTopSitesSpecPrefix));
+  struct {
+    const char* expected;
+    const char* query;
+  } test_cases[] = {
+    // Already is canonical: redirects.
+    {"http://www.google.com/", "http://www.google.com"},
+    // Exact match with stored URL: redirects.
+    {"http://www.google.com/", "http://www.google.com/test?q=3"},
+    // Prefix match: redirects.
+    {"http://www.google.com/", "http://www.google.com/test"},
+    // Competing prefix match: redirects to closest.
+    {"http://www.google.com/2", "http://www.google.com/test/q"},
+    // Multiple prefix matches: redirects to first.
+    {"http://www.google.com/2", "http://www.google.com/test/y"},
+    // No prefix match: as-is.
+    {"http://www.google.com/no-match", "http://www.google.com/no-match"},
+    // String prefix match but not URL-prefix match: as-is.
+    {"http://www.google.com/t", "http://www.google.com/t"},
+    // Different protocol: as-is.
+    {"https://www.google.com/test", "https://www.google.com/test"},
+    // Smart enough to know that port 80 is HTTP: redirects.
+    {"http://www.google.com/", "http://www.google.com:80/test"},
+    // Exact match, unaffected by "http://www.google.com/sh/1": redirects.
+    {"http://www.google.com/sh", "http://www.google.com/sh/1/2"},
+    // Suffix match only: as-is
+    {"http://www.google.com/sh/1/2/3", "http://www.google.com/sh/1/2/3"},
+    // Exact match, unaffected by "http://www.google.com/sh": redirects.
+    {"http://www.google.com/sh/1", "http://www.google.com/sh/1"},
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_cases); ++i) {
+    std::string expected(test_cases[i].expected);
+    std::string query(test_cases[i].query);
+    EXPECT_EQ(expected, cache_.GetCanonicalURLForPrefix(GURL(query)).spec())
+      << " for test_case[" << i << "]";
+  }
+}
+
+}  // namespace
+
+}  // namespace history

chrome/browser/history/top_sites_impl.cc

@@ -212,8 +212,11 @@ void TopSitesImpl::GetMostVisitedURLs(
 }
 
 bool TopSitesImpl::GetPageThumbnail(
-    const GURL& url, scoped_refptr<base::RefCountedMemory>* bytes) {
+    const GURL& url,
+    bool prefix_match,
+    scoped_refptr<base::RefCountedMemory>* bytes) {
   // WARNING: this may be invoked on any thread.
+  // Perform exact match.
   {
     base::AutoLock lock(lock_);
     if (thread_safe_cache_->GetPageThumbnail(url, bytes))
@@ -231,6 +234,18 @@ bool TopSitesImpl::GetPageThumbnail(
     }
   }
 
+  if (prefix_match) {
+    // Still not found, so strip "?query#ref", and perform prefix match.
+    GURL::Replacements replacements;
+    replacements.ClearQuery();
+    replacements.ClearRef();
+    GURL url_stripped(url.ReplaceComponents(replacements));
+    base::AutoLock lock(lock_);
+    GURL canonical_url(
+        thread_safe_cache_->GetCanonicalURLForPrefix(url_stripped));
+    if (thread_safe_cache_->GetPageThumbnail(canonical_url, bytes))
+      return true;
+  }
   return false;
 }
 

chrome/browser/history/top_sites_impl.h

@@ -65,7 +65,9 @@ class TopSitesImpl : public TopSites {
   virtual void GetMostVisitedURLs(
       const GetMostVisitedURLsCallback& callback) OVERRIDE;
   virtual bool GetPageThumbnail(
-      const GURL& url, scoped_refptr<base::RefCountedMemory>* bytes) OVERRIDE;
+      const GURL& url,
+      bool prefix_match,
+      scoped_refptr<base::RefCountedMemory>* bytes) OVERRIDE;
   virtual bool GetPageThumbnailScore(const GURL& url,
                                      ThumbnailScore* score) OVERRIDE;
   virtual bool GetTemporaryPageThumbnailScore(const GURL& url,

chrome/browser/history/top_sites_impl_unittest.cc

@@ -148,7 +148,7 @@ class TopSitesImplTest : public HistoryUnitTestBase {
   // Gets the thumbnail for |url| from TopSites.
   SkBitmap GetThumbnail(const GURL& url) {
     scoped_refptr<base::RefCountedMemory> data;
-    return top_sites()->GetPageThumbnail(url, &data) ?
+    return top_sites()->GetPageThumbnail(url, false, &data) ?
         ExtractThumbnail(*data.get()) : SkBitmap();
   }
 
@@ -478,13 +478,13 @@ TEST_F(TopSitesImplTest, ThumbnailRemoved) {
 
   // Make sure the thumbnail was actually set.
   scoped_refptr<base::RefCountedMemory> result;
-  EXPECT_TRUE(top_sites()->GetPageThumbnail(url, &result));
+  EXPECT_TRUE(top_sites()->GetPageThumbnail(url, false, &result));
   EXPECT_TRUE(ThumbnailEqualsBytes(thumbnail, result.get()));
 
   // Reset the thumbnails and make sure we don't get it back.
   SetTopSites(MostVisitedURLList());
   RefreshTopSitesAndRecreate();
-  EXPECT_FALSE(top_sites()->GetPageThumbnail(url, &result));
+  EXPECT_FALSE(top_sites()->GetPageThumbnail(url, false, &result));
 }
 
 // Tests GetPageThumbnail.
@@ -509,19 +509,21 @@ TEST_F(TopSitesImplTest, GetPageThumbnail) {
 
   scoped_refptr<base::RefCountedMemory> result;
   EXPECT_TRUE(top_sites()->SetPageThumbnail(url1.url, thumbnail, score));
-  EXPECT_TRUE(top_sites()->GetPageThumbnail(url1.url, &result));
+  EXPECT_TRUE(top_sites()->GetPageThumbnail(url1.url, false, &result));
 
   EXPECT_TRUE(top_sites()->SetPageThumbnail(GURL("http://gmail.com"),
                                            thumbnail, score));
   EXPECT_TRUE(top_sites()->GetPageThumbnail(GURL("http://gmail.com"),
+                                            false,
                                             &result));
   // Get a thumbnail via a redirect.
   EXPECT_TRUE(top_sites()->GetPageThumbnail(GURL("http://mail.google.com"),
+                                            false,
                                             &result));
 
   EXPECT_TRUE(top_sites()->SetPageThumbnail(GURL("http://mail.google.com"),
                                            thumbnail, score));
-  EXPECT_TRUE(top_sites()->GetPageThumbnail(url2.url, &result));
+  EXPECT_TRUE(top_sites()->GetPageThumbnail(url2.url, false, &result));
 
   EXPECT_TRUE(ThumbnailEqualsBytes(thumbnail, result.get()));
 }
@@ -582,7 +584,7 @@ TEST_F(TopSitesImplTest, SaveToDB) {
     ASSERT_NO_FATAL_FAILURE(ContainsPrepopulatePages(querier, 1));
 
     scoped_refptr<base::RefCountedMemory> read_data;
-    EXPECT_TRUE(top_sites()->GetPageThumbnail(asdf_url, &read_data));
+    EXPECT_TRUE(top_sites()->GetPageThumbnail(asdf_url, false, &read_data));
     EXPECT_TRUE(ThumbnailEqualsBytes(tmp_bitmap, read_data.get()));
   }
 
@@ -647,7 +649,7 @@ TEST_F(TopSitesImplTest, RealDatabase) {
     ASSERT_NO_FATAL_FAILURE(ContainsPrepopulatePages(querier, 1));
 
     scoped_refptr<base::RefCountedMemory> read_data;
-    EXPECT_TRUE(top_sites()->GetPageThumbnail(asdf_url, &read_data));
+    EXPECT_TRUE(top_sites()->GetPageThumbnail(asdf_url, false, &read_data));
     EXPECT_TRUE(ThumbnailEqualsBytes(asdf_thumbnail, read_data.get()));
   }
 
@@ -679,7 +681,7 @@ TEST_F(TopSitesImplTest, RealDatabase) {
     EXPECT_EQ(google1_url, querier.urls()[0].url);
     EXPECT_EQ(google_title, querier.urls()[0].title);
     ASSERT_EQ(3u, querier.urls()[0].redirects.size());
-    EXPECT_TRUE(top_sites()->GetPageThumbnail(google3_url, &read_data));
+    EXPECT_TRUE(top_sites()->GetPageThumbnail(google3_url, false, &read_data));
     EXPECT_TRUE(ThumbnailEqualsBytes(google_thumbnail, read_data.get()));
 
     EXPECT_EQ(asdf_url, querier.urls()[1].url);
@@ -701,7 +703,7 @@ TEST_F(TopSitesImplTest, RealDatabase) {
   RefreshTopSitesAndRecreate();
   {
     scoped_refptr<base::RefCountedMemory> read_data;
-    EXPECT_TRUE(top_sites()->GetPageThumbnail(google3_url, &read_data));
+    EXPECT_TRUE(top_sites()->GetPageThumbnail(google3_url, false, &read_data));
     EXPECT_TRUE(ThumbnailEqualsBytes(weewar_bitmap, read_data.get()));
   }
 
@@ -721,7 +723,7 @@ TEST_F(TopSitesImplTest, RealDatabase) {
   RefreshTopSitesAndRecreate();
   {
     scoped_refptr<base::RefCountedMemory> read_data;
-    EXPECT_TRUE(top_sites()->GetPageThumbnail(google3_url, &read_data));
+    EXPECT_TRUE(top_sites()->GetPageThumbnail(google3_url, false, &read_data));
     EXPECT_FALSE(ThumbnailEqualsBytes(weewar_bitmap, read_data.get()));
     EXPECT_TRUE(ThumbnailEqualsBytes(green_bitmap, read_data.get()));
   }
@@ -964,7 +966,7 @@ TEST_F(TopSitesImplTest, AddTemporaryThumbnail) {
 
   // We shouldn't get the thumnail back though (the url isn't in to sites yet).
   scoped_refptr<base::RefCountedMemory> out;
-  EXPECT_FALSE(top_sites()->GetPageThumbnail(unknown_url, &out));
+  EXPECT_FALSE(top_sites()->GetPageThumbnail(unknown_url, false, &out));
   // But we should be able to get the temporary page thumbnail score.
   ThumbnailScore out_score;
   EXPECT_TRUE(top_sites()->GetTemporaryPageThumbnailScore(unknown_url,
@@ -983,7 +985,7 @@ TEST_F(TopSitesImplTest, AddTemporaryThumbnail) {
   // Update URLs. This should result in using thumbnail.
   SetTopSites(list);
 
-  ASSERT_TRUE(top_sites()->GetPageThumbnail(unknown_url, &out));
+  ASSERT_TRUE(top_sites()->GetPageThumbnail(unknown_url, false, &out));
   EXPECT_TRUE(ThumbnailEqualsBytes(thumbnail, out.get()));
 }
 

chrome/browser/history/url_utils.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/history/url_utils.h"
+
+#include <algorithm>
+
+namespace history {
+
+namespace {
+
+// Comparator to enforce '\0' < '?' < '#' < '/' < other characters.
+int GetURLCharPriority(char ch) {
+  switch (ch) {
+    case '\0': return 0;
+    case '?': return 1;
+    case '#': return 2;
+    case '/': return 3;
+  }
+  return 4;
+}
+
+}  // namespace
+
+// Instead of splitting URLs and extract path components, we can implement
+// CanonicalURLStringCompare() using string operations only. The key idea is,
+// treating '/' to be less than any valid path characters would make it behave
+// as a separator, so e.g., "test" < "test-case" would be enforced by
+// "test/..." < "test-case/...". We also force "?" < "/", so "test?query" <
+// "test/stuff". Since the routine is merely lexicographical string comparison
+// with remapping of chracter ordering, so it is a valid strict-weak ordering.
+bool CanonicalURLStringCompare(const std::string& s1, const std::string& s2) {
+  const std::string::value_type* ch1 = s1.c_str();
+  const std::string::value_type* ch2 = s2.c_str();
+  while (*ch1 && *ch2 && *ch1 == *ch2) {
+    ++ch1;
+    ++ch2;
+  }
+  int pri_diff = GetURLCharPriority(*ch1) - GetURLCharPriority(*ch2);
+  // We want false to be returned if |pri_diff| > 0.
+  return (pri_diff != 0) ? pri_diff < 0 : *ch1 < *ch2;
+}
+
+bool UrlIsPrefix(const GURL& url1, const GURL& url2) {
+  if (url1.scheme() != url2.scheme() || url1.host() != url2.host() ||
+      url1.port() != url2.port()) {
+    return false;
+  }
+  // Only need to compare path now. Note that queries are ignored.
+  std::string p1(url1.path());
+  std::string p2(url2.path());
+  if (p1.length() > p2.length())
+    return false;
+  std::pair<std::string::iterator, std::string::iterator> first_diff =
+      std::mismatch(p1.begin(), p1.end(), p2.begin());
+  // Necessary condition: |p1| is a string prefix of |p2|.
+  if (first_diff.first != p1.end())
+    return false;  // E.g.: (|p1| = "/test", |p2| = "/exam") => false.
+
+  // |p1| is string prefix.
+  if (first_diff.second == p2.end())  // Is exact match?
+    return true;  // E.g.: ("/test", "/test") => true.
+  // |p1| is strict string prefix, check full match of last path component.
+  if (!p1.empty() && *p1.rbegin() == '/')  // Ends in '/'?
+    return true;  // E.g.: ("/test/", "/test/stuff") => true.
+
+  // Finally, |p1| does not end in "/": check first extra character in |p2|.
+  // E.g.: ("/test", "/test/stuff") => true; ("/test", "/testing") => false.
+  return *(first_diff.second) == '/';
+}
+
+}  // namespace history

chrome/browser/history/url_utils.h

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_HISTORY_URL_UTILS_H_
+#define CHROME_BROWSER_HISTORY_URL_UTILS_H_
+
+#include <string>
+
+#include "chrome/browser/history/history_types.h"
+
+namespace history {
+
+// CanonicalURLStringCompare performs lexicographical comparison of two strings
+// that represent valid URLs, so that if the pre-path (scheme, host, and port)
+// parts are equal, then the path parts are compared by treating path components
+// (delimited by "/") as separate tokens that form units of comparison.
+// For example, let us compare |s1| and |s2|, with
+//   |s1| = "http://www.google.com:80/base/test/ab/cd?query/stuff"
+//   |s2| = "http://www.google.com:80/base/test-case/yz#ref/stuff"
+// The pre-path parts "http://www.google.com:80/" match. We treat the paths as
+//   |s1| => ["base", "test", "ab", "cd"]
+//   |s2| => ["base", "test-case", "yz"]
+// Components 1 "base" are identical. Components 2 yield "test" < "test-case",
+// so we consider |s1| < |s2|, and return true. Note that naive string
+// comparison would yield the opposite (|s1| > |s2|), since '/' > '-' in ASCII.
+// Note that path can be terminated by "?query" or "#ref". The post-path parts
+// are compared in an arbitrary (but consistent) way.
+bool CanonicalURLStringCompare(const std::string& s1, const std::string& s2);
+
+// Returns whether or not |url1| is a "URL prefix" of |url2|. Criteria:
+// - Scheme, host, port: exact match required.
+// - Path: treated as a list of path components (e.g., ["a", "bb"] for "/a/bb"),
+//   and |url1|'s list must be a prefix of |url2|'s list.
+// - Query and ref: ignored.
+// Note that "http://www.google.com/test" is NOT a prefix of
+// "http://www.google.com/testing", although "test" is a prefix of "testing".
+bool UrlIsPrefix(const GURL& url1, const GURL& url2);
+
+}  // namespace history
+
+#endif  // CHROME_BROWSER_HISTORY_URL_UTILS_H_

chrome/browser/history/url_utils_unittest.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/history/url_utils.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace history {
+
+namespace {
+
+TEST(UrlUtilsTest, CanonicalURLStringCompare) {
+  // Comprehensive test by comparing each pair in sorted list. O(n^2).
+  const char* sorted_list[] = {
+    "http://www.gogle.com/redirects_to_google",
+    "http://www.google.com",
+    "http://www.google.com/",
+    "http://www.google.com/?q",
+    "http://www.google.com/A",
+    "http://www.google.com/index.html",
+    "http://www.google.com/test",
+    "http://www.google.com/test?query",
+    "http://www.google.com/test?r=3",
+    "http://www.google.com/test#hash",
+    "http://www.google.com/test/?query",
+    "http://www.google.com/test/#hash",
+    "http://www.google.com/test/zzzzz",
+    "http://www.google.com/test$dollar",
+    "http://www.google.com/test%E9%9B%80",
+    "http://www.google.com/test-case",
+    "http://www.google.com:80/",
+    "https://www.google.com",
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(sorted_list); ++i) {
+    EXPECT_FALSE(CanonicalURLStringCompare(sorted_list[i], sorted_list[i]))
+        << " for \"" << sorted_list[i] << "\" < \"" << sorted_list[i] << "\"";
+    // Every disjoint pair-wise comparison.
+    for (size_t j = i + 1; j < ARRAYSIZE_UNSAFE(sorted_list); ++j) {
+      EXPECT_TRUE(CanonicalURLStringCompare(sorted_list[i], sorted_list[j]))
+          << " for \"" << sorted_list[i] << "\" < \"" << sorted_list[j] << "\"";
+      EXPECT_FALSE(CanonicalURLStringCompare(sorted_list[j], sorted_list[i]))
+          << " for \"" << sorted_list[j] << "\" < \"" << sorted_list[i] << "\"";
+    }
+  }
+}
+
+TEST(UrlUtilsTest, UrlIsPrefix) {
+  struct {
+    const char* s1;
+    const char* s2;
+  } true_cases[] = {
+    {"http://www.google.com", "http://www.google.com"},
+    {"http://www.google.com/a/b", "http://www.google.com/a/b"},
+    {"http://www.google.com?test=3", "http://www.google.com/"},
+    {"http://www.google.com/#hash", "http://www.google.com/?q"},
+    {"http://www.google.com/", "http://www.google.com/test/with/dir/"},
+    {"http://www.google.com:360", "http://www.google.com:360/?q=1234"},
+    {"http://www.google.com:80", "http://www.google.com/gurl/is/smart"},
+    {"http://www.google.com:80/", "http://www.google.com/"},
+    {"http://www.google.com/test", "http://www.google.com/test/with/dir/"},
+    {"http://www.google.com/test/", "http://www.google.com/test/with/dir"},
+    {"http://www.google.com/test?", "http://www.google.com/test/with/dir/"},
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(true_cases); ++i) {
+    EXPECT_TRUE(UrlIsPrefix(GURL(true_cases[i].s1), GURL(true_cases[i].s2)))
+        << " for true_cases[" << i << "]";
+  }
+  struct {
+    const char* s1;
+    const char* s2;
+  } false_cases[] = {
+    {"http://www.google.com/test", "http://www.google.com"},
+    {"http://www.google.com/a/b/", "http://www.google.com/a/b"},  // Arguable.
+    {"http://www.google.co", "http://www.google.com"},
+    {"http://google.com", "http://www.google.com"},
+    {"http://www.google.com", "https://www.google.com"},
+    {"http://www.google.com/path", "http://www.google.com:137/path"},
+    {"http://www.google.com/same/dir", "http://www.youtube.com/same/dir"},
+    {"http://www.google.com/te", "http://www.google.com/test"},
+    {"http://www.google.com/test", "http://www.google.com/test-bed"},
+    {"http://www.google.com/test-", "http://www.google.com/test?"},
+  };
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(false_cases); ++i) {
+    EXPECT_FALSE(UrlIsPrefix(GURL(false_cases[i].s1), GURL(false_cases[i].s2)))
+        << " for false_cases[" << i << "]";
+  }
+}
+
+}  // namespace
+
+}  // namespace history

chrome/browser/search/instant_service.cc

@@ -93,7 +93,8 @@ InstantService::InstantService(Profile* profile)
   content::URLDataSource::Add(profile, new ThemeSource(profile));
 #endif  // defined(ENABLE_THEMES)
 
-  content::URLDataSource::Add(profile, new ThumbnailSource(profile));
+  content::URLDataSource::Add(profile, new ThumbnailSource(profile, false));
+  content::URLDataSource::Add(profile, new ThumbnailSource(profile, true));
   content::URLDataSource::Add(profile, new FaviconSource(
       profile, FaviconSource::FAVICON));
   content::URLDataSource::Add(profile, new LocalNtpSource(profile));

chrome/browser/thumbnails/thumbnail_service.h

@@ -39,10 +39,13 @@ class ThumbnailService : public RefcountedBrowserContextKeyedService {
 
   // Gets a thumbnail for a given page. Returns true iff we have the thumbnail.
   // This may be invoked on any thread.
+  // If an exact thumbnail URL match fails, |prefix_match| specifies whether or
+  // not to try harder by matching the query thumbnail URL as URL prefix.
   // As this method may be invoked on any thread the ref count needs to be
   // incremented before this method returns, so this takes a scoped_refptr*.
   virtual bool GetPageThumbnail(
       const GURL& url,
+      bool prefix_match,
       scoped_refptr<base::RefCountedMemory>* bytes) = 0;
 
   // Returns true if the page thumbnail should be updated.
@@ -52,6 +55,6 @@ class ThumbnailService : public RefcountedBrowserContextKeyedService {
   virtual ~ThumbnailService() {}
 };
 
-}
+}  // namespace thumbnails
 
 #endif  // CHROME_BROWSER_THUMBNAILS_THUMBNAIL_SERVICE_H_

chrome/browser/thumbnails/thumbnail_service_impl.cc

@@ -28,13 +28,13 @@ bool IsThumbnailRetargetingEnabled() {
       switches::kEnableThumbnailRetargeting);
 }
 
-}
+}  // namespace
 
 namespace thumbnails {
 
 ThumbnailServiceImpl::ThumbnailServiceImpl(Profile* profile)
     : top_sites_(profile->GetTopSites()),
-      use_thumbnail_retargeting_(IsThumbnailRetargetingEnabled()){
+      use_thumbnail_retargeting_(IsThumbnailRetargetingEnabled()) {
 }
 
 ThumbnailServiceImpl::~ThumbnailServiceImpl() {
@@ -51,12 +51,13 @@ bool ThumbnailServiceImpl::SetPageThumbnail(const ThumbnailingContext& context,
 
 bool ThumbnailServiceImpl::GetPageThumbnail(
     const GURL& url,
+    bool prefix_match,
     scoped_refptr<base::RefCountedMemory>* bytes) {
   scoped_refptr<history::TopSites> local_ptr(top_sites_);
   if (local_ptr.get() == NULL)
     return false;
 
-  return local_ptr->GetPageThumbnail(url, bytes);
+  return local_ptr->GetPageThumbnail(url, prefix_match, bytes);
 }
 
 ThumbnailingAlgorithm* ThumbnailServiceImpl::GetThumbnailingAlgorithm()

chrome/browser/thumbnails/thumbnail_service_impl.h

@@ -28,6 +28,7 @@ class ThumbnailServiceImpl : public ThumbnailService {
   virtual ThumbnailingAlgorithm* GetThumbnailingAlgorithm() const OVERRIDE;
   virtual bool GetPageThumbnail(
       const GURL& url,
+      bool prefix_match,
       scoped_refptr<base::RefCountedMemory>* bytes) OVERRIDE;
   virtual bool ShouldAcquirePageThumbnail(const GURL& url) OVERRIDE;
 
@@ -43,6 +44,6 @@ class ThumbnailServiceImpl : public ThumbnailService {
   DISALLOW_COPY_AND_ASSIGN(ThumbnailServiceImpl);
 };
 
-}
+}  // namespace thumbnails
 
 #endif  // CHROME_BROWSER_THUMBNAILS_THUMBNAIL_SERVICE_IMPL_H_

chrome/browser/ui/webui/ntp/most_visited_handler.cc

@@ -77,8 +77,11 @@ MostVisitedHandler::~MostVisitedHandler() {
 void MostVisitedHandler::RegisterMessages() {
   Profile* profile = Profile::FromWebUI(web_ui());
   // Set up our sources for thumbnail and favicon data.
-  ThumbnailSource* thumbnail_src = new ThumbnailSource(profile);
-  content::URLDataSource::Add(profile, thumbnail_src);
+  ThumbnailSource* thumbnail_src_exact = new ThumbnailSource(profile, false);
+  content::URLDataSource::Add(profile, thumbnail_src_exact);
+
+  ThumbnailSource* thumbnail_src_prefix = new ThumbnailSource(profile, true);
+  content::URLDataSource::Add(profile, thumbnail_src_prefix);
 
 #if defined(OS_ANDROID)
   // Register chrome://touch-icon as a data source for touch icons or favicons.

chrome/browser/ui/webui/ntp/suggestions_page_handler.cc

@@ -63,7 +63,7 @@ SuggestionsHandler::~SuggestionsHandler() {
 void SuggestionsHandler::RegisterMessages() {
   Profile* profile = Profile::FromWebUI(web_ui());
   // Set up our sources for thumbnail and favicon data.
-  content::URLDataSource::Add(profile, new ThumbnailSource(profile));
+  content::URLDataSource::Add(profile, new ThumbnailSource(profile, false));
   content::URLDataSource::Add(
       profile, new FaviconSource(profile, FaviconSource::FAVICON));
 

chrome/browser/ui/webui/ntp/thumbnail_source.cc

@@ -20,16 +20,18 @@
 using content::BrowserThread;
 
 // Set ThumbnailService now as Profile isn't thread safe.
-ThumbnailSource::ThumbnailSource(Profile* profile)
+ThumbnailSource::ThumbnailSource(Profile* profile, bool prefix_match)
     : thumbnail_service_(ThumbnailServiceFactory::GetForProfile(profile)),
-      profile_(profile) {
+      profile_(profile),
+      prefix_match_(prefix_match) {
 }
 
 ThumbnailSource::~ThumbnailSource() {
 }
 
 std::string ThumbnailSource::GetSource() const {
-  return chrome::kChromeUIThumbnailHost;
+  return prefix_match_ ?
+      chrome::kChromeUIThumbnailHost2 : chrome::kChromeUIThumbnailHost;
 }
 
 void ThumbnailSource::StartDataRequest(
@@ -38,7 +40,7 @@ void ThumbnailSource::StartDataRequest(
     int render_view_id,
     const content::URLDataSource::GotDataCallback& callback) {
   scoped_refptr<base::RefCountedMemory> data;
-  if (thumbnail_service_->GetPageThumbnail(GURL(path), &data)) {
+  if (thumbnail_service_->GetPageThumbnail(GURL(path), prefix_match_, &data)) {
     // We have the thumbnail.
     callback.Run(data.get());
   } else {

chrome/browser/ui/webui/ntp/thumbnail_source.h

@@ -25,7 +25,7 @@ class ThumbnailService;
 // thumbnails and the history/top-sites backend that serves these.
 class ThumbnailSource : public content::URLDataSource {
  public:
-  explicit ThumbnailSource(Profile* profile);
+  ThumbnailSource(Profile* profile, bool prefix_match);
 
   // content::URLDataSource implementation.
   virtual std::string GetSource() const OVERRIDE;
@@ -53,6 +53,11 @@ class ThumbnailSource : public content::URLDataSource {
   // Only used when servicing requests on the UI thread.
   Profile* const profile_;
 
+  // If an exact thumbnail URL match fails, specifies whether or not to try
+  // harder by matching the query thumbnail URL as URL prefix. This affects
+  // GetSource().
+  const bool prefix_match_;
+
   DISALLOW_COPY_AND_ASSIGN(ThumbnailSource);
 };
 

chrome/chrome_browser.gypi

@@ -823,6 +823,8 @@
         'browser/history/url_database.h',
         'browser/history/url_index_private_data.cc',
         'browser/history/url_index_private_data.h',
+        'browser/history/url_utils.cc',
+        'browser/history/url_utils.h',
         'browser/history/visit_database.cc',
         'browser/history/visit_database.h',
         'browser/history/visit_filter.cc',

chrome/chrome_tests_unit.gypi

@@ -934,10 +934,12 @@
         'browser/history/shortcuts_database_unittest.cc',
         'browser/history/snippet_unittest.cc',
         'browser/history/thumbnail_database_unittest.cc',
+        'browser/history/top_sites_cache_unittest.cc',
         'browser/history/top_sites_database_unittest.cc',
         'browser/history/top_sites_impl_unittest.cc',
         'browser/history/typed_url_syncable_service_unittest.cc',
         'browser/history/url_database_unittest.cc',
+        'browser/history/url_utils_unittest.cc',
         'browser/history/visit_database_unittest.cc',
         'browser/history/visit_filter_unittest.cc',
         'browser/history/visit_tracker_unittest.cc',

chrome/common/url_constants.cc

@@ -221,6 +221,7 @@ const char kChromeUITaskManagerHost[] = "tasks";
 const char kChromeUITermsHost[] = "terms";
 const char kChromeUIThemeHost[] = "theme";
 const char kChromeUIThumbnailHost[] = "thumb";
+const char kChromeUIThumbnailHost2[] = "thumb2";
 const char kChromeUITouchIconHost[] = "touch-icon";
 const char kChromeUITranslateInternalsHost[] = "translate-internals";
 const char kChromeUIUberFrameHost[] = "uber-frame";

chrome/common/url_constants.h

@@ -213,6 +213,7 @@ extern const char kChromeUITaskManagerHost[];
 extern const char kChromeUITermsHost[];
 extern const char kChromeUIThemeHost[];
 extern const char kChromeUIThumbnailHost[];
+extern const char kChromeUIThumbnailHost2[];
 extern const char kChromeUITouchIconHost[];
 extern const char kChromeUITranslateInternalsHost[];
 extern const char kChromeUIUberFrameHost[];