summaryrefslogtreecommitdiffstats
path: root/chrome/browser/history
diff options
context:
space:
mode:
authorbeaudoin@chromium.org <beaudoin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-11-07 15:35:37 +0000
committerbeaudoin@chromium.org <beaudoin@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-11-07 15:35:37 +0000
commitbc8f8bcbcb2705065e97e02aefb8fbce6c00f928 (patch)
treeb838681a7ad77de08fb025d0baa38d3bcd3baf8d /chrome/browser/history
parent69e3e004ca55e180b07df05f1d8364b29f5e135f (diff)
downloadchromium_src-bc8f8bcbcb2705065e97e02aefb8fbce6c00f928.zip
chromium_src-bc8f8bcbcb2705065e97e02aefb8fbce6c00f928.tar.gz
chromium_src-bc8f8bcbcb2705065e97e02aefb8fbce6c00f928.tar.bz2
Removed checking for template URL specialization in TopSites.
When asked to do |prefix_match|, TopSitesImpl::GetPageThumbnail no longer uses URL specialization (eg. appending path suffixes) since it caused bad thumbnails to be shown in some cases. BUG= Review URL: https://codereview.chromium.org/61763006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@233617 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/history')
-rw-r--r--chrome/browser/history/top_sites_cache.cc29
-rw-r--r--chrome/browser/history/top_sites_cache.h15
-rw-r--r--chrome/browser/history/top_sites_cache_unittest.cc55
-rw-r--r--chrome/browser/history/top_sites_impl.cc7
4 files changed, 10 insertions, 96 deletions
diff --git a/chrome/browser/history/top_sites_cache.cc b/chrome/browser/history/top_sites_cache.cc
index 27d0c86..5914d99 100644
--- a/chrome/browser/history/top_sites_cache.cc
+++ b/chrome/browser/history/top_sites_cache.cc
@@ -73,35 +73,6 @@ const GURL& TopSitesCache::GetCanonicalURL(const GURL& url) const {
return it == canonical_urls_.end() ? url : it->first.first->url;
}
-GURL TopSitesCache::GetSpecializedCanonicalURL(const GURL& url) const {
- // Perform effective binary search for URL prefix search.
- CanonicalURLs::const_iterator it =
- canonical_urls_.upper_bound(CanonicalURLQuery(url).entry());
-
- // Check whether |prev_it| equals to |url|, ignoring "?query#ref". This
- // handles exact match and equality matches with earlier "?query#ref" part.
- if (it != canonical_urls_.begin()) {
- CanonicalURLs::const_iterator prev_it = it;
- --prev_it;
- if (url.ReplaceComponents(clear_query_ref_) ==
- GetURLFromIterator(prev_it).ReplaceComponents(clear_query_ref_)) {
- return prev_it->first.first->url;
- }
- }
-
- // Check whether |url| is a URL prefix of |it|. This handles strict
- // specialized match and equality matches with later "?query#ref".
- if (it != canonical_urls_.end()) {
- GURL compare_url(GetURLFromIterator(it));
- if (HaveSameSchemeHostAndPort(url, compare_url) &&
- IsPathPrefix(url.path(), compare_url.path())) {
- return it->first.first->url;
- }
- }
-
- return GURL::EmptyGURL();
-}
-
GURL TopSitesCache::GetGeneralizedCanonicalURL(const GURL& url) const {
CanonicalURLs::const_iterator it_hi =
canonical_urls_.lower_bound(CanonicalURLQuery(url).entry());
diff --git a/chrome/browser/history/top_sites_cache.h b/chrome/browser/history/top_sites_cache.h
index 0ce7308..f66d0f0 100644
--- a/chrome/browser/history/top_sites_cache.h
+++ b/chrome/browser/history/top_sites_cache.h
@@ -30,9 +30,6 @@ namespace history {
//
// The rule to "match" URL in |canonical_urls_| always favors exact match.
// - In GetCanonicalURL(), exact match is the only case examined.
-// - In GetSpecializedCanonicalURL(), we also perform "specialized" URL matches,
-// i.e., stored URLs in |canonical_urls_| of which the input URL is a
-// URL prefix, ignoring "?query#ref".
// - In GetGeneralizedCanonicalURL(), we also perform "generalized" URL matches,
// i.e., stored URLs in |canonical_urls_| that are prefixes of input URL,
// ignoring "?query#ref".
@@ -70,14 +67,10 @@ class TopSitesCache {
// Returns the canonical URL for |url|.
const GURL& GetCanonicalURL(const GURL& url) const;
- // Searches for a URL in |canonical_urls_| that has |url| as a URL prefix.
- // Prefers an exact match if it exists, or the least specialized match while
- // ignoring "?query#ref". Returns the result to if match is found, otherwise
- // returns an empty GURL.
- GURL GetSpecializedCanonicalURL(const GURL& url) const;
-
- // Similar to GetSpecializedCanonicalURL(), but searches for a URL in
- // |canonical_urls_| that is a URL prefix of |url|, and leaset generalized.
+ // Searches for a URL in |canonical_urls_| that is a URL prefix of |url|.
+ // Prefers an exact match if it exists, or the least generalized match while
+ // ignoring "?query#ref". Returns the resulting canonical URL if match is
+ // found, otherwise returns an empty GURL.
GURL GetGeneralizedCanonicalURL(const GURL& url) const;
// Returns true if |url| is known.
diff --git a/chrome/browser/history/top_sites_cache_unittest.cc b/chrome/browser/history/top_sites_cache_unittest.cc
index 0348dc4..a4a8690 100644
--- a/chrome/browser/history/top_sites_cache_unittest.cc
+++ b/chrome/browser/history/top_sites_cache_unittest.cc
@@ -142,49 +142,9 @@ TEST_F(TopSitesCacheTest, GetCanonicalURLExactMatch) {
// Get the answer from direct lookup.
GURL stored_url(s);
GURL expected(cache_.GetCanonicalURL(stored_url));
- // Test specialization.
- GURL result1(cache_.GetSpecializedCanonicalURL(stored_url));
- EXPECT_EQ(expected, result1) << " for kTopSitesSpecPrefix[" << i << "]";
// Test generalization.
- GURL result2(cache_.GetGeneralizedCanonicalURL(stored_url));
- EXPECT_EQ(expected, result2) << " for kTopSitesSpecPrefix[" << i << "]";
- }
-}
-
-TEST_F(TopSitesCacheTest, GetSpecializedCanonicalURL) {
- InitTopSiteCache(kTopSitesSpecPrefix, arraysize(kTopSitesSpecPrefix));
- struct {
- const char* expected;
- const char* query;
- } test_cases[] = {
- // Exact match after trimming "?query": redirects.
- {"http://www.google.com/", "http://www.google.com/test"},
- // Specialized match: redirects.
- {"http://www.google.com/sh", "http://www.google.com/sh/1/2"},
- // Specialized match with trailing "/": redirects.
- {"http://www.google.com/sh", "http://www.google.com/sh/1/2/"},
- // Unique specialization match: redirects.
- {"http://www.google.com/", "http://www.chromium.org/a"},
- // Multiple exact matches after trimming: redirects to first.
- {"http://www.google.com/2", "http://www.google.com/test/y"},
- // Multiple specialized matches: redirects to least specialized.
- {"http://www.google.com/2", "http://www.google.com/test/q"},
- // No specialized match: fails.
- {"", "http://www.google.com/no-match"},
- // String prefix match but not URL-prefix match: fails.
- {"", "http://www.google.com/t"},
- // Different protocol: fails.
- {"", "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"},
- // Generalization match only: fails.
- {"", "http://www.google.com/sh/1/2/3/4"},
- };
- 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);
- GURL result(cache_.GetSpecializedCanonicalURL(GURL(query)));
- EXPECT_EQ(expected, result.spec()) << " for test_case[" << i << "]";
+ GURL result(cache_.GetGeneralizedCanonicalURL(stored_url));
+ EXPECT_EQ(expected, result) << " for kTopSitesSpecPrefix[" << i << "]";
}
}
@@ -240,8 +200,8 @@ TEST_F(TopSitesCacheTest, GetGeneralizedCanonicalURL) {
}
}
-// This tests a special case where there are 2 specialized and generalized
-// matches, and both should be checked to find the correct match.
+// This tests a special case where there are 2 generalized matches, and both
+// should be checked to find the correct match.
TEST_F(TopSitesCacheTest, GetPrefixCanonicalURLDiffByQuery) {
const char* top_sites_spec[] = {
"http://www.dest.com/1",
@@ -251,7 +211,6 @@ TEST_F(TopSitesCacheTest, GetPrefixCanonicalURLDiffByQuery) {
};
InitTopSiteCache(top_sites_spec, arraysize(top_sites_spec));
- // Shared by GetSpecializedCanonicalURL() and GetGeneralizedCanonicalURL
struct {
const char* expected;
const char* query;
@@ -269,10 +228,8 @@ TEST_F(TopSitesCacheTest, GetPrefixCanonicalURLDiffByQuery) {
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);
- GURL result1(cache_.GetSpecializedCanonicalURL(GURL(query)));
- EXPECT_EQ(expected, result1.spec()) << " for test_case[" << i << "]";
- GURL result2(cache_.GetGeneralizedCanonicalURL(GURL(query)));
- EXPECT_EQ(expected, result2.spec()) << " for test_case[" << i << "]";
+ GURL result(cache_.GetGeneralizedCanonicalURL(GURL(query)));
+ EXPECT_EQ(expected, result.spec()) << " for test_case[" << i << "]";
}
}
diff --git a/chrome/browser/history/top_sites_impl.cc b/chrome/browser/history/top_sites_impl.cc
index a084716..18f2e21 100644
--- a/chrome/browser/history/top_sites_impl.cc
+++ b/chrome/browser/history/top_sites_impl.cc
@@ -247,13 +247,6 @@ bool TopSitesImpl::GetPageThumbnail(
base::AutoLock lock(lock_);
GURL canonical_url;
- // Test whether |url| is prefix of any stored URL.
- canonical_url = thread_safe_cache_->GetSpecializedCanonicalURL(*it);
- if (!canonical_url.is_empty() &&
- thread_safe_cache_->GetPageThumbnail(canonical_url, bytes)) {
- return true;
- }
-
// Test whether any stored URL is a prefix of |url|.
canonical_url = thread_safe_cache_->GetGeneralizedCanonicalURL(*it);
if (!canonical_url.is_empty() &&
generated by cgit v1.1 at 2025-03-02 10:02:59 +0000