Implementing URL prefix match for history thumbnail cache.

chrome/browser/history/top_sites_cache.h

Index: chrome/browser/history/top_sites_cache.h
diff --git a/chrome/browser/history/top_sites_cache.h b/chrome/browser/history/top_sites_cache.h
index 822d18903dc00e05ceb538e3975f71b2a6a8e3cf..7e189f2163e36da11980fa77af653b72d2a9388d 100644
--- a/chrome/browser/history/top_sites_cache.h
+++ b/chrome/browser/history/top_sites_cache.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_HISTORY_TOP_SITES_CACHE_H_
 
 #include <map>
+#include <string>
 #include <utility>
 
 #include "base/memory/ref_counted.h"
@@ -13,6 +14,20 @@
 
 namespace history {
 
+// The entries in CanonicalURLs, see CanonicalURLs for details. The second
+// argument gives the index of the URL into MostVisitedURLs redirects.
+typedef std::pair<MostVisitedURL*, size_t> CanonicalURLEntry;
+
+// Defines the "path-prefix" compare of two URLs
+// Comparator used for CanonicalURLs.
+class CanonicalURLComparator {
+ public:
+  bool operator()(const CanonicalURLEntry& e1,
+                  const CanonicalURLEntry& e2) const;
+  static bool CompareString(const std::string& s1,
+                            const std::string& s2);
+};
+
 // TopSitesCache caches the top sites and thumbnails for TopSites.
 class TopSitesCache {
  public:
@@ -37,6 +52,12 @@ class TopSitesCache {
   bool GetPageThumbnail(const GURL& url,
                         scoped_refptr<base::RefCountedMemory>* bytes);
 
+  // Similar to GetPageThumbnail(), but rather than requiring URL exact match,

[beaudoin, 2013/09/03 23:22:39]

URL exact match --> an exact match

[huangs, 2013/09/04 02:02:05]

Done.

+  // only requires the specified URL to be the prefix of a URL in TopSites.
+  // If multiple matches exist, considers the lexicographically earliest one.

[beaudoin, 2013/09/03 23:22:39]

considers [...] --> returns the thumbnail for the first match in lexicographical
order.

[huangs, 2013/09/04 02:02:05]

Done.

+  bool GetPageThumbnailForPrefix(const GURL& prefix_url,
+                                 scoped_refptr<base::RefCountedMemory>* bytes);
+
   // Fetches the thumbnail score for the specified url. Returns true if
   // there is a thumbnail score for the specified url.
   bool GetPageThumbnailScore(const GURL& url, ThumbnailScore* score);
@@ -44,26 +65,27 @@ class TopSitesCache {
   // Returns the canonical URL for |url|.
   const GURL& GetCanonicalURL(const GURL& url);
 
+  // Returns the canonical URL for |url_prefix|, which specifies a prefix for
+  // stored URLs in |canonical_urls_|.

[beaudoin, 2013/09/03 23:22:39]

I think the comment about favoring the first URL in lexicographical order is
more important here. Rephrase a little, I suggest:

Returns the canonical URL for |url_prefix|. If |url_prefix| matches multiple
entries then the first one in lexicographical order is returned.

(Is it the first one in lexicographical order from |canonical_urls_| or the
canonical url of the first match in lexicographical order??)

[huangs, 2013/09/04 02:02:05]

Also performs canonicalization. Updated comments.

+  const GURL& GetCanonicalURLForPrefix(const GURL& url_prefix);
+
   // Returns true if |url| is known.
   bool IsKnownURL(const GURL& url);
 
   // Returns the index into |top_sites_| for |url|.
   size_t GetURLIndex(const GURL& url);
 
- private:
-  // The entries in CanonicalURLs, see CanonicalURLs for details. The second
-  // argument gives the index of the URL into MostVisitedURLs redirects.
-  typedef std::pair<MostVisitedURL*, size_t> CanonicalURLEntry;
-
-  // Comparator used for CanonicalURLs.
-  class CanonicalURLComparator {
-   public:
-    bool operator()(const CanonicalURLEntry& e1,
-                    const CanonicalURLEntry& e2) const {
-      return e1.first->redirects[e1.second] < e2.first->redirects[e2.second];
-    }
-  };
+  // Returns whether or not |url1| is a prefix of |url2|. Criteria:
+  // - Scheme, host, port: exact match required.
+  // - Path: considered as a list of path components (e.g., ["a", "bb"] in
+  //   "/a/bb"), and |url1|'s list must be a prefix of |url2|'s list.
+  // - Query: ignored.
+  // Note that "http://www.google.com/test" is NOT a prefix of
+  // "http://www.google.com/testing", even though "test" is prefix of "testing".

[beaudoin, 2013/09/03 23:22:39]

Nit: a prefix

[huangs, 2013/09/04 02:02:05]

Done.

+  // Also, "http://www.google.com" will not match "http://www.google.com:80".
+  static bool UrlIsPrefix(const GURL& url1, const GURL& url2);
 
+ private:
   // This is used to map from redirect url to the MostVisitedURL the redirect is
   // from. Ideally this would be map<GURL, size_t> (second param indexing into
   // top_sites_), but this results in duplicating all redirect urls. As some
@@ -80,8 +102,11 @@ 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.
-  CanonicalURLs::iterator GetCanonicalURLsIterator(const GURL& url);
+  // Returns the iterator into canonical_urls_ for the |url|. If |match_prefix|
+  // is true, then returns the first iterator into canonical_urls_ that has
+  // |urL| as a path-prefix.

[beaudoin, 2013/09/03 23:22:39]

Nit: |url|

[huangs, 2013/09/04 02:02:05]

Done.

+  CanonicalURLs::iterator GetCanonicalURLsIterator(const GURL& url,
+                                                   bool match_prefix);
 
   // The top sites.
   MostVisitedURLList top_sites_;