diff options
6 files changed, 56 insertions, 23 deletions
diff --git a/chrome/browser/autocomplete/history_quick_provider.cc b/chrome/browser/autocomplete/history_quick_provider.cc index 1ecb81f..3c368185 100644 --- a/chrome/browser/autocomplete/history_quick_provider.cc +++ b/chrome/browser/autocomplete/history_quick_provider.cc @@ -136,12 +136,12 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch( match.contents = net::FormatUrl(info.url(), languages_, format_types, UnescapeRule::SPACES, NULL, NULL, NULL); match.contents_class = SpansFromTermMatch(history_match.url_matches, - match.contents.size(), 0); + match.contents.size()); // Format the description autocomplete presentation. match.description = info.title(); match.description_class = SpansFromTermMatch(history_match.title_matches, - match.description.size(), 0); + match.description.size()); return match; } @@ -184,26 +184,31 @@ int HistoryQuickProvider::CalculateRelevance(int raw_score, // static ACMatchClassifications HistoryQuickProvider::SpansFromTermMatch( const history::TermMatches& matches, - size_t text_length, - size_t adjust) { + size_t text_length) { ACMatchClassifications spans; if (matches.empty()) { if (text_length) spans.push_back(ACMatchClassification(0, ACMatchClassification::DIM)); return spans; } - if (matches[0].offset > adjust) + if (matches[0].offset) spans.push_back(ACMatchClassification(0, ACMatchClassification::NONE)); size_t match_count = matches.size(); for (size_t i = 0; i < match_count;) { - size_t offset = matches[i].offset - adjust; + size_t offset = matches[i].offset; + // TODO(mrossetti): Remove the following 'if' when http://crbug.com/77210 + // has been properly fixed. This guards against trying to highlight + // substrings which fall off the end of the string as a result of having + // encoded characters in the string. + if (offset >= text_length) + return spans; spans.push_back(ACMatchClassification(offset, ACMatchClassification::MATCH)); // Skip all adjacent matches. do { offset += matches[i].length; ++i; - } while ((i < match_count) && (offset == matches[i].offset - adjust)); + } while ((i < match_count) && (offset == matches[i].offset)); if (offset < text_length) { spans.push_back(ACMatchClassification(offset, ACMatchClassification::NONE)); diff --git a/chrome/browser/autocomplete/history_quick_provider.h b/chrome/browser/autocomplete/history_quick_provider.h index 74ad56c..3b0277a 100644 --- a/chrome/browser/autocomplete/history_quick_provider.h +++ b/chrome/browser/autocomplete/history_quick_provider.h @@ -61,13 +61,10 @@ class HistoryQuickProvider : public HistoryProvider { history::InMemoryURLIndex* GetIndex(); // Fill and return an ACMatchClassifications structure given the term - // matches (|matches|) to highlight where terms were found. |adjust| is - // subtracted form each offset and is used to account for any leading - // 'http://' in the potential result. + // matches (|matches|) to highlight where terms were found. static ACMatchClassifications SpansFromTermMatch( const history::TermMatches& matches, - size_t text_length, - size_t adjust); + size_t text_length); // Only for use in unittests. Takes ownership of |index|. void SetIndexForTesting(history::InMemoryURLIndex* index); diff --git a/chrome/browser/autocomplete/history_quick_provider_unittest.cc b/chrome/browser/autocomplete/history_quick_provider_unittest.cc index 463c43b..38ed75c 100644 --- a/chrome/browser/autocomplete/history_quick_provider_unittest.cc +++ b/chrome/browser/autocomplete/history_quick_provider_unittest.cc @@ -65,6 +65,8 @@ struct TestURLInfo { {"http://abcdefxyzghijklmnopqrstuvw.com/a", "An XYZ", 1, 1, 0}, {"http://abcxyzdefghijklmnopqrstuvw.com/a", "An XYZ", 1, 1, 0}, {"http://xyzabcdefghijklmnopqrstuvw.com/a", "An XYZ", 1, 1, 0}, + {"http://cda.com/Dogs%20Cats%20Gorillas%20Sea%20Slugs%20and%20Mice", + "Dogs & Cats & Mice", 1, 1, 0}, }; class HistoryQuickProviderTest : public TestingBrowserProcessTest, @@ -110,6 +112,8 @@ class HistoryQuickProviderTest : public TestingBrowserProcessTest, scoped_ptr<TestingProfile> profile_; HistoryService* history_service_; + ACMatches ac_matches_; // The resulting matches after running RunTest. + private: scoped_refptr<HistoryQuickProvider> provider_; }; @@ -177,23 +181,23 @@ void HistoryQuickProviderTest::RunTest(const string16 text, provider_->Start(input, false); EXPECT_TRUE(provider_->done()); - ACMatches matches = provider_->matches(); + ac_matches_ = provider_->matches(); // If the number of expected and actual matches aren't equal then we need // test no further, but let's do anyway so that we know which URLs failed. - EXPECT_EQ(expected_urls.size(), matches.size()); + EXPECT_EQ(expected_urls.size(), ac_matches_.size()); // Verify that all expected URLs were found and that all found URLs // were expected. std::set<std::string> leftovers = for_each(expected_urls.begin(), expected_urls.end(), - SetShouldContain(matches)).LeftOvers(); + SetShouldContain(ac_matches_)).LeftOvers(); EXPECT_TRUE(leftovers.empty()); // See if we got the expected top scorer. - if (!matches.empty()) { - std::partial_sort(matches.begin(), matches.begin() + 1, - matches.end(), AutocompleteMatch::MoreRelevant); - EXPECT_EQ(expected_top_result, matches[0].destination_url.spec()); + if (!ac_matches_.empty()) { + std::partial_sort(ac_matches_.begin(), ac_matches_.begin() + 1, + ac_matches_.end(), AutocompleteMatch::MoreRelevant); + EXPECT_EQ(expected_top_result, ac_matches_[0].destination_url.spec()); } } @@ -240,6 +244,27 @@ TEST_F(HistoryQuickProviderTest, RecencyMatch) { RunTest(text, expected_urls, "http://startest.com/y/a"); } +TEST_F(HistoryQuickProviderTest, EncodingLimitMatch) { + string16 text(ASCIIToUTF16("ice")); + std::vector<std::string> expected_urls; + std::string url( + "http://cda.com/Dogs%20Cats%20Gorillas%20Sea%20Slugs%20and%20Mice"); + expected_urls.push_back(url); + RunTest(text, expected_urls, url); + // Verify that the matches' ACMatchClassifications offsets are in range. + ACMatchClassifications content(ac_matches_[0].contents_class); + // The max offset accounts for 6 occurrences of '%20' plus the 'http://'. + const size_t max_offset = url.size() - ((6 * 2) + 7); + for (ACMatchClassifications::const_iterator citer = content.begin(); + citer != content.end(); ++citer) + EXPECT_GT(max_offset, citer->offset); + ACMatchClassifications description(ac_matches_[0].description_class); + std::string page_title("Dogs & Cats & Mice"); + for (ACMatchClassifications::const_iterator diter = content.begin(); + diter != content.end(); ++diter) + EXPECT_GT(page_title.size(), diter->offset); +} + TEST_F(HistoryQuickProviderTest, Spans) { // Test SpansFromTermMatch history::TermMatches matches_a; @@ -251,7 +276,7 @@ TEST_F(HistoryQuickProviderTest, Spans) { matches_a.push_back(history::TermMatch(3, 10, 1)); matches_a.push_back(history::TermMatch(4, 14, 5)); ACMatchClassifications spans_a = - HistoryQuickProvider::SpansFromTermMatch(matches_a, 20, 0); + HistoryQuickProvider::SpansFromTermMatch(matches_a, 20); // ACMatch spans should be: 'NM-NM---N-M-N--M----N-' ASSERT_EQ(9U, spans_a.size()); EXPECT_EQ(0U, spans_a[0].offset); @@ -278,7 +303,7 @@ TEST_F(HistoryQuickProviderTest, Spans) { matches_b.push_back(history::TermMatch(1, 0, 2)); matches_b.push_back(history::TermMatch(2, 3, 2)); ACMatchClassifications spans_b = - HistoryQuickProvider::SpansFromTermMatch(matches_b, 5, 0); + HistoryQuickProvider::SpansFromTermMatch(matches_b, 5); // ACMatch spans should be: 'M-NM-' ASSERT_EQ(3U, spans_b.size()); EXPECT_EQ(0U, spans_b[0].offset); diff --git a/chrome/browser/history/in_memory_url_index.cc b/chrome/browser/history/in_memory_url_index.cc index 6d9a44f..9d52aa2 100644 --- a/chrome/browser/history/in_memory_url_index.cc +++ b/chrome/browser/history/in_memory_url_index.cc @@ -206,7 +206,7 @@ bool InMemoryURLIndex::RestoreFromCacheFile() { // SQLite table checksums automatically stored. base::TimeTicks beginning_time = base::TimeTicks::Now(); FilePath file_path; - if (!GetCacheFilePath(&file_path)) + if (!GetCacheFilePath(&file_path) || !file_util::PathExists(file_path)) return false; std::string data; if (!file_util::ReadFileToString(file_path, &data)) { diff --git a/chrome/browser/history/in_memory_url_index_unittest.cc b/chrome/browser/history/in_memory_url_index_unittest.cc index e7b76c8..dee196c 100644 --- a/chrome/browser/history/in_memory_url_index_unittest.cc +++ b/chrome/browser/history/in_memory_url_index_unittest.cc @@ -234,6 +234,12 @@ TEST_F(InMemoryURLIndexTest, Retrieval) { matches[0].url_info.url().spec()); // Note: URL gets lowercased. EXPECT_EQ(ASCIIToUTF16("Practically Useless Search Result"), matches[0].url_info.title()); + + // Search which will match at the end of an URL with encoded characters. + terms.clear(); + terms.push_back(ASCIIToUTF16("ice")); + matches = url_index_->HistoryItemsForTerms(terms); + ASSERT_EQ(1U, matches.size()); } TEST_F(ExpandedInMemoryURLIndexTest, ShortCircuit) { diff --git a/chrome/test/data/History/url_history_provider_test.db.txt b/chrome/test/data/History/url_history_provider_test.db.txt index 44ab44f..26be362 100644 --- a/chrome/test/data/History/url_history_provider_test.db.txt +++ b/chrome/test/data/History/url_history_provider_test.db.txt @@ -55,7 +55,7 @@ INSERT INTO "urls" VALUES(26,'http://www.codeproject.com/','Your Development Res INSERT INTO "urls" VALUES(27,'http://www.tomshardware.com/us/#redir','Tom''s Hardware: Hardware News, Tests and Reviews',6,6,0,0,65); INSERT INTO "urls" VALUES(28,'http://www.ddj.com/windows/184416623','Dr. Dobb''s | Avoiding the Visual C++ Runtime Library | 2 1, 2003',6,6,0,0,0); INSERT INTO "urls" VALUES(29,'http://svcs.cnn.com/weather/getForecast?time=34&mode=json_html&zipCode=336736767676&locCode=EGLL&celcius=true&csiID=csi2','',6,6,0,1,0); -INSERT INTO "urls" VALUES(30,'http://www.drudgery.com/','Life in the Slow Lane',3,2,2,0,0); +INSERT INTO "urls" VALUES(30,'http://www.drudgery.com/Dogs%20Cats%20Gorillas%20Dolphine%20Sea%20Slugsand%20Mice','Life in the Slow Lane',3,2,2,0,0); INSERT INTO "urls" VALUES(31,'http://www.redrudgerydo.com/','Music of the Wild Landscape',0,0,6,0,0); INSERT INTO "urls" VALUES(32,'https://NearlyPerfectResult.com/','Practically Perfect Search Result',99,99,0,0,0); INSERT INTO "urls" VALUES(33,'http://QuiteUselessSearchResultxyz.com/','Practically Useless Search Result',4,0,99,0,0); |