diff options
author | mrossetti@chromium.org <mrossetti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-15 18:51:16 +0000 |
---|---|---|
committer | mrossetti@chromium.org <mrossetti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-10-15 18:51:16 +0000 |
commit | 0822d88ca6c5451ab3553cca1a603ebfc575dc40 (patch) | |
tree | 41f8667ff00a0027c3bca91cbeedfe88fe7157d8 /chrome/browser/autocomplete | |
parent | f941f47a837bc6d98937acc6a13603bb7c06d286 (diff) | |
download | chromium_src-0822d88ca6c5451ab3553cca1a603ebfc575dc40.zip chromium_src-0822d88ca6c5451ab3553cca1a603ebfc575dc40.tar.gz chromium_src-0822d88ca6c5451ab3553cca1a603ebfc575dc40.tar.bz2 |
HQP Refactoring (in Preparation for SQLite Cache)
1. Move ownership of the InMemoryURLIndex from the InMemoryHistoryBackend to the HistoryBackend where it truly belongs.
2. Encapsulate the private, persistent data for the InMemoryURLIndex in a new class, URLIndexPrivateData.
3. Handle (by notification) URL visits, updates and deletes. Refactor use of NOTIFICATION_HISTORY_URLS_DELETED to provide the deleted URLRow so that row ID is available.
4. Correctly handle the adding and removing of page title words when a URL change is detected.
5. Move most of the support types, including the new URLIndexPrivateData class, into a new file, in_memory_url_index_types.h.
6. Replace static class member functions with non-friend, non-class functions for better flexibility.
7. Move convenience types out from InMemoryURLIndex class up into history namespace.
8. Rename convenience types to generalize their intent.
9. Other small cleanups.
BUG=96731, 92718
TEST=Unit tests updated.
TBR=atwilson,brettw
Previously reviewed and LG'ed as http://codereview.chromium.org/8120004/.
Review URL: http://codereview.chromium.org/8291005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@105678 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/autocomplete')
6 files changed, 36 insertions, 32 deletions
diff --git a/chrome/browser/autocomplete/autocomplete.cc b/chrome/browser/autocomplete/autocomplete.cc index 6dbb9a1..8b6618c 100644 --- a/chrome/browser/autocomplete/autocomplete.cc +++ b/chrome/browser/autocomplete/autocomplete.cc @@ -531,6 +531,8 @@ void AutocompleteProvider::Stop() { } void AutocompleteProvider::DeleteMatch(const AutocompleteMatch& match) { + DLOG(WARNING) << "The AutocompleteProvider '" << name() + << "' has not implemented DeleteMatch."; } AutocompleteProvider::~AutocompleteProvider() { diff --git a/chrome/browser/autocomplete/history_provider.cc b/chrome/browser/autocomplete/history_provider.cc index 9fe0754..8140aea 100644 --- a/chrome/browser/autocomplete/history_provider.cc +++ b/chrome/browser/autocomplete/history_provider.cc @@ -31,17 +31,16 @@ void HistoryProvider::DeleteMatch(const AutocompleteMatch& match) { profile_->GetHistoryService(Profile::EXPLICIT_ACCESS); // Delete the match from the history DB. - GURL selected_url(match.destination_url); - if (!history_service || !selected_url.is_valid()) { - NOTREACHED() << "Can't delete requested URL"; - return; - } - history_service->DeleteURL(selected_url); + DCHECK(history_service); + DCHECK(match.destination_url.is_valid()); + history_service->DeleteURL(match.destination_url); + DeleteMatchFromMatches(match); +} - // Delete the match from the current set of matches. +void HistoryProvider::DeleteMatchFromMatches(const AutocompleteMatch& match) { bool found = false; for (ACMatches::iterator i(matches_.begin()); i != matches_.end(); ++i) { - if (i->destination_url == selected_url && i->type == match.type) { + if (i->destination_url == match.destination_url && i->type == match.type) { found = true; if (i->is_history_what_you_typed_match || i->starred) { // We can't get rid of What-You-Typed or Bookmarked matches, diff --git a/chrome/browser/autocomplete/history_provider.h b/chrome/browser/autocomplete/history_provider.h index 231a7b1..6cc0f80 100644 --- a/chrome/browser/autocomplete/history_provider.h +++ b/chrome/browser/autocomplete/history_provider.h @@ -54,6 +54,10 @@ class HistoryProvider : public AutocompleteProvider { // |input.prevent_inline_autocomplete()| is true, or the input text contains // trailing whitespace. bool PreventInlineAutocomplete(const AutocompleteInput& input); + + // Finds and removes the match from the current collection of matches and + // backing data. + void DeleteMatchFromMatches(const AutocompleteMatch& match); }; #endif // CHROME_BROWSER_AUTOCOMPLETE_HISTORY_PROVIDER_H_ diff --git a/chrome/browser/autocomplete/history_quick_provider.cc b/chrome/browser/autocomplete/history_quick_provider.cc index ae42585..8b53a02 100644 --- a/chrome/browser/autocomplete/history_quick_provider.cc +++ b/chrome/browser/autocomplete/history_quick_provider.cc @@ -16,6 +16,7 @@ #include "base/utf_string_conversions.h" #include "chrome/browser/history/history.h" #include "chrome/browser/history/in_memory_url_index.h" +#include "chrome/browser/history/in_memory_url_index_types.h" #include "chrome/browser/net/url_fixer_upper.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" @@ -81,8 +82,7 @@ void HistoryQuickProvider::Start(const AutocompleteInput& input, } } -// HistoryQuickProvider matches are currently not deletable. -// TODO(mrossetti): Determine when a match should be deletable. +// TODO(mrossetti): Implement this function. (Will happen in next CL.) void HistoryQuickProvider::DeleteMatch(const AutocompleteMatch& match) {} void HistoryQuickProvider::DoAutocomplete() { @@ -90,8 +90,8 @@ void HistoryQuickProvider::DoAutocomplete() { string16 term_string = autocomplete_input_.text(); term_string = net::UnescapeURLComponent(term_string, UnescapeRule::SPACES | UnescapeRule::URL_SPECIAL_CHARS); - history::InMemoryURLIndex::String16Vector terms( - InMemoryURLIndex::WordVectorFromString16(term_string, false)); + history::String16Vector terms( + history::String16VectorFromString16(term_string, false)); ScoredHistoryMatches matches = GetIndex()->HistoryItemsForTerms(terms); if (matches.empty()) return; @@ -131,13 +131,12 @@ AutocompleteMatch HistoryQuickProvider::QuickMatchToACMatch( // Format the URL autocomplete presentation. std::vector<size_t> offsets = - InMemoryURLIndex::OffsetsFromTermMatches(history_match.url_matches); + history::OffsetsFromTermMatches(history_match.url_matches); match.contents = net::FormatUrlWithOffsets(info.url(), languages_, net::kFormatUrlOmitAll, UnescapeRule::SPACES, NULL, NULL, &offsets); history::TermMatches new_matches = - InMemoryURLIndex::ReplaceOffsetsInTermMatches(history_match.url_matches, - offsets); + ReplaceOffsetsInTermMatches(history_match.url_matches, offsets); match.contents_class = SpansFromTermMatch(new_matches, match.contents.length(), true); match.fill_into_edit = match.contents; @@ -170,7 +169,8 @@ history::InMemoryURLIndex* HistoryQuickProvider::GetIndex() { return history_service->InMemoryIndex(); } -void HistoryQuickProvider::SetIndexForTesting( +// TODO(mrossetti): This will be inlined in the next CL. +void HistoryQuickProvider::set_index( history::InMemoryURLIndex* index) { DCHECK(index); index_for_testing_.reset(index); diff --git a/chrome/browser/autocomplete/history_quick_provider.h b/chrome/browser/autocomplete/history_quick_provider.h index 8264669..4beac05 100644 --- a/chrome/browser/autocomplete/history_quick_provider.h +++ b/chrome/browser/autocomplete/history_quick_provider.h @@ -37,9 +37,6 @@ class HistoryQuickProvider : public HistoryProvider { virtual void DeleteMatch(const AutocompleteMatch& match) OVERRIDE; - // Performs the autocomplete matching and scoring. - void DoAutocomplete(); - // Disable this provider. For unit testing purposes only. This is required // because this provider is closely associated with the HistoryURLProvider // and in order to properly test the latter the HistoryQuickProvider must @@ -52,6 +49,9 @@ class HistoryQuickProvider : public HistoryProvider { FRIEND_TEST_ALL_PREFIXES(HistoryQuickProviderTest, Spans); FRIEND_TEST_ALL_PREFIXES(HistoryQuickProviderTest, Relevance); + // Performs the autocomplete matching and scoring. + void DoAutocomplete(); + // Creates an AutocompleteMatch from |history_match|. |max_match_score| gives // the maximum possible score for the match. |history_matches| is the full set // of matches to compare each match to when calculating confidence. @@ -81,7 +81,8 @@ class HistoryQuickProvider : public HistoryProvider { bool is_url); // Only for use in unittests. Takes ownership of |index|. - void SetIndexForTesting(history::InMemoryURLIndex* index); + void set_index(history::InMemoryURLIndex* index); + AutocompleteInput autocomplete_input_; std::string languages_; diff --git a/chrome/browser/autocomplete/history_quick_provider_unittest.cc b/chrome/browser/autocomplete/history_quick_provider_unittest.cc index f936860..65b7654 100644 --- a/chrome/browser/autocomplete/history_quick_provider_unittest.cc +++ b/chrome/browser/autocomplete/history_quick_provider_unittest.cc @@ -142,6 +142,13 @@ void HistoryQuickProviderTest::OnProviderUpdate(bool updated_matches) { void HistoryQuickProviderTest::FillData() { history::URLDatabase* db = history_service_->InMemoryDatabase(); ASSERT_TRUE(db != NULL); + + history::InMemoryURLIndex* index = + new history::InMemoryURLIndex(profile_.get(), + FilePath(FILE_PATH_LITERAL("/dummy"))); + PrefService* prefs = profile_->GetPrefs(); + std::string languages(prefs->GetString(prefs::kAcceptLanguages)); + index->Init(db, languages); for (size_t i = 0; i < arraysize(quick_test_db); ++i) { const TestURLInfo& cur = quick_test_db[i]; const GURL current_url(cur.url); @@ -153,20 +160,11 @@ void HistoryQuickProviderTest::FillData() { url_info.set_typed_count(cur.typed_count); url_info.set_last_visit(visit_time); url_info.set_hidden(false); - EXPECT_TRUE(db->AddURL(url_info)); - - history_service_->AddPageWithDetails(current_url, UTF8ToUTF16(cur.title), - cur.visit_count, cur.typed_count, - visit_time, false, - history::SOURCE_BROWSED); + url_info.set_id(i); + index->UpdateURL(url_info); } - history::InMemoryURLIndex* index = - new history::InMemoryURLIndex(FilePath(FILE_PATH_LITERAL("/dummy"))); - PrefService* prefs = profile_->GetPrefs(); - std::string languages(prefs->GetString(prefs::kAcceptLanguages)); - index->Init(db, languages); - provider_->SetIndexForTesting(index); + provider_->set_index(index); } HistoryQuickProviderTest::SetShouldContain::SetShouldContain( |