diff options
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_match.cc | 48 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/autocomplete_match.h | 15 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/shortcuts_provider.cc | 48 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/shortcuts_provider.h | 12 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/shortcuts_provider_shortcut.cc | 130 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/shortcuts_provider_shortcut.h | 86 | ||||
| -rw-r--r-- | chrome/browser/autocomplete/shortcuts_provider_unittest.cc | 68 | ||||
| -rw-r--r-- | chrome/browser/history/shortcuts_backend.cc | 175 | ||||
| -rw-r--r-- | chrome/browser/history/shortcuts_backend.h | 64 | ||||
| -rw-r--r-- | chrome/browser/history/shortcuts_backend_unittest.cc | 73 | ||||
| -rw-r--r-- | chrome/browser/history/shortcuts_database.cc | 63 | ||||
| -rw-r--r-- | chrome/browser/history/shortcuts_database.h | 13 | ||||
| -rw-r--r-- | chrome/browser/history/shortcuts_database_unittest.cc | 50 | ||||
| -rw-r--r-- | chrome/chrome_browser.gypi | 2 |
14 files changed, 343 insertions, 504 deletions
diff --git a/chrome/browser/autocomplete/autocomplete_match.cc b/chrome/browser/autocomplete/autocomplete_match.cc index 14684a5..4d4f256 100644 --- a/chrome/browser/autocomplete/autocomplete_match.cc +++ b/chrome/browser/autocomplete/autocomplete_match.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "base/logging.h" +#include "base/string_number_conversions.h" #include "base/string_util.h" #include "chrome/browser/autocomplete/autocomplete_match.h" #include "chrome/browser/search_engines/template_url.h" @@ -216,6 +217,53 @@ void AutocompleteMatch::ClassifyLocationInString( } // static +std::string AutocompleteMatch::ClassificationsToString( + const ACMatchClassifications& classifications) { + std::string serialized_classifications; + for (size_t i = 0; i < classifications.size(); ++i) { + if (i) + serialized_classifications += ','; + serialized_classifications += base::IntToString(classifications[i].offset) + + ',' + base::IntToString(classifications[i].style); + } + return serialized_classifications; +} + +// static +ACMatchClassifications AutocompleteMatch::ClassificationsFromString( + const std::string& serialized_classifications) { + ACMatchClassifications classifications; + std::vector<std::string> tokens; + Tokenize(serialized_classifications, ",", &tokens); + DCHECK(!(tokens.size() & 1)); // The number of tokens should be even. + for (size_t i = 0; i < tokens.size(); i += 2) { + int classification_offset = 0; + int classification_style = ACMatchClassification::NONE; + if (!base::StringToInt(tokens[i], &classification_offset) || + !base::StringToInt(tokens[i + 1], &classification_style)) { + NOTREACHED(); + return classifications; + } + classifications.push_back(ACMatchClassification(classification_offset, + classification_style)); + } + return classifications; +} + +// static +void AutocompleteMatch::AddLastClassificationIfNecessary( + ACMatchClassifications* classifications, + size_t offset, + int style) { + DCHECK(classifications); + if (classifications->empty() || classifications->back().style != style) { + DCHECK(classifications->empty() || + (offset > classifications->back().offset)); + classifications->push_back(ACMatchClassification(offset, style)); + } +} + +// static string16 AutocompleteMatch::SanitizeString(const string16& text) { // NOTE: This logic is mirrored by |sanitizeString()| in // schema_generated_bindings.js. diff --git a/chrome/browser/autocomplete/autocomplete_match.h b/chrome/browser/autocomplete/autocomplete_match.h index 096ee71..a0260ae 100644 --- a/chrome/browser/autocomplete/autocomplete_match.h +++ b/chrome/browser/autocomplete/autocomplete_match.h @@ -134,6 +134,21 @@ struct AutocompleteMatch { int style, ACMatchClassifications* classifications); + // Converts classifications to and from a serialized string representation + // (using comma-separated integers to sequentially list positions and styles). + static std::string ClassificationsToString( + const ACMatchClassifications& classifications); + static ACMatchClassifications ClassificationsFromString( + const std::string& serialized_classifications); + + // Adds a classification to the end of |classifications| iff its style is + // different from the last existing classification. |offset| must be larger + // than the offset of the last classification in |classifications|. + static void AddLastClassificationIfNecessary( + ACMatchClassifications* classifications, + size_t offset, + int style); + // Removes invalid characters from |text|. Should be called on strings coming // from external sources (such as extensions) before assigning to |contents| // or |description|. diff --git a/chrome/browser/autocomplete/shortcuts_provider.cc b/chrome/browser/autocomplete/shortcuts_provider.cc index bbc02e7b..96bb9d1 100644 --- a/chrome/browser/autocomplete/shortcuts_provider.cc +++ b/chrome/browser/autocomplete/shortcuts_provider.cc @@ -30,9 +30,6 @@ #include "net/base/escape.h" #include "net/base/net_util.h" -using shortcuts_provider::Shortcut; -using shortcuts_provider::ShortcutMap; - namespace { class RemoveMatchPredicate { @@ -137,10 +134,11 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) { string16 term_string(base::i18n::ToLower(input.text())); DCHECK(!term_string.empty()); - for (ShortcutMap::const_iterator it = FindFirstMatch(term_string); + for (history::ShortcutsBackend::ShortcutMap::const_iterator it = + FindFirstMatch(term_string); it != shortcuts_backend_->shortcuts_map().end() && StartsWith(it->first, term_string, true); ++it) - matches_.push_back(ShortcutToACMatch(input, term_string, it)); + matches_.push_back(ShortcutToACMatch(input, term_string, it->second)); std::partial_sort(matches_.begin(), matches_.begin() + std::min(AutocompleteProvider::kMaxMatches, matches_.size()), @@ -154,21 +152,21 @@ void ShortcutsProvider::GetMatches(const AutocompleteInput& input) { AutocompleteMatch ShortcutsProvider::ShortcutToACMatch( const AutocompleteInput& input, const string16& term_string, - ShortcutMap::const_iterator it) { - AutocompleteMatch match(this, CalculateScore(term_string, it->second), + const history::ShortcutsBackend::Shortcut& shortcut) { + AutocompleteMatch match(this, CalculateScore(term_string, shortcut), true, AutocompleteMatch::HISTORY_TITLE); - match.destination_url = it->second.url; + match.destination_url = shortcut.url; DCHECK(match.destination_url.is_valid()); - match.fill_into_edit = UTF8ToUTF16(it->second.url.spec()); + match.fill_into_edit = UTF8ToUTF16(shortcut.url.spec()); - match.contents = it->second.contents; + match.contents = shortcut.contents; match.contents_class = ClassifyAllMatchesInString(term_string, match.contents, - it->second.contents_class); + shortcut.contents_class); - match.description = it->second.description; + match.description = shortcut.description; match.description_class = ClassifyAllMatchesInString( - term_string, match.description, it->second.description_class); + term_string, match.description, shortcut.description_class); return match; } @@ -223,11 +221,11 @@ ACMatchClassifications ShortcutsProvider::ClassifyAllMatchesInString( if (!match_class.empty() && match_class.back().offset == match_start) match_class.pop_back(); - shortcuts_provider::AddLastMatchIfNeeded(&match_class, match_start, - ACMatchClassification::MATCH); + AutocompleteMatch::AddLastClassificationIfNecessary(&match_class, + match_start, ACMatchClassification::MATCH); if (match_end < text_lowercase.length()) { - shortcuts_provider::AddLastMatchIfNeeded(&match_class, match_end, - ACMatchClassification::NONE); + AutocompleteMatch::AddLastClassificationIfNecessary(&match_class, + match_end, ACMatchClassification::NONE); } last_position = match_end; @@ -240,9 +238,8 @@ ACMatchClassifications ShortcutsProvider::ClassifyAllMatchesInString( ACMatchClassifications output; for (ACMatchClassifications::const_iterator i = original_class.begin(), j = match_class.begin(); i != original_class.end();) { - shortcuts_provider::AddLastMatchIfNeeded(&output, - std::max(i->offset, j->offset), - i->style | j->style); + AutocompleteMatch::AddLastClassificationIfNecessary(&output, + std::max(i->offset, j->offset), i->style | j->style); const size_t next_i_offset = (i + 1) == original_class.end() ? static_cast<size_t>(-1) : (i + 1)->offset; const size_t next_j_offset = (j + 1) == match_class.end() ? @@ -256,9 +253,9 @@ ACMatchClassifications ShortcutsProvider::ClassifyAllMatchesInString( return output; } -ShortcutMap::const_iterator ShortcutsProvider::FindFirstMatch( - const string16& keyword) { - ShortcutMap::const_iterator it = +history::ShortcutsBackend::ShortcutMap::const_iterator + ShortcutsProvider::FindFirstMatch(const string16& keyword) { + history::ShortcutsBackend::ShortcutMap::const_iterator it = shortcuts_backend_->shortcuts_map().lower_bound(keyword); // Lower bound not necessarily matches the keyword, check for item pointed by // the lower bound iterator to at least start with keyword. @@ -268,8 +265,9 @@ ShortcutMap::const_iterator ShortcutsProvider::FindFirstMatch( } // static -int ShortcutsProvider::CalculateScore(const string16& terms, - const Shortcut& shortcut) { +int ShortcutsProvider::CalculateScore( + const string16& terms, + const history::ShortcutsBackend::Shortcut& shortcut) { DCHECK(!terms.empty()); DCHECK_LE(terms.length(), shortcut.text.length()); diff --git a/chrome/browser/autocomplete/shortcuts_provider.h b/chrome/browser/autocomplete/shortcuts_provider.h index d5e1129..46260de 100644 --- a/chrome/browser/autocomplete/shortcuts_provider.h +++ b/chrome/browser/autocomplete/shortcuts_provider.h @@ -6,16 +6,13 @@ #define CHROME_BROWSER_AUTOCOMPLETE_SHORTCUTS_PROVIDER_H_ #pragma once -#include <map> #include <set> #include <string> -#include <vector> #include "base/gtest_prod_util.h" #include "base/time.h" #include "chrome/browser/autocomplete/autocomplete_match.h" #include "chrome/browser/autocomplete/history_provider.h" -#include "chrome/browser/autocomplete/shortcuts_provider_shortcut.h" #include "chrome/browser/history/shortcuts_backend.h" class Profile; @@ -56,7 +53,7 @@ class ShortcutsProvider AutocompleteMatch ShortcutToACMatch( const AutocompleteInput& input, const string16& terms, - shortcuts_provider::ShortcutMap::const_iterator it); + const history::ShortcutsBackend::Shortcut& shortcut); // Given |text| and a corresponding base set of classifications // |original_class|, adds ACMatchClassification::MATCH markers for all @@ -76,11 +73,12 @@ class ShortcutsProvider // Returns iterator to first item in |shortcuts_map_| matching |keyword|. // Returns shortcuts_map_.end() if there are no matches. - shortcuts_provider::ShortcutMap::const_iterator FindFirstMatch( + history::ShortcutsBackend::ShortcutMap::const_iterator FindFirstMatch( const string16& keyword); - static int CalculateScore(const string16& terms, - const shortcuts_provider::Shortcut& shortcut); + static int CalculateScore( + const string16& terms, + const history::ShortcutsBackend::Shortcut& shortcut); // For unit-test only. void set_shortcuts_backend(history::ShortcutsBackend* shortcuts_backend); diff --git a/chrome/browser/autocomplete/shortcuts_provider_shortcut.cc b/chrome/browser/autocomplete/shortcuts_provider_shortcut.cc deleted file mode 100644 index 634c9a3..0000000 --- a/chrome/browser/autocomplete/shortcuts_provider_shortcut.cc +++ /dev/null @@ -1,130 +0,0 @@ -// Copyright (c) 2012 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/autocomplete/shortcuts_provider_shortcut.h" - -#include "base/string_number_conversions.h" -#include "base/string_util.h" -#include "base/time.h" -#include "base/utf_string_conversions.h" -#include "chrome/browser/autocomplete/shortcuts_provider.h" - -namespace { - -// Takes Match classification vector and removes all matched positions, -// compacting repetitions if necessary. -void StripMatchMarkersFromClassifications(ACMatchClassifications* matches) { - DCHECK(matches); - ACMatchClassifications unmatched; - for (ACMatchClassifications::iterator i = matches->begin(); - i != matches->end(); ++i) { - shortcuts_provider::AddLastMatchIfNeeded(&unmatched, i->offset, - i->style & ~ACMatchClassification::MATCH); - } - matches->swap(unmatched); -} - -} // namespace - -namespace shortcuts_provider { - -Shortcut::Shortcut(const string16& text, - const GURL& url, - const string16& contents, - const ACMatchClassifications& in_contents_class, - const string16& description, - const ACMatchClassifications& in_description_class) - : text(text), - url(url), - contents(contents), - contents_class(in_contents_class), - description(description), - description_class(in_description_class), - last_access_time(base::Time::Now()), - number_of_hits(1) { - StripMatchMarkersFromClassifications(&contents_class); - StripMatchMarkersFromClassifications(&description_class); -} - -Shortcut::Shortcut(const std::string& id, - const string16& text, - const string16& url, - const string16& contents, - const string16& contents_class, - const string16& description, - const string16& description_class, - int64 time_of_last_access, - int number_of_hits) - : id(id), - text(text), - url(url), - contents(contents), - contents_class(SpansFromString(contents_class)), - description(description), - description_class(SpansFromString(description_class)), - last_access_time(base::Time::FromInternalValue(time_of_last_access)), - number_of_hits(1) {} - -Shortcut::Shortcut() - : last_access_time(base::Time::Now()), - number_of_hits(0) {} - -Shortcut::~Shortcut() {} - -string16 Shortcut::contents_class_as_str() const { - return SpansToString(contents_class); -} - -string16 Shortcut::description_class_as_str() const { - return SpansToString(description_class); -} - -string16 SpansToString(const ACMatchClassifications& value) { - string16 spans; - string16 comma(ASCIIToUTF16(",")); - for (size_t i = 0; i < value.size(); ++i) { - if (i) - spans.append(comma); - spans.append(base::IntToString16(value[i].offset)); - spans.append(comma); - spans.append(base::IntToString16(value[i].style)); - } - return spans; -} - -ACMatchClassifications SpansFromString(const string16& value) { - ACMatchClassifications spans; - std::vector<string16> tokens; - Tokenize(value, ASCIIToUTF16(","), &tokens); - // The number of tokens should be even. - if ((tokens.size() & 1) == 1) { - NOTREACHED(); - return spans; - } - for (size_t i = 0; i < tokens.size(); i += 2) { - int span_start = 0; - int span_type = ACMatchClassification::NONE; - if (!base::StringToInt(tokens[i], &span_start) || - !base::StringToInt(tokens[i + 1], &span_type)) { - NOTREACHED(); - return spans; - } - spans.push_back(ACMatchClassification(span_start, span_type)); - } - return spans; -} - -// Adds match at the end if and only if its style is different from the last -// match. -void AddLastMatchIfNeeded(ACMatchClassifications* matches, - size_t position, - int style) { - DCHECK(matches); - if (matches->empty() || matches->back().style != style) { - if (!matches->empty()) - DCHECK_GT(position, matches->back().offset); - matches->push_back(ACMatchClassification(position, style)); - } -} - -} // namespace shortcuts_provider diff --git a/chrome/browser/autocomplete/shortcuts_provider_shortcut.h b/chrome/browser/autocomplete/shortcuts_provider_shortcut.h deleted file mode 100644 index aadb230..0000000 --- a/chrome/browser/autocomplete/shortcuts_provider_shortcut.h +++ /dev/null @@ -1,86 +0,0 @@ -// Copyright (c) 2012 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_AUTOCOMPLETE_SHORTCUTS_PROVIDER_SHORTCUT_H_ -#define CHROME_BROWSER_AUTOCOMPLETE_SHORTCUTS_PROVIDER_SHORTCUT_H_ -#pragma once - -#include <map> -#include <string> -#include <vector> - -#include "base/string16.h" -#include "base/time.h" -#include "chrome/browser/autocomplete/autocomplete_match.h" -#include "googleurl/src/gurl.h" - -namespace shortcuts_provider { - -// The following struct encapsulates one previously selected omnibox shortcut. -struct Shortcut { - Shortcut(const string16& text, - const GURL& url, - const string16& contents, - const ACMatchClassifications& contents_class, - const string16& description, - const ACMatchClassifications& description_class); - // This constructor is used for creation of the structure from DB data. - Shortcut(const std::string& id, - const string16& text, - const string16& url, - const string16& contents, - const string16& contents_class, - const string16& description, - const string16& description_class, - int64 time_of_last_access, - int number_of_hits); - // Required for STL, we don't use this directly. - Shortcut(); - ~Shortcut(); - - string16 contents_class_as_str() const; - string16 description_class_as_str() const; - - std::string id; // Unique guid for the shortcut. - string16 text; // The user's original input string. - GURL url; // The corresponding destination URL. - - // Contents and description from the original match, along with their - // corresponding markup. We need these in order to correctly mark which - // parts are URLs, dim, etc. However, we strip all MATCH classifications - // from these since we'll mark the matching portions ourselves as we match - // the user's current typing against these Shortcuts. - string16 contents; - ACMatchClassifications contents_class; - string16 description; - ACMatchClassifications description_class; - - base::Time last_access_time; // Last time shortcut was selected. - int number_of_hits; // How many times shortcut was selected. -}; - -// Maps the original match (|text| in the Shortcut) to Shortcut for quick -// search. -typedef std::multimap<string16, Shortcut> ShortcutMap; - -// Quick access guid maps - first one for loading, the second one is a shadow -// map for access. -typedef std::map<std::string, Shortcut> GuidToShortcutMap; -typedef std::map<std::string, ShortcutMap::iterator> GuidToShortcutsIteratorMap; - -// Helpers dealing with database update. -// Converts spans vector to comma-separated string. -string16 SpansToString(const ACMatchClassifications& value); -// Coverts comma-separated unsigned integer values into spans vector. -ACMatchClassifications SpansFromString(const string16& value); - -// Helper for initialization and update. -// Adds match at the end if and only if its style is different from the last -// match. -void AddLastMatchIfNeeded(ACMatchClassifications* matches, - size_t position, - int style); -} // namespace shortcuts_provider - -#endif // CHROME_BROWSER_AUTOCOMPLETE_SHORTCUTS_PROVIDER_SHORTCUT_H_ diff --git a/chrome/browser/autocomplete/shortcuts_provider_unittest.cc b/chrome/browser/autocomplete/shortcuts_provider_unittest.cc index 33598b6..07d3223 100644 --- a/chrome/browser/autocomplete/shortcuts_provider_unittest.cc +++ b/chrome/browser/autocomplete/shortcuts_provider_unittest.cc @@ -28,9 +28,6 @@ #include "content/test/test_browser_thread.h" #include "testing/gtest/include/gtest/gtest.h" -using base::Time; -using base::TimeDelta; - using content::BrowserThread; namespace { @@ -145,7 +142,7 @@ class ShortcutsProviderTest : public testing::Test, void TearDown(); // Fills test data into the provider. - void FillData(); + void FillData(TestShortcutInfo* db, size_t db_size); // Runs an autocomplete query on |text| and checks to see that the returned // results' destination URLs match those provided. |expected_urls| does not @@ -181,31 +178,24 @@ void ShortcutsProviderTest::SetUp() { mock_backend_ = new history::ShortcutsBackend(FilePath(), profile_.get()); mock_backend_->Init(); provider_->set_shortcuts_backend(mock_backend_.get()); - FillData(); + FillData(shortcut_test_db, arraysize(shortcut_test_db)); } void ShortcutsProviderTest::TearDown() { provider_ = NULL; } -void ShortcutsProviderTest::FillData() { +void ShortcutsProviderTest::FillData(TestShortcutInfo* db, size_t db_size) { DCHECK(provider_.get()); - mock_backend_->DeleteAllShortcuts(); - for (size_t i = 0; i < arraysize(shortcut_test_db); ++i) { - const TestShortcutInfo& cur = shortcut_test_db[i]; - const GURL current_url(cur.url); - Time visit_time = Time::Now() - TimeDelta::FromDays(cur.days_from_now); - shortcuts_provider::Shortcut shortcut( - ASCIIToUTF16(cur.title), - current_url, - ASCIIToUTF16(cur.contents), - shortcuts_provider::SpansFromString(ASCIIToUTF16(cur.contents_class)), + for (size_t i = 0; i < db_size; ++i) { + const TestShortcutInfo& cur = db[i]; + history::ShortcutsBackend::Shortcut shortcut(cur.guid, + ASCIIToUTF16(cur.title), GURL(cur.url), ASCIIToUTF16(cur.contents), + AutocompleteMatch::ClassificationsFromString(cur.contents_class), ASCIIToUTF16(cur.description), - shortcuts_provider::SpansFromString( - ASCIIToUTF16(cur.description_class))); - shortcut.last_access_time = visit_time; - shortcut.number_of_hits = cur.typed_count; - shortcut.id = cur.guid; + AutocompleteMatch::ClassificationsFromString(cur.description_class), + base::Time::Now() - base::TimeDelta::FromDays(cur.days_from_now), + cur.typed_count); mock_backend_->AddShortcut(shortcut); } } @@ -565,15 +555,12 @@ TEST_F(ShortcutsProviderTest, CalculateScore) { ACMatchClassification(0, ACMatchClassification::NONE)); spans_description.push_back( ACMatchClassification(2, ACMatchClassification::MATCH)); - shortcuts_provider::Shortcut shortcut(ASCIIToUTF16("test"), - GURL("http://www.test.com"), - ASCIIToUTF16("www.test.com"), - spans_content, - ASCIIToUTF16("A test"), - spans_description); + history::ShortcutsBackend::Shortcut shortcut(std::string(), + ASCIIToUTF16("test"), GURL("http://www.test.com"), + ASCIIToUTF16("www.test.com"), spans_content, ASCIIToUTF16("A test"), + spans_description, base::Time::Now(), 1); // Maximal score. - shortcut.last_access_time = Time::Now(); const int kMaxScore = ShortcutsProvider::CalculateScore( ASCIIToUTF16("test"), shortcut); @@ -589,20 +576,20 @@ TEST_F(ShortcutsProviderTest, CalculateScore) { EXPECT_LT(score_one_quarter, score_one_half); // Should decay with time - one week. - shortcut.last_access_time = Time::Now() - TimeDelta::FromDays(7); + shortcut.last_access_time = base::Time::Now() - base::TimeDelta::FromDays(7); int score_week_old = ShortcutsProvider::CalculateScore(ASCIIToUTF16("test"), shortcut); EXPECT_LT(score_week_old, kMaxScore); // Should decay more in two weeks. - shortcut.last_access_time = Time::Now() - TimeDelta::FromDays(14); + shortcut.last_access_time = base::Time::Now() - base::TimeDelta::FromDays(14); int score_two_weeks_old = ShortcutsProvider::CalculateScore(ASCIIToUTF16("test"), shortcut); EXPECT_LT(score_two_weeks_old, score_week_old); // But not if it was activly clicked on. 2 hits slow decaying power. shortcut.number_of_hits = 2; - shortcut.last_access_time = Time::Now() - TimeDelta::FromDays(14); + shortcut.last_access_time = base::Time::Now() - base::TimeDelta::FromDays(14); int score_popular_two_weeks_old = ShortcutsProvider::CalculateScore(ASCIIToUTF16("test"), shortcut); EXPECT_LT(score_two_weeks_old, score_popular_two_weeks_old); @@ -611,7 +598,7 @@ TEST_F(ShortcutsProviderTest, CalculateScore) { // 3 hits slow decaying power even more. shortcut.number_of_hits = 3; - shortcut.last_access_time = Time::Now() - TimeDelta::FromDays(14); + shortcut.last_access_time = base::Time::Now() - base::TimeDelta::FromDays(14); int score_more_popular_two_weeks_old = ShortcutsProvider::CalculateScore(ASCIIToUTF16("test"), shortcut); EXPECT_LT(score_two_weeks_old, score_more_popular_two_weeks_old); @@ -639,22 +626,7 @@ TEST_F(ShortcutsProviderTest, DeleteMatch) { size_t original_shortcuts_count = provider_->shortcuts_backend_->shortcuts_map().size(); - for (size_t i = 0; i < arraysize(shortcuts_to_test_delete); ++i) { - const TestShortcutInfo& cur = shortcuts_to_test_delete[i]; - const GURL current_url(cur.url); - Time visit_time = Time::Now() - TimeDelta::FromDays(cur.days_from_now); - shortcuts_provider::Shortcut shortcut( - ASCIIToUTF16(cur.title), - current_url, - ASCIIToUTF16(cur.contents), - shortcuts_provider::SpansFromString(ASCIIToUTF16(cur.contents_class)), - ASCIIToUTF16(cur.description), - shortcuts_provider::SpansFromString( - ASCIIToUTF16(cur.description_class))); - shortcut.last_access_time = visit_time; - shortcut.id = cur.guid; - mock_backend_->AddShortcut(shortcut); - } + FillData(shortcuts_to_test_delete, arraysize(shortcuts_to_test_delete)); EXPECT_EQ(original_shortcuts_count + 3, provider_->shortcuts_backend_->shortcuts_map().size()); diff --git a/chrome/browser/history/shortcuts_backend.cc b/chrome/browser/history/shortcuts_backend.cc index 5af231a..db350e5 100644 --- a/chrome/browser/history/shortcuts_backend.cc +++ b/chrome/browser/history/shortcuts_backend.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -26,8 +26,61 @@ using content::BrowserThread; +namespace { + +// Takes Match classification vector and removes all matched positions, +// compacting repetitions if necessary. +void StripMatchMarkersFromClassifications(ACMatchClassifications* matches) { + DCHECK(matches); + ACMatchClassifications unmatched; + for (ACMatchClassifications::iterator i = matches->begin(); + i != matches->end(); ++i) { + AutocompleteMatch::AddLastClassificationIfNecessary(&unmatched, i->offset, + i->style & ~ACMatchClassification::MATCH); + } + matches->swap(unmatched); +} + +} // namespace + namespace history { +// ShortcutsBackend::Shortcut ------------------------------------------------- + +ShortcutsBackend::Shortcut::Shortcut( + const std::string& id, + const string16& text, + const GURL& url, + const string16& contents, + const ACMatchClassifications& contents_class, + const string16& description, + const ACMatchClassifications& description_class, + const base::Time& last_access_time, + int number_of_hits) + : id(id), + text(text), + url(url), + contents(contents), + contents_class(contents_class), + description(description), + description_class(description_class), + last_access_time(last_access_time), + number_of_hits(number_of_hits) { + StripMatchMarkersFromClassifications(&this->contents_class); + StripMatchMarkersFromClassifications(&this->description_class); +} + +ShortcutsBackend::Shortcut::Shortcut() + : last_access_time(base::Time::Now()), + number_of_hits(0) { +} + +ShortcutsBackend::Shortcut::~Shortcut() { +} + + +// ShortcutsBackend ----------------------------------------------------------- + ShortcutsBackend::ShortcutsBackend(const FilePath& db_folder_path, Profile *profile) : current_state_(NOT_INITIALIZED), @@ -45,23 +98,20 @@ ShortcutsBackend::ShortcutsBackend(const FilePath& db_folder_path, ShortcutsBackend::~ShortcutsBackend() {} bool ShortcutsBackend::Init() { - if (current_state_ == NOT_INITIALIZED) { - current_state_ = INITIALIZING; - if (no_db_access_) { - current_state_ = INITIALIZED; - return true; - } else { - return BrowserThread::PostTask( - BrowserThread::DB, FROM_HERE, - base::Bind(&ShortcutsBackend::InitInternal, this)); - } - } else { + if (current_state_ != NOT_INITIALIZED) return false; + + if (no_db_access_) { + current_state_ = INITIALIZED; + return true; } + + current_state_ = INITIALIZING; + return BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, + base::Bind(&ShortcutsBackend::InitInternal, this)); } -bool ShortcutsBackend::AddShortcut( - const shortcuts_provider::Shortcut& shortcut) { +bool ShortcutsBackend::AddShortcut(const Shortcut& shortcut) { if (!initialized()) return false; DCHECK(guid_map_.find(shortcut.id) == guid_map_.end()); @@ -69,32 +119,24 @@ bool ShortcutsBackend::AddShortcut( std::make_pair(base::i18n::ToLower(shortcut.text), shortcut)); FOR_EACH_OBSERVER(ShortcutsBackendObserver, observer_list_, OnShortcutsChanged()); - if (no_db_access_) - return true; - return BrowserThread::PostTask( - BrowserThread::DB, FROM_HERE, + return no_db_access_ || BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, base::Bind(base::IgnoreResult(&ShortcutsDatabase::AddShortcut), db_.get(), shortcut)); } -bool ShortcutsBackend::UpdateShortcut( - const shortcuts_provider::Shortcut& shortcut) { +bool ShortcutsBackend::UpdateShortcut(const Shortcut& shortcut) { if (!initialized()) return false; - shortcuts_provider::GuidToShortcutsIteratorMap::iterator it = - guid_map_.find(shortcut.id); + GuidToShortcutsIteratorMap::iterator it = guid_map_.find(shortcut.id); if (it != guid_map_.end()) shortcuts_map_.erase(it->second); guid_map_[shortcut.id] = shortcuts_map_.insert( std::make_pair(base::i18n::ToLower(shortcut.text), shortcut)); FOR_EACH_OBSERVER(ShortcutsBackendObserver, observer_list_, OnShortcutsChanged()); - if (no_db_access_) - return true; - return BrowserThread::PostTask( - BrowserThread::DB, FROM_HERE, - base::Bind(base::IgnoreResult(&ShortcutsDatabase::UpdateShortcut), - db_.get(), shortcut)); + return no_db_access_ || BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, + base::Bind(base::IgnoreResult(&ShortcutsDatabase::UpdateShortcut), + db_.get(), shortcut)); } bool ShortcutsBackend::DeleteShortcutsWithIds( @@ -102,8 +144,7 @@ bool ShortcutsBackend::DeleteShortcutsWithIds( if (!initialized()) return false; for (size_t i = 0; i < shortcut_ids.size(); ++i) { - shortcuts_provider::GuidToShortcutsIteratorMap::iterator it = - guid_map_.find(shortcut_ids[i]); + GuidToShortcutsIteratorMap::iterator it = guid_map_.find(shortcut_ids[i]); if (it != guid_map_.end()) { shortcuts_map_.erase(it->second); guid_map_.erase(it); @@ -111,21 +152,16 @@ bool ShortcutsBackend::DeleteShortcutsWithIds( } FOR_EACH_OBSERVER(ShortcutsBackendObserver, observer_list_, OnShortcutsChanged()); - if (no_db_access_) - return true; - return BrowserThread::PostTask( - BrowserThread::DB, FROM_HERE, - base::Bind( - base::IgnoreResult(&ShortcutsDatabase::DeleteShortcutsWithIds), - db_.get(), shortcut_ids)); + return no_db_access_ || BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, + base::Bind(base::IgnoreResult(&ShortcutsDatabase::DeleteShortcutsWithIds), + db_.get(), shortcut_ids)); } bool ShortcutsBackend::DeleteShortcutsWithUrl(const GURL& shortcut_url) { if (!initialized()) return false; std::vector<std::string> shortcut_ids; - for (shortcuts_provider::GuidToShortcutsIteratorMap::iterator - it = guid_map_.begin(); + for (GuidToShortcutsIteratorMap::iterator it = guid_map_.begin(); it != guid_map_.end();) { if (it->second->second.url == shortcut_url) { shortcut_ids.push_back(it->first); @@ -137,13 +173,9 @@ bool ShortcutsBackend::DeleteShortcutsWithUrl(const GURL& shortcut_url) { } FOR_EACH_OBSERVER(ShortcutsBackendObserver, observer_list_, OnShortcutsChanged()); - if (no_db_access_) - return true; - return BrowserThread::PostTask( - BrowserThread::DB, FROM_HERE, - base::Bind( - base::IgnoreResult(&ShortcutsDatabase::DeleteShortcutsWithUrl), - db_.get(), shortcut_url.spec())); + return no_db_access_ || BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, + base::Bind(base::IgnoreResult(&ShortcutsDatabase::DeleteShortcutsWithUrl), + db_.get(), shortcut_url.spec())); } bool ShortcutsBackend::DeleteAllShortcuts() { @@ -153,29 +185,24 @@ bool ShortcutsBackend::DeleteAllShortcuts() { guid_map_.clear(); FOR_EACH_OBSERVER(ShortcutsBackendObserver, observer_list_, OnShortcutsChanged()); - if (no_db_access_) - return true; - return BrowserThread::PostTask( - BrowserThread::DB, FROM_HERE, - base::Bind( - base::IgnoreResult(&ShortcutsDatabase::DeleteAllShortcuts), - db_.get())); + return no_db_access_ || BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, + base::Bind(base::IgnoreResult(&ShortcutsDatabase::DeleteAllShortcuts), + db_.get())); } void ShortcutsBackend::InitInternal() { DCHECK(current_state_ == INITIALIZING); db_->Init(); - shortcuts_provider::GuidToShortcutMap shortcuts; + ShortcutsDatabase::GuidToShortcutMap shortcuts; db_->LoadShortcuts(&shortcuts); - temp_shortcuts_map_.reset(new shortcuts_provider::ShortcutMap); - temp_guid_map_.reset(new shortcuts_provider::GuidToShortcutsIteratorMap); - for (shortcuts_provider::GuidToShortcutMap::iterator it = shortcuts.begin(); + temp_shortcuts_map_.reset(new ShortcutMap); + temp_guid_map_.reset(new GuidToShortcutsIteratorMap); + for (ShortcutsDatabase::GuidToShortcutMap::iterator it = shortcuts.begin(); it != shortcuts.end(); ++it) { (*temp_guid_map_)[it->first] = temp_shortcuts_map_->insert( std::make_pair(base::i18n::ToLower(it->second.text), it->second)); } - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(&ShortcutsBackend::InitCompleted, this)); } @@ -204,8 +231,7 @@ void ShortcutsBackend::Observe(int type, content::Details<const history::URLsDeletedDetails>(details)->urls; std::vector<std::string> shortcut_ids; - for (shortcuts_provider::GuidToShortcutsIteratorMap::iterator - it = guid_map_.begin(); + for (GuidToShortcutsIteratorMap::iterator it = guid_map_.begin(); it != guid_map_.end(); ++it) { if (urls.find(it->second->second.url) != urls.end()) shortcut_ids.push_back(it->first); @@ -219,32 +245,21 @@ void ShortcutsBackend::Observe(int type, AutocompleteLog* log = content::Details<AutocompleteLog>(details).ptr(); string16 text_lowercase(base::i18n::ToLower(log->text)); - int number_of_hits = 1; - std::string id; const AutocompleteMatch& match(log->result.match_at(log->selected_index)); - for (shortcuts_provider::ShortcutMap::iterator it = - shortcuts_map_.lower_bound(text_lowercase); + for (ShortcutMap::iterator it = shortcuts_map_.lower_bound(text_lowercase); it != shortcuts_map_.end() && StartsWith(it->first, text_lowercase, true); ++it) { if (match.destination_url == it->second.url) { - number_of_hits = it->second.number_of_hits + 1; - id = it->second.id; - break; + UpdateShortcut(Shortcut(it->second.id, log->text, match.destination_url, + match.contents, match.contents_class, match.description, + match.description_class, base::Time::Now(), + it->second.number_of_hits + 1)); + return; } } - shortcuts_provider::Shortcut shortcut(log->text, match.destination_url, + AddShortcut(Shortcut(guid::GenerateGUID(), log->text, match.destination_url, match.contents, match.contents_class, match.description, - match.description_class); - shortcut.number_of_hits = number_of_hits; - if (number_of_hits == 1) - shortcut.id = guid::GenerateGUID(); - else - shortcut.id = id; - - if (number_of_hits == 1) - AddShortcut(shortcut); - else - UpdateShortcut(shortcut); + match.description_class, base::Time::Now(), 1)); } } // namespace history diff --git a/chrome/browser/history/shortcuts_backend.h b/chrome/browser/history/shortcuts_backend.h index 00191c2..66892c7 100644 --- a/chrome/browser/history/shortcuts_backend.h +++ b/chrome/browser/history/shortcuts_backend.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -17,8 +17,8 @@ #include "base/observer_list.h" #include "base/string16.h" #include "base/synchronization/lock.h" -#include "chrome/browser/autocomplete/shortcuts_provider_shortcut.h" -#include "chrome/browser/history/shortcuts_database.h" +#include "base/time.h" +#include "chrome/browser/autocomplete/autocomplete_match.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "googleurl/src/gurl.h" @@ -27,11 +27,48 @@ class Profile; namespace history { +class ShortcutsDatabase; + // This class manages the shortcut provider backend - access to database on the // db thread, etc. class ShortcutsBackend : public base::RefCountedThreadSafe<ShortcutsBackend>, public content::NotificationObserver { public: + // The following struct encapsulates one previously selected omnibox shortcut. + struct Shortcut { + Shortcut(const std::string& id, + const string16& text, + const GURL& url, + const string16& contents, + const ACMatchClassifications& contents_class, + const string16& description, + const ACMatchClassifications& description_class, + const base::Time& last_access_time, + int number_of_hits); + // Required for STL, we don't use this directly. + Shortcut(); + ~Shortcut(); + + std::string id; // Unique guid for the shortcut. + string16 text; // The user's original input string. + GURL url; // The corresponding destination URL. + + // Contents and description from the original match, along with their + // corresponding markup. We need these in order to correctly mark which + // parts are URLs, dim, etc. However, we strip all MATCH classifications + // from these since we'll mark the matching portions ourselves as we match + // the user's current typing against these Shortcuts. + string16 contents; + ACMatchClassifications contents_class; + string16 description; + ACMatchClassifications description_class; + + base::Time last_access_time; // Last time shortcut was selected. + int number_of_hits; // How many times shortcut was selected. + }; + + typedef std::multimap<string16, ShortcutsBackend::Shortcut> ShortcutMap; + // |profile| is necessary for profile notifications only and can be NULL in // unit-tests. |db_folder_path| could be an empty path only in unit-tests as // well. It means there is no database created, all things are done in memory. @@ -59,10 +96,10 @@ class ShortcutsBackend : public base::RefCountedThreadSafe<ShortcutsBackend>, // All of the public functions *must* be called on UI thread only! // Adds the Shortcut to the database. - bool AddShortcut(const shortcuts_provider::Shortcut& shortcut); + bool AddShortcut(const ShortcutsBackend::Shortcut& shortcut); // Updates timing and selection count for the Shortcut. - bool UpdateShortcut(const shortcuts_provider::Shortcut& shortcut); + bool UpdateShortcut(const ShortcutsBackend::Shortcut& shortcut); // Deletes the Shortcuts with the id. bool DeleteShortcutsWithIds(const std::vector<std::string>& shortcut_ids); @@ -73,14 +110,10 @@ class ShortcutsBackend : public base::RefCountedThreadSafe<ShortcutsBackend>, // Deletes all of the shortcuts. bool DeleteAllShortcuts(); - const shortcuts_provider::ShortcutMap& shortcuts_map() const { + const ShortcutMap& shortcuts_map() const { return shortcuts_map_; } - const shortcuts_provider::GuidToShortcutsIteratorMap& guid_map() const { - return guid_map_; - } - void AddObserver(ShortcutsBackendObserver* obs) { observer_list_.AddObserver(obs); } @@ -90,6 +123,9 @@ class ShortcutsBackend : public base::RefCountedThreadSafe<ShortcutsBackend>, } private: + typedef std::map<std::string, ShortcutMap::iterator> + GuidToShortcutsIteratorMap; + // Internal initialization of the back-end. Posted by Init() to the DB thread. // On completion posts InitCompleted() back to UI thread. void InitInternal(); @@ -115,12 +151,12 @@ class ShortcutsBackend : public base::RefCountedThreadSafe<ShortcutsBackend>, // The |temp_shortcuts_map_| and |temp_guid_map_| used for temporary storage // between InitInternal() and InitComplete() to avoid doing a potentially huge // copy. - scoped_ptr<shortcuts_provider::ShortcutMap> temp_shortcuts_map_; - scoped_ptr<shortcuts_provider::GuidToShortcutsIteratorMap> temp_guid_map_; + scoped_ptr<ShortcutMap> temp_shortcuts_map_; + scoped_ptr<GuidToShortcutsIteratorMap> temp_guid_map_; - shortcuts_provider::ShortcutMap shortcuts_map_; + ShortcutMap shortcuts_map_; // This is a helper map for quick access to a shortcut by guid. - shortcuts_provider::GuidToShortcutsIteratorMap guid_map_; + GuidToShortcutsIteratorMap guid_map_; content::NotificationRegistrar notification_registrar_; diff --git a/chrome/browser/history/shortcuts_backend_unittest.cc b/chrome/browser/history/shortcuts_backend_unittest.cc index ad67f87..ad51166 100644 --- a/chrome/browser/history/shortcuts_backend_unittest.cc +++ b/chrome/browser/history/shortcuts_backend_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -9,7 +9,6 @@ #include "base/stringprintf.h" #include "base/time.h" #include "base/utf_string_conversions.h" -#include "chrome/browser/autocomplete/shortcuts_provider_shortcut.h" #include "chrome/browser/history/shortcuts_backend.h" #include "chrome/browser/history/shortcuts_database.h" #include "chrome/common/guid.h" @@ -19,8 +18,6 @@ #include "testing/gtest/include/gtest/gtest.h" using content::BrowserThread; -using shortcuts_provider::Shortcut; -using shortcuts_provider::ShortcutMap; namespace history { @@ -82,15 +79,16 @@ void ShortcutsBackendTest::InitBackend() { TEST_F(ShortcutsBackendTest, AddAndUpdateShortcut) { InitBackend(); - Shortcut shortcut("BD85DBA2-8C29-49F9-84AE-48E1E90880DF", - ASCIIToUTF16("goog"), ASCIIToUTF16("http://www.google.com"), - ASCIIToUTF16("Google"), ASCIIToUTF16("0,1"), - ASCIIToUTF16("Google"), ASCIIToUTF16("0,1"), - base::Time::Now().ToInternalValue(), - 100); + ShortcutsBackend::Shortcut shortcut("BD85DBA2-8C29-49F9-84AE-48E1E90880DF", + ASCIIToUTF16("goog"), GURL("http://www.google.com"), + ASCIIToUTF16("Google"), + AutocompleteMatch::ClassificationsFromString("0,1"), + ASCIIToUTF16("Google"), + AutocompleteMatch::ClassificationsFromString("0,1"), base::Time::Now(), + 100); EXPECT_TRUE(backend_->AddShortcut(shortcut)); - const ShortcutMap& shortcuts = backend_->shortcuts_map(); + const ShortcutsBackend::ShortcutMap& shortcuts = backend_->shortcuts_map(); ASSERT_TRUE(shortcuts.end() != shortcuts.find(shortcut.text)); EXPECT_EQ(shortcut.id, shortcuts.find(shortcut.text)->second.id); EXPECT_EQ(shortcut.contents, shortcuts.find(shortcut.text)->second.contents); @@ -102,40 +100,41 @@ TEST_F(ShortcutsBackendTest, AddAndUpdateShortcut) { TEST_F(ShortcutsBackendTest, DeleteShortcuts) { InitBackend(); - Shortcut shortcut1("BD85DBA2-8C29-49F9-84AE-48E1E90880DF", - ASCIIToUTF16("goog"), - ASCIIToUTF16("http://www.google.com"), - ASCIIToUTF16("Google"), ASCIIToUTF16("0,1,4,0"), - ASCIIToUTF16("Google"), ASCIIToUTF16("0,3,4,1"), - base::Time::Now().ToInternalValue(), - 100); + ShortcutsBackend::Shortcut shortcut1("BD85DBA2-8C29-49F9-84AE-48E1E90880DF", + ASCIIToUTF16("goog"), GURL("http://www.google.com"), + ASCIIToUTF16("Google"), + AutocompleteMatch::ClassificationsFromString("0,1,4,0"), + ASCIIToUTF16("Google"), + AutocompleteMatch::ClassificationsFromString("0,3,4,1"), + base::Time::Now(), 100); EXPECT_TRUE(backend_->AddShortcut(shortcut1)); - Shortcut shortcut2("BD85DBA2-8C29-49F9-84AE-48E1E90880E0", - ASCIIToUTF16("gle"), ASCIIToUTF16("http://www.google.com"), - ASCIIToUTF16("Google"), ASCIIToUTF16("0,1"), - ASCIIToUTF16("Google"), ASCIIToUTF16("0,1"), - base::Time::Now().ToInternalValue(), - 100); + ShortcutsBackend::Shortcut shortcut2("BD85DBA2-8C29-49F9-84AE-48E1E90880E0", + ASCIIToUTF16("gle"), GURL("http://www.google.com"), + ASCIIToUTF16("Google"), + AutocompleteMatch::ClassificationsFromString("0,1"), + ASCIIToUTF16("Google"), + AutocompleteMatch::ClassificationsFromString("0,1"), base::Time::Now(), + 100); EXPECT_TRUE(backend_->AddShortcut(shortcut2)); - Shortcut shortcut3("BD85DBA2-8C29-49F9-84AE-48E1E90880E1", - ASCIIToUTF16("sp"), ASCIIToUTF16("http://www.sport.com"), - ASCIIToUTF16("Sports"), ASCIIToUTF16("0,1"), - ASCIIToUTF16("Sport news"), ASCIIToUTF16("0,1"), - base::Time::Now().ToInternalValue(), - 10); + ShortcutsBackend::Shortcut shortcut3("BD85DBA2-8C29-49F9-84AE-48E1E90880E1", + ASCIIToUTF16("sp"), GURL("http://www.sport.com"), ASCIIToUTF16("Sports"), + AutocompleteMatch::ClassificationsFromString("0,1"), + ASCIIToUTF16("Sport news"), + AutocompleteMatch::ClassificationsFromString("0,1"), base::Time::Now(), + 10); EXPECT_TRUE(backend_->AddShortcut(shortcut3)); - Shortcut shortcut4("BD85DBA2-8C29-49F9-84AE-48E1E90880E2", - ASCIIToUTF16("mov"), ASCIIToUTF16("http://www.film.com"), - ASCIIToUTF16("Movies"), ASCIIToUTF16("0,1"), - ASCIIToUTF16("Movie news"), ASCIIToUTF16("0,1"), - base::Time::Now().ToInternalValue(), - 10); + ShortcutsBackend::Shortcut shortcut4("BD85DBA2-8C29-49F9-84AE-48E1E90880E2", + ASCIIToUTF16("mov"), GURL("http://www.film.com"), ASCIIToUTF16("Movies"), + AutocompleteMatch::ClassificationsFromString("0,1"), + ASCIIToUTF16("Movie news"), + AutocompleteMatch::ClassificationsFromString("0,1"), base::Time::Now(), + 10); EXPECT_TRUE(backend_->AddShortcut(shortcut4)); - const ShortcutMap& shortcuts = backend_->shortcuts_map(); + const ShortcutsBackend::ShortcutMap& shortcuts = backend_->shortcuts_map(); ASSERT_EQ(4U, shortcuts.size()); EXPECT_EQ(shortcut1.id, shortcuts.find(shortcut1.text)->second.id); diff --git a/chrome/browser/history/shortcuts_database.cc b/chrome/browser/history/shortcuts_database.cc index 877c28b..035d82b 100644 --- a/chrome/browser/history/shortcuts_database.cc +++ b/chrome/browser/history/shortcuts_database.cc @@ -9,58 +9,35 @@ #include <vector> #include "base/logging.h" -#include "base/string_number_conversions.h" #include "base/stringprintf.h" #include "base/time.h" -#include "base/utf_string_conversions.h" -#include "chrome/browser/autocomplete/shortcuts_provider.h" #include "chrome/common/guid.h" #include "sql/statement.h" -using base::Time; - namespace { // Using define instead of const char, so I could use ## in the statements. #define kShortcutsDBName "omni_box_shortcuts" -// The maximum length allowed for form data. -const size_t kMaxDataLength = 2048; // 2K is a hard limit on URLs URI. - -string16 LimitDataSize(const string16& data) { - if (data.size() > kMaxDataLength) - return data.substr(0, kMaxDataLength); - - return data; -} - -void BindShortcutToStatement(const shortcuts_provider::Shortcut& shortcut, - sql::Statement* s) { +void BindShortcutToStatement( + const history::ShortcutsBackend::Shortcut& shortcut, + sql::Statement* s) { DCHECK(guid::IsValidGUID(shortcut.id)); s->BindString(0, shortcut.id); - s->BindString16(1, LimitDataSize(shortcut.text)); - s->BindString16(2, LimitDataSize(UTF8ToUTF16(shortcut.url.spec()))); - s->BindString16(3, LimitDataSize(shortcut.contents)); - s->BindString16(4, LimitDataSize(shortcut.contents_class_as_str())); - s->BindString16(5, LimitDataSize(shortcut.description)); - s->BindString16(6, LimitDataSize(shortcut.description_class_as_str())); + s->BindString16(1, shortcut.text); + s->BindString(2, shortcut.url.spec()); + s->BindString16(3, shortcut.contents); + s->BindString(4, + AutocompleteMatch::ClassificationsToString(shortcut.contents_class)); + s->BindString16(5, shortcut.description); + s->BindString(6, + AutocompleteMatch::ClassificationsToString(shortcut.description_class)); s->BindInt64(7, shortcut.last_access_time.ToInternalValue()); s->BindInt(8, shortcut.number_of_hits); } -shortcuts_provider::Shortcut ShortcutFromStatement(const sql::Statement& s) { - return shortcuts_provider::Shortcut(s.ColumnString(0), - s.ColumnString16(1), - s.ColumnString16(2), - s.ColumnString16(3), - s.ColumnString16(4), - s.ColumnString16(5), - s.ColumnString16(6), - s.ColumnInt64(7), - s.ColumnInt(8)); -} - -bool DeleteShortcut(const char* field_name, const std::string& id, +bool DeleteShortcut(const char* field_name, + const std::string& id, sql::Connection& db) { sql::Statement s(db.GetUniqueStatement( base::StringPrintf("DELETE FROM %s WHERE %s = ?", kShortcutsDBName, @@ -104,7 +81,7 @@ bool ShortcutsDatabase::Init() { } bool ShortcutsDatabase::AddShortcut( - const shortcuts_provider::Shortcut& shortcut) { + const ShortcutsBackend::Shortcut& shortcut) { sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE, "INSERT INTO " kShortcutsDBName " (id, text, url, contents, contents_class, description," @@ -116,7 +93,7 @@ bool ShortcutsDatabase::AddShortcut( } bool ShortcutsDatabase::UpdateShortcut( - const shortcuts_provider::Shortcut& shortcut) { + const ShortcutsBackend::Shortcut& shortcut) { sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE, "UPDATE " kShortcutsDBName " " "SET id=?, text=?, url=?, contents=?, contents_class=?," @@ -158,8 +135,7 @@ bool ShortcutsDatabase::DeleteAllShortcuts() { } // Loads all of the shortcuts. -bool ShortcutsDatabase::LoadShortcuts( - std::map<std::string, shortcuts_provider::Shortcut>* shortcuts) { +bool ShortcutsDatabase::LoadShortcuts(GuidToShortcutMap* shortcuts) { DCHECK(shortcuts); sql::Statement s(db_.GetCachedStatement(SQL_FROM_HERE, "SELECT id, text, url, contents, contents_class, " @@ -172,7 +148,12 @@ bool ShortcutsDatabase::LoadShortcuts( shortcuts->clear(); while (s.Step()) { shortcuts->insert(std::make_pair(s.ColumnString(0), - ShortcutFromStatement(s))); + ShortcutsBackend::Shortcut(s.ColumnString(0), s.ColumnString16(1), + GURL(s.ColumnString(2)), s.ColumnString16(3), + AutocompleteMatch::ClassificationsFromString(s.ColumnString(4)), + s.ColumnString16(5), + AutocompleteMatch::ClassificationsFromString(s.ColumnString(6)), + base::Time::FromInternalValue(s.ColumnInt64(7)), s.ColumnInt(8)))); } return true; } diff --git a/chrome/browser/history/shortcuts_database.h b/chrome/browser/history/shortcuts_database.h index 4cd1fe5..2d73d22 100644 --- a/chrome/browser/history/shortcuts_database.h +++ b/chrome/browser/history/shortcuts_database.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -14,7 +14,7 @@ #include "base/gtest_prod_util.h" #include "base/memory/ref_counted.h" #include "base/string16.h" -#include "chrome/browser/autocomplete/shortcuts_provider_shortcut.h" +#include "chrome/browser/history/shortcuts_backend.h" #include "googleurl/src/gurl.h" #include "sql/connection.h" @@ -40,16 +40,18 @@ namespace history { // number_of_hits Number of times that the entry has been selected. class ShortcutsDatabase : public base::RefCountedThreadSafe<ShortcutsDatabase> { public: + typedef std::map<std::string, ShortcutsBackend::Shortcut> GuidToShortcutMap; + explicit ShortcutsDatabase(const FilePath& folder_path); virtual ~ShortcutsDatabase(); bool Init(); // Adds the ShortcutsProvider::Shortcut to the database. - bool AddShortcut(const shortcuts_provider::Shortcut& shortcut); + bool AddShortcut(const ShortcutsBackend::Shortcut& shortcut); // Updates timing and selection count for the ShortcutsProvider::Shortcut. - bool UpdateShortcut(const shortcuts_provider::Shortcut& shortcut); + bool UpdateShortcut(const ShortcutsBackend::Shortcut& shortcut); // Deletes the ShortcutsProvider::Shortcuts with the id. bool DeleteShortcutsWithIds(const std::vector<std::string>& shortcut_ids); @@ -61,8 +63,7 @@ class ShortcutsDatabase : public base::RefCountedThreadSafe<ShortcutsDatabase> { bool DeleteAllShortcuts(); // Loads all of the shortcuts. - bool LoadShortcuts( - std::map<std::string, shortcuts_provider::Shortcut>* shortcuts); + bool LoadShortcuts(GuidToShortcutMap* shortcuts); private: // Ensures that the table is present. diff --git a/chrome/browser/history/shortcuts_database_unittest.cc b/chrome/browser/history/shortcuts_database_unittest.cc index 216df83..135ee19 100644 --- a/chrome/browser/history/shortcuts_database_unittest.cc +++ b/chrome/browser/history/shortcuts_database_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 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. @@ -8,18 +8,12 @@ #include "base/stringprintf.h" #include "base/time.h" #include "base/utf_string_conversions.h" -#include "chrome/browser/autocomplete/shortcuts_provider_shortcut.h" #include "chrome/browser/history/shortcuts_database.h" #include "chrome/common/guid.h" #include "sql/statement.h" #include "testing/gtest/include/gtest/gtest.h" -using base::Time; -using base::TimeDelta; -using shortcuts_provider::Shortcut; -using shortcuts_provider::ShortcutMap; - namespace history { struct ShortcutsDatabaseTestInfo { @@ -54,7 +48,8 @@ class ShortcutsDatabaseTest : public testing::Test { void ClearDB(); size_t CountRecords() const; - Shortcut ShortcutFromTestInfo(const ShortcutsDatabaseTestInfo& info); + ShortcutsBackend::Shortcut ShortcutFromTestInfo( + const ShortcutsDatabaseTestInfo& info); void AddAll(); @@ -86,18 +81,15 @@ size_t ShortcutsDatabaseTest::CountRecords() const { return static_cast<size_t>(s.ColumnInt(0)); } -Shortcut ShortcutsDatabaseTest::ShortcutFromTestInfo( +ShortcutsBackend::Shortcut ShortcutsDatabaseTest::ShortcutFromTestInfo( const ShortcutsDatabaseTestInfo& info) { - Time visit_time = Time::Now() - TimeDelta::FromDays(info.days_from_now); - return Shortcut(info.guid, - ASCIIToUTF16(info.title), - ASCIIToUTF16(info.url), - ASCIIToUTF16(info.contents), - ASCIIToUTF16(info.contents_class), - ASCIIToUTF16(info.description), - ASCIIToUTF16(info.description_class), - visit_time.ToInternalValue(), - info.typed_count); + return ShortcutsBackend::Shortcut(info.guid, ASCIIToUTF16(info.title), + GURL(info.url), ASCIIToUTF16(info.contents), + AutocompleteMatch::ClassificationsFromString(info.contents_class), + ASCIIToUTF16(info.description), + AutocompleteMatch::ClassificationsFromString(info.description_class), + base::Time::Now() - base::TimeDelta::FromDays(info.days_from_now), + info.typed_count); } void ShortcutsDatabaseTest::AddAll() { @@ -121,12 +113,14 @@ TEST_F(ShortcutsDatabaseTest, AddShortcut) { TEST_F(ShortcutsDatabaseTest, UpdateShortcut) { AddAll(); - Shortcut shortcut(ShortcutFromTestInfo(shortcut_test_db[1])); + ShortcutsBackend::Shortcut shortcut( + ShortcutFromTestInfo(shortcut_test_db[1])); shortcut.contents = ASCIIToUTF16("gro.todhsals"); EXPECT_TRUE(db_->UpdateShortcut(shortcut)); - std::map<std::string, Shortcut> shortcuts; + ShortcutsDatabase::GuidToShortcutMap shortcuts; EXPECT_TRUE(db_->LoadShortcuts(&shortcuts)); - std::map<std::string, Shortcut>::iterator it = shortcuts.find(shortcut.id); + ShortcutsDatabase::GuidToShortcutMap::iterator it = + shortcuts.find(shortcut.id); EXPECT_TRUE(it != shortcuts.end()); EXPECT_TRUE(it->second.contents == shortcut.contents); } @@ -139,10 +133,10 @@ TEST_F(ShortcutsDatabaseTest, DeleteShortcutsWithIds) { EXPECT_TRUE(db_->DeleteShortcutsWithIds(shortcut_ids)); EXPECT_EQ(arraysize(shortcut_test_db) - 2, CountRecords()); - std::map<std::string, Shortcut> shortcuts; + ShortcutsDatabase::GuidToShortcutMap shortcuts; EXPECT_TRUE(db_->LoadShortcuts(&shortcuts)); - std::map<std::string, Shortcut>::iterator it = + ShortcutsDatabase::GuidToShortcutMap::iterator it = shortcuts.find(shortcut_test_db[0].guid); EXPECT_TRUE(it == shortcuts.end()); @@ -159,10 +153,10 @@ TEST_F(ShortcutsDatabaseTest, DeleteShortcutsWithUrl) { EXPECT_TRUE(db_->DeleteShortcutsWithUrl("http://slashdot.org/")); EXPECT_EQ(arraysize(shortcut_test_db) - 2, CountRecords()); - std::map<std::string, Shortcut> shortcuts; + ShortcutsDatabase::GuidToShortcutMap shortcuts; EXPECT_TRUE(db_->LoadShortcuts(&shortcuts)); - std::map<std::string, Shortcut>::iterator it = + ShortcutsDatabase::GuidToShortcutMap::iterator it = shortcuts.find(shortcut_test_db[0].guid); EXPECT_TRUE(it != shortcuts.end()); @@ -175,7 +169,7 @@ TEST_F(ShortcutsDatabaseTest, DeleteShortcutsWithUrl) { TEST_F(ShortcutsDatabaseTest, LoadShortcuts) { AddAll(); - std::map<std::string, Shortcut> shortcuts; + ShortcutsDatabase::GuidToShortcutMap shortcuts; EXPECT_TRUE(db_->LoadShortcuts(&shortcuts)); for (size_t i = 0; i < arraysize(shortcut_test_db); ++i) { @@ -187,7 +181,7 @@ TEST_F(ShortcutsDatabaseTest, LoadShortcuts) { TEST_F(ShortcutsDatabaseTest, DeleteAllShortcuts) { AddAll(); - std::map<std::string, Shortcut> shortcuts; + ShortcutsDatabase::GuidToShortcutMap shortcuts; EXPECT_TRUE(db_->LoadShortcuts(&shortcuts)); EXPECT_EQ(arraysize(shortcut_test_db), shortcuts.size()); EXPECT_TRUE(db_->DeleteAllShortcuts()); diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 6aa16dc..b841751 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -138,8 +138,6 @@ 'browser/autocomplete/search_provider.h', 'browser/autocomplete/shortcuts_provider.cc', 'browser/autocomplete/shortcuts_provider.h', - 'browser/autocomplete/shortcuts_provider_shortcut.cc', - 'browser/autocomplete/shortcuts_provider_shortcut.h', 'browser/autocomplete_history_manager.cc', 'browser/autocomplete_history_manager.h', 'browser/autofill/address.cc', |