diff options
46 files changed, 1367 insertions, 1407 deletions
diff --git a/chrome/browser/autocomplete/autocomplete_unittest.cc b/chrome/browser/autocomplete/autocomplete_unittest.cc index 9e45132..9caf94b 100644 --- a/chrome/browser/autocomplete/autocomplete_unittest.cc +++ b/chrome/browser/autocomplete/autocomplete_unittest.cc @@ -182,8 +182,9 @@ void AutocompleteProviderTest:: profile_.CreateTemplateURLService(); // Reset the default TemplateURL. - TemplateURL* default_t_url = new TemplateURL(); - default_t_url->SetURL("http://defaultturl/{searchTerms}"); + TemplateURLData data; + data.SetURL("http://defaultturl/{searchTerms}"); + TemplateURL* default_t_url = new TemplateURL(data); TemplateURLService* turl_model = TemplateURLServiceFactory::GetForProfile(&profile_); turl_model->Add(default_t_url); @@ -192,10 +193,10 @@ void AutocompleteProviderTest:: ASSERT_NE(0, default_provider_id); // Create another TemplateURL for KeywordProvider. - TemplateURL* keyword_t_url = new TemplateURL(); - keyword_t_url->set_short_name(ASCIIToUTF16("k")); - keyword_t_url->set_keyword(ASCIIToUTF16("k")); - keyword_t_url->SetURL("http://keyword/{searchTerms}"); + data.short_name = ASCIIToUTF16("k"); + data.SetKeyword(ASCIIToUTF16("k")); + data.SetURL("http://keyword/{searchTerms}"); + TemplateURL* keyword_t_url = new TemplateURL(data); turl_model->Add(keyword_t_url); ASSERT_NE(0, keyword_t_url->id()); @@ -223,18 +224,19 @@ void AutocompleteProviderTest:: TemplateURLServiceFactory::GetForProfile(&profile_); // Create a TemplateURL for KeywordProvider. - TemplateURL* keyword_t_url = new TemplateURL(); - keyword_t_url->set_short_name(ASCIIToUTF16("foo.com")); - keyword_t_url->set_keyword(ASCIIToUTF16("foo.com")); - keyword_t_url->SetURL("http://foo.com/{searchTerms}"); + TemplateURLData data; + data.short_name = ASCIIToUTF16("foo.com"); + data.SetKeyword(ASCIIToUTF16("foo.com")); + data.SetURL("http://foo.com/{searchTerms}"); + TemplateURL* keyword_t_url = new TemplateURL(data); turl_model->Add(keyword_t_url); ASSERT_NE(0, keyword_t_url->id()); // Create another TemplateURL for KeywordProvider. - keyword_t_url = new TemplateURL(); - keyword_t_url->set_short_name(ASCIIToUTF16("bar.com")); - keyword_t_url->set_keyword(ASCIIToUTF16("bar.com")); - keyword_t_url->SetURL("http://bar.com/{searchTerms}"); + data.short_name = ASCIIToUTF16("bar.com"); + data.SetKeyword(ASCIIToUTF16("bar.com")); + data.SetURL("http://bar.com/{searchTerms}"); + keyword_t_url = new TemplateURL(data); turl_model->Add(keyword_t_url); ASSERT_NE(0, keyword_t_url->id()); diff --git a/chrome/browser/autocomplete/keyword_provider_unittest.cc b/chrome/browser/autocomplete/keyword_provider_unittest.cc index 6fceace..d2a4fff 100644 --- a/chrome/browser/autocomplete/keyword_provider_unittest.cc +++ b/chrome/browser/autocomplete/keyword_provider_unittest.cc @@ -201,12 +201,12 @@ TEST_F(KeywordProviderTest, DISABLED_Description) { } TEST_F(KeywordProviderTest, AddKeyword) { - TemplateURL* template_url = new TemplateURL(); + TemplateURLData data; + data.short_name = ASCIIToUTF16("Test"); string16 keyword(ASCIIToUTF16("foo")); - std::string url("http://www.google.com/foo?q={searchTerms}"); - template_url->SetURL(url); - template_url->set_keyword(keyword); - template_url->set_short_name(ASCIIToUTF16("Test")); + data.SetKeyword(keyword); + data.SetURL("http://www.google.com/foo?q={searchTerms}"); + TemplateURL* template_url = new TemplateURL(data); model_->Add(template_url); ASSERT_TRUE(template_url == model_->GetTemplateURLForKeyword(keyword)); } diff --git a/chrome/browser/autocomplete/search_provider.h b/chrome/browser/autocomplete/search_provider.h index 9c5d26a..5af9882 100644 --- a/chrome/browser/autocomplete/search_provider.h +++ b/chrome/browser/autocomplete/search_provider.h @@ -90,7 +90,12 @@ class SearchProvider : public AutocompleteProvider, // . The keyword provider. This is used if the user has typed in a keyword. class Providers { public: - Providers() : default_provider_(NULL), keyword_provider_(NULL) {} + Providers() + : cached_default_provider_(TemplateURLData()), + cached_keyword_provider_(TemplateURLData()), + default_provider_(NULL), + keyword_provider_(NULL) { + } // Returns true if the specified providers match the two providers managed // by this class. diff --git a/chrome/browser/autocomplete/search_provider_unittest.cc b/chrome/browser/autocomplete/search_provider_unittest.cc index d10d466..f2e8bba 100644 --- a/chrome/browser/autocomplete/search_provider_unittest.cc +++ b/chrome/browser/autocomplete/search_provider_unittest.cc @@ -121,10 +121,11 @@ void SearchProviderTest::SetUp() { turl_model->Load(); // Reset the default TemplateURL. - default_t_url_ = new TemplateURL(); - default_t_url_->SetURL("http://defaultturl/{searchTerms}"); - default_t_url_->SetSuggestionsURL("http://defaultturl2/{searchTerms}"); - default_t_url_->set_short_name(ASCIIToUTF16("t")); + TemplateURLData data; + data.short_name = ASCIIToUTF16("t"); + data.SetURL("http://defaultturl/{searchTerms}"); + data.suggestions_url = "http://defaultturl2/{searchTerms}"; + default_t_url_ = new TemplateURL(data); turl_model->Add(default_t_url_); turl_model->SetDefaultSearchProvider(default_t_url_); TemplateURLID default_provider_id = default_t_url_->id(); @@ -134,11 +135,11 @@ void SearchProviderTest::SetUp() { term1_url_ = AddSearchToHistory(default_t_url_, term1_, 1); // Create another TemplateURL. - keyword_t_url_ = new TemplateURL(); - keyword_t_url_->set_keyword(ASCIIToUTF16("k")); - keyword_t_url_->set_short_name(ASCIIToUTF16("k")); - keyword_t_url_->SetURL("http://keyword/{searchTerms}"); - keyword_t_url_->SetSuggestionsURL("http://suggest_keyword/{searchTerms}"); + data.short_name = ASCIIToUTF16("k"); + data.SetKeyword(ASCIIToUTF16("k")); + data.SetURL("http://keyword/{searchTerms}"); + data.suggestions_url = "http://suggest_keyword/{searchTerms}"; + keyword_t_url_ = new TemplateURL(data); turl_model->Add(keyword_t_url_); ASSERT_NE(0, keyword_t_url_->id()); diff --git a/chrome/browser/importer/firefox2_importer.cc b/chrome/browser/importer/firefox2_importer.cc index fe6cbb1..91bb725 100644 --- a/chrome/browser/importer/firefox2_importer.cc +++ b/chrome/browser/importer/firefox2_importer.cc @@ -134,13 +134,13 @@ TemplateURL* Firefox2Importer::CreateTemplateURL(const string16& title, if (keyword.empty() || !url.is_valid()) return NULL; - TemplateURL* t_url = new TemplateURL(); + TemplateURLData data; // We set short name by using the title if it exists. // Otherwise, we use the shortcut. - t_url->set_short_name(title.empty() ? keyword : title); - t_url->set_keyword(keyword); - t_url->SetURL(TemplateURLRef::DisplayURLToURLRef(UTF8ToUTF16(url.spec()))); - return t_url; + data.short_name = title.empty() ? keyword : title; + data.SetKeyword(keyword); + data.SetURL(TemplateURLRef::DisplayURLToURLRef(UTF8ToUTF16(url.spec()))); + return new TemplateURL(data); } // static diff --git a/chrome/browser/importer/firefox_importer_utils.cc b/chrome/browser/importer/firefox_importer_utils.cc index 5904940..5cceeb2 100644 --- a/chrome/browser/importer/firefox_importer_utils.cc +++ b/chrome/browser/importer/firefox_importer_utils.cc @@ -200,8 +200,8 @@ void ParseSearchEnginesFromXMLFiles(const std::vector<FilePath>& xml_files, file_iter != xml_files.end(); ++file_iter) { file_util::ReadFileToString(*file_iter, &content); FirefoxURLParameterFilter param_filter; - TemplateURL* template_url = TemplateURLParser::Parse(NULL, content.data(), - content.length(), ¶m_filter); + TemplateURL* template_url = TemplateURLParser::Parse(NULL, true, + content.data(), content.length(), ¶m_filter); if (template_url) { SearchEnginesMap::iterator iter = search_engine_for_url.find(template_url->url()); @@ -216,7 +216,6 @@ void ParseSearchEnginesFromXMLFiles(const std::vector<FilePath>& xml_files, delete iter->second; iter->second = template_url; } - iter->second->set_show_in_default_list(true); if (default_turl == search_engine_for_url.end()) default_turl = iter; } diff --git a/chrome/browser/importer/ie_importer.cc b/chrome/browser/importer/ie_importer.cc index 63823dd..0730fb8 100644 --- a/chrome/browser/importer/ie_importer.cc +++ b/chrome/browser/importer/ie_importer.cc @@ -584,14 +584,13 @@ void IEImporter::ImportSearchEngines() { // First time we see that URL. GURL gurl(url); if (gurl.is_valid()) { - TemplateURL* template_url = new TemplateURL(); - template_url->set_short_name(name); - template_url->SetURL(url); - // Give this a keyword to facilitate tab-to-search, if possible. - template_url->set_keyword(TemplateURLService::GenerateKeyword(gurl, - false)); - template_url->set_show_in_default_list(true); - search_engines_map.insert(std::make_pair(url, template_url)); + TemplateURLData data; + data.short_name = name; + data.SetKeyword(TemplateURLService::GenerateKeyword(gurl, false)); + data.SetURL(url); + data.show_in_default_list = true; + t_iter = search_engines_map.insert(std::make_pair(url, + new TemplateURL(data))).first; } } } diff --git a/chrome/browser/importer/profile_import_process_messages.h b/chrome/browser/importer/profile_import_process_messages.h index ff9407a..ceaf9e0 100644 --- a/chrome/browser/importer/profile_import_process_messages.h +++ b/chrome/browser/importer/profile_import_process_messages.h @@ -189,6 +189,63 @@ struct ParamTraits<history::ImportedFaviconUsage> { } }; // ParamTraits<history::ImportedFaviconUsage +// Traits for TemplateURLData +template <> +struct ParamTraits<TemplateURLData> { + typedef TemplateURLData param_type; + static void Write(Message* m, const param_type& p) { + WriteParam(m, p.short_name); + WriteParam(m, p.raw_keyword()); + WriteParam(m, p.autogenerate_keyword()); + WriteParam(m, p.url()); + WriteParam(m, p.suggestions_url); + WriteParam(m, p.instant_url); + WriteParam(m, p.favicon_url); + WriteParam(m, p.originating_url); + WriteParam(m, p.show_in_default_list); + WriteParam(m, p.safe_for_autoreplace); + WriteParam(m, p.input_encodings); + WriteParam(m, p.id); + WriteParam(m, p.date_created); + WriteParam(m, p.last_modified); + WriteParam(m, p.created_by_policy); + WriteParam(m, p.usage_count); + WriteParam(m, p.prepopulate_id); + WriteParam(m, p.sync_guid); + } + static bool Read(const Message* m, PickleIterator* iter, param_type* p) { + string16 keyword; + bool autogenerate_keyword; + std::string url; + if (!ReadParam(m, iter, &p->short_name) || + !ReadParam(m, iter, &keyword) || + !ReadParam(m, iter, &autogenerate_keyword) || + !ReadParam(m, iter, &url) || + !ReadParam(m, iter, &p->suggestions_url) || + !ReadParam(m, iter, &p->instant_url) || + !ReadParam(m, iter, &p->favicon_url) || + !ReadParam(m, iter, &p->originating_url) || + !ReadParam(m, iter, &p->show_in_default_list) || + !ReadParam(m, iter, &p->safe_for_autoreplace) || + !ReadParam(m, iter, &p->input_encodings) || + !ReadParam(m, iter, &p->id) || + !ReadParam(m, iter, &p->date_created) || + !ReadParam(m, iter, &p->last_modified) || + !ReadParam(m, iter, &p->created_by_policy) || + !ReadParam(m, iter, &p->usage_count) || + !ReadParam(m, iter, &p->prepopulate_id) || + !ReadParam(m, iter, &p->sync_guid)) + return false; + p->SetKeyword(keyword); + p->SetAutogenerateKeyword(autogenerate_keyword); + p->SetURL(url); + return true; + } + static void Log(const param_type& p, std::string* l) { + l->append("<TemplateURLData>"); + } +}; + // Traits for TemplateURL*. // WARNING: These will cause us to allocate a new TemplateURL on the heap on the // receiver side. Any messages using this type must have handlers that are @@ -198,67 +255,13 @@ template <> struct ParamTraits<TemplateURL*> { typedef TemplateURL* param_type; static void Write(Message* m, const param_type& p) { - WriteParam(m, p->short_name()); - WriteParam(m, p->url()); - WriteParam(m, p->suggestions_url()); - WriteParam(m, p->instant_url()); - WriteParam(m, p->originating_url()); - WriteParam(m, p->keyword()); - WriteParam(m, p->autogenerate_keyword()); - WriteParam(m, p->show_in_default_list()); - WriteParam(m, p->safe_for_autoreplace()); - WriteParam(m, p->favicon_url()); - WriteParam(m, p->input_encodings()); - WriteParam(m, p->date_created()); - WriteParam(m, p->last_modified()); - WriteParam(m, p->usage_count()); - WriteParam(m, p->prepopulate_id()); + WriteParam(m, p->data()); } static bool Read(const Message* m, PickleIterator* iter, param_type* p) { - string16 short_name; - std::string url; - std::string suggestions_url; - std::string instant_url; - GURL originating_url; - string16 keyword; - bool autogenerate_keyword; - bool show_in_default_list; - bool safe_for_autoreplace; - GURL favicon_url; - base::Time date_created; - base::Time last_modified; - int usage_count; - int prepopulate_id; - if (!ReadParam(m, iter, &short_name) || - !ReadParam(m, iter, &url) || - !ReadParam(m, iter, &suggestions_url) || - !ReadParam(m, iter, &instant_url) || - !ReadParam(m, iter, &originating_url) || - !ReadParam(m, iter, &keyword) || - !ReadParam(m, iter, &autogenerate_keyword) || - !ReadParam(m, iter, &show_in_default_list) || - !ReadParam(m, iter, &safe_for_autoreplace) || - !ReadParam(m, iter, &favicon_url) || - !ReadParam(m, iter, &date_created) || - !ReadParam(m, iter, &last_modified) || - !ReadParam(m, iter, &usage_count) || - !ReadParam(m, iter, &prepopulate_id)) + TemplateURLData data; + if (!ReadParam(m, iter, &data)) return false; - *p = new TemplateURL(); - (*p)->set_short_name(short_name); - (*p)->SetURL(url); - (*p)->SetSuggestionsURL(suggestions_url); - (*p)->SetInstantURL(suggestions_url); - (*p)->set_originating_url(originating_url); - (*p)->set_keyword(keyword); - (*p)->set_autogenerate_keyword(autogenerate_keyword); - (*p)->set_show_in_default_list(show_in_default_list); - (*p)->set_safe_for_autoreplace(safe_for_autoreplace); - (*p)->set_favicon_url(favicon_url); - (*p)->set_date_created(date_created); - (*p)->set_last_modified(last_modified); - (*p)->set_usage_count(usage_count); - (*p)->SetPrepopulateId(prepopulate_id); + *p = new TemplateURL(data); return true; } static void Log(const param_type& p, std::string* l) { diff --git a/chrome/browser/instant/instant_browsertest.cc b/chrome/browser/instant/instant_browsertest.cc index 1c96356..eec6b18 100644 --- a/chrome/browser/instant/instant_browsertest.cc +++ b/chrome/browser/instant/instant_browsertest.cc @@ -68,19 +68,15 @@ class InstantTest : public InProcessBrowserTest { observer.Wait(); } - // TemplateURLService takes ownership of this. - TemplateURL* template_url = new TemplateURL(); - - std::string url = base::StringPrintf( - "http://%s:%d/files/%s?q={searchTerms}", + TemplateURLData data; + data.short_name = ASCIIToUTF16("foo"); + data.SetKeyword(ASCIIToUTF16("foo")); + data.SetURL(base::StringPrintf("http://%s:%d/files/%s?q={searchTerms}", test_server()->host_port_pair().host().c_str(), - test_server()->host_port_pair().port(), - page.c_str()); - template_url->SetURL(url); - template_url->SetInstantURL(url); - template_url->set_keyword(ASCIIToUTF16("foo")); - template_url->set_short_name(ASCIIToUTF16("foo")); - + test_server()->host_port_pair().port(), page.c_str())); + data.instant_url = data.url(); + // TemplateURLService takes ownership of this. + TemplateURL* template_url = new TemplateURL(data); model->Add(template_url); model->SetDefaultSearchProvider(template_url); } diff --git a/chrome/browser/policy/configuration_policy_handler.cc b/chrome/browser/policy/configuration_policy_handler.cc index 238f5df..418a3ee 100644 --- a/chrome/browser/policy/configuration_policy_handler.cc +++ b/chrome/browser/policy/configuration_policy_handler.cc @@ -597,10 +597,10 @@ bool DefaultSearchPolicyHandler::DefaultSearchURLIsValid( *url_value = policies.GetValue(key::kDefaultSearchProviderSearchURL); if (!*url_value || !(*url_value)->GetAsString(url_string)) return false; - TemplateURL t_url; - t_url.SetURL(*url_string); + TemplateURLData data; + data.SetURL(*url_string); SearchTermsData search_terms_data; - return t_url.url_ref().SupportsReplacementUsingTermsData(search_terms_data); + return TemplateURL(data).SupportsReplacementUsingTermsData(search_terms_data); } void DefaultSearchPolicyHandler::EnsureStringPrefExists( diff --git a/chrome/browser/protector/default_search_provider_change_browsertest.cc b/chrome/browser/protector/default_search_provider_change_browsertest.cc index 5a50ce5..4dae17c 100644 --- a/chrome/browser/protector/default_search_provider_change_browsertest.cc +++ b/chrome/browser/protector/default_search_provider_change_browsertest.cc @@ -40,6 +40,20 @@ const std::string http_example_com = "http://example.com/%s"; const string16 example_net = ASCIIToUTF16("Example.net"); const std::string http_example_net = "http://example.net/%s"; +// Convenience function. +TemplateURL* MakeTemplateURL(const string16& short_name, + const string16& keyword, + const std::string& url) { + TemplateURLData data; + data.short_name = short_name; + if (keyword.empty()) + data.SetAutogenerateKeyword(true); + else + data.SetKeyword(keyword); + data.SetURL(url); + return new TemplateURL(data); +} + }; class DefaultSearchProviderChangeTest : public InProcessBrowserTest { @@ -61,19 +75,6 @@ class DefaultSearchProviderChangeTest : public InProcessBrowserTest { EXPECT_CALL(*mock_protector_service_, Shutdown()); } - TemplateURL* MakeTemplateURL(const string16& short_name, - const string16& keyword, - const std::string& search_url) { - TemplateURL* url = new TemplateURL; - url->set_short_name(short_name); - if (keyword.empty()) - url->set_autogenerate_keyword(true); - else - url->set_keyword(keyword); - url->SetURL(search_url); - return url; - } - const TemplateURL* FindTemplateURL(const std::string& search_url) { TemplateURLService::TemplateURLVector urls = turl_service_->GetTemplateURLs(); @@ -85,12 +86,6 @@ class DefaultSearchProviderChangeTest : public InProcessBrowserTest { return NULL; } - // Adds a copy of |turl| that will be owned by TemplateURLService. - void AddCopy(TemplateURL* turl) { - TemplateURL* turl_copy = new TemplateURL(*turl); - turl_service_->Add(turl_copy); - } - void AddAndSetDefault(TemplateURL* turl) { turl_service_->Add(turl); turl_service_->SetDefaultSearchProvider(turl); @@ -158,7 +153,7 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, BackupValid) { int current_histogram_id = protector::GetSearchProviderHistogramID(current_url); - AddCopy(backup_url); + turl_service_->Add(new TemplateURL(backup_url->data())); AddAndSetDefault(current_url); scoped_ptr<BaseSettingChange> change( @@ -211,7 +206,7 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, BackupValidLongNames) { { // Backup name too long. - AddCopy(backup_url_long); + turl_service_->Add(new TemplateURL(backup_url_long->data())); AddAndSetDefault(current_url); scoped_ptr<BaseSettingChange> change( @@ -228,7 +223,7 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, BackupValidLongNames) { { // Current name too long. - AddCopy(backup_url); + turl_service_->Add(new TemplateURL(backup_url->data())); AddAndSetDefault(current_url_long); scoped_ptr<BaseSettingChange> change( @@ -360,7 +355,7 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, MakeTemplateURL(example_info, ASCIIToUTF16("a"), http_example_info); int backup_histogram_id = protector::GetSearchProviderHistogramID(backup_url); - AddCopy(backup_url); + turl_service_->Add(new TemplateURL(backup_url->data())); turl_service_->SetDefaultSearchProvider(NULL); scoped_ptr<BaseSettingChange> change( @@ -486,7 +481,7 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, TemplateURL* new_url = MakeTemplateURL(example_net, ASCIIToUTF16("c"), http_example_net); - AddCopy(backup_url); + turl_service_->Add(new TemplateURL(backup_url->data())); AddAndSetDefault(current_url); scoped_ptr<BaseSettingChange> change( @@ -512,7 +507,7 @@ IN_PROC_BROWSER_TEST_F(DefaultSearchProviderChangeTest, TemplateURL* current_url = MakeTemplateURL(example_com, ASCIIToUTF16("b"), http_example_com); - AddCopy(backup_url); + turl_service_->Add(new TemplateURL(backup_url->data())); AddAndSetDefault(current_url); scoped_ptr<BaseSettingChange> change( diff --git a/chrome/browser/search_engines/search_host_to_urls_map.cc b/chrome/browser/search_engines/search_host_to_urls_map.cc index 3810f06..de42cc3 100644 --- a/chrome/browser/search_engines/search_host_to_urls_map.cc +++ b/chrome/browser/search_engines/search_host_to_urls_map.cc @@ -59,23 +59,6 @@ void SearchHostToURLsMap::Remove(const TemplateURL* template_url) { host_to_urls_map_.erase(host_to_urls_map_.find(host)); } -void SearchHostToURLsMap::Update(const TemplateURL* existing_turl, - const TemplateURL& new_values, - const SearchTermsData& search_terms_data) { - DCHECK(initialized_); - DCHECK(existing_turl); - - Remove(existing_turl); - - // Use the information from new_values but preserve existing_turl's id. - TemplateURLID previous_id = existing_turl->id(); - TemplateURL* modifiable_turl = const_cast<TemplateURL*>(existing_turl); - *modifiable_turl = new_values; - modifiable_turl->set_id(previous_id); - - Add(existing_turl, search_terms_data); -} - void SearchHostToURLsMap::UpdateGoogleBaseURLs( const SearchTermsData& search_terms_data) { DCHECK(initialized_); diff --git a/chrome/browser/search_engines/search_host_to_urls_map.h b/chrome/browser/search_engines/search_host_to_urls_map.h index e4692fb..cd21a87 100644 --- a/chrome/browser/search_engines/search_host_to_urls_map.h +++ b/chrome/browser/search_engines/search_host_to_urls_map.h @@ -1,4 +1,4 @@ -// Copyright (c) 2010 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. @@ -38,13 +38,6 @@ class SearchHostToURLsMap { // Removes the TemplateURL from the lookup. void Remove(const TemplateURL* template_url); - // Updates information in an existing TemplateURL. Note: Using Remove and - // then Add separately would lead to race conditions in reading because the - // lock would be released inbetween the calls. - void Update(const TemplateURL* existing_turl, - const TemplateURL& new_values, - const SearchTermsData& search_terms_data); - // Updates all search providers which have a google base url. void UpdateGoogleBaseURLs(const SearchTermsData& search_terms_data); diff --git a/chrome/browser/search_engines/search_host_to_urls_map_unittest.cc b/chrome/browser/search_engines/search_host_to_urls_map_unittest.cc index 3ac1caf..04c0d00 100644 --- a/chrome/browser/search_engines/search_host_to_urls_map_unittest.cc +++ b/chrome/browser/search_engines/search_host_to_urls_map_unittest.cc @@ -29,11 +29,11 @@ class SearchHostToURLsMapTest : public testing::Test { void SearchHostToURLsMapTest::SetUp() { // Add some entries to the search host map. host_ = "www.unittest.com"; - t_urls_[0].reset(new TemplateURL()); - t_urls_[0]->SetURL("http://" + host_ + "/path1"); - t_urls_[1].reset(new TemplateURL()); - t_urls_[1]->SetURL("http://" + host_ + "/path2"); - + TemplateURLData data; + data.SetURL("http://" + host_ + "/path1"); + t_urls_[0].reset(new TemplateURL(data)); + data.SetURL("http://" + host_ + "/path2"); + t_urls_[1].reset(new TemplateURL(data)); std::vector<const TemplateURL*> template_urls; template_urls.push_back(t_urls_[0].get()); template_urls.push_back(t_urls_[1].get()); @@ -45,8 +45,9 @@ void SearchHostToURLsMapTest::SetUp() { TEST_F(SearchHostToURLsMapTest, Add) { std::string new_host = "example.com"; - TemplateURL new_t_url; - new_t_url.SetURL("http://" + new_host + "/"); + TemplateURLData data; + data.SetURL("http://" + new_host + "/"); + TemplateURL new_t_url(data); UIThreadSearchTermsData search_terms_data; provider_map_->Add(&new_t_url, search_terms_data); @@ -70,18 +71,6 @@ TEST_F(SearchHostToURLsMapTest, Remove) { ASSERT_EQ(1, url_count); } -TEST_F(SearchHostToURLsMapTest, Update) { - std::string new_host = "example.com"; - TemplateURL new_values; - new_values.SetURL("http://" + new_host + "/"); - - UIThreadSearchTermsData search_terms_data; - provider_map_->Update(t_urls_[0].get(), new_values, search_terms_data); - - ASSERT_EQ(t_urls_[0].get(), provider_map_->GetTemplateURLForHost(new_host)); - ASSERT_EQ(t_urls_[1].get(), provider_map_->GetTemplateURLForHost(host_)); -} - TEST_F(SearchHostToURLsMapTest, UpdateGoogleBaseURLs) { UIThreadSearchTermsData search_terms_data; std::string google_base_url = "google.com"; @@ -89,8 +78,9 @@ TEST_F(SearchHostToURLsMapTest, UpdateGoogleBaseURLs) { new std::string("http://" + google_base_url +"/")); // Add in a url with the templated Google base url. - TemplateURL new_t_url; - new_t_url.SetURL("{google:baseURL}?q={searchTerms}"); + TemplateURLData data; + data.SetURL("{google:baseURL}?q={searchTerms}"); + TemplateURL new_t_url(data); provider_map_->Add(&new_t_url, search_terms_data); ASSERT_EQ(&new_t_url, provider_map_->GetTemplateURLForHost(google_base_url)); diff --git a/chrome/browser/search_engines/search_provider_install_data_unittest.cc b/chrome/browser/search_engines/search_provider_install_data_unittest.cc index 3d8c584..1d2ccbd 100644 --- a/chrome/browser/search_engines/search_provider_install_data_unittest.cc +++ b/chrome/browser/search_engines/search_provider_install_data_unittest.cc @@ -232,10 +232,11 @@ void SearchProviderInstallDataTest::SimulateDefaultSearchIsManaged( TemplateURL* SearchProviderInstallDataTest::AddNewTemplateURL( const std::string& url, const string16& keyword) { - TemplateURL* t_url = new TemplateURL(); - t_url->set_short_name(keyword); - t_url->set_keyword(keyword); - t_url->SetURL(url); + TemplateURLData data; + data.short_name = keyword; + data.SetKeyword(keyword); + data.SetURL(url); + TemplateURL* t_url = new TemplateURL(data); util_.model()->Add(t_url); return t_url; } diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc index b00ffc4..06f3fdf 100644 --- a/chrome/browser/search_engines/template_url.cc +++ b/chrome/browser/search_engines/template_url.cc @@ -22,8 +22,6 @@ #include "net/base/escape.h" #include "ui/base/l10n/l10n_util.h" -using content::UserMetricsAction; - // TODO(pastarmovj): Remove google_update_settings and user_metrics when the // CollectRLZMetrics function is not needed anymore. @@ -568,77 +566,90 @@ void TemplateURLRef::ParseHostAndSearchTermKey( } +// TemplateURLData ------------------------------------------------------------ + +TemplateURLData::TemplateURLData() + : show_in_default_list(false), + safe_for_autoreplace(false), + id(0), + date_created(base::Time::Now()), + last_modified(base::Time::Now()), + created_by_policy(false), + usage_count(0), + prepopulate_id(0), + sync_guid(guid::GenerateGUID()), + autogenerate_keyword_(false), + keyword_generated_(false) { +} + +TemplateURLData::~TemplateURLData() { +} + +void TemplateURLData::SetKeyword(const string16& keyword) const { + // Case sensitive keyword matching is confusing. As such, we force all + // keywords to be lower case. + keyword_ = base::i18n::ToLower(keyword); + autogenerate_keyword_ = false; +} + +const string16& TemplateURLData::keyword(const TemplateURL* t_url) const { + EnsureKeyword(t_url); + return keyword_; +} + +void TemplateURLData::SetAutogenerateKeyword(bool autogenerate_keyword) const { + autogenerate_keyword_ = autogenerate_keyword; + if (autogenerate_keyword_) { + keyword_.clear(); + keyword_generated_ = false; + } +} + +void TemplateURLData::EnsureKeyword(const TemplateURL* t_url) const { + if (autogenerate_keyword_ && !keyword_generated_) { + // Generate a keyword and cache it. + keyword_ = base::i18n::ToLower(TemplateURLService::GenerateKeyword( + TemplateURLService::GenerateSearchURL(t_url).GetWithEmptyPath(), true)); + keyword_generated_ = true; + } +} + +void TemplateURLData::SetURL(const std::string& url) { + url_ = url; +} + + // TemplateURL ---------------------------------------------------------------- -TemplateURL::TemplateURL() - : autogenerate_keyword_(false), - keyword_generated_(false), - show_in_default_list_(false), - safe_for_autoreplace_(false), - id_(0), - date_created_(base::Time::Now()), - last_modified_(base::Time::Now()), - created_by_policy_(false), - usage_count_(0), - prepopulate_id_(0), - sync_guid_(guid::GenerateGUID()), +TemplateURL::TemplateURL(const TemplateURLData& data) + : data_(data), url_ref_(ALLOW_THIS_IN_INITIALIZER_LIST(this), TemplateURLRef::SEARCH), suggestions_url_ref_(ALLOW_THIS_IN_INITIALIZER_LIST(this), TemplateURLRef::SUGGEST), instant_url_ref_(ALLOW_THIS_IN_INITIALIZER_LIST(this), TemplateURLRef::INSTANT) { + SetPrepopulateId(data_.prepopulate_id); } TemplateURL::TemplateURL(const TemplateURL& other) - : short_name_(other.short_name_), - url_(other.url_), - suggestions_url_(other.suggestions_url_), - instant_url_(other.instant_url_), - originating_url_(other.originating_url_), - keyword_(other.keyword_), - autogenerate_keyword_(other.autogenerate_keyword_), - keyword_generated_(other.keyword_generated_), - show_in_default_list_(other.show_in_default_list_), - safe_for_autoreplace_(other.safe_for_autoreplace_), - favicon_url_(other.favicon_url_), - input_encodings_(other.input_encodings_), - id_(other.id_), - date_created_(other.date_created_), - last_modified_(other.last_modified_), - created_by_policy_(other.created_by_policy_), - usage_count_(other.usage_count_), - sync_guid_(other.sync_guid_), + : data_(other.data_), url_ref_(ALLOW_THIS_IN_INITIALIZER_LIST(this), TemplateURLRef::SEARCH), suggestions_url_ref_(ALLOW_THIS_IN_INITIALIZER_LIST(this), TemplateURLRef::SUGGEST), instant_url_ref_(ALLOW_THIS_IN_INITIALIZER_LIST(this), TemplateURLRef::INSTANT) { - CopyURLRefs(other); + SetPrepopulateId(data_.prepopulate_id); } TemplateURL& TemplateURL::operator=(const TemplateURL& other) { if (this == &other) return *this; - short_name_ = other.short_name_; - url_ = other.url_; - suggestions_url_ = other.suggestions_url_; - instant_url_ = other.instant_url_; - originating_url_ = other.originating_url_; - keyword_ = other.keyword_; - autogenerate_keyword_ = other.autogenerate_keyword_; - keyword_generated_ = other.keyword_generated_; - show_in_default_list_ = other.show_in_default_list_; - safe_for_autoreplace_ = other.safe_for_autoreplace_; - favicon_url_ = other.favicon_url_; - input_encodings_ = other.input_encodings_; - id_ = other.id_; - date_created_ = other.date_created_; - last_modified_ = other.last_modified_; - created_by_policy_ = other.created_by_policy_; - usage_count_ = other.usage_count_; - sync_guid_ = other.sync_guid_; - CopyURLRefs(other); + data_ = other.data_; + url_ref_.InvalidateCachedValues(); + suggestions_url_ref_.InvalidateCachedValues(); + instant_url_ref_.InvalidateCachedValues(); + SetPrepopulateId(data_.prepopulate_id); return *this; } @@ -662,60 +673,41 @@ GURL TemplateURL::GenerateFaviconURL(const GURL& url) { } string16 TemplateURL::AdjustedShortNameForLocaleDirection() const { - string16 bidi_safe_short_name = short_name_; + string16 bidi_safe_short_name = data_.short_name; base::i18n::AdjustStringForLocaleDirection(&bidi_safe_short_name); return bidi_safe_short_name; } -void TemplateURL::SetURL(const std::string& url) { - url_ = url; - url_ref_.InvalidateCachedValues(); -} - -void TemplateURL::SetSuggestionsURL(const std::string& url) { - suggestions_url_ = url; - suggestions_url_ref_.InvalidateCachedValues(); -} - -void TemplateURL::SetInstantURL(const std::string& url) { - instant_url_ = url; - instant_url_ref_.InvalidateCachedValues(); +bool TemplateURL::ShowInDefaultList() const { + return data_.show_in_default_list && url_ref_.SupportsReplacement(); } -void TemplateURL::set_keyword(const string16& keyword) { - // Case sensitive keyword matching is confusing. As such, we force all - // keywords to be lower case. - keyword_ = base::i18n::ToLower(keyword); - autogenerate_keyword_ = false; +bool TemplateURL::SupportsReplacement() const { + UIThreadSearchTermsData search_terms_data; + return SupportsReplacementUsingTermsData(search_terms_data); } -const string16& TemplateURL::keyword() const { - EnsureKeyword(); - return keyword_; +bool TemplateURL::SupportsReplacementUsingTermsData( + const SearchTermsData& search_terms_data) const { + return url_ref_.SupportsReplacementUsingTermsData(search_terms_data); } -void TemplateURL::EnsureKeyword() const { - if (autogenerate_keyword_ && !keyword_generated_) { - // Generate a keyword and cache it. - keyword_ = TemplateURLService::GenerateKeyword( - TemplateURLService::GenerateSearchURL(this).GetWithEmptyPath(), true); - keyword_generated_ = true; - } +std::string TemplateURL::GetExtensionId() const { + DCHECK(IsExtensionKeyword()); + return GURL(data_.url()).host(); } -bool TemplateURL::ShowInDefaultList() const { - return show_in_default_list() && url_ref_.SupportsReplacement(); +bool TemplateURL::IsExtensionKeyword() const { + return GURL(data_.url()).SchemeIs(chrome::kExtensionScheme); } -void TemplateURL::CopyURLRefs(const TemplateURL& other) { +void TemplateURL::SetURL(const std::string& url) { + data_.SetURL(url); url_ref_.InvalidateCachedValues(); - suggestions_url_ref_.InvalidateCachedValues(); - instant_url_ref_.InvalidateCachedValues(); - SetPrepopulateId(other.prepopulate_id_); } void TemplateURL::SetPrepopulateId(int id) { - prepopulate_id_ = id; + data_.prepopulate_id = id; const bool prepopulated = id > 0; suggestions_url_ref_.prepopulated_ = prepopulated; url_ref_.prepopulated_ = prepopulated; @@ -726,27 +718,5 @@ void TemplateURL::InvalidateCachedValues() const { url_ref_.InvalidateCachedValues(); suggestions_url_ref_.InvalidateCachedValues(); instant_url_ref_.InvalidateCachedValues(); - if (autogenerate_keyword_) { - keyword_.clear(); - keyword_generated_ = false; - } -} - -bool TemplateURL::SupportsReplacement() const { - UIThreadSearchTermsData search_terms_data; - return SupportsReplacementUsingTermsData(search_terms_data); -} - -bool TemplateURL::SupportsReplacementUsingTermsData( - const SearchTermsData& search_terms_data) const { - return url_ref_.SupportsReplacementUsingTermsData(search_terms_data); -} - -std::string TemplateURL::GetExtensionId() const { - DCHECK(IsExtensionKeyword()); - return GURL(url_).host(); -} - -bool TemplateURL::IsExtensionKeyword() const { - return GURL(url_).SchemeIs(chrome::kExtensionScheme); + data_.SetAutogenerateKeyword(data_.autogenerate_keyword()); } diff --git a/chrome/browser/search_engines/template_url.h b/chrome/browser/search_engines/template_url.h index 78889a6..3e56c73 100644 --- a/chrome/browser/search_engines/template_url.h +++ b/chrome/browser/search_engines/template_url.h @@ -14,31 +14,20 @@ #include "chrome/browser/search_engines/template_url_id.h" #include "googleurl/src/gurl.h" -class PrefService; class Profile; class SearchTermsData; class TemplateURL; -class TemplateURLParsingContext; -class WebDataService; - -// TemplateURL represents the relevant portions of the Open Search Description -// Document (http://www.opensearch.org/Specifications/OpenSearch). -// The main use case for TemplateURL is to use the TemplateURLRef returned by -// suggestions_url or url for keyword/suggestion expansion: -// . suggestions_url describes a URL that is ideal for as you type suggestions. -// The returned results are in the mime type application/x-suggestions+json. -// . url describes a URL that may be used as a shortcut. Returned results are -// are text/html. -// Before using either one, make sure it's non-NULL, and if you intend to use -// it to replace search terms, make sure SupportsReplacement returns true. -// To use either URL invoke the ReplaceSearchTerms method on the corresponding -// TemplateURLRef. -// -// For files parsed from the Web, be sure and invoke IsValid. IsValid returns -// true if the URL could be parsed. + + +// TemplateURLRef ------------------------------------------------------------- + +// A TemplateURLRef represents a single URL within the larger TemplateURL class +// (which represents an entire "search engine", see below). If +// SupportsReplacement() is true, this URL has placeholders in it, for which +// callers can substitute values to get a "real" URL using ReplaceSearchTerms(). // -// Both TemplateURL and TemplateURLRef have value semantics. This allows the -// UI to create a copy while the user modifies the values. +// TemplateURLRefs always have a non-NULL |owner_| TemplateURL, which they +// access in order to get at important data like the underlying URL string. class TemplateURLRef { public: // Magic numbers to pass to ReplaceSearchTerms() for the |accepted_suggestion| @@ -246,156 +235,189 @@ class TemplateURLRef { DISALLOW_COPY_AND_ASSIGN(TemplateURLRef); }; -// Describes the relevant portions of a single OSD document. -class TemplateURL { - public: - TemplateURL(); - - TemplateURL(const TemplateURL& other); - TemplateURL& operator=(const TemplateURL& other); - ~TemplateURL(); +// TemplateURLData ------------------------------------------------------------ - // Generates a favicon URL from the specified url. - static GURL GenerateFaviconURL(const GURL& url); +// The data for the TemplateURL. Separating this into its own class allows most +// users to do SSA-style usage of TemplateURL: construct a TemplateURLData with +// whatever fields are desired, then create an immutable TemplateURL from it. +struct TemplateURLData { + TemplateURLData(); + ~TemplateURLData(); // A short description of the template. This is the name we show to the user - // in various places that use keywords. For example, the location bar shows - // this when the user selects the keyword. - void set_short_name(const string16& short_name) { - short_name_ = short_name; - } - const string16& short_name() const { return short_name_; } - // An accessor for the short_name, but adjusted so it can be appropriately - // displayed even if it is LTR and the UI is RTL. - string16 AdjustedShortNameForLocaleDirection() const; + // in various places that use TemplateURLs. For example, the location bar + // shows this when the user selects a substituting match. + string16 short_name; + + // The shortcut for this TemplateURL. May be empty. + void SetKeyword(const string16& keyword) const; + const string16& keyword(const TemplateURL* t_url) const; + // TODO(pkasting): This should only be necessary until we eliminate keyword + // autogeneration. + const string16& raw_keyword() const { return keyword_; } + + // Whether to autogenerate a keyword in TemplateURL::GetKeyword(). Most + // consumers should not need this. + // NOTE: Calling SetKeyword() turns this back off. Manual and automatic + // keywords are mutually exclusive. + void SetAutogenerateKeyword(bool autogenerate_keyword) const; + bool autogenerate_keyword() const { return autogenerate_keyword_; } - // Parameterized URL for providing the results. + // Ensures that the keyword is generated. Most consumers should not need this + // because it is done automatically. Use this method on the UI thread, so + // the keyword may be accessed on another thread. + void EnsureKeyword(const TemplateURL* t_url) const; + + // The raw URL for the TemplateURL, which may not be valid as-is (e.g. because + // it requires substitutions first). void SetURL(const std::string& url); const std::string& url() const { return url_; } - // URL providing JSON results. This is typically used to provide suggestions - // as you type. - void SetSuggestionsURL(const std::string& url); - const std::string& suggestions_url() const { return suggestions_url_; } + // Optional additional raw URLs. + std::string suggestions_url; + std::string instant_url; - // Parameterized URL for instant results. - void SetInstantURL(const std::string& url); - const std::string& instant_url() const { return instant_url_; } + // Optional favicon for the TemplateURL. + GURL favicon_url; // URL to the OSD file this came from. May be empty. - void set_originating_url(const GURL& url) { - originating_url_ = url; - } - const GURL& originating_url() const { return originating_url_; } - - // The shortcut for this template url. May be empty. - void set_keyword(const string16& keyword); - const string16& keyword() const; + GURL originating_url; - // Whether to autogenerate a keyword from the url() in GetKeyword(). Most - // consumers should not need this. - // NOTE: Calling set_keyword() turns this back off. Manual and automatic - // keywords are mutually exclusive. - void set_autogenerate_keyword(bool autogenerate_keyword) { - autogenerate_keyword_ = autogenerate_keyword; - if (autogenerate_keyword_) { - keyword_.clear(); - keyword_generated_ = false; - } - } - bool autogenerate_keyword() const { - return autogenerate_keyword_; - } - - // Ensures that the keyword is generated. Most consumers should not need this - // because it is done automatically. Use this method on the UI thread, so - // the keyword may be accessed on another thread. - void EnsureKeyword() const; - - // Whether this keyword is shown in the default list of search providers. This - // is just a property and does not indicate whether this TemplateURL has - // a TemplateURLRef that supports replacement. Use ShowInDefaultList to - // test both. - // The default value is false. - void set_show_in_default_list(bool show_in_default_list) { - show_in_default_list_ = show_in_default_list; - } - bool show_in_default_list() const { return show_in_default_list_; } - - // Returns true if show_in_default_list() is true and this TemplateURL has a - // TemplateURLRef that supports replacement. - bool ShowInDefaultList() const; + // Whether this TemplateURL is shown in the default list of search providers. + // This is just a property and does not indicate whether the TemplateURL has a + // TemplateURLRef that supports replacement. Use + // TemplateURL::ShowInDefaultList() to test both. + bool show_in_default_list; // Whether it's safe for auto-modification code (the autogenerator and the // code that imports data from other browsers) to replace the TemplateURL. - // This should be set to false for any keyword the user edits, or any keyword - // that the user clearly manually edited in the past, like a bookmark keyword - // from another browser. - void set_safe_for_autoreplace(bool safe_for_autoreplace) { - safe_for_autoreplace_ = safe_for_autoreplace; - } - bool safe_for_autoreplace() const { return safe_for_autoreplace_; } + // This should be set to false for any TemplateURL the user edits, or any + // TemplateURL that the user clearly manually edited in the past, like a + // bookmark keyword from another browser. + bool safe_for_autoreplace; + + // The list of supported encodings for the search terms. This may be empty, + // which indicates the terms should be encoded with UTF-8. + std::vector<std::string> input_encodings; - // The favicon. This is optional. - void set_favicon_url(const GURL& url) { favicon_url_ = url; } - const GURL& favicon_url() const { return favicon_url_; } + // Unique identifier of this TemplateURL. The unique ID is set by the + // TemplateURLService when the TemplateURL is added to it. + TemplateURLID id; - // Date this keyword was created. + // Date this TemplateURL was created. // - // NOTE: this may be 0, which indicates the keyword was created before we + // NOTE: this may be 0, which indicates the TemplateURL was created before we // started tracking creation time. - void set_date_created(base::Time time) { date_created_ = time; } - base::Time date_created() const { return date_created_; } + base::Time date_created; - // The last time this keyword was modified by a user, since creation. + // The last time this TemplateURL was modified by a user, since creation. // // NOTE: Like date_created above, this may be 0. - void set_last_modified(base::Time time) { last_modified_ = time; } - base::Time last_modified() const { return last_modified_; } + base::Time last_modified; // True if this TemplateURL was automatically created by the administrator via // group policy. - void set_created_by_policy(bool created_by_policy) { - created_by_policy_ = created_by_policy; - } - bool created_by_policy() const { return created_by_policy_; } + bool created_by_policy; - // Number of times this keyword has been explicitly used to load a URL. We - // don't increment this for uses as the "default search engine" since that's - // not really "explicit" usage and incrementing would result in pinning the - // user's default search engine(s) to the top of the list of searches on the - // New Tab page, de-emphasizing the omnibox as "where you go to search". - void set_usage_count(int count) { usage_count_ = count; } - int usage_count() const { return usage_count_; } + // Number of times this TemplateURL has been explicitly used to load a URL. + // We don't increment this for uses as the "default search engine" since + // that's not really "explicit" usage and incrementing would result in pinning + // the user's default search engine(s) to the top of the list of searches on + // the New Tab page, de-emphasizing the omnibox as "where you go to search". + int usage_count; - // The list of supported encodings for the search terms. This may be empty, - // which indicates the terms should be encoded with UTF-8. - void set_input_encodings(const std::vector<std::string>& encodings) { - input_encodings_ = encodings; - } - void add_input_encoding(const std::string& encoding) { - input_encodings_.push_back(encoding); + // If this TemplateURL comes from prepopulated data the prepopulate_id is > 0. + int prepopulate_id; + + // The primary unique identifier for Sync. This set on all TemplateURLs + // regardless of whether they have been associated with Sync. + std::string sync_guid; + + private: + // Private so we can enforce using the setters. + // TODO(pkasting): For now these setters are not critical, but later we will + // begin using them to ensure that these fields are non-empty. + mutable string16 keyword_; + std::string url_; + + // TODO(pkasting): These fields will go away soon. + mutable bool autogenerate_keyword_; + // True if the keyword was generated. This is used to avoid multiple attempts + // if generating a keyword failed. + mutable bool keyword_generated_; +}; + + +// TemplateURL ---------------------------------------------------------------- + +// A TemplateURL represents a single "search engine", defined primarily as a +// subset of the Open Search Description Document +// (http://www.opensearch.org/Specifications/OpenSearch) plus some extensions. +// One TemplateURL contains several TemplateURLRefs, which correspond to various +// different capabilities (e.g. doing searches or getting suggestions), as well +// as a TemplateURLData containing other details like the name, keyword, etc. +// +// TemplateURLs are intended to be read-only for most users; the only public +// non-const method is the Profile getter, which returns a non-const Profile*. +// The TemplateURLService, which handles storing and manipulating TemplateURLs, +// is made a friend so that it can be the exception to this pattern. +class TemplateURL { + public: + explicit TemplateURL(const TemplateURLData& data); + + TemplateURL(const TemplateURL& other); + TemplateURL& operator=(const TemplateURL& other); + + ~TemplateURL(); + + // Generates a favicon URL from the specified url. + static GURL GenerateFaviconURL(const GURL& url); + + const TemplateURLData& data() const { return data_; } + + const string16& short_name() const { return data_.short_name; } + // An accessor for the short_name, but adjusted so it can be appropriately + // displayed even if it is LTR and the UI is RTL. + string16 AdjustedShortNameForLocaleDirection() const; + + const string16& keyword() const { return data_.keyword(this); } + bool autogenerate_keyword() const { + return data_.autogenerate_keyword(); } + void EnsureKeyword() const { data_.EnsureKeyword(this); } + + const std::string& url() const { return data_.url(); } + const std::string& suggestions_url() const { return data_.suggestions_url; } + const std::string& instant_url() const { return data_.instant_url; } + const GURL& favicon_url() const { return data_.favicon_url; } + + const GURL& originating_url() const { return data_.originating_url; } + + bool show_in_default_list() const { return data_.show_in_default_list; } + + // Returns true if show_in_default_list() is true and this TemplateURL has a + // TemplateURLRef that supports replacement. + bool ShowInDefaultList() const; + + bool safe_for_autoreplace() const { return data_.safe_for_autoreplace; } + const std::vector<std::string>& input_encodings() const { - return input_encodings_; + return data_.input_encodings; } - // Returns the unique identifier of this TemplateURL. The unique ID is set - // by the TemplateURLService when the TemplateURL is added to it. - TemplateURLID id() const { return id_; } + TemplateURLID id() const { return data_.id; } - // Copies the data from |other|'s TemplateURLRef members into |this|. - void CopyURLRefs(const TemplateURL& other); + base::Time date_created() const { return data_.date_created; } + base::Time last_modified() const { return data_.last_modified; } - // If this TemplateURL comes from prepopulated data the prepopulate_id is > 0. - // SetPrepopulateId also sets any TemplateURLRef's prepopulated flag to true - // if |id| > 0 and false otherwise. - void SetPrepopulateId(int id); - int prepopulate_id() const { return prepopulate_id_; } + bool created_by_policy() const { return data_.created_by_policy; } + + int usage_count() const { return data_.usage_count; } - const std::string& sync_guid() const { return sync_guid_; } - void set_sync_guid(const std::string& guid) { sync_guid_ = guid; } + int prepopulate_id() const { return data_.prepopulate_id; } + + const std::string& sync_guid() const { return data_.sync_guid; } const TemplateURLRef& url_ref() const { return url_ref_; } const TemplateURLRef& suggestions_url_ref() const { @@ -414,51 +436,15 @@ class TemplateURL { bool IsExtensionKeyword() const; private: - friend void MergeEnginesFromPrepopulateData( - PrefService* prefs, - WebDataService* service, - std::vector<TemplateURL*>* template_urls, - const TemplateURL** default_search_provider); - friend class KeywordTable; - friend class KeywordTableTest; - friend class SearchHostToURLsMap; - friend class TemplateURLParsingContext; friend class TemplateURLService; - FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, - ResolveSyncKeywordConflict); + + void SetURL(const std::string& url); + void SetPrepopulateId(int id); // Invalidates cached values on this object and its child TemplateURLRefs. void InvalidateCachedValues() const; - // Unique identifier, used when archived to the database. - void set_id(TemplateURLID id) { id_ = id; } - - string16 short_name_; - std::string url_; - std::string suggestions_url_; - std::string instant_url_; - GURL originating_url_; - mutable string16 keyword_; - bool autogenerate_keyword_; // If this is set, |keyword_| holds the cached - // generated keyword if available. - mutable bool keyword_generated_; // True if the keyword was generated. This - // is used to avoid multiple attempts if - // generating a keyword failed. - bool show_in_default_list_; - bool safe_for_autoreplace_; - GURL favicon_url_; - // List of supported input encodings. - std::vector<std::string> input_encodings_; - TemplateURLID id_; - base::Time date_created_; - base::Time last_modified_; - bool created_by_policy_; - int usage_count_; - int prepopulate_id_; - // The primary unique identifier for Sync. This is only set on TemplateURLs - // that have been associated with Sync. - std::string sync_guid_; - + TemplateURLData data_; TemplateURLRef url_ref_; TemplateURLRef suggestions_url_ref_; TemplateURLRef instant_url_ref_; diff --git a/chrome/browser/search_engines/template_url_fetcher.cc b/chrome/browser/search_engines/template_url_fetcher.cc index ac1ff53..b00b1a3 100644 --- a/chrome/browser/search_engines/template_url_fetcher.cc +++ b/chrome/browser/search_engines/template_url_fetcher.cc @@ -147,8 +147,8 @@ void TemplateURLFetcher::RequestDelegate::OnURLFetchComplete( return; } - template_url_.reset(TemplateURLParser::Parse(fetcher_->profile(), data.data(), - data.length(), NULL)); + template_url_.reset(TemplateURLParser::Parse(fetcher_->profile(), false, + data.data(), data.length(), NULL)); if (!template_url_.get() || !template_url_->url_ref().SupportsReplacement()) { fetcher_->RequestCompleted(this); // WARNING: RequestCompleted deletes us. @@ -194,18 +194,19 @@ void TemplateURLFetcher::RequestDelegate::AddSearchProvider() { // The short name is what is shown to the user. We preserve original names // since it is better when generated keyword in many cases. - template_url_->set_keyword(keyword_); - template_url_->set_originating_url(osdd_url_); + TemplateURLData data(template_url_->data()); + data.SetKeyword(keyword_); + data.originating_url = osdd_url_; // The page may have specified a URL to use for favicons, if not, set it. - if (!template_url_->favicon_url().is_valid()) - template_url_->set_favicon_url(favicon_url_); + if (!data.favicon_url.is_valid()) + data.favicon_url = favicon_url_; switch (provider_type_) { case AUTODETECTED_PROVIDER: // Mark the keyword as replaceable so it can be removed if necessary. - template_url_->set_safe_for_autoreplace(true); - model->Add(template_url_.release()); + data.safe_for_autoreplace = true; + model->Add(new TemplateURL(data)); break; case EXPLICIT_PROVIDER: @@ -215,7 +216,7 @@ void TemplateURLFetcher::RequestDelegate::AddSearchProvider() { // The source TabContents' delegate takes care of adding the URL to the // model, which takes ownership, or of deleting it if the add is // cancelled. - callbacks_->ConfirmAddSearchProvider(template_url_.release(), + callbacks_->ConfirmAddSearchProvider(new TemplateURL(data), fetcher_->profile()); break; diff --git a/chrome/browser/search_engines/template_url_fetcher_unittest.cc b/chrome/browser/search_engines/template_url_fetcher_unittest.cc index 324e337..9a9712c 100644 --- a/chrome/browser/search_engines/template_url_fetcher_unittest.cc +++ b/chrome/browser/search_engines/template_url_fetcher_unittest.cc @@ -288,11 +288,11 @@ TEST_F(TemplateURLFetcherTest, ExplicitBeforeLoadTest) { TEST_F(TemplateURLFetcherTest, DuplicateKeywordsTest) { string16 keyword(ASCIIToUTF16("test")); - TemplateURL* t_url = new TemplateURL(); - t_url->SetURL("http://example.com/"); - t_url->set_keyword(keyword); - t_url->set_short_name(keyword); - test_util_.model()->Add(t_url); + TemplateURLData data; + data.short_name = keyword; + data.SetKeyword(keyword); + data.SetURL("http://example.com/"); + test_util_.model()->Add(new TemplateURL(data)); test_util_.ChangeModelToLoadState(); ASSERT_TRUE(test_util_.model()->GetTemplateURLForKeyword(keyword)); diff --git a/chrome/browser/search_engines/template_url_parser.cc b/chrome/browser/search_engines/template_url_parser.cc index 974c771..7ae6a91 100644 --- a/chrome/browser/search_engines/template_url_parser.cc +++ b/chrome/browser/search_engines/template_url_parser.cc @@ -145,7 +145,7 @@ class TemplateURLParsingContext { // This will be NULL if parsing failed or if the results were invalid for some // reason (e.g. the resulting URL was not HTTP[S], a name wasn't supplied, // etc.). - TemplateURL* GetTemplateURL(Profile* profile); + TemplateURL* GetTemplateURL(Profile* profile, bool show_in_default_list); private: // Key is UTF8 encoded. @@ -163,7 +163,9 @@ class TemplateURLParsingContext { static ElementNameToElementTypeMap* kElementNameToElementTypeMap; - scoped_ptr<TemplateURL> url_; + // Data that gets updated as we parse, and is converted to a TemplateURL by + // GetTemplateURL(). + TemplateURLData data_; std::vector<ElementType> elements_; bool image_is_valid_for_favicon_; @@ -197,8 +199,7 @@ TemplateURLParsingContext::ElementNameToElementTypeMap* TemplateURLParsingContext::TemplateURLParsingContext( TemplateURLParser::ParameterFilter* parameter_filter) - : url_(new TemplateURL()), - image_is_valid_for_favicon_(false), + : image_is_valid_for_favicon_(false), parameter_filter_(parameter_filter), method_(GET), suggestion_method_(GET), @@ -206,9 +207,9 @@ TemplateURLParsingContext::TemplateURLParsingContext( derive_image_from_url_(false) { if (kElementNameToElementTypeMap == NULL) InitMapping(); - // When combined with proscriptions elsewhere against updating url_->url_ to - // the empty string, this call ensures url_->url() will never be NULL. - url_->SetURL("x"); + // When combined with proscriptions elsewhere against updating data_->url_ to + // the empty string, this call ensures data_->url() will never be NULL. + data_.SetURL("x"); } // static @@ -249,7 +250,7 @@ void TemplateURLParsingContext::EndElementImpl(void* ctx, const xmlChar* name) { reinterpret_cast<TemplateURLParsingContext*>(ctx); switch (context->GetKnownType()) { case TemplateURLParsingContext::SHORT_NAME: - context->url_->short_name_ = context->string_; + context->data_.short_name = context->string_; break; case TemplateURLParsingContext::IMAGE: { GURL image_url(UTF16ToUTF8(context->string_)); @@ -261,7 +262,7 @@ void TemplateURLParsingContext::EndElementImpl(void* ctx, const xmlChar* name) { } else if (context->image_is_valid_for_favicon_ && image_url.is_valid() && (image_url.SchemeIs(chrome::kHttpScheme) || image_url.SchemeIs(chrome::kHttpsScheme))) { - context->url_->set_favicon_url(image_url); + context->data_.favicon_url = image_url; } context->image_is_valid_for_favicon_ = false; break; @@ -269,7 +270,7 @@ void TemplateURLParsingContext::EndElementImpl(void* ctx, const xmlChar* name) { case TemplateURLParsingContext::INPUT_ENCODING: { std::string input_encoding = UTF16ToASCII(context->string_); if (IsValidEncodingString(input_encoding)) - context->url_->input_encodings_.push_back(input_encoding); + context->data_.input_encodings.push_back(input_encoding); break; } case TemplateURLParsingContext::URL: @@ -290,30 +291,33 @@ void TemplateURLParsingContext::CharactersImpl(void* ctx, UTF8ToUTF16(std::string(reinterpret_cast<const char*>(ch), len)); } -TemplateURL* TemplateURLParsingContext::GetTemplateURL(Profile* profile) { +TemplateURL* TemplateURLParsingContext::GetTemplateURL( + Profile* profile, + bool show_in_default_list) { // Basic legality checks. - if (url_->short_name_.empty() || !IsHTTPRef(url_->url()) || - !IsHTTPRef(url_->suggestions_url())) + if (data_.short_name.empty() || !IsHTTPRef(data_.url()) || + !IsHTTPRef(data_.suggestions_url)) return NULL; // If the image was a data URL, use the favicon from the search URL instead. // (see TODO inEndElementImpl()). - GURL url(url_->url()); - if (derive_image_from_url_ && url_->favicon_url().is_empty()) - url_->set_favicon_url(TemplateURL::GenerateFaviconURL(url)); + GURL url(data_.url()); + if (derive_image_from_url_ && data_.favicon_url.is_empty()) + data_.favicon_url = TemplateURL::GenerateFaviconURL(url); // TODO(jcampan): http://b/issue?id=1196285 we do not support search engines // that use POST yet. if (method_ == TemplateURLParsingContext::POST) return NULL; if (suggestion_method_ == TemplateURLParsingContext::POST) - url_->SetSuggestionsURL(std::string()); + data_.suggestions_url.clear(); // Give this a keyword to facilitate tab-to-search. string16 keyword(TemplateURLService::GenerateKeyword(url, false)); DCHECK(!keyword.empty()); - url_->set_keyword(keyword); - return url_.release(); + data_.SetKeyword(keyword); + data_.show_in_default_list = show_in_default_list; + return new TemplateURL(data_); } // static @@ -353,12 +357,12 @@ void TemplateURLParsingContext::ParseURL(const xmlChar** atts) { } if (is_html_url && !template_url.empty()) { - url_->SetURL(template_url); + data_.SetURL(template_url); is_suggest_url_ = false; if (is_post) method_ = POST; } else if (is_suggest_url) { - url_->SetSuggestionsURL(template_url); + data_.suggestions_url = template_url; is_suggest_url_ = true; if (is_post) suggestion_method_ = POST; @@ -413,7 +417,7 @@ void TemplateURLParsingContext::ProcessURLParams() { if (!parameter_filter_ && extra_params_.empty()) return; - GURL url(is_suggest_url_ ? url_->suggestions_url() : url_->url()); + GURL url(is_suggest_url_ ? data_.suggestions_url : data_.url()); if (url.is_empty()) return; @@ -451,9 +455,9 @@ void TemplateURLParsingContext::ProcessURLParams() { repl.SetQueryStr(new_query); url = url.ReplaceComponents(repl); if (is_suggest_url_) - url_->SetSuggestionsURL(url.spec()); + data_.suggestions_url = url.spec(); else if (url.is_valid()) - url_->SetURL(url.spec()); + data_.SetURL(url.spec()); } } @@ -472,6 +476,7 @@ TemplateURLParsingContext::ElementType // static TemplateURL* TemplateURLParser::Parse( Profile* profile, + bool show_in_default_list, const char* data, size_t length, TemplateURLParser::ParameterFilter* param_filter) { @@ -489,5 +494,5 @@ TemplateURL* TemplateURLParser::Parse( xmlSAXUserParseMemory(&sax_handler, &context, data, static_cast<int>(length)); xmlSubstituteEntitiesDefault(last_sub_entities_value); - return context.GetTemplateURL(profile); + return context.GetTemplateURL(profile, show_in_default_list); } diff --git a/chrome/browser/search_engines/template_url_parser.h b/chrome/browser/search_engines/template_url_parser.h index 9459f2c..3191e5d 100644 --- a/chrome/browser/search_engines/template_url_parser.h +++ b/chrome/browser/search_engines/template_url_parser.h @@ -37,6 +37,7 @@ class TemplateURLParser { // from another browser, we remove any parameter identifying that browser. If // set to NULL, the URL is not modified. static TemplateURL* Parse(Profile* profile, + bool show_in_default_list, const char* data, size_t length, ParameterFilter* parameter_filter); diff --git a/chrome/browser/search_engines/template_url_parser_unittest.cc b/chrome/browser/search_engines/template_url_parser_unittest.cc index 2ad1ae1..4cc4ed2 100644 --- a/chrome/browser/search_engines/template_url_parser_unittest.cc +++ b/chrome/browser/search_engines/template_url_parser_unittest.cc @@ -99,7 +99,7 @@ void TemplateURLParserTest::ParseFile( std::string contents; ASSERT_TRUE(file_util::ReadFileToString(full_path, &contents)); - template_url_.reset(TemplateURLParser::Parse(NULL, contents.data(), + template_url_.reset(TemplateURLParser::Parse(NULL, false, contents.data(), contents.length(), filter)); } diff --git a/chrome/browser/search_engines/template_url_prepopulate_data.cc b/chrome/browser/search_engines/template_url_prepopulate_data.cc index 5be3bae..5f785ce 100644 --- a/chrome/browser/search_engines/template_url_prepopulate_data.cc +++ b/chrome/browser/search_engines/template_url_prepopulate_data.cc @@ -3153,30 +3153,28 @@ int GetDataVersion(PrefService* prefs) { TemplateURL* MakePrepopulatedTemplateURL(const string16& name, const string16& keyword, const base::StringPiece& search_url, - const base::StringPiece& favicon_url, const base::StringPiece& suggest_url, const base::StringPiece& instant_url, + const base::StringPiece& favicon_url, const base::StringPiece& encoding, int id) { - TemplateURL* new_turl = new TemplateURL(); - new_turl->SetURL(search_url.as_string()); - new_turl->set_favicon_url(GURL(favicon_url.as_string())); - new_turl->SetSuggestionsURL(suggest_url.as_string()); - new_turl->SetInstantURL(instant_url.as_string()); - new_turl->set_short_name(name); + TemplateURLData data; + data.short_name = name; if (keyword.empty()) - new_turl->set_autogenerate_keyword(true); + data.SetAutogenerateKeyword(true); else - new_turl->set_keyword(keyword); - new_turl->set_show_in_default_list(true); - new_turl->set_safe_for_autoreplace(true); - new_turl->set_date_created(base::Time()); - new_turl->set_last_modified(base::Time()); - std::vector<std::string> turl_encodings; - turl_encodings.push_back(encoding.as_string()); - new_turl->set_input_encodings(turl_encodings); - new_turl->SetPrepopulateId(id); - return new_turl; + data.SetKeyword(keyword); + data.SetURL(search_url.as_string()); + data.suggestions_url = suggest_url.as_string(); + data.instant_url = instant_url.as_string(); + data.favicon_url = GURL(favicon_url.as_string()); + data.show_in_default_list = true; + data.safe_for_autoreplace = true; + data.input_encodings.push_back(encoding.as_string()); + data.date_created = base::Time(); + data.last_modified = base::Time(); + data.prepopulate_id = id; + return new TemplateURL(data); } void GetPrepopulatedTemplateFromPrefs(PrefService* prefs, @@ -3220,7 +3218,7 @@ void GetPrepopulatedTemplateFromPrefs(PrefService* prefs, continue; } t_urls->push_back(MakePrepopulatedTemplateURL(name, keyword, search_url, - favicon_url, suggest_url, instant_url, encoding, id)); + suggest_url, instant_url, favicon_url, encoding, id)); } } @@ -3228,8 +3226,8 @@ void GetPrepopulatedTemplateFromPrefs(PrefService* prefs, TemplateURL* MakePrepopulatedTemplateURLFromPrepopulateEngine( const PrepopulatedEngine& engine) { return MakePrepopulatedTemplateURL(WideToUTF16(engine.name), - WideToUTF16(engine.keyword), engine.search_url, engine.favicon_url, - engine.suggest_url, engine.instant_url, engine.encoding, engine.id); + WideToUTF16(engine.keyword), engine.search_url, engine.suggest_url, + engine.instant_url, engine.favicon_url, engine.encoding, engine.id); } void GetPrepopulatedEngines(PrefService* prefs, diff --git a/chrome/browser/search_engines/template_url_service.cc b/chrome/browser/search_engines/template_url_service.cc index bb3f4c3..6667f84 100644 --- a/chrome/browser/search_engines/template_url_service.cc +++ b/chrome/browser/search_engines/template_url_service.cc @@ -68,17 +68,16 @@ bool TemplateURLsHaveSamePrefs(const TemplateURL* url1, const TemplateURL* url2) { if (url1 == url2) return true; - return NULL != url1 && - NULL != url2 && - url1->short_name() == url2->short_name() && - url1->keyword() == url2->keyword() && - url1->url() == url2->url() && - url1->suggestions_url() == url2->suggestions_url() && - url1->instant_url() == url2->instant_url() && - url1->favicon_url() == url2->favicon_url() && - url1->safe_for_autoreplace() == url2->safe_for_autoreplace() && - url1->show_in_default_list() == url2->show_in_default_list() && - url1->input_encodings() == url2->input_encodings(); + return (url1 != NULL) && (url2 != NULL) && + (url1->short_name() == url2->short_name()) && + (url1->keyword() == url2->keyword()) && + (url1->url() == url2->url()) && + (url1->suggestions_url() == url2->suggestions_url()) && + (url1->instant_url() == url2->instant_url()) && + (url1->favicon_url() == url2->favicon_url()) && + (url1->safe_for_autoreplace() == url2->safe_for_autoreplace()) && + (url1->show_in_default_list() == url2->show_in_default_list()) && + (url1->input_encodings() == url2->input_encodings()); } } // namespace @@ -316,6 +315,17 @@ void TemplateURLService::Add(TemplateURL* template_url) { NotifyObservers(); } +void TemplateURLService::AddWithOverrides(const TemplateURL* template_url, + const string16& short_name, + const string16& keyword, + const std::string& url) { + TemplateURL* modifiable_url = const_cast<TemplateURL*>(template_url); + modifiable_url->data_.short_name = short_name; + modifiable_url->data_.SetKeyword(keyword); + modifiable_url->SetURL(url); + Add(modifiable_url); +} + void TemplateURLService::Remove(const TemplateURL* template_url) { RemoveNoNotify(template_url); NotifyObservers(); @@ -368,15 +378,14 @@ void TemplateURLService::RegisterExtensionKeyword(const Extension* extension) { const TemplateURL* existing_url = GetTemplateURLForExtension(extension); string16 keyword = UTF8ToUTF16(extension->omnibox_keyword()); - scoped_ptr<TemplateURL> template_url(new TemplateURL); - template_url->set_short_name(UTF8ToUTF16(extension->name())); - template_url->set_keyword(keyword); + TemplateURLData data; + data.short_name = UTF8ToUTF16(extension->name()); + data.SetKeyword(UTF8ToUTF16(extension->omnibox_keyword())); // This URL is not actually used for navigation. It holds the extension's // ID, as well as forcing the TemplateURL to be treated as a search keyword. - template_url->SetURL( - std::string(chrome::kExtensionScheme) + "://" + - extension->id() + "/?q={searchTerms}"); - template_url->set_safe_for_autoreplace(false); + data.SetURL(std::string(chrome::kExtensionScheme) + "://" + extension->id() + + "/?q={searchTerms}"); + scoped_ptr<TemplateURL> template_url(new TemplateURL(data)); if (existing_url) { // TODO(mpcomplete): only replace if the user hasn't changed the keyword. @@ -413,26 +422,27 @@ std::vector<const TemplateURL*> TemplateURLService::GetTemplateURLs() const { void TemplateURLService::IncrementUsageCount(const TemplateURL* url) { DCHECK(url && std::find(template_urls_.begin(), template_urls_.end(), url) != - template_urls_.end()); - const_cast<TemplateURL*>(url)->set_usage_count(url->usage_count() + 1); + template_urls_.end()); + ++const_cast<TemplateURL*>(url)->data_.usage_count; if (service_.get()) - service_.get()->UpdateKeyword(*url); + service_->UpdateKeyword(*url); } void TemplateURLService::ResetTemplateURL(const TemplateURL* url, const string16& title, const string16& keyword, const std::string& search_url) { - TemplateURL new_url(*url); - new_url.set_short_name(title); - new_url.set_keyword(keyword); - if (new_url.url() != search_url) { - new_url.SetURL(search_url); + TemplateURLData data(url->data()); + data.short_name = title; + data.SetKeyword(keyword); + if (search_url != data.url()) { + data.SetURL(search_url); // The urls have changed, reset the favicon url. - new_url.set_favicon_url(GURL()); + data.favicon_url = GURL(); } - new_url.set_safe_for_autoreplace(false); - new_url.set_last_modified(time_provider_()); + data.safe_for_autoreplace = false; + data.last_modified = time_provider_(); + TemplateURL new_url(data); UpdateNoNotify(url, new_url); NotifyObservers(); } @@ -528,7 +538,7 @@ void TemplateURLService::OnWebDataServiceRequestDone( &default_search_provider, &new_resource_keyword_version); - bool database_specified_a_default = NULL != default_search_provider; + bool database_specified_a_default = (default_search_provider != NULL); // Check if default search provider is now managed. scoped_ptr<TemplateURL> default_from_prefs; @@ -568,16 +578,15 @@ void TemplateURLService::OnWebDataServiceRequestDone( // Reuse it. } else { // The value from the preferences takes over. - // - // AddNoNotify will take ownership of default_from_prefs so it is safe to - // release. If it's null, there's no ownership to worry about :-) - TemplateURL* managed_default = default_from_prefs.release(); - if (managed_default) { - managed_default->set_created_by_policy(true); - managed_default->set_id(kInvalidTemplateURLID); + default_search_provider = NULL; + if (default_from_prefs.get()) { + TemplateURLData data(default_from_prefs->data()); + data.created_by_policy = true; + data.id = kInvalidTemplateURLID; + TemplateURL* managed_default = new TemplateURL(data); AddNoNotify(managed_default); + default_search_provider = managed_default; } - default_search_provider = managed_default; } // Note that this saves the default search provider to prefs. SetDefaultSearchProviderNoNotify(default_search_provider); @@ -589,7 +598,7 @@ void TemplateURLService::OnWebDataServiceRequestDone( default_search_provider = synced_default; pending_synced_default_search_ = false; } else if (database_specified_a_default && - NULL == default_search_provider && + default_search_provider == NULL && !template_urls.empty()) { default_search_provider = template_urls[0]; } @@ -759,14 +768,12 @@ SyncError TemplateURLService::ProcessSyncChanges( iter != change_list.end(); ++iter) { DCHECK_EQ(syncable::SEARCH_ENGINES, iter->sync_data().GetDataType()); - scoped_ptr<TemplateURL> turl( - CreateTemplateURLFromSyncData(iter->sync_data())); - if (!turl.get()) { - NOTREACHED() << "Failed to read search engine."; - continue; - } + std::string guid = + iter->sync_data().GetSpecifics().search_engine().sync_guid(); + const TemplateURL* existing_turl = GetTemplateURLForGUID(guid); + scoped_ptr<TemplateURL> turl(CreateTemplateURLFromTemplateURLAndSyncData( + existing_turl, iter->sync_data())); - const TemplateURL* existing_turl = GetTemplateURLForGUID(turl->sync_guid()); const TemplateURL* existing_keyword_turl = GetTemplateURLForKeyword(turl->keyword()); @@ -790,8 +797,9 @@ SyncError TemplateURLService::ProcessSyncChanges( if (existing_keyword_turl) ResolveSyncKeywordConflict(turl.get(), &new_changes); // Force the local ID to kInvalidTemplateURLID so we can add it. - turl->set_id(kInvalidTemplateURLID); - Add(turl.release()); + TemplateURLData data(turl->data()); + data.id = kInvalidTemplateURLID; + Add(new TemplateURL(data)); // Possibly set the newly added |turl| as the default search provider. SetDefaultSearchProviderIfNewlySynced(guid); @@ -799,11 +807,9 @@ SyncError TemplateURLService::ProcessSyncChanges( existing_turl) { // Possibly resolve a keyword conflict if they have the same keywords but // are not the same entry. - TemplateURL updated_turl(*existing_turl); - UpdateTemplateURLWithSyncData(&updated_turl, iter->sync_data()); if (existing_keyword_turl && existing_keyword_turl != existing_turl) - ResolveSyncKeywordConflict(&updated_turl, &new_changes); - UpdateNoNotify(existing_turl, updated_turl); + ResolveSyncKeywordConflict(turl.get(), &new_changes); + UpdateNoNotify(existing_turl, *turl); NotifyObservers(); } else { // Something really unexpected happened. Either we received an @@ -862,10 +868,9 @@ SyncError TemplateURLService::MergeDataAndStartSyncing( for (SyncDataMap::const_iterator iter = sync_data_map.begin(); iter != sync_data_map.end(); ++iter) { - scoped_ptr<TemplateURL> sync_turl( - CreateTemplateURLFromSyncData(iter->second)); - DCHECK(sync_turl.get()); const TemplateURL* local_turl = GetTemplateURLForGUID(iter->first); + scoped_ptr<TemplateURL> sync_turl( + CreateTemplateURLFromTemplateURLAndSyncData(local_turl, iter->second)); if (sync_turl->sync_guid().empty()) { // Due to a bug, older search engine entries with no sync GUID @@ -883,9 +888,7 @@ SyncError TemplateURLService::MergeDataAndStartSyncing( // TemplateURLID and the TemplateURL may have to be reparsed. This // also makes the local data's last_modified timestamp equal to Sync's, // avoiding an Update on the next MergeData call. - TemplateURL updated_turl(*local_turl); - UpdateTemplateURLWithSyncData(&updated_turl, iter->second); - UpdateNoNotify(local_turl, updated_turl); + UpdateNoNotify(local_turl, *sync_turl); NotifyObservers(); } else if (sync_turl->last_modified() < local_turl->last_modified()) { // Otherwise, we know we have newer data, so update Sync with our @@ -916,8 +919,9 @@ SyncError TemplateURLService::MergeDataAndStartSyncing( // the cloud. ResolveSyncKeywordConflict(sync_turl.get(), &new_changes); // Force the local ID to kInvalidTemplateURLID so we can add it. - sync_turl->set_id(kInvalidTemplateURLID); - Add(sync_turl.release()); + TemplateURLData data(sync_turl->data()); + data.id = kInvalidTemplateURLID; + Add(new TemplateURL(data)); // Possibly set the newly added |turl| as the default search provider. SetDefaultSearchProviderIfNewlySynced(guid); @@ -1001,11 +1005,34 @@ SyncData TemplateURLService::CreateSyncDataFromTemplateURL( } // static -TemplateURL* TemplateURLService::CreateTemplateURLFromSyncData( +TemplateURL* TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData( + const TemplateURL* existing_turl, const SyncData& sync_data) { - TemplateURL* turl = new TemplateURL(); - UpdateTemplateURLWithSyncData(turl, sync_data); - return turl; + sync_pb::SearchEngineSpecifics specifics = + sync_data.GetSpecifics().search_engine(); + + TemplateURLData data; + data.short_name = UTF8ToUTF16(specifics.short_name()); + data.originating_url = GURL(specifics.originating_url()); + data.SetKeyword(UTF8ToUTF16(specifics.keyword())); + data.SetAutogenerateKeyword(specifics.autogenerate_keyword()); + data.SetURL(specifics.url()); + data.suggestions_url = specifics.suggestions_url(); + data.instant_url = specifics.instant_url(); + data.favicon_url = GURL(specifics.favicon_url()); + data.show_in_default_list = specifics.show_in_default_list(); + data.safe_for_autoreplace = specifics.safe_for_autoreplace(); + base::SplitString(specifics.input_encodings(), ';', &data.input_encodings); + data.date_created = base::Time::FromInternalValue(specifics.date_created()); + data.last_modified = base::Time::FromInternalValue(specifics.last_modified()); + data.prepopulate_id = specifics.prepopulate_id(); + data.sync_guid = specifics.sync_guid(); + if (existing_turl) { + data.id = existing_turl->id(); + data.created_by_policy = existing_turl->created_by_policy(); + data.usage_count = existing_turl->usage_count(); + } + return new TemplateURL(data); } // static @@ -1066,11 +1093,11 @@ void TemplateURLService::Init(const Initializer* initializers, // TemplateURLService ends up owning the TemplateURL, don't try and free // it. - TemplateURL* template_url = new TemplateURL(); - template_url->set_keyword(UTF8ToUTF16(initializers[i].keyword)); - template_url->set_short_name(UTF8ToUTF16(initializers[i].content)); - template_url->SetURL(osd_url); - AddNoNotify(template_url); + TemplateURLData data; + data.short_name = UTF8ToUTF16(initializers[i].content); + data.SetKeyword(UTF8ToUTF16(initializers[i].keyword)); + data.SetURL(osd_url); + AddNoNotify(new TemplateURL(data)); } } @@ -1251,27 +1278,26 @@ bool TemplateURLService::LoadDefaultSearchProviderFromPrefs( std::string prepopulate_id = prefs->GetString(prefs::kDefaultSearchProviderPrepopulateID); - default_provider->reset(new TemplateURL()); - (*default_provider)->set_short_name(name); - (*default_provider)->SetURL(search_url); - (*default_provider)->SetSuggestionsURL(suggest_url); - (*default_provider)->SetInstantURL(instant_url); - (*default_provider)->set_keyword(keyword); - (*default_provider)->set_favicon_url(GURL(icon_url)); - std::vector<std::string> encodings_vector; - base::SplitString(encodings, ';', &encodings_vector); - (*default_provider)->set_input_encodings(encodings_vector); + TemplateURLData data; + data.short_name = name; + data.SetKeyword(keyword); + data.SetURL(search_url); + data.suggestions_url = suggest_url; + data.instant_url = instant_url; + data.favicon_url = GURL(icon_url); + data.show_in_default_list = true; + base::SplitString(encodings, ';', &data.input_encodings); if (!id_string.empty() && !*is_managed) { int64 value; base::StringToInt64(id_string, &value); - (*default_provider)->set_id(value); + data.id = value; } if (!prepopulate_id.empty() && !*is_managed) { int value; base::StringToInt(prepopulate_id, &value); - (*default_provider)->SetPrepopulateId(value); + data.prepopulate_id = value; } - (*default_provider)->set_show_in_default_list(true); + default_provider->reset(new TemplateURL(data)); return true; } @@ -1305,17 +1331,21 @@ void TemplateURLService::UpdateNoNotify(const TemplateURL* existing_turl, const TemplateURL& new_values) { DCHECK(loaded_); DCHECK(existing_turl); - DCHECK(std::find(template_urls_.begin(), template_urls_.end(), existing_turl) - != template_urls_.end()); + DCHECK(std::find(template_urls_.begin(), template_urls_.end(), + existing_turl) != template_urls_.end()); if (!existing_turl->keyword().empty()) keyword_to_template_map_.erase(existing_turl->keyword()); if (!existing_turl->sync_guid().empty()) guid_to_template_map_.erase(existing_turl->sync_guid()); - // This call handles copying over the values (while retaining the id). + provider_map_.Remove(existing_turl); + TemplateURLID previous_id = existing_turl->id(); + TemplateURL* modifiable_turl = const_cast<TemplateURL*>(existing_turl); + *modifiable_turl = new_values; + modifiable_turl->data_.id = previous_id; UIThreadSearchTermsData search_terms_data; - provider_map_.Update(existing_turl, new_values, search_terms_data); + provider_map_.Add(existing_turl, search_terms_data); if (!existing_turl->keyword().empty()) keyword_to_template_map_[existing_turl->keyword()] = existing_turl; @@ -1520,14 +1550,18 @@ void TemplateURLService::UpdateDefaultSearch() { SetDefaultSearchProviderNoNotify(NULL); RemoveNoNotify(old_default); } else if (default_search_provider_) { - new_default_from_prefs->set_created_by_policy(true); - UpdateNoNotify(default_search_provider_, *new_default_from_prefs.get()); + TemplateURLData data(new_default_from_prefs->data()); + data.created_by_policy = true; + TemplateURL new_values(data); + UpdateNoNotify(default_search_provider_, new_values); } else { // AddNoNotify will take ownership of new_template, so it's safe to // release. - TemplateURL* new_template = new_default_from_prefs.release(); - if (new_template) { - new_template->set_created_by_policy(true); + TemplateURL* new_template = NULL; + if (new_default_from_prefs.get()) { + TemplateURLData data(new_default_from_prefs->data()); + data.created_by_policy = true; + new_template = new TemplateURL(data); AddNoNotify(new_template); } SetDefaultSearchProviderNoNotify(new_template); @@ -1538,9 +1572,11 @@ void TemplateURLService::UpdateDefaultSearch() { is_default_search_managed_ = new_is_default_managed; // AddNoNotify will take ownership of new_template, so it's safe to // release. - TemplateURL* new_template = new_default_from_prefs.release(); - if (new_template) { - new_template->set_created_by_policy(true); + TemplateURL* new_template = NULL; + if (new_default_from_prefs.get()) { + TemplateURLData data(new_default_from_prefs->data()); + data.created_by_policy = true; + new_template = new TemplateURL(data); AddNoNotify(new_template); } SetDefaultSearchProviderNoNotify(new_template); @@ -1550,7 +1586,7 @@ void TemplateURLService::UpdateDefaultSearch() { is_default_search_managed_ = new_is_default_managed; // If we had a default, delete the previous default if created by policy // and set a likely default. - if (NULL != default_search_provider_ && + if ((default_search_provider_ != NULL) && default_search_provider_->created_by_policy()) { const TemplateURL* old_default = default_search_provider_; default_search_provider_ = NULL; @@ -1570,17 +1606,17 @@ void TemplateURLService::UpdateDefaultSearch() { void TemplateURLService::SetDefaultSearchProviderNoNotify( const TemplateURL* url) { - DCHECK(!url || std::find(template_urls_.begin(), template_urls_.end(), url) - != template_urls_.end()); + DCHECK(!url || std::find(template_urls_.begin(), template_urls_.end(), url) != + template_urls_.end()); default_search_provider_ = url; if (url) { TemplateURL* modifiable_url = const_cast<TemplateURL*>(url); // Don't mark the url as edited, otherwise we won't be able to rev the // template urls we ship with. - modifiable_url->set_show_in_default_list(true); + modifiable_url->data_.show_in_default_list = true; if (service_.get()) - service_.get()->UpdateKeyword(*url); + service_->UpdateKeyword(*url); if (url->url_ref().HasGoogleBaseURLs()) { GoogleURLTracker::RequestServerCheck(); @@ -1617,9 +1653,9 @@ void TemplateURLService::SetDefaultSearchProviderNoNotify( void TemplateURLService::AddNoNotify(TemplateURL* template_url) { DCHECK(template_url); DCHECK_EQ(kInvalidTemplateURLID, template_url->id()); - DCHECK(std::find(template_urls_.begin(), template_urls_.end(), template_url) - == template_urls_.end()); - template_url->set_id(++next_id_); + DCHECK(std::find(template_urls_.begin(), template_urls_.end(), + template_url) == template_urls_.end()); + template_url->data_.id = ++next_id_; template_urls_.push_back(template_url); AddToMaps(template_url); @@ -1632,9 +1668,8 @@ void TemplateURLService::AddNoNotify(TemplateURL* template_url) { } void TemplateURLService::RemoveNoNotify(const TemplateURL* template_url) { - TemplateURLVector::iterator i = std::find(template_urls_.begin(), - template_urls_.end(), - template_url); + TemplateURLVector::iterator i = + std::find(template_urls_.begin(), template_urls_.end(), template_url); if (i == template_urls_.end()) return; @@ -1650,7 +1685,7 @@ void TemplateURLService::RemoveNoNotify(const TemplateURL* template_url) { template_urls_.erase(i); if (service_.get()) - service_->RemoveKeyword(*template_url); + service_->RemoveKeyword(template_url->id()); // Inform sync of the deletion. ProcessTemplateURLChange(template_url, SyncChange::ACTION_DELETE); @@ -1715,7 +1750,7 @@ void TemplateURLService::RemoveProvidersCreatedByPolicy( i = template_urls->erase(i); if (service_.get()) - service_->RemoveKeyword(*template_url); + service_->RemoveKeyword(template_url->id()); delete template_url; } else { ++i; @@ -1727,8 +1762,9 @@ void TemplateURLService::ResetTemplateURLGUID(const TemplateURL* url, const std::string& guid) { DCHECK(!guid.empty()); - TemplateURL new_url(*url); - new_url.set_sync_guid(guid); + TemplateURLData data(url->data()); + data.sync_guid = guid; + TemplateURL new_url(data); UpdateNoNotify(url, new_url); } @@ -1772,15 +1808,16 @@ bool TemplateURLService::ResolveSyncKeywordConflict( existing_turl->created_by_policy()) { string16 new_keyword = UniquifyKeyword(*sync_turl); DCHECK(!GetTemplateURLForKeyword(new_keyword)); - sync_turl->set_keyword(new_keyword); + sync_turl->data_.SetKeyword(new_keyword); // If we update the cloud TURL, we need to push an update back to sync // informing it that something has changed. SyncData sync_data = CreateSyncDataFromTemplateURL(*sync_turl); change_list->push_back(SyncChange(SyncChange::ACTION_UPDATE, sync_data)); } else { string16 new_keyword = UniquifyKeyword(*existing_turl); - TemplateURL new_turl(*existing_turl); - new_turl.set_keyword(new_keyword); + TemplateURLData data(existing_turl->data()); + data.SetKeyword(new_keyword); + TemplateURL new_turl(data); UpdateNoNotify(existing_turl, new_turl); NotifyObservers(); } @@ -1808,30 +1845,24 @@ void TemplateURLService::MergeSyncAndLocalURLDuplicates( DCHECK(sync_turl); DCHECK(local_turl); DCHECK(change_list); - scoped_ptr<TemplateURL> scoped_sync_turl(sync_turl); - - if (scoped_sync_turl->last_modified() > local_turl->last_modified()) { + if (sync_turl->last_modified() > local_turl->last_modified()) { // Fully replace local_url with Sync's copy. Note that because use Add // rather than ResetTemplateURL, |sync_url| is added with a fresh // TemplateURLID. We don't need to sync the new ID back to the server since // it's only relevant locally. bool delete_default = (local_turl == GetDefaultSearchProvider()); - if (delete_default && is_default_search_managed_) { - NOTREACHED() << "Tried to delete managed default search provider"; - } else { - if (delete_default) - default_search_provider_ = NULL; + DCHECK(!delete_default || !is_default_search_managed_); + if (delete_default) + default_search_provider_ = NULL; - Remove(local_turl); + Remove(local_turl); - // Force the local ID to kInvalidTemplateURLID so we can add it. - scoped_sync_turl->set_id(kInvalidTemplateURLID); - TemplateURL* temp = scoped_sync_turl.release(); - Add(temp); - if (delete_default) - SetDefaultSearchProvider(temp); - } + // Force the local ID to kInvalidTemplateURLID so we can add it. + sync_turl->data_.id = kInvalidTemplateURLID; + Add(scoped_sync_turl.release()); + if (delete_default) + SetDefaultSearchProvider(sync_turl); } else { // Change the local TURL's GUID to the server's GUID and push an update to // Sync. This ensures that the rest of local_url's fields are sync'd up to @@ -1879,36 +1910,9 @@ void TemplateURLService::PatchMissingSyncGUIDs( TemplateURL* template_url = *i; DCHECK(template_url); if (template_url->sync_guid().empty()) { - template_url->set_sync_guid(guid::GenerateGUID()); + template_url->data_.sync_guid = guid::GenerateGUID(); if (service_.get()) service_->UpdateKeyword(*template_url); } } } - -// static -void TemplateURLService::UpdateTemplateURLWithSyncData( - TemplateURL* dst, - const SyncData& sync_data) { - sync_pb::SearchEngineSpecifics specifics = - sync_data.GetSpecifics().search_engine(); - dst->set_short_name(UTF8ToUTF16(specifics.short_name())); - dst->set_keyword(UTF8ToUTF16(specifics.keyword())); - dst->set_favicon_url(GURL(specifics.favicon_url())); - dst->SetURL(specifics.url()); - dst->set_safe_for_autoreplace(specifics.safe_for_autoreplace()); - dst->set_originating_url(GURL(specifics.originating_url())); - dst->set_date_created( - base::Time::FromInternalValue(specifics.date_created())); - std::vector<std::string> input_encodings; - base::SplitString(specifics.input_encodings(), ';', &input_encodings); - dst->set_input_encodings(input_encodings); - dst->set_show_in_default_list(specifics.show_in_default_list()); - dst->SetSuggestionsURL(specifics.suggestions_url()); - dst->SetPrepopulateId(specifics.prepopulate_id()); - dst->set_autogenerate_keyword(specifics.autogenerate_keyword()); - dst->SetInstantURL(specifics.instant_url()); - dst->set_last_modified( - base::Time::FromInternalValue(specifics.last_modified())); - dst->set_sync_guid(specifics.sync_guid()); -} diff --git a/chrome/browser/search_engines/template_url_service.h b/chrome/browser/search_engines/template_url_service.h index 9fd8eed..5fcfe74 100644 --- a/chrome/browser/search_engines/template_url_service.h +++ b/chrome/browser/search_engines/template_url_service.h @@ -141,6 +141,13 @@ class TemplateURLService : public WebDataServiceConsumer, // Takes ownership of |template_url| and adds it to this model. void Add(TemplateURL* template_url); + // Like Add(), but overwrites the |template_url|'s values with the provided + // ones. + void AddWithOverrides(const TemplateURL* template_url, + const string16& short_name, + const string16& keyword, + const std::string& url); + // Removes the keyword from the model. This deletes the supplied TemplateURL. // This fails if the supplied template_url is the default search provider. void Remove(const TemplateURL* template_url); @@ -290,10 +297,12 @@ class TemplateURLService : public WebDataServiceConsumer, // from |turl|. static SyncData CreateSyncDataFromTemplateURL(const TemplateURL& turl); - // Returns a heap-allocated TemplateURL, populated by |sync_data|'s fields. - // This does the opposite of CreateSyncDataFromTemplateURL. The caller owns - // the returned TemplateURL*. - static TemplateURL* CreateTemplateURLFromSyncData(const SyncData& sync_data); + // Creates a new heap-allocated TemplateURL* which is populated by overlaying + // |sync_data| atop |existing_turl|. |existing_turl| may be NULL; if not it + // remains unmodified. The caller owns the returned TemplateURL*. + static TemplateURL* CreateTemplateURLFromTemplateURLAndSyncData( + const TemplateURL* existing_turl, + const SyncData& sync_data); // Returns a map mapping Sync GUIDs to pointers to SyncData. static SyncDataMap CreateGUIDToSyncDataMap(const SyncDataList& sync_data); @@ -507,11 +516,6 @@ class TemplateURLService : public WebDataServiceConsumer, // where old entries were being pushed to Sync without a sync_guid. void PatchMissingSyncGUIDs(std::vector<TemplateURL*>* template_urls); - // Overwrites |dst|'s synced fields with values from |sync_data|. This does - // commit |dst| to the TemplateURLService. - static void UpdateTemplateURLWithSyncData(TemplateURL* dst, - const SyncData& sync_data); - content::NotificationRegistrar registrar_; // Mapping from keyword to the TemplateURL. diff --git a/chrome/browser/search_engines/template_url_service_sync_unittest.cc b/chrome/browser/search_engines/template_url_service_sync_unittest.cc index 8179cea..2cbe233 100644 --- a/chrome/browser/search_engines/template_url_service_sync_unittest.cc +++ b/chrome/browser/search_engines/template_url_service_sync_unittest.cc @@ -44,16 +44,18 @@ void SetManagedDefaultSearchPreferences(TemplateURLService* turl_service, TestingProfile* profile, bool enabled, const std::string& name, + const std::string& keyword, const std::string& search_url, const std::string& suggest_url, const std::string& icon_url, - const std::string& encodings, - const std::string& keyword) { + const std::string& encodings) { TestingPrefService* pref_service = profile->GetTestingPrefService(); pref_service->SetManagedPref(prefs::kDefaultSearchProviderEnabled, Value::CreateBooleanValue(enabled)); pref_service->SetManagedPref(prefs::kDefaultSearchProviderName, Value::CreateStringValue(name)); + pref_service->SetManagedPref(prefs::kDefaultSearchProviderKeyword, + Value::CreateStringValue(keyword)); pref_service->SetManagedPref(prefs::kDefaultSearchProviderSearchURL, Value::CreateStringValue(search_url)); pref_service->SetManagedPref(prefs::kDefaultSearchProviderSuggestURL, @@ -62,8 +64,6 @@ void SetManagedDefaultSearchPreferences(TemplateURLService* turl_service, Value::CreateStringValue(icon_url)); pref_service->SetManagedPref(prefs::kDefaultSearchProviderEncodings, Value::CreateStringValue(encodings)); - pref_service->SetManagedPref(prefs::kDefaultSearchProviderKeyword, - Value::CreateStringValue(keyword)); } // Remove all the managed preferences for the default search provider and @@ -71,13 +71,13 @@ void SetManagedDefaultSearchPreferences(TemplateURLService* turl_service, void RemoveManagedDefaultSearchPreferences(TemplateURLService* turl_service, TestingProfile* profile) { TestingPrefService* pref_service = profile->GetTestingPrefService(); - pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderSearchURL); pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderEnabled); pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderName); + pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderKeyword); + pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderSearchURL); pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderSuggestURL); pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderIconURL); pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderEncodings); - pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderKeyword); pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderID); pref_service->RemoveManagedPref(prefs::kDefaultSearchProviderPrepopulateID); } @@ -267,30 +267,30 @@ TemplateURL* TemplateURLServiceSyncTest::CreateTestTemplateURL( const std::string& guid, time_t last_mod, bool created_by_policy) const { - TemplateURL* turl = new TemplateURL(); - turl->set_short_name(ASCIIToUTF16("unittest")); - turl->set_keyword(keyword); - turl->set_safe_for_autoreplace(true); - turl->set_date_created(Time::FromTimeT(100)); - turl->set_last_modified(Time::FromTimeT(last_mod)); - turl->set_created_by_policy(created_by_policy); - turl->SetPrepopulateId(999999); + TemplateURLData data; + data.short_name = ASCIIToUTF16("unittest"); + data.SetKeyword(keyword); + data.SetURL(url); + data.favicon_url = GURL("http://favicon.url"); + data.safe_for_autoreplace = true; + data.date_created = Time::FromTimeT(100); + data.last_modified = Time::FromTimeT(last_mod); + data.created_by_policy = created_by_policy; + data.prepopulate_id = 999999; if (!guid.empty()) - turl->set_sync_guid(guid); - turl->SetURL(url); - turl->set_favicon_url(GURL("http://favicon.url")); - return turl; + data.sync_guid = guid; + return new TemplateURL(data); } void TemplateURLServiceSyncTest::AssertEquals(const TemplateURL& expected, const TemplateURL& actual) const { ASSERT_EQ(expected.short_name(), actual.short_name()); + ASSERT_EQ(expected.keyword(), actual.keyword()); ASSERT_EQ(expected.url(), actual.url()); ASSERT_EQ(expected.suggestions_url(), actual.suggestions_url()); - ASSERT_EQ(expected.keyword(), actual.keyword()); + ASSERT_EQ(expected.favicon_url(), actual.favicon_url()); ASSERT_EQ(expected.show_in_default_list(), actual.show_in_default_list()); ASSERT_EQ(expected.safe_for_autoreplace(), actual.safe_for_autoreplace()); - ASSERT_EQ(expected.favicon_url(), actual.favicon_url()); ASSERT_EQ(expected.input_encodings(), actual.input_encodings()); ASSERT_EQ(expected.date_created(), actual.date_created()); ASSERT_EQ(expected.last_modified(), actual.last_modified()); @@ -341,7 +341,8 @@ SyncDataList TemplateURLServiceSyncTest::CreateInitialSyncData() const { TemplateURL* TemplateURLServiceSyncTest::Deserialize( const SyncData& sync_data) { - return TemplateURLService::CreateTemplateURLFromSyncData(sync_data); + return TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData(NULL, + sync_data); } @@ -1097,14 +1098,17 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwiceWithSameSyncData) { // We should have updated the original TemplateURL with Sync's version. // Keep a copy of it so we can compare it after we re-merge. - ASSERT_TRUE(model()->GetTemplateURLForGUID("key1")); - TemplateURL updated_turl(*model()->GetTemplateURLForGUID("key1")); - EXPECT_EQ(Time::FromTimeT(90), updated_turl.last_modified()); + const TemplateURL* key1_url = model()->GetTemplateURLForGUID("key1"); + ASSERT_TRUE(key1_url); + scoped_ptr<TemplateURL> updated_turl(new TemplateURL(key1_url->data())); + EXPECT_EQ(Time::FromTimeT(90), updated_turl->last_modified()); // Modify a single field of the initial data. This should not be updated in // the second merge, as the last_modified timestamp remains the same. scoped_ptr<TemplateURL> temp_turl(Deserialize(initial_data[0])); - temp_turl->set_short_name(ASCIIToUTF16("SomethingDifferent")); + TemplateURLData data(temp_turl->data()); + data.short_name = ASCIIToUTF16("SomethingDifferent"); + temp_turl.reset(new TemplateURL(data)); initial_data.clear(); initial_data.push_back( TemplateURLService::CreateSyncDataFromTemplateURL(*temp_turl)); @@ -1121,7 +1125,7 @@ TEST_F(TemplateURLServiceSyncTest, MergeTwiceWithSameSyncData) { // Check that the TemplateURL was not modified. const TemplateURL* reupdated_turl = model()->GetTemplateURLForGUID("key1"); ASSERT_TRUE(reupdated_turl); - AssertEquals(updated_turl, *reupdated_turl); + AssertEquals(*updated_turl, *reupdated_turl); } TEST_F(TemplateURLServiceSyncTest, SyncedDefaultGUIDArrivesFirst) { diff --git a/chrome/browser/search_engines/template_url_service_unittest.cc b/chrome/browser/search_engines/template_url_service_unittest.cc index 7552a1f..553ebfb 100644 --- a/chrome/browser/search_engines/template_url_service_unittest.cc +++ b/chrome/browser/search_engines/template_url_service_unittest.cc @@ -42,16 +42,18 @@ namespace { // current data. The caller owns the returned TemplateURL*. TemplateURL* CreatePreloadedTemplateURL(bool safe_for_autoreplace, int prepopulate_id) { - TemplateURL* t_url = new TemplateURL(); - t_url->set_keyword(ASCIIToUTF16("unittest")); - t_url->set_short_name(ASCIIToUTF16("unittest")); - t_url->SetPrepopulateId(prepopulate_id); - t_url->set_safe_for_autoreplace(safe_for_autoreplace); - t_url->set_date_created(Time::FromTimeT(100)); - t_url->set_last_modified(Time::FromTimeT(100)); - t_url->SetURL("http://www.unittest.com/"); - t_url->set_favicon_url(GURL("http://favicon.url")); - return t_url; + TemplateURLData data; + data.short_name = ASCIIToUTF16("unittest"); + data.SetKeyword(ASCIIToUTF16("unittest")); + data.SetURL("http://www.unittest.com/"); + data.favicon_url = GURL("http://favicon.url"); + data.show_in_default_list = true; + data.safe_for_autoreplace = safe_for_autoreplace; + data.input_encodings.push_back("UTF-8"); + data.date_created = Time::FromTimeT(100); + data.last_modified = Time::FromTimeT(100); + data.prepopulate_id = prepopulate_id; + return new TemplateURL(data); } @@ -102,9 +104,10 @@ void TestGenerateSearchURL::RunTest() { // gtest documentation. bool everything_passed = true; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(generate_url_cases); ++i) { - TemplateURL t_url; + TemplateURLData data; if (generate_url_cases[i].url) - t_url.SetURL(generate_url_cases[i].url); + data.SetURL(generate_url_cases[i].url); + TemplateURL t_url(data); std::string result = (search_terms_data_ ? TemplateURLService::GenerateSearchURLUsingTermsData(&t_url, *search_terms_data_) : @@ -158,14 +161,14 @@ class TemplateURLServiceTest : public testing::Test { virtual void SetUp(); virtual void TearDown(); - TemplateURL* AddKeywordWithDate(const std::string& keyword, + TemplateURL* AddKeywordWithDate(const std::string& short_name, + const std::string& keyword, bool autogenerate_keyword, const std::string& url, const std::string& suggest_url, const std::string& favicon_url, - const std::string& encodings, - const std::string& short_name, bool safe_for_autoreplace, + const std::string& encodings, Time date_created, Time last_modified); @@ -180,11 +183,11 @@ class TemplateURLServiceTest : public testing::Test { // notification. void SetManagedDefaultSearchPreferences(bool enabled, const std::string& name, + const std::string& keyword, const std::string& search_url, const std::string& suggest_url, const std::string& icon_url, - const std::string& encodings, - const std::string& keyword); + const std::string& encodings); // Remove all the managed preferences for the default search provider and // trigger notification. @@ -228,29 +231,28 @@ void TemplateURLServiceTest::TearDown() { } TemplateURL* TemplateURLServiceTest::AddKeywordWithDate( + const std::string& short_name, const std::string& keyword, bool autogenerate_keyword, const std::string& url, const std::string& suggest_url, const std::string& favicon_url, - const std::string& encodings, - const std::string& short_name, bool safe_for_autoreplace, + const std::string& encodings, Time date_created, Time last_modified) { - TemplateURL* t_url = new TemplateURL(); - t_url->set_short_name(UTF8ToUTF16(short_name)); - t_url->set_keyword(UTF8ToUTF16(keyword)); - t_url->set_autogenerate_keyword(autogenerate_keyword); - t_url->set_safe_for_autoreplace(safe_for_autoreplace); - std::vector<std::string> encodings_vector; - base::SplitString(encodings, ';', &encodings_vector); - t_url->set_input_encodings(encodings_vector); - t_url->set_date_created(date_created); - t_url->set_last_modified(last_modified); - t_url->SetSuggestionsURL(suggest_url); - t_url->SetURL(url); - t_url->set_favicon_url(GURL(favicon_url)); + TemplateURLData data; + data.short_name = UTF8ToUTF16(short_name); + data.SetKeyword(UTF8ToUTF16(keyword)); + data.SetAutogenerateKeyword(autogenerate_keyword); + data.SetURL(url); + data.suggestions_url = suggest_url; + data.favicon_url = GURL(favicon_url); + data.safe_for_autoreplace = safe_for_autoreplace; + base::SplitString(encodings, ';', &data.input_encodings); + data.date_created = date_created; + data.last_modified = last_modified; + TemplateURL* t_url = new TemplateURL(data); model()->Add(t_url); EXPECT_NE(0, t_url->id()); return t_url; @@ -259,16 +261,16 @@ TemplateURL* TemplateURLServiceTest::AddKeywordWithDate( void TemplateURLServiceTest::AssertEquals(const TemplateURL& expected, const TemplateURL& actual) { ASSERT_EQ(expected.short_name(), actual.short_name()); + ASSERT_EQ(expected.keyword(), actual.keyword()); ASSERT_EQ(expected.url(), actual.url()); ASSERT_EQ(expected.suggestions_url(), actual.suggestions_url()); - ASSERT_EQ(expected.keyword(), actual.keyword()); + ASSERT_EQ(expected.favicon_url(), actual.favicon_url()); ASSERT_EQ(expected.show_in_default_list(), actual.show_in_default_list()); ASSERT_EQ(expected.safe_for_autoreplace(), actual.safe_for_autoreplace()); - ASSERT_EQ(expected.favicon_url(), actual.favicon_url()); - ASSERT_EQ(expected.date_created(), actual.date_created()); - ASSERT_EQ(expected.last_modified(), actual.last_modified()); ASSERT_EQ(expected.input_encodings(), actual.input_encodings()); ASSERT_EQ(expected.id(), actual.id()); + ASSERT_EQ(expected.date_created(), actual.date_created()); + ASSERT_EQ(expected.last_modified(), actual.last_modified()); ASSERT_EQ(expected.sync_guid(), actual.sync_guid()); } @@ -277,28 +279,30 @@ void TemplateURLServiceTest::ExpectSimilar(const TemplateURL* expected, ASSERT_TRUE(expected != NULL); ASSERT_TRUE(actual != NULL); EXPECT_EQ(expected->short_name(), actual->short_name()); + EXPECT_EQ(expected->keyword(), actual->keyword()); EXPECT_EQ(expected->url(), actual->url()); EXPECT_EQ(expected->suggestions_url(), actual->suggestions_url()); - EXPECT_EQ(expected->keyword(), actual->keyword()); + EXPECT_EQ(expected->favicon_url(), actual->favicon_url()); EXPECT_EQ(expected->show_in_default_list(), actual->show_in_default_list()); EXPECT_EQ(expected->safe_for_autoreplace(), actual->safe_for_autoreplace()); - EXPECT_EQ(expected->favicon_url(), actual->favicon_url()); EXPECT_EQ(expected->input_encodings(), actual->input_encodings()); } void TemplateURLServiceTest::SetManagedDefaultSearchPreferences( bool enabled, const std::string& name, + const std::string& keyword, const std::string& search_url, const std::string& suggest_url, const std::string& icon_url, - const std::string& encodings, - const std::string& keyword) { + const std::string& encodings) { TestingPrefService* service = test_util_.profile()->GetTestingPrefService(); service->SetManagedPref(prefs::kDefaultSearchProviderEnabled, Value::CreateBooleanValue(enabled)); service->SetManagedPref(prefs::kDefaultSearchProviderName, Value::CreateStringValue(name)); + service->SetManagedPref(prefs::kDefaultSearchProviderKeyword, + Value::CreateStringValue(keyword)); service->SetManagedPref(prefs::kDefaultSearchProviderSearchURL, Value::CreateStringValue(search_url)); service->SetManagedPref(prefs::kDefaultSearchProviderSuggestURL, @@ -307,19 +311,17 @@ void TemplateURLServiceTest::SetManagedDefaultSearchPreferences( Value::CreateStringValue(icon_url)); service->SetManagedPref(prefs::kDefaultSearchProviderEncodings, Value::CreateStringValue(encodings)); - service->SetManagedPref(prefs::kDefaultSearchProviderKeyword, - Value::CreateStringValue(keyword)); } void TemplateURLServiceTest::RemoveManagedDefaultSearchPreferences() { TestingPrefService* service = test_util_.profile()->GetTestingPrefService(); - service->RemoveManagedPref(prefs::kDefaultSearchProviderSearchURL); service->RemoveManagedPref(prefs::kDefaultSearchProviderEnabled); service->RemoveManagedPref(prefs::kDefaultSearchProviderName); + service->RemoveManagedPref(prefs::kDefaultSearchProviderKeyword); + service->RemoveManagedPref(prefs::kDefaultSearchProviderSearchURL); service->RemoveManagedPref(prefs::kDefaultSearchProviderSuggestURL); service->RemoveManagedPref(prefs::kDefaultSearchProviderIconURL); service->RemoveManagedPref(prefs::kDefaultSearchProviderEncodings); - service->RemoveManagedPref(prefs::kDefaultSearchProviderKeyword); service->RemoveManagedPref(prefs::kDefaultSearchProviderID); service->RemoveManagedPref(prefs::kDefaultSearchProviderPrepopulateID); } @@ -399,15 +401,16 @@ TEST_F(TemplateURLServiceTest, AddUpdateRemove) { test_util_.VerifyLoad(); const size_t initial_count = model()->GetTemplateURLs().size(); - TemplateURL* t_url = new TemplateURL(); - t_url->set_short_name(ASCIIToUTF16("google")); - t_url->set_keyword(ASCIIToUTF16("keyword")); - t_url->set_safe_for_autoreplace(true); - t_url->set_date_created(Time::FromTimeT(100)); - t_url->set_last_modified(Time::FromTimeT(100)); - t_url->set_sync_guid("00000000-0000-0000-0000-000000000001"); - t_url->SetURL("http://www.google.com/foo/bar"); - t_url->set_favicon_url(GURL("http://favicon.url")); + TemplateURLData data; + data.short_name = ASCIIToUTF16("google"); + data.SetKeyword(ASCIIToUTF16("keyword")); + data.SetURL("http://www.google.com/foo/bar"); + data.favicon_url = GURL("http://favicon.url"); + data.safe_for_autoreplace = true; + data.date_created = Time::FromTimeT(100); + data.last_modified = Time::FromTimeT(100); + data.sync_guid = "00000000-0000-0000-0000-000000000001"; + TemplateURL* t_url = new TemplateURL(data); model()->Add(t_url); ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("keyword"), GURL(), NULL)); @@ -418,7 +421,7 @@ TEST_F(TemplateURLServiceTest, AddUpdateRemove) { // We need to make a second copy as the model takes ownership of |t_url| and // will delete it. We have to do this after calling Add() since that gives // |t_url| its ID. - TemplateURL cloned_url(*t_url); + scoped_ptr<TemplateURL> cloned_url(new TemplateURL(t_url->data())); // Reload the model to verify it was actually saved to the database. test_util_.ResetModel(true); @@ -426,7 +429,7 @@ TEST_F(TemplateURLServiceTest, AddUpdateRemove) { const TemplateURL* loaded_url = model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword")); ASSERT_TRUE(loaded_url != NULL); - AssertEquals(cloned_url, *loaded_url); + AssertEquals(*cloned_url, *loaded_url); ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("keyword"), GURL(), NULL)); @@ -447,13 +450,13 @@ TEST_F(TemplateURLServiceTest, AddUpdateRemove) { ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("keyword"), GURL(), NULL)); ASSERT_FALSE(model()->CanReplaceKeyword(ASCIIToUTF16("b"), GURL(), NULL)); - cloned_url = *loaded_url; + cloned_url.reset(new TemplateURL(loaded_url->data())); test_util_.BlockTillServiceProcessesRequests(); test_util_.ResetModel(true); ASSERT_EQ(initial_count + 1, model()->GetTemplateURLs().size()); loaded_url = model()->GetTemplateURLForKeyword(ASCIIToUTF16("b")); ASSERT_TRUE(loaded_url != NULL); - AssertEquals(cloned_url, *loaded_url); + AssertEquals(*cloned_url, *loaded_url); // We changed a TemplateURL in the service, so ensure that the time was // updated. ASSERT_EQ(base::Time::FromDoubleT(1337), loaded_url->last_modified()); @@ -521,24 +524,20 @@ TEST_F(TemplateURLServiceTest, ClearBrowsingData_Keywords) { EXPECT_EQ(0U, model()->GetTemplateURLs().size()); // Create one with a 0 time. - AddKeywordWithDate("key1", false, "http://foo1", "http://suggest1", - "http://icon1", "UTF-8;UTF-16", "name1", true, Time(), - Time()); + AddKeywordWithDate("name1", "key1", false, "http://foo1", "http://suggest1", + "http://icon1", true, "UTF-8;UTF-16", Time(), Time()); // Create one for now and +/- 1 day. - AddKeywordWithDate("key2", false, "http://foo2", "http://suggest2", - "http://icon2", "UTF-8;UTF-16", "name2", true, - now - one_day, Time()); - AddKeywordWithDate("key3", false, "http://foo3", std::string(), std::string(), - std::string(), "name3", true, now, Time()); - AddKeywordWithDate("key4", false, "http://foo4", std::string(), std::string(), - std::string(), "name4", true, now + one_day, Time()); + AddKeywordWithDate("name2", "key2", false, "http://foo2", "http://suggest2", + "http://icon2", true, "UTF-8;UTF-16", now - one_day, Time()); + AddKeywordWithDate("name3", "key3", false, "http://foo3", std::string(), + std::string(), true, std::string(), now, Time()); + AddKeywordWithDate("name4", "key4", false, "http://foo4", std::string(), + std::string(), true, std::string(), now + one_day, Time()); // Try the other three states. - AddKeywordWithDate("key5", false, "http://foo5", "http://suggest5", - "http://icon5", "UTF-8;UTF-16", "name5", false, now, - Time()); - AddKeywordWithDate("key6", false, "http://foo6", "http://suggest6", - "http://icon6", "UTF-8;UTF-16", "name6", false, - month_ago, Time()); + AddKeywordWithDate("name5", "key5", false, "http://foo5", "http://suggest5", + "http://icon5", false, "UTF-8;UTF-16", now, Time()); + AddKeywordWithDate("name6", "key6", false, "http://foo6", "http://suggest6", + "http://icon6", false, "UTF-8;UTF-16", month_ago, Time()); // We just added a few items, validate them. EXPECT_EQ(6U, model()->GetTemplateURLs().size()); @@ -583,13 +582,12 @@ TEST_F(TemplateURLServiceTest, ClearBrowsingData_KeywordsForOrigin) { EXPECT_EQ(0U, model()->GetTemplateURLs().size()); // Create one for now and +/- 1 day. - AddKeywordWithDate("key1", false, "http://foo1", "http://suggest1", - "http://icon2", "UTF-8;UTF-16", "name2", true, - now - one_day, Time()); - AddKeywordWithDate("key2", false, "http://foo2", std::string(), std::string(), - std::string(), "name2", true, now, Time()); - AddKeywordWithDate("key3", false, "http://foo3", std::string(), std::string(), - std::string(), "name3", true, now + one_day, Time()); + AddKeywordWithDate("name1", "key1", false, "http://foo1", "http://suggest1", + "http://icon2", true, "UTF-8;UTF-16", now - one_day, Time()); + AddKeywordWithDate("name2", "key2", false, "http://foo2", std::string(), + std::string(), true, std::string(), now, Time()); + AddKeywordWithDate("name3", "key3", false, "http://foo3", std::string(), + std::string(), true, std::string(), now + one_day, Time()); // We just added a few items, validate them. EXPECT_EQ(3U, model()->GetTemplateURLs().size()); @@ -626,13 +624,14 @@ TEST_F(TemplateURLServiceTest, Reset) { // Add a new TemplateURL. test_util_.VerifyLoad(); const size_t initial_count = model()->GetTemplateURLs().size(); - TemplateURL* t_url = new TemplateURL(); - t_url->set_short_name(ASCIIToUTF16("google")); - t_url->set_keyword(ASCIIToUTF16("keyword")); - t_url->set_date_created(Time::FromTimeT(100)); - t_url->set_last_modified(Time::FromTimeT(100)); - t_url->SetURL("http://www.google.com/foo/bar"); - t_url->set_favicon_url(GURL("http://favicon.url")); + TemplateURLData data; + data.short_name = ASCIIToUTF16("google"); + data.SetKeyword(ASCIIToUTF16("keyword")); + data.SetURL("http://www.google.com/foo/bar"); + data.favicon_url = GURL("http://favicon.url"); + data.date_created = Time::FromTimeT(100); + data.last_modified = Time::FromTimeT(100); + TemplateURL* t_url = new TemplateURL(data); model()->Add(t_url); VerifyObserverCount(1); @@ -656,14 +655,14 @@ TEST_F(TemplateURLServiceTest, Reset) { ASSERT_TRUE( model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword")) == NULL); - TemplateURL last_url(*t_url); + scoped_ptr<TemplateURL> cloned_url(new TemplateURL(t_url->data())); // Reload the model from the database and make sure the change took. test_util_.ResetModel(true); EXPECT_EQ(initial_count + 1, model()->GetTemplateURLs().size()); const TemplateURL* read_url = model()->GetTemplateURLForKeyword(new_keyword); ASSERT_TRUE(read_url); - AssertEquals(last_url, *read_url); + AssertEquals(*cloned_url, *read_url); ASSERT_EQ(base::Time::FromDoubleT(1337), read_url->last_modified()); } @@ -671,9 +670,8 @@ TEST_F(TemplateURLServiceTest, DefaultSearchProvider) { // Add a new TemplateURL. test_util_.VerifyLoad(); const size_t initial_count = model()->GetTemplateURLs().size(); - TemplateURL* t_url = AddKeywordWithDate("key1", false, "http://foo1", - "http://sugg1", "http://icon1", "UTF-8;UTF-16", "name1", true, Time(), - Time()); + TemplateURL* t_url = AddKeywordWithDate("name1", "key1", false, "http://foo1", + "http://sugg1", "http://icon1", true, "UTF-8;UTF-16", Time(), Time()); test_util_.ResetObserverCount(); model()->SetDefaultSearchProvider(t_url); @@ -685,13 +683,13 @@ TEST_F(TemplateURLServiceTest, DefaultSearchProvider) { VerifyObserverCount(1); test_util_.BlockTillServiceProcessesRequests(); - TemplateURL cloned_url(*t_url); + scoped_ptr<TemplateURL> cloned_url(new TemplateURL(t_url->data())); // Make sure when we reload we get a default search provider. test_util_.ResetModel(true); EXPECT_EQ(initial_count + 1, model()->GetTemplateURLs().size()); ASSERT_TRUE(model()->GetDefaultSearchProvider()); - AssertEquals(cloned_url, *model()->GetDefaultSearchProvider()); + AssertEquals(*cloned_url, *model()->GetDefaultSearchProvider()); } TEST_F(TemplateURLServiceTest, TemplateURLWithNoKeyword) { @@ -699,8 +697,8 @@ TEST_F(TemplateURLServiceTest, TemplateURLWithNoKeyword) { const size_t initial_count = model()->GetTemplateURLs().size(); - AddKeywordWithDate("", false, "http://foo1", "http://sugg1", - "http://icon1", "UTF-8;UTF-16", "name1", true, Time(), Time()); + AddKeywordWithDate("name1", std::string(), false, "http://foo1", + "http://sugg1", "http://icon1", true, "UTF-8;UTF-16", Time(), Time()); // We just added a few items, validate them. ASSERT_EQ(initial_count + 1, model()->GetTemplateURLs().size()); @@ -723,9 +721,8 @@ TEST_F(TemplateURLServiceTest, TemplateURLWithNoKeyword) { TEST_F(TemplateURLServiceTest, CantReplaceWithSameKeyword) { test_util_.ChangeModelToLoadState(); ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("foo"), GURL(), NULL)); - TemplateURL* t_url = AddKeywordWithDate("foo", false, "http://foo1", - "http://sugg1", "http://icon1", "UTF-8;UTF-16", "name1", true, Time(), - Time()); + TemplateURL* t_url = AddKeywordWithDate("name1", "foo", false, "http://foo1", + "http://sugg1", "http://icon1", true, "UTF-8;UTF-16", Time(), Time()); // Can still replace, newly added template url is marked safe to replace. ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("foo"), @@ -744,9 +741,9 @@ TEST_F(TemplateURLServiceTest, CantReplaceWithSameHosts) { test_util_.ChangeModelToLoadState(); ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("foo"), GURL("http://foo.com"), NULL)); - TemplateURL* t_url = AddKeywordWithDate("foo", false, "http://foo.com", - "http://sugg1", "http://icon1", "UTF-8;UTF-16", "name1", true, Time(), - Time()); + TemplateURL* t_url = AddKeywordWithDate("name1", "foo", false, + "http://foo.com", "http://sugg1", "http://icon1", true, "UTF-8;UTF-16", + Time(), Time()); // Can still replace, newly added template url is marked safe to replace. ASSERT_TRUE(model()->CanReplaceKeyword(ASCIIToUTF16("bar"), @@ -774,20 +771,21 @@ TEST_F(TemplateURLServiceTest, HasDefaultSearchProvider) { TEST_F(TemplateURLServiceTest, DefaultSearchProviderLoadedFromPrefs) { test_util_.VerifyLoad(); - TemplateURL* t_url = new TemplateURL(); - t_url->set_short_name(ASCIIToUTF16("a")); - t_url->set_safe_for_autoreplace(true); - t_url->set_date_created(Time::FromTimeT(100)); - t_url->set_last_modified(Time::FromTimeT(100)); - t_url->SetSuggestionsURL("http://url2"); - t_url->SetURL("http://url"); - t_url->SetInstantURL("http://instant"); + TemplateURLData data; + data.short_name = ASCIIToUTF16("a"); + data.safe_for_autoreplace = true; + data.SetURL("http://url"); + data.suggestions_url = "http://url2"; + data.instant_url = "http://instant"; + data.date_created = Time::FromTimeT(100); + data.last_modified = Time::FromTimeT(100); + TemplateURL* t_url = new TemplateURL(data); model()->Add(t_url); const TemplateURLID id = t_url->id(); model()->SetDefaultSearchProvider(t_url); test_util_.BlockTillServiceProcessesRequests(); - TemplateURL first_default_search_provider(*t_url); + scoped_ptr<TemplateURL> cloned_url(new TemplateURL(t_url->data())); // Reset the model and don't load it. The template url we set as the default // should be pulled from prefs now. @@ -807,8 +805,7 @@ TEST_F(TemplateURLServiceTest, DefaultSearchProviderLoadedFromPrefs) { test_util_.VerifyLoad(); ASSERT_TRUE(model()->GetDefaultSearchProvider()); - AssertEquals(first_default_search_provider, - *model()->GetDefaultSearchProvider()); + AssertEquals(*cloned_url, *model()->GetDefaultSearchProvider()); } TEST_F(TemplateURLServiceTest, BuildQueryTerms) { @@ -872,9 +869,8 @@ TEST_F(TemplateURLServiceTest, UpdateKeywordSearchTermsForURL) { }; test_util_.ChangeModelToLoadState(); - AddKeywordWithDate("x", false, "http://x/foo?q={searchTerms}", - "http://sugg1", "http://icon1", "UTF-8;UTF-16", "name", false, Time(), - Time()); + AddKeywordWithDate("name", "x", false, "http://x/foo?q={searchTerms}", + "http://sugg1", "http://icon1", false, "UTF-8;UTF-16", Time(), Time()); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(data); ++i) { history::URLVisitedDetails details; @@ -895,8 +891,8 @@ TEST_F(TemplateURLServiceTest, DontUpdateKeywordSearchForNonReplaceable) { }; test_util_.ChangeModelToLoadState(); - AddKeywordWithDate("x", false, "http://x/foo", "http://sugg1", - "http://icon1", "UTF-8;UTF-16", "name", false, Time(), Time()); + AddKeywordWithDate("name", "x", false, "http://x/foo", "http://sugg1", + "http://icon1", false, "UTF-8;UTF-16", Time(), Time()); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(data); ++i) { history::URLVisitedDetails details; @@ -913,9 +909,9 @@ TEST_F(TemplateURLServiceTest, ChangeGoogleBaseValue) { // test. test_util_.ChangeModelToLoadState(); test_util_.SetGoogleBaseURL("http://google.com/"); - const TemplateURL* t_url = AddKeywordWithDate("google.com", true, + const TemplateURL* t_url = AddKeywordWithDate("name", "google.com", true, "{google:baseURL}?q={searchTerms}", "http://sugg1", "http://icon1", - "UTF-8;UTF-16", "name", false, Time(), Time()); + false, "UTF-8;UTF-16", Time(), Time()); ASSERT_EQ(t_url, model()->GetTemplateURLForHost("google.com")); EXPECT_EQ("google.com", t_url->url_ref().GetHost()); EXPECT_EQ(ASCIIToUTF16("google.com"), t_url->keyword()); @@ -960,10 +956,9 @@ TEST_F(TemplateURLServiceTest, GenerateVisitOnKeyword) { test_util_.profile()->CreateHistoryService(true, false); // Create a keyword. - TemplateURL* t_url = AddKeywordWithDate( - "keyword", false, "http://foo.com/foo?query={searchTerms}", - "http://sugg1", "http://icon1", "UTF-8;UTF-16", "keyword", - true, base::Time::Now(), base::Time::Now()); + TemplateURL* t_url = AddKeywordWithDate("keyword", "keyword", false, + "http://foo.com/foo?query={searchTerms}", "http://sugg1", "http://icon1", + true, "UTF-8;UTF-16", base::Time::Now(), base::Time::Now()); // Add a visit that matches the url of the keyword. HistoryService* history = @@ -1031,7 +1026,7 @@ TEST_F(TemplateURLServiceTest, LoadRetainsModifiedProvider) { model()->Add(t_url); // Do the copy after t_url is added so that the id is set. - TemplateURL copy_t_url = *t_url; + scoped_ptr<TemplateURL> cloned_url(new TemplateURL(t_url->data())); ASSERT_EQ(t_url, model()->GetTemplateURLForKeyword(ASCIIToUTF16("unittest"))); // Wait for any saves to finish. @@ -1042,7 +1037,7 @@ TEST_F(TemplateURLServiceTest, LoadRetainsModifiedProvider) { const TemplateURL* url_for_unittest = model()->GetTemplateURLForKeyword(ASCIIToUTF16("unittest")); ASSERT_TRUE(url_for_unittest != NULL); - AssertEquals(copy_t_url, *url_for_unittest); + AssertEquals(*cloned_url, *url_for_unittest); // Wait for any saves to finish. test_util_.BlockTillServiceProcessesRequests(); @@ -1059,8 +1054,9 @@ TEST_F(TemplateURLServiceTest, LoadRetainsModifiedProvider) { TEST_F(TemplateURLServiceTest, LoadSavesPrepopulatedDefaultSearchProvider) { test_util_.VerifyLoad(); // Verify that the default search provider is set to something. - ASSERT_TRUE(model()->GetDefaultSearchProvider() != NULL); - TemplateURL default_url = *model()->GetDefaultSearchProvider(); + const TemplateURL* default_search = model()->GetDefaultSearchProvider(); + ASSERT_TRUE(default_search != NULL); + scoped_ptr<TemplateURL> cloned_url(new TemplateURL(default_search->data())); // Wait for any saves to finish. test_util_.BlockTillServiceProcessesRequests(); @@ -1068,8 +1064,9 @@ TEST_F(TemplateURLServiceTest, LoadSavesPrepopulatedDefaultSearchProvider) { // Reload the model and check that the default search provider // was properly saved. test_util_.ResetModel(true); - ASSERT_TRUE(model()->GetDefaultSearchProvider() != NULL); - AssertEquals(default_url, *model()->GetDefaultSearchProvider()); + default_search = model()->GetDefaultSearchProvider(); + ASSERT_TRUE(default_search != NULL); + AssertEquals(*cloned_url, *default_search); } // Make sure that the load routine doesn't delete @@ -1085,7 +1082,7 @@ TEST_F(TemplateURLServiceTest, LoadRetainsDefaultProvider) { model()->SetDefaultSearchProvider(t_url); // Do the copy after t_url is added and set as default so that its // internal state is correct. - TemplateURL copy_t_url = *t_url; + scoped_ptr<TemplateURL> cloned_url(new TemplateURL(t_url->data())); ASSERT_EQ(t_url, model()->GetTemplateURLForKeyword(ASCIIToUTF16("unittest"))); ASSERT_EQ(t_url, model()->GetDefaultSearchProvider()); @@ -1098,7 +1095,7 @@ TEST_F(TemplateURLServiceTest, LoadRetainsDefaultProvider) { const TemplateURL* keyword_url = model()->GetTemplateURLForKeyword(ASCIIToUTF16("unittest")); ASSERT_TRUE(keyword_url != NULL); - AssertEquals(copy_t_url, *keyword_url); + AssertEquals(*cloned_url, *keyword_url); ASSERT_EQ(keyword_url, model()->GetDefaultSearchProvider()); } @@ -1111,7 +1108,7 @@ TEST_F(TemplateURLServiceTest, LoadRetainsDefaultProvider) { const TemplateURL* keyword_url = model()->GetTemplateURLForKeyword(ASCIIToUTF16("unittest")); ASSERT_TRUE(keyword_url != NULL); - AssertEquals(copy_t_url, *keyword_url); + AssertEquals(*cloned_url, *keyword_url); ASSERT_EQ(keyword_url, model()->GetDefaultSearchProvider()); } } @@ -1154,14 +1151,15 @@ TEST_F(TemplateURLServiceTest, LoadEnsuresDefaultSearchProviderExists) { // during this operation. TEST_F(TemplateURLServiceTest, LoadDoesAutoKeywordUpdate) { string16 prepopulated_url; - TemplateURL* t_url = CreateReplaceablePreloadedTemplateURL(false, - 0, &prepopulated_url); - t_url->SetURL("{google:baseURL}?q={searchTerms}"); - t_url->set_autogenerate_keyword(true); + scoped_ptr<TemplateURL> t_url( + CreateReplaceablePreloadedTemplateURL(false, 0, &prepopulated_url)); + TemplateURLData data(t_url->data()); + data.SetAutogenerateKeyword(true); + data.SetURL("{google:baseURL}?q={searchTerms}"); // Then add it to the model and save it all. test_util_.ChangeModelToLoadState(); - model()->Add(t_url); + model()->Add(new TemplateURL(data)); test_util_.BlockTillServiceProcessesRequests(); // Now reload the model and verify that the merge updates the url. @@ -1198,9 +1196,9 @@ TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) { test_util_.ResetObserverCount(); // Set a regular default search provider. - TemplateURL* regular_default = AddKeywordWithDate("key1", false, - "http://foo1", "http://sugg1", "http://icon1", "UTF-8;UTF-16", "name1", - true, Time(), Time()); + TemplateURL* regular_default = AddKeywordWithDate("name1", "key1", false, + "http://foo1", "http://sugg1", "http://icon1", true, "UTF-8;UTF-16", + Time(), Time()); VerifyObserverCount(1); model()->SetDefaultSearchProvider(regular_default); // Adding the URL and setting the default search provider should have caused @@ -1215,22 +1213,21 @@ TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) { const char kSearchURL[] = "http://test.com/search?t={searchTerms}"; const char kIconURL[] = "http://test.com/icon.jpg"; const char kEncodings[] = "UTF-16;UTF-32"; - SetManagedDefaultSearchPreferences(true, kName, kSearchURL, std::string(), - kIconURL, kEncodings, kKeyword); + SetManagedDefaultSearchPreferences(true, kName, kKeyword, kSearchURL, + std::string(), kIconURL, kEncodings); VerifyObserverFired(); EXPECT_TRUE(model()->is_default_search_managed()); EXPECT_EQ(initial_count + 2, model()->GetTemplateURLs().size()); // Verify that the default manager we are getting is the managed one. - scoped_ptr<TemplateURL> expected_managed_default1(new TemplateURL()); - expected_managed_default1->set_short_name(ASCIIToUTF16(kName)); - expected_managed_default1->set_keyword(ASCIIToUTF16(kKeyword)); - expected_managed_default1->set_show_in_default_list(true); - std::vector<std::string> encodings_vector; - base::SplitString(kEncodings, ';', &encodings_vector); - expected_managed_default1->set_input_encodings(encodings_vector); - expected_managed_default1->SetURL(kSearchURL); - expected_managed_default1->set_favicon_url(GURL(kIconURL)); + TemplateURLData data; + data.short_name = ASCIIToUTF16(kName); + data.SetKeyword(ASCIIToUTF16(kKeyword)); + data.SetURL(kSearchURL); + data.favicon_url = GURL(kIconURL); + data.show_in_default_list = true; + base::SplitString(kEncodings, ';', &data.input_encodings); + scoped_ptr<TemplateURL> expected_managed_default1(new TemplateURL(data)); const TemplateURL* actual_managed_default = model()->GetDefaultSearchProvider(); ExpectSimilar(expected_managed_default1.get(), actual_managed_default); @@ -1241,19 +1238,20 @@ TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) { const char kNewKeyword[] = "other.com"; const char kNewSearchURL[] = "http://other.com/search?t={searchTerms}"; const char kNewSuggestURL[] = "http://other.com/suggest?t={searchTerms}"; - SetManagedDefaultSearchPreferences(true, kNewName, kNewSearchURL, - kNewSuggestURL, std::string(), std::string(), kNewKeyword); + SetManagedDefaultSearchPreferences(true, kNewName, kNewKeyword, kNewSearchURL, + kNewSuggestURL, std::string(), std::string()); VerifyObserverFired(); EXPECT_TRUE(model()->is_default_search_managed()); EXPECT_EQ(initial_count + 2, model()->GetTemplateURLs().size()); // Verify that the default manager we are now getting is the correct one. - scoped_ptr<TemplateURL> expected_managed_default2(new TemplateURL()); - expected_managed_default2->set_short_name(ASCIIToUTF16(kNewName)); - expected_managed_default2->set_keyword(ASCIIToUTF16(kNewKeyword)); - expected_managed_default2->set_show_in_default_list(true); - expected_managed_default2->SetSuggestionsURL(kNewSuggestURL); - expected_managed_default2->SetURL(kNewSearchURL); + TemplateURLData data2; + data2.short_name = ASCIIToUTF16(kNewName); + data2.SetKeyword(ASCIIToUTF16(kNewKeyword)); + data2.SetURL(kNewSearchURL); + data2.suggestions_url = kNewSuggestURL; + data2.show_in_default_list = true; + scoped_ptr<TemplateURL> expected_managed_default2(new TemplateURL(data2)); actual_managed_default = model()->GetDefaultSearchProvider(); ExpectSimilar(expected_managed_default2.get(), actual_managed_default); EXPECT_EQ(actual_managed_default->show_in_default_list(), true); @@ -1279,8 +1277,8 @@ TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) { EXPECT_EQ(initial_count + 1, model()->GetTemplateURLs().size()); // Re-enable it. - SetManagedDefaultSearchPreferences(true, kName, kSearchURL, std::string(), - kIconURL, kEncodings, kKeyword); + SetManagedDefaultSearchPreferences(true, kName, kKeyword, kSearchURL, + std::string(), kIconURL, kEncodings); VerifyObserverFired(); EXPECT_TRUE(model()->is_default_search_managed()); EXPECT_EQ(initial_count + 2, model()->GetTemplateURLs().size()); @@ -1297,9 +1295,8 @@ TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) { // First, remove the preferences, reset the model, and set a default. RemoveManagedDefaultSearchPreferences(); test_util_.ResetModel(true); - TemplateURL* t_url = AddKeywordWithDate("key1", false, "http://foo1", - "http://sugg1", "http://icon1", "UTF-8;UTF-16", "name1", true, Time(), - Time()); + TemplateURL* t_url = AddKeywordWithDate("name1", "key1", false, "http://foo1", + "http://sugg1", "http://icon1", true, "UTF-8;UTF-16", Time(), Time()); model()->SetDefaultSearchProvider(t_url); EXPECT_EQ(t_url, model()->GetDefaultSearchProvider()); @@ -1319,11 +1316,12 @@ TEST_F(TemplateURLServiceTest, PatchEmptySyncGUID) { test_util_.VerifyLoad(); const size_t initial_count = model()->GetTemplateURLs().size(); - TemplateURL* t_url = new TemplateURL(); - t_url->set_short_name(ASCIIToUTF16("google")); - t_url->set_keyword(ASCIIToUTF16("keyword")); - t_url->set_sync_guid(std::string()); - t_url->SetURL("http://www.google.com/foo/bar"); + TemplateURLData data; + data.short_name = ASCIIToUTF16("google"); + data.SetKeyword(ASCIIToUTF16("keyword")); + data.SetURL("http://www.google.com/foo/bar"); + data.sync_guid.clear(); + TemplateURL* t_url = new TemplateURL(data); model()->Add(t_url); VerifyObserverCount(1); diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc index 479f306..68be193 100644 --- a/chrome/browser/search_engines/template_url_unittest.cc +++ b/chrome/browser/search_engines/template_url_unittest.cc @@ -58,15 +58,16 @@ void TemplateURLTest::CheckSuggestBaseURL( // Actual tests --------------------------------------------------------------- TEST_F(TemplateURLTest, Defaults) { - TemplateURL url; - EXPECT_FALSE(url.show_in_default_list()); - EXPECT_FALSE(url.safe_for_autoreplace()); - EXPECT_EQ(0, url.prepopulate_id()); + TemplateURLData data; + EXPECT_FALSE(data.show_in_default_list); + EXPECT_FALSE(data.safe_for_autoreplace); + EXPECT_EQ(0, data.prepopulate_id); } TEST_F(TemplateURLTest, TestValidWithComplete) { - TemplateURL url; - url.SetURL("{searchTerms}"); + TemplateURLData data; + data.SetURL("{searchTerms}"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); } @@ -88,8 +89,9 @@ TEST_F(TemplateURLTest, URLRefTestSearchTerms) { }; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(search_term_cases); ++i) { const SearchTermsCase& value = search_term_cases[i]; - TemplateURL url; - url.SetURL(value.url); + TemplateURLData data; + data.SetURL(value.url); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); std::string result = url.url_ref().ReplaceSearchTerms(value.terms, @@ -101,8 +103,9 @@ TEST_F(TemplateURLTest, URLRefTestSearchTerms) { } TEST_F(TemplateURLTest, URLRefTestCount) { - TemplateURL url; - url.SetURL("http://foo{searchTerms}{count?}"); + TemplateURLData data; + data.SetURL("http://foo{searchTerms}{count?}"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), @@ -112,8 +115,9 @@ TEST_F(TemplateURLTest, URLRefTestCount) { } TEST_F(TemplateURLTest, URLRefTestCount2) { - TemplateURL url; - url.SetURL("http://foo{searchTerms}{count}"); + TemplateURLData data; + data.SetURL("http://foo{searchTerms}{count}"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), @@ -123,8 +127,9 @@ TEST_F(TemplateURLTest, URLRefTestCount2) { } TEST_F(TemplateURLTest, URLRefTestIndices) { - TemplateURL url; - url.SetURL("http://foo{searchTerms}x{startIndex?}y{startPage?}"); + TemplateURLData data; + data.SetURL("http://foo{searchTerms}x{startIndex?}y{startPage?}"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), @@ -134,8 +139,9 @@ TEST_F(TemplateURLTest, URLRefTestIndices) { } TEST_F(TemplateURLTest, URLRefTestIndices2) { - TemplateURL url; - url.SetURL("http://foo{searchTerms}x{startIndex}y{startPage}"); + TemplateURLData data; + data.SetURL("http://foo{searchTerms}x{startIndex}y{startPage}"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), @@ -145,8 +151,9 @@ TEST_F(TemplateURLTest, URLRefTestIndices2) { } TEST_F(TemplateURLTest, URLRefTestEncoding) { - TemplateURL url; - url.SetURL("http://foo{searchTerms}x{inputEncoding?}y{outputEncoding?}a"); + TemplateURLData data; + data.SetURL("http://foo{searchTerms}x{inputEncoding?}y{outputEncoding?}a"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), @@ -158,8 +165,9 @@ TEST_F(TemplateURLTest, URLRefTestEncoding) { // Test that setting the prepopulate ID from TemplateURL causes the stored // TemplateURLRef to handle parsing the URL parameters differently. TEST_F(TemplateURLTest, SetPrepopulatedAndParse) { - TemplateURL url; - url.SetURL("http://foo{fhqwhgads}"); + TemplateURLData data; + data.SetURL("http://foo{fhqwhgads}"); + TemplateURL url(data); TemplateURLRef::Replacements replacements; bool valid = false; EXPECT_EQ("http://foo{fhqwhgads}", @@ -167,16 +175,18 @@ TEST_F(TemplateURLTest, SetPrepopulatedAndParse) { EXPECT_TRUE(replacements.empty()); EXPECT_TRUE(valid); - url.SetPrepopulateId(123); - EXPECT_EQ("http://foo", - url.url_ref().ParseURL("http://foo{fhqwhgads}", &replacements, &valid)); + data.prepopulate_id = 123; + TemplateURL url2(data); + EXPECT_EQ("http://foo", url2.url_ref().ParseURL("http://foo{fhqwhgads}", + &replacements, &valid)); EXPECT_TRUE(replacements.empty()); EXPECT_TRUE(valid); } TEST_F(TemplateURLTest, InputEncodingBeforeSearchTerm) { - TemplateURL url; - url.SetURL("http://foox{inputEncoding?}a{searchTerms}y{outputEncoding?}b"); + TemplateURLData data; + data.SetURL("http://foox{inputEncoding?}a{searchTerms}y{outputEncoding?}b"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), @@ -186,8 +196,9 @@ TEST_F(TemplateURLTest, InputEncodingBeforeSearchTerm) { } TEST_F(TemplateURLTest, URLRefTestEncoding2) { - TemplateURL url; - url.SetURL("http://foo{searchTerms}x{inputEncoding}y{outputEncoding}a"); + TemplateURLData data; + data.SetURL("http://foo{searchTerms}x{inputEncoding}y{outputEncoding}a"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("X"), @@ -209,10 +220,11 @@ TEST_F(TemplateURLTest, URLRefTestSearchTermsUsingTermsData) { }; TestSearchTermsData search_terms_data("http://example.com/e/"); - TemplateURL url; + TemplateURLData data; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(search_term_cases); ++i) { const SearchTermsCase& value = search_term_cases[i]; - url.SetURL(value.url); + data.SetURL(value.url); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTermsUsingTermsData(value.terms, @@ -244,9 +256,10 @@ TEST_F(TemplateURLTest, URLRefTermToWide) { }; // Set one input encoding: big-5. This is so we can test fallback to UTF-8. - TemplateURL url; - url.SetURL("http://foo?q={searchTerms}"); - url.add_input_encoding("big-5"); + TemplateURLData data; + data.SetURL("http://foo?q={searchTerms}"); + data.input_encodings.push_back("big-5"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(to_wide_cases); i++) { @@ -270,9 +283,10 @@ TEST_F(TemplateURLTest, DisplayURLToURLRef) { { "http://foo{searchTerms}{language}", ASCIIToUTF16("http://foo%s{language}") }, }; + TemplateURLData data; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { - TemplateURL url; - url.SetURL(test_data[i].url); + data.SetURL(test_data[i].url); + TemplateURL url(data); EXPECT_EQ(test_data[i].expected_result, url.url_ref().DisplayURL()); EXPECT_EQ(test_data[i].url, TemplateURLRef::DisplayURLToURLRef(url.url_ref().DisplayURL())); @@ -309,10 +323,11 @@ TEST_F(TemplateURLTest, ReplaceSearchTerms) { { "http://foo/{inputEncoding}a{language}a{searchTerms}a", "http://foo/UTF-8a{language}aXa" }, }; - TemplateURL url; - url.add_input_encoding("UTF-8"); + TemplateURLData data; + data.input_encodings.push_back("UTF-8"); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { - url.SetURL(test_data[i].url); + data.SetURL(test_data[i].url); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); EXPECT_TRUE(url.url_ref().SupportsReplacement()); std::string expected_result = test_data[i].expected_result; @@ -348,10 +363,12 @@ TEST_F(TemplateURLTest, ReplaceArbitrarySearchTerms) { "http://foo/{searchTerms}/bar", "http://foo/%82%A0%20%82%A2/bar"}, }; + TemplateURLData data; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { - TemplateURL url; - url.SetURL(test_data[i].url); - url.add_input_encoding(test_data[i].encoding); + data.SetURL(test_data[i].url); + data.input_encodings.clear(); + data.input_encodings.push_back(test_data[i].encoding); + TemplateURL url(data); GURL result(url.url_ref().ReplaceSearchTerms(test_data[i].search_term, TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); ASSERT_TRUE(result.is_valid()); @@ -376,10 +393,11 @@ TEST_F(TemplateURLTest, Suggestions) { { 0, string16(), "http://bar/foo?aq=0&oq=&q=foobar" }, { 1, ASCIIToUTF16("foo"), "http://bar/foo?aq=1&oq=foo&q=foobar" }, }; - TemplateURL url; - url.add_input_encoding("UTF-8"); - url.SetURL("http://bar/foo?{google:acceptedSuggestion}" - "{google:originalQueryForSuggestion}q={searchTerms}"); + TemplateURLData data; + data.SetURL("http://bar/foo?{google:acceptedSuggestion}" + "{google:originalQueryForSuggestion}q={searchTerms}"); + data.input_encodings.push_back("UTF-8"); + TemplateURL url(data); ASSERT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { @@ -402,8 +420,9 @@ TEST_F(TemplateURLTest, RLZ) { } #endif - TemplateURL url; - url.SetURL("http://bar/?{google:RLZ}{searchTerms}"); + TemplateURLData data; + data.SetURL("http://bar/?{google:RLZ}{searchTerms}"); + TemplateURL url(data); EXPECT_TRUE(url.url_ref().IsValid()); ASSERT_TRUE(url.url_ref().SupportsReplacement()); GURL result(url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("x"), @@ -423,7 +442,7 @@ TEST_F(TemplateURLTest, HostAndSearchTermKey) { const std::string host; const std::string path; const std::string search_term_key; - } data[] = { + } test_data[] = { { "http://blah/?foo=bar&q={searchTerms}&b=x", "blah", "/", "q"}, // No query key should result in empty values. @@ -444,12 +463,13 @@ TEST_F(TemplateURLTest, HostAndSearchTermKey) { { "http://blah/?q=stock:{searchTerms}", "blah", "/", "q"}, }; - for (size_t i = 0; i < ARRAYSIZE_UNSAFE(data); ++i) { - TemplateURL url; - url.SetURL(data[i].url); - EXPECT_EQ(data[i].host, url.url_ref().GetHost()); - EXPECT_EQ(data[i].path, url.url_ref().GetPath()); - EXPECT_EQ(data[i].search_term_key, url.url_ref().GetSearchTermKey()); + for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { + TemplateURLData data; + data.SetURL(test_data[i].url); + TemplateURL url(data); + EXPECT_EQ(test_data[i].host, url.url_ref().GetHost()); + EXPECT_EQ(test_data[i].path, url.url_ref().GetPath()); + EXPECT_EQ(test_data[i].search_term_key, url.url_ref().GetSearchTermKey()); } } @@ -470,23 +490,24 @@ TEST_F(TemplateURLTest, GoogleBaseSuggestURL) { } TEST_F(TemplateURLTest, Keyword) { - TemplateURL url; - url.SetURL("http://www.google.com/search"); - EXPECT_FALSE(url.autogenerate_keyword()); - url.set_keyword(ASCIIToUTF16("foo")); - EXPECT_EQ(ASCIIToUTF16("foo"), url.keyword()); - url.set_autogenerate_keyword(true); - EXPECT_TRUE(url.autogenerate_keyword()); - EXPECT_EQ(ASCIIToUTF16("google.com"), url.keyword()); - url.set_keyword(ASCIIToUTF16("foo")); - EXPECT_FALSE(url.autogenerate_keyword()); - EXPECT_EQ(ASCIIToUTF16("foo"), url.keyword()); + TemplateURLData data; + data.SetURL("http://www.google.com/search"); + EXPECT_FALSE(data.autogenerate_keyword()); + data.SetKeyword(ASCIIToUTF16("foo")); + EXPECT_EQ(ASCIIToUTF16("foo"), TemplateURL(data).keyword()); + data.SetAutogenerateKeyword(true); + EXPECT_TRUE(data.autogenerate_keyword()); + EXPECT_EQ(ASCIIToUTF16("google.com"), TemplateURL(data).keyword()); + data.SetKeyword(ASCIIToUTF16("foo")); + EXPECT_FALSE(data.autogenerate_keyword()); + EXPECT_EQ(ASCIIToUTF16("foo"), TemplateURL(data).keyword()); } TEST_F(TemplateURLTest, ParseParameterKnown) { std::string parsed_url("{searchTerms}"); - TemplateURL url; - url.SetURL(parsed_url); + TemplateURLData data; + data.SetURL(parsed_url); + TemplateURL url(data); TemplateURLRef::Replacements replacements; EXPECT_TRUE(url.url_ref().ParseParameter(0, 12, &parsed_url, &replacements)); EXPECT_EQ(std::string(), parsed_url); @@ -497,8 +518,9 @@ TEST_F(TemplateURLTest, ParseParameterKnown) { TEST_F(TemplateURLTest, ParseParameterUnknown) { std::string parsed_url("{fhqwhgads}"); - TemplateURL url; - url.SetURL(parsed_url); + TemplateURLData data; + data.SetURL(parsed_url); + TemplateURL url(data); TemplateURLRef::Replacements replacements; // By default, TemplateURLRef should not consider itself prepopulated. @@ -509,14 +531,17 @@ TEST_F(TemplateURLTest, ParseParameterUnknown) { // If the TemplateURLRef is prepopulated, we should remove unknown parameters. parsed_url = "{fhqwhgads}"; - url.SetPrepopulateId(1); - EXPECT_FALSE(url.url_ref().ParseParameter(0, 10, &parsed_url, &replacements)); + data.prepopulate_id = 1; + TemplateURL url2(data); + EXPECT_FALSE(url2.url_ref().ParseParameter(0, 10, &parsed_url, + &replacements)); EXPECT_EQ(std::string(), parsed_url); EXPECT_TRUE(replacements.empty()); } TEST_F(TemplateURLTest, ParseURLEmpty) { - TemplateURL url; + TemplateURLData data; + TemplateURL url(data); TemplateURLRef::Replacements replacements; bool valid = false; EXPECT_EQ(std::string(), @@ -526,8 +551,9 @@ TEST_F(TemplateURLTest, ParseURLEmpty) { } TEST_F(TemplateURLTest, ParseURLNoTemplateEnd) { - TemplateURL url; - url.SetURL("{"); + TemplateURLData data; + data.SetURL("{"); + TemplateURL url(data); TemplateURLRef::Replacements replacements; bool valid = false; EXPECT_EQ(std::string(), url.url_ref().ParseURL("{", &replacements, &valid)); @@ -536,8 +562,9 @@ TEST_F(TemplateURLTest, ParseURLNoTemplateEnd) { } TEST_F(TemplateURLTest, ParseURLNoKnownParameters) { - TemplateURL url; - url.SetURL("{}"); + TemplateURLData data; + data.SetURL("{}"); + TemplateURL url(data); TemplateURLRef::Replacements replacements; bool valid = false; EXPECT_EQ("{}", url.url_ref().ParseURL("{}", &replacements, &valid)); @@ -546,8 +573,9 @@ TEST_F(TemplateURLTest, ParseURLNoKnownParameters) { } TEST_F(TemplateURLTest, ParseURLTwoParameters) { - TemplateURL url; - url.SetURL("{}{{%s}}"); + TemplateURLData data; + data.SetURL("{}{{%s}}"); + TemplateURL url(data); TemplateURLRef::Replacements replacements; bool valid = false; EXPECT_EQ("{}{}", @@ -559,8 +587,9 @@ TEST_F(TemplateURLTest, ParseURLTwoParameters) { } TEST_F(TemplateURLTest, ParseURLNestedParameter) { - TemplateURL url; - url.SetURL("{%s"); + TemplateURLData data; + data.SetURL("{%s"); + TemplateURL url(data); TemplateURLRef::Replacements replacements; bool valid = false; EXPECT_EQ("{", diff --git a/chrome/browser/search_engines/util.cc b/chrome/browser/search_engines/util.cc index 991b0fb..b284aec 100644 --- a/chrome/browser/search_engines/util.cc +++ b/chrome/browser/search_engines/util.cc @@ -53,7 +53,7 @@ static void RemoveDuplicatePrepopulateIDs( if (prepopulate_id) { if (ids.find(prepopulate_id) != ids.end()) { if (service) - service->RemoveKeyword(**i); + service->RemoveKeyword((*i)->id()); delete *i; i = template_urls->erase(i); } else { @@ -91,7 +91,8 @@ void MergeEnginesFromPrepopulateData( DCHECK(template_urls); DCHECK(default_search_provider); - // Build a map from prepopulate id to TemplateURL of existing urls. + // Create a map to hold all provided |template_urls| that originally came from + // prepopulate data (i.e. have a non-zero prepopulate_id()). typedef std::map<int, TemplateURL*> IDMap; IDMap id_to_turl; for (std::vector<TemplateURL*>::iterator i(template_urls->begin()); @@ -101,66 +102,71 @@ void MergeEnginesFromPrepopulateData( id_to_turl[prepopulate_id] = *i; } + // Get the current set of prepopulatd URLs. std::vector<TemplateURL*> prepopulated_urls; size_t default_search_index; TemplateURLPrepopulateData::GetPrepopulatedEngines(prefs, - &prepopulated_urls, - &default_search_index); - - std::set<int> updated_ids; + &prepopulated_urls, &default_search_index); + + // For each current prepopulated URL, check whether |template_urls| contained + // a matching prepopulated URL. If so, update the passed-in URL to match the + // current data. (If the passed-in URL was user-edited, we persist the user's + // name and keyword.) If not, add the prepopulated URL to |template_urls|. + // Along the way, point |default_search_provider| at the default prepopulated + // URL, if the user hasn't already set another URL as default. for (size_t i = 0; i < prepopulated_urls.size(); ++i) { // We take ownership of |prepopulated_urls[i]|. scoped_ptr<TemplateURL> prepopulated_url(prepopulated_urls[i]); const int prepopulated_id = prepopulated_url->prepopulate_id(); - if (!prepopulated_id || updated_ids.count(prepopulated_id)) { - // Prepopulate engines need a unique id. - NOTREACHED(); - continue; - } + DCHECK_NE(0, prepopulated_id); - TemplateURL* existing_url = NULL; + TemplateURL* url_in_vector = NULL; IDMap::iterator existing_url_iter(id_to_turl.find(prepopulated_id)); if (existing_url_iter != id_to_turl.end()) { - existing_url = existing_url_iter->second; + // Update the data store with the new prepopulated data. Preserve user + // edits to the name and keyword. + TemplateURLData data(prepopulated_url->data()); + scoped_ptr<TemplateURL> existing_url(existing_url_iter->second); + id_to_turl.erase(existing_url_iter); if (!existing_url->safe_for_autoreplace()) { - // User edited the entry, preserve the keyword and description. - prepopulated_url->set_safe_for_autoreplace(false); - prepopulated_url->set_keyword(existing_url->keyword()); - prepopulated_url->set_autogenerate_keyword( - existing_url->autogenerate_keyword()); - prepopulated_url->set_short_name(existing_url->short_name()); - } - prepopulated_url->set_id(existing_url->id()); - - *existing_url = *prepopulated_url; - if (service) { - service->UpdateKeyword(*existing_url); + data.safe_for_autoreplace = false; + data.SetKeyword(existing_url->keyword()); + data.SetAutogenerateKeyword(existing_url->autogenerate_keyword()); + data.short_name = existing_url->short_name(); } - id_to_turl.erase(existing_url_iter); + data.id = existing_url->id(); + url_in_vector = new TemplateURL(data); + if (service) + service->UpdateKeyword(*url_in_vector); + + // Replace the entry in |template_urls| with the updated one. + std::vector<TemplateURL*>::iterator j = std::find(template_urls->begin(), + template_urls->end(), existing_url.get()); + *j = url_in_vector; + if (*default_search_provider == existing_url.get()) + *default_search_provider = url_in_vector; } else { - existing_url = prepopulated_url.get(); template_urls->push_back(prepopulated_url.release()); + url_in_vector = template_urls->back(); } - DCHECK(existing_url); + DCHECK(url_in_vector); if (i == default_search_index && !*default_search_provider) - *default_search_provider = existing_url; - - updated_ids.insert(prepopulated_id); + *default_search_provider = url_in_vector; } - // Remove any prepopulated engines which are no longer in the master list, as - // long as the user hasn't modified them or made them the default engine. + // The block above removed all the URLs from the |id_to_turl| map that were + // found in the prepopulate data. Any remaining URLs that haven't been + // user-edited or made default can be removed from the data store. for (IDMap::iterator i(id_to_turl.begin()); i != id_to_turl.end(); ++i) { const TemplateURL* template_url = i->second; if ((template_url->safe_for_autoreplace()) && (template_url != *default_search_provider)) { - std::vector<TemplateURL*>::iterator i = std::find(template_urls->begin(), - template_urls->end(), - template_url); - DCHECK(i != template_urls->end()); - template_urls->erase(i); + std::vector<TemplateURL*>::iterator j = + std::find(template_urls->begin(), template_urls->end(), template_url); + DCHECK(j != template_urls->end()); + template_urls->erase(j); if (service) - service->RemoveKeyword(*template_url); + service->RemoveKeyword(template_url->id()); delete template_url; } } @@ -185,7 +191,10 @@ void GetSearchProvidersUsingKeywordResult( WDKeywordsResult keyword_result = reinterpret_cast< const WDResult<WDKeywordsResult>*>(&result)->GetValue(); - template_urls->swap(keyword_result.keywords); + for (KeywordTable::Keywords::const_iterator i( + keyword_result.keywords.begin()); i != keyword_result.keywords.end(); + ++i) + template_urls->push_back(new TemplateURL(*i)); const int resource_keyword_version = TemplateURLPrepopulateData::GetDataVersion(prefs); @@ -222,8 +231,10 @@ bool DidDefaultSearchProviderChange( if (!keyword_result.did_default_search_provider_change) return false; - backup_default_search_provider->reset( - keyword_result.default_search_provider_backup); + if (keyword_result.backup_valid) { + backup_default_search_provider->reset(new TemplateURL( + keyword_result.default_search_provider_backup)); + } return true; } diff --git a/chrome/browser/sync/test/integration/search_engines_helper.cc b/chrome/browser/sync/test/integration/search_engines_helper.cc index cb1c6c5..dc54be1 100644 --- a/chrome/browser/sync/test/integration/search_engines_helper.cc +++ b/chrome/browser/sync/test/integration/search_engines_helper.cc @@ -177,25 +177,25 @@ TemplateURL* CreateTestTemplateURL(int seed) { TemplateURL* CreateTestTemplateURL(int seed, const string16& keyword, const std::string& sync_guid) { - return CreateTestTemplateURL(seed, - base::StringPrintf("http://www.test%d.com/", seed), keyword, sync_guid); + return CreateTestTemplateURL(seed, keyword, + base::StringPrintf("http://www.test%d.com/", seed), sync_guid); } TemplateURL* CreateTestTemplateURL(int seed, - const std::string& url, const string16& keyword, + const std::string& url, const std::string& sync_guid) { - TemplateURL* turl = new TemplateURL(); - turl->set_short_name(CreateKeyword(seed)); - turl->set_keyword(keyword); - turl->set_safe_for_autoreplace(true); - turl->set_date_created(base::Time::FromTimeT(100)); - turl->set_last_modified(base::Time::FromTimeT(100)); - turl->SetPrepopulateId(999999); - turl->set_sync_guid(sync_guid); - turl->SetURL(url); - turl->set_favicon_url(GURL("http://favicon.url")); - return turl; + TemplateURLData data; + data.short_name = CreateKeyword(seed); + data.SetKeyword(keyword); + data.safe_for_autoreplace = true; + data.SetURL(url); + data.favicon_url = GURL("http://favicon.url"); + data.date_created = base::Time::FromTimeT(100); + data.last_modified = base::Time::FromTimeT(100); + data.prepopulate_id = 999999; + data.sync_guid = sync_guid; + return new TemplateURL(data); } void AddSearchEngine(int profile, int seed) { diff --git a/chrome/browser/sync/test/integration/search_engines_helper.h b/chrome/browser/sync/test/integration/search_engines_helper.h index 8e4ca3e..f404864 100644 --- a/chrome/browser/sync/test/integration/search_engines_helper.h +++ b/chrome/browser/sync/test/integration/search_engines_helper.h @@ -49,8 +49,8 @@ TemplateURL* CreateTestTemplateURL(int seed, const string16& keyword, const std::string& sync_guid); TemplateURL* CreateTestTemplateURL(int seed, - const std::string& url, const string16& keyword, + const std::string& url, const std::string& sync_guid); // Add a search engine based on a seed to the service at index |profile| and the diff --git a/chrome/browser/sync/test/integration/single_client_search_engines_sync_test.cc b/chrome/browser/sync/test/integration/single_client_search_engines_sync_test.cc index df9558e..48e60a6 100644 --- a/chrome/browser/sync/test/integration/single_client_search_engines_sync_test.cc +++ b/chrome/browser/sync/test/integration/single_client_search_engines_sync_test.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. @@ -7,12 +7,6 @@ #include "chrome/browser/sync/test/integration/search_engines_helper.h" #include "chrome/browser/sync/test/integration/sync_test.h" -using search_engines_helper::AddSearchEngine; -using search_engines_helper::CreateTestTemplateURL; -using search_engines_helper::GetServiceForProfile; -using search_engines_helper::GetVerifierService; -using search_engines_helper::ServiceMatchesVerifier; - class SingleClientSearchEnginesSyncTest : public SyncTest { public: SingleClientSearchEnginesSyncTest() : SyncTest(SINGLE_CLIENT) {} @@ -24,11 +18,11 @@ class SingleClientSearchEnginesSyncTest : public SyncTest { IN_PROC_BROWSER_TEST_F(SingleClientSearchEnginesSyncTest, Sanity) { ASSERT_TRUE(SetupSync()) << "SetupSync() failed."; - ASSERT_TRUE(ServiceMatchesVerifier(0)); + ASSERT_TRUE(search_engines_helper::ServiceMatchesVerifier(0)); - AddSearchEngine(0, 0); + search_engines_helper::AddSearchEngine(0, 0); ASSERT_TRUE(GetClient(0)->AwaitFullSyncCompletion( "Waiting for search engines to update.")); - ASSERT_TRUE(ServiceMatchesVerifier(0)); + ASSERT_TRUE(search_engines_helper::ServiceMatchesVerifier(0)); } diff --git a/chrome/browser/sync/test/integration/two_client_search_engines_sync_test.cc b/chrome/browser/sync/test/integration/two_client_search_engines_sync_test.cc index 30e7a57..67c0106 100644 --- a/chrome/browser/sync/test/integration/two_client_search_engines_sync_test.cc +++ b/chrome/browser/sync/test/integration/two_client_search_engines_sync_test.cc @@ -13,7 +13,6 @@ using search_engines_helper::AddSearchEngine; using search_engines_helper::AllServicesMatch; using search_engines_helper::ChangeDefaultSearchProvider; using search_engines_helper::CreateTestTemplateURL; -using search_engines_helper::DeleteSearchEngineByKeyword; using search_engines_helper::DeleteSearchEngineBySeed; using search_engines_helper::EditSearchEngine; using search_engines_helper::GetServiceForProfile; @@ -178,7 +177,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientSearchEnginesSyncTest, MergeMultiple) { AddSearchEngine(1, 2); AddSearchEngine(1, 3); GetServiceForProfile(1)->Add(CreateTestTemplateURL(0, - "http://www.somethingelse.com/", ASCIIToUTF16("somethingelse.com"), + ASCIIToUTF16("somethingelse.com"), "http://www.somethingelse.com/", "somethingelse")); ASSERT_TRUE(AwaitQuiescence()); diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc index f06e473..5519ea0 100644 --- a/chrome/browser/tab_contents/render_view_context_menu.cc +++ b/chrome/browser/tab_contents/render_view_context_menu.cc @@ -1828,24 +1828,23 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) { return; model->Load(); - scoped_ptr<TemplateURL> template_url(new TemplateURL); - string16 keyword = - net::StripWWW(UTF8ToUTF16((params_.page_url.host()))); - template_url->set_short_name(keyword); - template_url->set_keyword(keyword); - template_url->SetURL(params_.keyword_url.spec()); - template_url->set_favicon_url(TemplateURL::GenerateFaviconURL( - params_.page_url.GetOrigin())); - TabContentsWrapper* tab_contents_wrapper = TabContentsWrapper::GetCurrentWrapperForContents( source_web_contents_); if (tab_contents_wrapper && tab_contents_wrapper->search_engine_tab_helper() && tab_contents_wrapper->search_engine_tab_helper()->delegate()) { - // Takes ownership of |template_url|. + string16 keyword(TemplateURLService::GenerateKeyword(params_.page_url, + false)); + TemplateURLData data; + data.short_name = keyword; + data.SetKeyword(keyword); + data.SetURL(params_.keyword_url.spec()); + data.favicon_url = + TemplateURL::GenerateFaviconURL(params_.page_url.GetOrigin()); + // Takes ownership of the TemplateURL. tab_contents_wrapper->search_engine_tab_helper()->delegate()-> - ConfirmAddSearchProvider(template_url.release(), profile_); + ConfirmAddSearchProvider(new TemplateURL(data), profile_); } break; } diff --git a/chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller_unittest.mm b/chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller_unittest.mm index 8cf6050..987679d 100644 --- a/chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller_unittest.mm +++ b/chrome/browser/ui/cocoa/browser/edit_search_engine_cocoa_controller_unittest.mm @@ -206,12 +206,13 @@ TEST_F(EditSearchEngineControllerTest, ValidateFields) { // Tests editing an existing TemplateURL. TEST_F(EditSearchEngineControllerTest, EditTemplateURL) { - TemplateURL url; - url.set_short_name(ASCIIToUTF16("Foobar")); - url.set_keyword(ASCIIToUTF16("keyword")); + TemplateURLData data; + data.short_name = ASCIIToUTF16("Foobar"); + data.SetKeyword(ASCIIToUTF16("keyword")); std::string urlString = TemplateURLRef::DisplayURLToURLRef( ASCIIToUTF16("http://foo-bar.com")); - url.SetURL(urlString); + data.SetURL(urlString); + TemplateURL url(data); FakeEditSearchEngineController *controller = [[FakeEditSearchEngineController alloc] initWithProfile:profile() delegate:nil diff --git a/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc b/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc index 3a1cf30..596bfe2 100644 --- a/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc +++ b/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc @@ -285,18 +285,16 @@ class OmniboxViewTest : public InProcessBrowserTest, i = builtins.begin(); i != builtins.end(); ++i) model->Remove(*i); - TemplateURL* template_url = new TemplateURL(); - template_url->SetURL(kSearchURL); - template_url->set_keyword(ASCIIToUTF16(kSearchKeyword)); - template_url->set_short_name(ASCIIToUTF16(kSearchShortName)); + TemplateURLData data; + data.short_name = ASCIIToUTF16(kSearchShortName); + data.SetKeyword(ASCIIToUTF16(kSearchKeyword)); + data.SetURL(kSearchURL); + TemplateURL* template_url = new TemplateURL(data); model->Add(template_url); model->SetDefaultSearchProvider(template_url); - TemplateURL* second_url = new TemplateURL(); - second_url->SetURL(kSearchURL); - second_url->set_keyword(ASCIIToUTF16(kSearchKeyword2)); - second_url->set_short_name(ASCIIToUTF16(kSearchShortName)); - model->Add(second_url); + data.SetKeyword(ASCIIToUTF16(kSearchKeyword2)); + model->Add(new TemplateURL(data)); } void AddHistoryEntry(const TestHistoryEntry& entry, const Time& time) { @@ -945,10 +943,11 @@ class OmniboxViewTest : public InProcessBrowserTest, TemplateURLServiceFactory::GetForProfile(browser()->profile()); // Add a non-default substituting keyword. - TemplateURL* template_url = new TemplateURL(); - template_url->SetURL("http://abc.com/{searchTerms}"); - template_url->set_keyword(UTF8ToUTF16(kSearchText)); - template_url->set_short_name(UTF8ToUTF16("Search abc")); + TemplateURLData data; + data.short_name = ASCIIToUTF16("Search abc"); + data.SetKeyword(ASCIIToUTF16(kSearchText)); + data.SetURL("http://abc.com/{searchTerms}"); + TemplateURL* template_url = new TemplateURL(data); template_url_service->Add(template_url); omnibox_view->SetUserText(string16()); @@ -970,11 +969,9 @@ class OmniboxViewTest : public InProcessBrowserTest, // Try a non-substituting keyword. template_url_service->Remove(template_url); - template_url = new TemplateURL(); - template_url->SetURL("http://abc.com/"); - template_url->set_keyword(UTF8ToUTF16(kSearchText)); - template_url->set_short_name(UTF8ToUTF16("abc")); - template_url_service->Add(template_url); + data.short_name = ASCIIToUTF16("abc"); + data.SetURL("http://abc.com/"); + template_url_service->Add(new TemplateURL(data)); // We always allow exact matches for non-substituting keywords. ASSERT_NO_FATAL_FAILURE(SendKeySequence(kSearchTextKeys)); diff --git a/chrome/browser/ui/search_engines/edit_search_engine_controller.cc b/chrome/browser/ui/search_engines/edit_search_engine_controller.cc index db46f3e..a5ce1c4 100644 --- a/chrome/browser/ui/search_engines/edit_search_engine_controller.cc +++ b/chrome/browser/ui/search_engines/edit_search_engine_controller.cc @@ -41,8 +41,9 @@ bool EditSearchEngineController::IsURLValid( // contains replacement strings. We do this by constructing a dummy // TemplateURL owner because |template_url_| might be NULL and we can't call // TemplateURLRef::IsValid() when its owner is NULL. - TemplateURL t_url; - t_url.SetURL(url); + TemplateURLData data; + data.SetURL(url); + TemplateURL t_url(data); const TemplateURLRef& template_ref = t_url.url_ref(); if (!template_ref.IsValid()) return false; @@ -97,15 +98,9 @@ void EditSearchEngineController::AcceptAddOrEdit( // Confiming an entry we got from JS. We have a template_url_, but it // hasn't yet been added to the model. DCHECK(template_url_); - // const_cast is ugly, but this is the same thing the TemplateURLService - // does in a similar situation (updating an existing TemplateURL with - // data from a new one). - TemplateURL* modifiable_url = const_cast<TemplateURL*>(template_url_); - modifiable_url->set_short_name(title_input); - modifiable_url->set_keyword(keyword_input); - modifiable_url->SetURL(url_string); // TemplateURLService takes ownership of template_url_. - template_url_service->Add(modifiable_url); + template_url_service->AddWithOverrides(template_url_, title_input, + keyword_input, url_string); content::RecordAction(UserMetricsAction("KeywordEditor_AddKeywordJS")); } else { // Adding or modifying an entry via the Delegate. @@ -134,8 +129,9 @@ std::string EditSearchEngineController::GetFixedUpURL( // Parse the string as a URL to determine the scheme. If we need to, add the // scheme. As the scheme may be expanded (as happens with {google:baseURL}) // we need to replace the search terms before testing for the scheme. - TemplateURL t_url; - t_url.SetURL(url); + TemplateURLData data; + data.SetURL(url); + TemplateURL t_url(data); std::string expanded_url(t_url.url_ref().ReplaceSearchTerms(ASCIIToUTF16("x"), TemplateURLRef::NO_SUGGESTIONS_AVAILABLE, string16())); url_parse::Parsed parts; diff --git a/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc b/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc index 90c7af8..d78f33e 100644 --- a/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc +++ b/chrome/browser/ui/search_engines/keyword_editor_controller_unittest.cc @@ -245,9 +245,10 @@ TEST_F(KeywordEditorControllerTest, MakeDefaultNoWebData) { // Mutates the TemplateURLService and make sure table model is updating // appropriately. TEST_F(KeywordEditorControllerTest, MutateTemplateURLService) { - TemplateURL* turl = new TemplateURL(); - turl->set_keyword(ASCIIToUTF16("a")); - turl->set_short_name(ASCIIToUTF16("b")); + TemplateURLData data; + data.short_name = ASCIIToUTF16("b"); + data.SetKeyword(ASCIIToUTF16("a")); + TemplateURL* turl = new TemplateURL(data); model_->Add(turl); // Table model should have updated. diff --git a/chrome/browser/ui/search_engines/search_engine_tab_helper.cc b/chrome/browser/ui/search_engines/search_engine_tab_helper.cc index 6c8d0cf..3065749 100644 --- a/chrome/browser/ui/search_engines/search_engine_tab_helper.cc +++ b/chrome/browser/ui/search_engines/search_engine_tab_helper.cc @@ -171,11 +171,10 @@ void SearchEngineTabHelper::GenerateKeywordIfNecessary( url_service->Remove(current_url); } - TemplateURL* new_url = new TemplateURL(); - new_url->set_short_name(keyword); - new_url->set_keyword(keyword); - new_url->set_safe_for_autoreplace(true); - new_url->add_input_encoding(params.searchable_form_encoding); + TemplateURLData data; + data.short_name = keyword; + data.SetKeyword(keyword); + data.SetURL(url.spec()); DCHECK(controller.GetLastCommittedEntry()); const GURL& current_favicon = controller.GetLastCommittedEntry()->GetFavicon().url; @@ -184,9 +183,9 @@ void SearchEngineTabHelper::GenerateKeywordIfNecessary( // latter. // TODO(sky): Need a way to set the favicon that doesn't involve generating // its url. - const GURL& favicon_url = current_favicon.is_valid() ? + data.favicon_url = current_favicon.is_valid() ? current_favicon : TemplateURL::GenerateFaviconURL(params.referrer.url); - new_url->SetURL(url.spec()); - new_url->set_favicon_url(favicon_url); - url_service->Add(new_url); + data.safe_for_autoreplace = true; + data.input_encodings.push_back(params.searchable_form_encoding); + url_service->Add(new TemplateURL(data)); } diff --git a/chrome/browser/ui/search_engines/template_url_table_model.cc b/chrome/browser/ui/search_engines/template_url_table_model.cc index c523ca5..83b566a 100644 --- a/chrome/browser/ui/search_engines/template_url_table_model.cc +++ b/chrome/browser/ui/search_engines/template_url_table_model.cc @@ -254,10 +254,11 @@ void TemplateURLTableModel::Add(int index, const std::string& url) { DCHECK(index >= 0 && index <= RowCount()); template_url_service_->RemoveObserver(this); - TemplateURL* turl = new TemplateURL(); - turl->set_short_name(short_name); - turl->set_keyword(keyword); - turl->SetURL(url); + TemplateURLData data; + data.short_name = short_name; + data.SetKeyword(keyword); + data.SetURL(url); + TemplateURL* turl = new TemplateURL(data); template_url_service_->Add(turl); ModelEntry* entry = new ModelEntry(this, turl); template_url_service_->AddObserver(this); diff --git a/chrome/browser/webdata/keyword_table.cc b/chrome/browser/webdata/keyword_table.cc index acf571d..0b59e71 100644 --- a/chrome/browser/webdata/keyword_table.cc +++ b/chrome/browser/webdata/keyword_table.cc @@ -47,37 +47,38 @@ const char kKeywordColumnsConcatenated[] = "id || short_name || keyword || " "suggest_url || prepopulate_id || autogenerate_keyword || logo_id || " "created_by_policy || instant_url || last_modified || sync_guid"; -// Inserts the data from |url| into |s|. |s| is assumed to have slots for all +// Inserts the data from |data| into |s|. |s| is assumed to have slots for all // the columns in the keyword table. |id_column| is the slot number to bind -// |url|'s id() to; |starting_column| is the slot number of the first of a +// |data|'s id() to; |starting_column| is the slot number of the first of a // contiguous set of slots to bind all the other fields to. void BindURLToStatement(const TemplateURL& url, sql::Statement* s, int id_column, int starting_column) { - s->BindInt64(id_column, url.id()); - s->BindString16(starting_column, url.short_name()); - s->BindString16(starting_column + 1, url.keyword()); - s->BindString(starting_column + 2, url.favicon_url().is_valid() ? - history::HistoryDatabase::GURLToDatabaseURL(url.favicon_url()) : + const TemplateURLData& data = url.data(); + s->BindInt64(id_column, data.id); + s->BindString16(starting_column, data.short_name); + s->BindString16(starting_column + 1, data.keyword(&url)); + s->BindString(starting_column + 2, data.favicon_url.is_valid() ? + history::HistoryDatabase::GURLToDatabaseURL(data.favicon_url) : std::string()); - s->BindString(starting_column + 3, url.url()); - s->BindBool(starting_column + 4, url.safe_for_autoreplace()); - s->BindString(starting_column + 5, url.originating_url().is_valid() ? - history::HistoryDatabase::GURLToDatabaseURL(url.originating_url()) : + s->BindString(starting_column + 3, data.url()); + s->BindBool(starting_column + 4, data.safe_for_autoreplace); + s->BindString(starting_column + 5, data.originating_url.is_valid() ? + history::HistoryDatabase::GURLToDatabaseURL(data.originating_url) : std::string()); - s->BindInt64(starting_column + 6, url.date_created().ToTimeT()); - s->BindInt(starting_column + 7, url.usage_count()); - s->BindString(starting_column + 8, JoinString(url.input_encodings(), ';')); - s->BindBool(starting_column + 9, url.show_in_default_list()); - s->BindString(starting_column + 10, url.suggestions_url()); - s->BindInt(starting_column + 11, url.prepopulate_id()); - s->BindInt(starting_column + 12, url.autogenerate_keyword() ? 1 : 0); + s->BindInt64(starting_column + 6, data.date_created.ToTimeT()); + s->BindInt(starting_column + 7, data.usage_count); + s->BindString(starting_column + 8, JoinString(data.input_encodings, ';')); + s->BindBool(starting_column + 9, data.show_in_default_list); + s->BindString(starting_column + 10, data.suggestions_url); + s->BindInt(starting_column + 11, data.prepopulate_id); + s->BindBool(starting_column + 12, data.autogenerate_keyword()); s->BindInt(starting_column + 13, 0); - s->BindBool(starting_column + 14, url.created_by_policy()); - s->BindString(starting_column + 15, url.instant_url()); - s->BindInt64(starting_column + 16, url.last_modified().ToTimeT()); - s->BindString(starting_column + 17, url.sync_guid()); + s->BindBool(starting_column + 14, data.created_by_policy); + s->BindString(starting_column + 15, data.instant_url); + s->BindInt64(starting_column + 16, data.last_modified.ToTimeT()); + s->BindString(starting_column + 17, data.sync_guid); } // Signs search provider id and returns its signature. @@ -148,8 +149,10 @@ bool KeywordTable::GetKeywords(Keywords* keywords) { " FROM keywords ORDER BY id ASC"); sql::Statement s(db_->GetUniqueStatement(query.c_str())); - while (s.Step()) - keywords->push_back(GetKeywordDataFromStatement(s)); + while (s.Step()) { + keywords->push_back(TemplateURLData()); + GetKeywordDataFromStatement(s, &keywords->back()); + } return s.Succeeded(); } @@ -180,14 +183,14 @@ int64 KeywordTable::GetDefaultSearchProviderID() { return value; } -TemplateURL* KeywordTable::GetDefaultSearchProviderBackup() { +bool KeywordTable::GetDefaultSearchProviderBackup(TemplateURLData* backup) { if (!IsBackupSignatureValid()) - return NULL; + return false; int64 backup_id = kInvalidTemplateURLID; if (!meta_table_->GetValue(kDefaultSearchIDBackupKey, &backup_id)) { LOG(ERROR) << "No default search id backup found."; - return NULL; + return false; } std::string query("SELECT " + std::string(kKeywordColumns) + " FROM keywords_backup WHERE id=?"); @@ -200,11 +203,11 @@ TemplateURL* KeywordTable::GetDefaultSearchProviderBackup() { return NULL; } - TemplateURL* template_url = GetKeywordDataFromStatement(s); + GetKeywordDataFromStatement(s, backup); // ID has no meaning for the backup and should be kInvalidTemplateURLID in // case the TemplateURL will be added to keywords if missing. - template_url->set_id(kInvalidTemplateURLID); - return template_url; + backup->id = kInvalidTemplateURLID; + return true; } bool KeywordTable::DidDefaultSearchProviderChange() { @@ -327,35 +330,27 @@ bool KeywordTable::MigrateToVersion44AddDefaultSearchProviderBackup() { } // static -TemplateURL* KeywordTable::GetKeywordDataFromStatement( - const sql::Statement& s) { - TemplateURL* url = new TemplateURL; - url->set_id(s.ColumnInt64(0)); - url->set_short_name(s.ColumnString16(1)); - url->set_keyword(s.ColumnString16(2)); - std::string tmp(s.ColumnString(3)); - if (!tmp.empty()) - url->set_favicon_url(GURL(tmp)); - url->SetURL(s.ColumnString(4)); - url->set_safe_for_autoreplace(s.ColumnBool(5)); - tmp = s.ColumnString(6); - if (!tmp.empty()) - url->set_originating_url(GURL(tmp)); - url->set_date_created(Time::FromTimeT(s.ColumnInt64(7))); - url->set_usage_count(s.ColumnInt(8)); - std::vector<std::string> encodings; - base::SplitString(s.ColumnString(9), ';', &encodings); - url->set_input_encodings(encodings); - url->set_show_in_default_list(s.ColumnBool(10)); - url->SetSuggestionsURL(s.ColumnString(11)); - url->SetPrepopulateId(s.ColumnInt(12)); - url->set_autogenerate_keyword(s.ColumnBool(13)); - url->set_created_by_policy(s.ColumnBool(15)); - url->SetInstantURL(s.ColumnString(16)); - url->set_last_modified(Time::FromTimeT(s.ColumnInt64(17))); - url->set_sync_guid(s.ColumnString(18)); - - return url; +void KeywordTable::GetKeywordDataFromStatement(const sql::Statement& s, + TemplateURLData* data) { + DCHECK(data); + data->short_name = s.ColumnString16(1); + data->SetKeyword(s.ColumnString16(2)); + data->SetAutogenerateKeyword(s.ColumnBool(13)); + data->SetURL(s.ColumnString(4)); + data->suggestions_url = s.ColumnString(11); + data->instant_url = s.ColumnString(16); + data->favicon_url = GURL(s.ColumnString(3)); + data->originating_url = GURL(s.ColumnString(6)); + data->show_in_default_list = s.ColumnBool(10); + data->safe_for_autoreplace = s.ColumnBool(5); + base::SplitString(s.ColumnString(9), ';', &data->input_encodings); + data->id = s.ColumnInt64(0); + data->date_created = Time::FromTimeT(s.ColumnInt64(7)); + data->last_modified = Time::FromTimeT(s.ColumnInt64(17)); + data->created_by_policy = s.ColumnBool(15); + data->usage_count = s.ColumnInt(8); + data->prepopulate_id = s.ColumnInt(12); + data->sync_guid = s.ColumnString(18); } bool KeywordTable::GetSignatureData(std::string* backup) { diff --git a/chrome/browser/webdata/keyword_table.h b/chrome/browser/webdata/keyword_table.h index 910e7b5..da62e4c 100644 --- a/chrome/browser/webdata/keyword_table.h +++ b/chrome/browser/webdata/keyword_table.h @@ -16,6 +16,7 @@ #include "chrome/browser/search_engines/template_url_id.h" class TemplateURL; +struct TemplateURLData; namespace sql { class Statement; @@ -27,7 +28,7 @@ class Statement; // Note: The database stores time in seconds, UTC. // // keywords Most of the columns mirror that of a field in -// TemplateURL. See TemplateURL for more details. +// TemplateURLData. See that struct for more details. // id // short_name // keyword @@ -43,16 +44,16 @@ class Statement; // input_encodings Semicolon separated list of supported input // encodings, may be empty. // suggest_url -// prepopulate_id See TemplateURL::prepopulate_id. +// prepopulate_id See TemplateURLData::prepopulate_id. // autogenerate_keyword // logo_id Deprecated, to be removed; see crbug.com/113248. -// created_by_policy See TemplateURL::created_by_policy. This was added -// in version 26. -// instant_url See TemplateURL::instant_url. This was added -// in version 29. -// last_modified See TemplateURL::last_modified. This was added in -// version 38. -// sync_guid See TemplateURL::sync_guid. This was added in +// created_by_policy See TemplateURLData::created_by_policy. This was +// added in version 26. +// instant_url See TemplateURLData::instant_url. This was added in +// version 29. +// last_modified See TemplateURLData::last_modified. This was added +// in version 38. +// sync_guid See TemplateURLData::sync_guid. This was added in // version 39. // // keywords_backup The full copy of the |keywords| table. Added in @@ -81,7 +82,7 @@ class Statement; // class KeywordTable : public WebDatabaseTable { public: - typedef std::vector<TemplateURL*> Keywords; + typedef std::vector<TemplateURLData> Keywords; // Constants exposed for the benefit of test code: @@ -118,13 +119,13 @@ class KeywordTable : public WebDatabaseTable { // Returns true on success. bool UpdateKeyword(const TemplateURL& url); - // ID (TemplateURL->id) of the default search provider. + // ID (TemplateURLData->id) of the default search provider. bool SetDefaultSearchProviderID(int64 id); int64 GetDefaultSearchProviderID(); - // Backup of the default search provider. NULL if the backup is invalid. The - // returned TemplateURL is owned by the caller. - TemplateURL* GetDefaultSearchProviderBackup(); + // If the default search provider backup is valid, returns true and copies the + // backup into |backup|. Otherwise returns false. + bool GetDefaultSearchProviderBackup(TemplateURLData* backup); // Returns true if the default search provider has been changed out from under // us. This can happen if another process modifies our database or the file @@ -150,9 +151,13 @@ class KeywordTable : public WebDatabaseTable { FRIEND_TEST_ALL_PREFIXES(KeywordTableTest, GetTableContents); FRIEND_TEST_ALL_PREFIXES(KeywordTableTest, GetTableContentsOrdering); - // Returns a new TemplateURL from the data in |s|. The caller owns the - // returned pointer. - static TemplateURL* GetKeywordDataFromStatement(const sql::Statement& s); + // NOTE: Since the table columns have changed in different versions, many + // functions below take a |table_version| argument which dictates which + // version number's column set to use. + + // Fills |data| with the data in |s|. + static void GetKeywordDataFromStatement(const sql::Statement& s, + TemplateURLData* data); // Returns contents of |keywords_backup| table and default search provider // id backup as a string through |data|. Return value is true on success, diff --git a/chrome/browser/webdata/keyword_table_unittest.cc b/chrome/browser/webdata/keyword_table_unittest.cc index dd4a379..cad3a40 100644 --- a/chrome/browser/webdata/keyword_table_unittest.cc +++ b/chrome/browser/webdata/keyword_table_unittest.cc @@ -40,10 +40,6 @@ class KeywordTableTest : public testing::Test { file_util::Delete(file_, false); } - static void SetID(int64 new_id, TemplateURL* url) { - url->set_id(new_id); - } - FilePath file_; private: @@ -56,61 +52,59 @@ TEST_F(KeywordTableTest, Keywords) { ASSERT_EQ(sql::INIT_OK, db.Init(file_)); KeywordTable* keyword_table = db.GetKeywordTable(); - TemplateURL keyword; - keyword.set_short_name(ASCIIToUTF16("short_name")); - keyword.set_originating_url(GURL("http://google.com/")); - keyword.set_keyword(ASCIIToUTF16("keyword")); - keyword.set_show_in_default_list(true); - keyword.set_safe_for_autoreplace(true); - keyword.add_input_encoding("UTF-8"); - keyword.add_input_encoding("UTF-16"); - SetID(1, &keyword); - keyword.set_date_created(Time::Now()); - keyword.set_last_modified( - keyword.date_created() + TimeDelta::FromSeconds(10)); - keyword.set_created_by_policy(true); - keyword.set_usage_count(32); - keyword.SetPrepopulateId(10); + TemplateURLData keyword; + keyword.short_name = ASCIIToUTF16("short_name"); + keyword.SetKeyword(ASCIIToUTF16("keyword")); keyword.SetURL("http://url/"); - keyword.SetInstantURL("http://instant/"); - keyword.set_favicon_url(GURL("http://favicon.url/")); - EXPECT_TRUE(keyword_table->AddKeyword(keyword)); + keyword.instant_url = "http://instant/"; + keyword.favicon_url = GURL("http://favicon.url/"); + keyword.originating_url = GURL("http://google.com/"); + keyword.show_in_default_list = true; + keyword.safe_for_autoreplace = true; + keyword.input_encodings.push_back("UTF-8"); + keyword.input_encodings.push_back("UTF-16"); + keyword.id = 1; + keyword.date_created = Time::Now(); + keyword.last_modified = keyword.date_created + TimeDelta::FromSeconds(10); + keyword.created_by_policy = true; + keyword.usage_count = 32; + keyword.prepopulate_id = 10; + TemplateURL url(keyword); + EXPECT_TRUE(keyword_table->AddKeyword(url)); KeywordTable::Keywords keywords; EXPECT_TRUE(keyword_table->GetKeywords(&keywords)); EXPECT_EQ(1U, keywords.size()); - const TemplateURL* restored_keyword = keywords.front(); + const TemplateURLData& restored_keyword = keywords.front(); - EXPECT_EQ(keyword.short_name(), restored_keyword->short_name()); - EXPECT_EQ(keyword.originating_url(), restored_keyword->originating_url()); + EXPECT_EQ(keyword.short_name, restored_keyword.short_name); + EXPECT_EQ(url.keyword(), TemplateURL(restored_keyword).keyword()); EXPECT_EQ(keyword.autogenerate_keyword(), - restored_keyword->autogenerate_keyword()); - EXPECT_EQ(keyword.keyword(), restored_keyword->keyword()); - EXPECT_EQ(keyword.show_in_default_list(), - restored_keyword->show_in_default_list()); - EXPECT_EQ(keyword.safe_for_autoreplace(), - restored_keyword->safe_for_autoreplace()); - EXPECT_EQ(keyword.input_encodings(), restored_keyword->input_encodings()); - EXPECT_EQ(keyword.id(), restored_keyword->id()); + restored_keyword.autogenerate_keyword()); + EXPECT_EQ(keyword.url(), restored_keyword.url()); + EXPECT_EQ(keyword.instant_url, restored_keyword.instant_url); + EXPECT_EQ(keyword.favicon_url, restored_keyword.favicon_url); + EXPECT_EQ(keyword.originating_url, restored_keyword.originating_url); + EXPECT_EQ(keyword.show_in_default_list, + restored_keyword.show_in_default_list); + EXPECT_EQ(keyword.safe_for_autoreplace, + restored_keyword.safe_for_autoreplace); + EXPECT_EQ(keyword.input_encodings, restored_keyword.input_encodings); + EXPECT_EQ(keyword.id, restored_keyword.id); // The database stores time only at the resolution of a second. - EXPECT_EQ(keyword.date_created().ToTimeT(), - restored_keyword->date_created().ToTimeT()); - EXPECT_EQ(keyword.last_modified().ToTimeT(), - restored_keyword->last_modified().ToTimeT()); - EXPECT_EQ(keyword.created_by_policy(), restored_keyword->created_by_policy()); - EXPECT_EQ(keyword.usage_count(), restored_keyword->usage_count()); - EXPECT_EQ(keyword.prepopulate_id(), restored_keyword->prepopulate_id()); - EXPECT_EQ(keyword.url(), restored_keyword->url()); - EXPECT_EQ(keyword.instant_url(), restored_keyword->instant_url()); - EXPECT_EQ(keyword.favicon_url(), restored_keyword->favicon_url()); - - EXPECT_TRUE(keyword_table->RemoveKeyword(restored_keyword->id())); - STLDeleteElements(&keywords); + EXPECT_EQ(keyword.date_created.ToTimeT(), + restored_keyword.date_created.ToTimeT()); + EXPECT_EQ(keyword.last_modified.ToTimeT(), + restored_keyword.last_modified.ToTimeT()); + EXPECT_EQ(keyword.created_by_policy, restored_keyword.created_by_policy); + EXPECT_EQ(keyword.usage_count, restored_keyword.usage_count); + EXPECT_EQ(keyword.prepopulate_id, restored_keyword.prepopulate_id); + + EXPECT_TRUE(keyword_table->RemoveKeyword(restored_keyword.id)); KeywordTable::Keywords empty_keywords; EXPECT_TRUE(keyword_table->GetKeywords(&empty_keywords)); EXPECT_EQ(0U, empty_keywords.size()); - STLDeleteElements(&empty_keywords); } TEST_F(KeywordTableTest, KeywordMisc) { @@ -121,25 +115,25 @@ TEST_F(KeywordTableTest, KeywordMisc) { ASSERT_EQ(kInvalidTemplateURLID, keyword_table->GetDefaultSearchProviderID()); ASSERT_EQ(0, keyword_table->GetBuiltinKeywordVersion()); - TemplateURL keyword; - keyword.set_short_name(ASCIIToUTF16("short_name")); - keyword.set_originating_url(GURL("http://google.com/")); - keyword.set_keyword(ASCIIToUTF16("keyword")); - keyword.set_show_in_default_list(true); - keyword.set_safe_for_autoreplace(true); - keyword.add_input_encoding("UTF-8"); - keyword.add_input_encoding("UTF-16"); - SetID(10, &keyword); - keyword.set_date_created(Time::Now()); - keyword.set_last_modified( - keyword.date_created() + TimeDelta::FromSeconds(10)); - keyword.set_created_by_policy(true); - keyword.set_usage_count(32); - keyword.SetPrepopulateId(10); + TemplateURLData keyword; + keyword.short_name = ASCIIToUTF16("short_name"); + keyword.SetKeyword(ASCIIToUTF16("keyword")); keyword.SetURL("http://url/"); - keyword.SetInstantURL("http://instant/"); - keyword.set_favicon_url(GURL("http://favicon.url/")); - ASSERT_TRUE(keyword_table->AddKeyword(keyword)); + keyword.instant_url = "http://instant/"; + keyword.favicon_url = GURL("http://favicon.url/"); + keyword.originating_url = GURL("http://google.com/"); + keyword.show_in_default_list = true; + keyword.safe_for_autoreplace = true; + keyword.input_encodings.push_back("UTF-8"); + keyword.input_encodings.push_back("UTF-16"); + keyword.id = 10; + keyword.date_created = Time::Now(); + keyword.last_modified = keyword.date_created + TimeDelta::FromSeconds(10); + keyword.created_by_policy = true; + keyword.usage_count = 32; + keyword.prepopulate_id = 10; + TemplateURL url(keyword); + ASSERT_TRUE(keyword_table->AddKeyword(url)); ASSERT_TRUE(keyword_table->SetDefaultSearchProviderID(10)); ASSERT_TRUE(keyword_table->SetBuiltinKeywordVersion(11)); @@ -156,32 +150,33 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) { EXPECT_EQ(kInvalidTemplateURLID, keyword_table->GetDefaultSearchProviderID()); - TemplateURL keyword; - keyword.set_short_name(ASCIIToUTF16("short_name")); - keyword.set_keyword(ASCIIToUTF16("keyword")); - keyword.set_show_in_default_list(true); - keyword.set_safe_for_autoreplace(true); - SetID(1, &keyword); - keyword.SetSuggestionsURL("url2"); + TemplateURLData keyword; + keyword.short_name = ASCIIToUTF16("short_name"); + keyword.SetKeyword(ASCIIToUTF16("keyword")); keyword.SetURL("http://url/"); - keyword.set_favicon_url(GURL("http://favicon.url/")); - EXPECT_TRUE(keyword_table->AddKeyword(keyword)); + keyword.suggestions_url = "url2"; + keyword.favicon_url = GURL("http://favicon.url/"); + keyword.show_in_default_list = true; + keyword.safe_for_autoreplace = true; + keyword.id = 1; + TemplateURL url(keyword); + EXPECT_TRUE(keyword_table->AddKeyword(url)); ASSERT_TRUE(keyword_table->SetDefaultSearchProviderID(1)); EXPECT_TRUE(keyword_table->IsBackupSignatureValid()); EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID()); - scoped_ptr<TemplateURL> backup_url( - keyword_table->GetDefaultSearchProviderBackup()); + TemplateURLData backup; + EXPECT_TRUE(keyword_table->GetDefaultSearchProviderBackup(&backup)); // Backup URL should have an invalid ID. - EXPECT_EQ(kInvalidTemplateURLID, backup_url->id()); - EXPECT_EQ(keyword.short_name(), backup_url->short_name()); - EXPECT_EQ(keyword.keyword(), backup_url->keyword()); - EXPECT_EQ(keyword.favicon_url(), backup_url->favicon_url()); - EXPECT_EQ(keyword.url(), backup_url->url()); - EXPECT_EQ(keyword.safe_for_autoreplace(), backup_url->safe_for_autoreplace()); - EXPECT_EQ(keyword.show_in_default_list(), backup_url->show_in_default_list()); - EXPECT_EQ(keyword.suggestions_url(), backup_url->suggestions_url()); + EXPECT_EQ(kInvalidTemplateURLID, backup.id); + EXPECT_EQ(keyword.short_name, backup.short_name); + EXPECT_EQ(url.keyword(), TemplateURL(backup).keyword()); + EXPECT_EQ(keyword.url(), backup.url()); + EXPECT_EQ(keyword.favicon_url, backup.favicon_url); + EXPECT_EQ(keyword.suggestions_url, backup.suggestions_url); + EXPECT_EQ(keyword.show_in_default_list, backup.show_in_default_list); + EXPECT_EQ(keyword.safe_for_autoreplace, backup.safe_for_autoreplace); EXPECT_FALSE(keyword_table->DidDefaultSearchProviderChange()); // Change the actual setting. @@ -190,15 +185,15 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) { EXPECT_TRUE(keyword_table->IsBackupSignatureValid()); EXPECT_EQ(2, keyword_table->GetDefaultSearchProviderID()); - backup_url.reset(keyword_table->GetDefaultSearchProviderBackup()); - EXPECT_EQ(kInvalidTemplateURLID, backup_url->id()); - EXPECT_EQ(keyword.short_name(), backup_url->short_name()); - EXPECT_EQ(keyword.keyword(), backup_url->keyword()); - EXPECT_EQ(keyword.favicon_url(), backup_url->favicon_url()); - EXPECT_EQ(keyword.url(), backup_url->url()); - EXPECT_EQ(keyword.safe_for_autoreplace(), backup_url->safe_for_autoreplace()); - EXPECT_EQ(keyword.show_in_default_list(), backup_url->show_in_default_list()); - EXPECT_EQ(keyword.suggestions_url(), backup_url->suggestions_url()); + EXPECT_TRUE(keyword_table->GetDefaultSearchProviderBackup(&backup)); + EXPECT_EQ(kInvalidTemplateURLID, backup.id); + EXPECT_EQ(keyword.short_name, backup.short_name); + EXPECT_EQ(url.keyword(), TemplateURL(backup).keyword()); + EXPECT_EQ(keyword.url(), backup.url()); + EXPECT_EQ(keyword.favicon_url, backup.favicon_url); + EXPECT_EQ(keyword.suggestions_url, backup.suggestions_url); + EXPECT_EQ(keyword.show_in_default_list, backup.show_in_default_list); + EXPECT_EQ(keyword.safe_for_autoreplace, backup.safe_for_autoreplace); EXPECT_TRUE(keyword_table->DidDefaultSearchProviderChange()); // Change the backup. @@ -208,7 +203,7 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) { KeywordTable::kDefaultSearchIDBackupKey, 2)); EXPECT_FALSE(keyword_table->IsBackupSignatureValid()); EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID()); - EXPECT_EQ(NULL, keyword_table->GetDefaultSearchProviderBackup()); + EXPECT_FALSE(keyword_table->GetDefaultSearchProviderBackup(&backup)); EXPECT_TRUE(keyword_table->DidDefaultSearchProviderChange()); // Change the signature. @@ -218,7 +213,7 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) { KeywordTable::kBackupSignatureKey, std::string())); EXPECT_FALSE(keyword_table->IsBackupSignatureValid()); EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID()); - EXPECT_EQ(NULL, keyword_table->GetDefaultSearchProviderBackup()); + EXPECT_FALSE(keyword_table->GetDefaultSearchProviderBackup(&backup)); EXPECT_TRUE(keyword_table->DidDefaultSearchProviderChange()); // Change keywords. @@ -229,24 +224,24 @@ TEST_F(KeywordTableTest, DefaultSearchProviderBackup) { EXPECT_TRUE(keyword_table->IsBackupSignatureValid()); EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID()); - backup_url.reset(keyword_table->GetDefaultSearchProviderBackup()); - EXPECT_EQ(kInvalidTemplateURLID, backup_url->id()); - EXPECT_EQ(keyword.short_name(), backup_url->short_name()); - EXPECT_EQ(keyword.keyword(), backup_url->keyword()); - EXPECT_EQ(keyword.favicon_url(), backup_url->favicon_url()); - EXPECT_EQ(keyword.url(), backup_url->url()); - EXPECT_EQ(keyword.safe_for_autoreplace(), backup_url->safe_for_autoreplace()); - EXPECT_EQ(keyword.show_in_default_list(), backup_url->show_in_default_list()); - EXPECT_EQ(keyword.suggestions_url(), backup_url->suggestions_url()); + EXPECT_TRUE(keyword_table->GetDefaultSearchProviderBackup(&backup)); + EXPECT_EQ(kInvalidTemplateURLID, backup.id); + EXPECT_EQ(keyword.short_name, backup.short_name); + EXPECT_EQ(url.keyword(), TemplateURL(backup).keyword()); + EXPECT_EQ(keyword.url(), backup.url()); + EXPECT_EQ(keyword.suggestions_url, backup.suggestions_url); + EXPECT_EQ(keyword.favicon_url, backup.favicon_url); + EXPECT_EQ(keyword.show_in_default_list, backup.show_in_default_list); + EXPECT_EQ(keyword.safe_for_autoreplace, backup.safe_for_autoreplace); EXPECT_TRUE(keyword_table->DidDefaultSearchProviderChange()); // Change keywords backup. sql::Statement remove_keyword_backup(keyword_table->db_->GetUniqueStatement( - "DELETE FROM keywords_backup WHERE id=1")); + "DELETE FROM keywords_backup WHERE id=1")); ASSERT_TRUE(remove_keyword_backup.Run()); EXPECT_FALSE(keyword_table->IsBackupSignatureValid()); EXPECT_EQ(1, keyword_table->GetDefaultSearchProviderID()); - EXPECT_EQ(NULL, keyword_table->GetDefaultSearchProviderBackup()); + EXPECT_FALSE(keyword_table->GetDefaultSearchProviderBackup(&backup)); EXPECT_TRUE(keyword_table->DidDefaultSearchProviderChange()); } @@ -255,29 +250,30 @@ TEST_F(KeywordTableTest, GetTableContents) { ASSERT_EQ(sql::INIT_OK, db.Init(file_)); KeywordTable* keyword_table = db.GetKeywordTable(); - TemplateURL keyword; - keyword.set_short_name(ASCIIToUTF16("short_name")); - keyword.set_keyword(ASCIIToUTF16("keyword")); - keyword.set_show_in_default_list(true); - keyword.set_safe_for_autoreplace(true); - SetID(1, &keyword); - keyword.set_date_created(base::Time::UnixEpoch()); - keyword.set_last_modified(base::Time::UnixEpoch()); - keyword.set_sync_guid("1234-5678-90AB-CDEF"); - keyword.SetSuggestionsURL("url2"); + TemplateURLData keyword; + keyword.short_name = ASCIIToUTF16("short_name"); + keyword.SetKeyword(ASCIIToUTF16("keyword")); keyword.SetURL("http://url/"); - keyword.set_favicon_url(GURL("http://favicon.url/")); - ASSERT_TRUE(keyword_table->AddKeyword(keyword)); - - keyword.set_originating_url(GURL("http://originating.url/")); - keyword.set_autogenerate_keyword(true); - EXPECT_EQ(ASCIIToUTF16("url"), keyword.keyword()); - keyword.add_input_encoding("Shift_JIS"); - SetID(2, &keyword); - keyword.SetPrepopulateId(5); - keyword.set_sync_guid("FEDC-BA09-8765-4321"); - keyword.SetInstantURL("http://instant2/"); - ASSERT_TRUE(keyword_table->AddKeyword(keyword)); + keyword.suggestions_url = "url2"; + keyword.favicon_url = GURL("http://favicon.url/"); + keyword.show_in_default_list = true; + keyword.safe_for_autoreplace = true; + keyword.id = 1; + keyword.date_created = base::Time::UnixEpoch(); + keyword.last_modified = base::Time::UnixEpoch(); + keyword.sync_guid = "1234-5678-90AB-CDEF"; + TemplateURL url(keyword); + EXPECT_TRUE(keyword_table->AddKeyword(url)); + + keyword.SetAutogenerateKeyword(true); + keyword.instant_url = "http://instant2/"; + keyword.originating_url = GURL("http://originating.url/"); + keyword.input_encodings.push_back("Shift_JIS"); + keyword.id = 2; + keyword.prepopulate_id = 5; + keyword.sync_guid = "FEDC-BA09-8765-4321"; + TemplateURL url2(keyword); + EXPECT_TRUE(keyword_table->AddKeyword(url2)); const char kTestContents[] = "1short_namekeywordhttp://favicon.url/" "http://url/1001url2000001234-5678-90AB-CDEF2short_nameurl" @@ -297,29 +293,30 @@ TEST_F(KeywordTableTest, GetTableContentsOrdering) { ASSERT_EQ(sql::INIT_OK, db.Init(file_)); KeywordTable* keyword_table = db.GetKeywordTable(); - TemplateURL keyword; - keyword.set_short_name(ASCIIToUTF16("short_name")); - keyword.set_keyword(ASCIIToUTF16("keyword")); - keyword.set_show_in_default_list(true); - keyword.set_safe_for_autoreplace(true); - SetID(2, &keyword); - keyword.set_date_created(base::Time::UnixEpoch()); - keyword.set_last_modified(base::Time::UnixEpoch()); - keyword.set_sync_guid("1234-5678-90AB-CDEF"); - keyword.SetSuggestionsURL("url2"); + TemplateURLData keyword; + keyword.short_name = ASCIIToUTF16("short_name"); + keyword.SetKeyword(ASCIIToUTF16("keyword")); keyword.SetURL("http://url/"); - keyword.set_favicon_url(GURL("http://favicon.url/")); - ASSERT_TRUE(keyword_table->AddKeyword(keyword)); - - keyword.set_originating_url(GURL("http://originating.url/")); - keyword.set_autogenerate_keyword(true); - EXPECT_EQ(ASCIIToUTF16("url"), keyword.keyword()); - keyword.add_input_encoding("Shift_JIS"); - SetID(1, &keyword); - keyword.SetPrepopulateId(5); - keyword.set_sync_guid("FEDC-BA09-8765-4321"); - keyword.SetInstantURL("http://instant2/"); - ASSERT_TRUE(keyword_table->AddKeyword(keyword)); + keyword.suggestions_url = "url2"; + keyword.favicon_url = GURL("http://favicon.url/"); + keyword.show_in_default_list = true; + keyword.safe_for_autoreplace = true; + keyword.id = 2; + keyword.date_created = base::Time::UnixEpoch(); + keyword.last_modified = base::Time::UnixEpoch(); + keyword.sync_guid = "1234-5678-90AB-CDEF"; + TemplateURL url(keyword); + EXPECT_TRUE(keyword_table->AddKeyword(url)); + + keyword.SetAutogenerateKeyword(true); + keyword.instant_url = "http://instant2/"; + keyword.originating_url = GURL("http://originating.url/"); + keyword.input_encodings.push_back("Shift_JIS"); + keyword.id = 1; + keyword.prepopulate_id = 5; + keyword.sync_guid = "FEDC-BA09-8765-4321"; + TemplateURL url2(keyword); + EXPECT_TRUE(keyword_table->AddKeyword(url2)); const char kTestContents[] = "1short_nameurlhttp://favicon.url/http://url/1" "http://originating.url/00Shift_JIS1url25100http://instant2/0" @@ -339,47 +336,46 @@ TEST_F(KeywordTableTest, UpdateKeyword) { ASSERT_EQ(sql::INIT_OK, db.Init(file_)); KeywordTable* keyword_table = db.GetKeywordTable(); - TemplateURL keyword; - keyword.set_short_name(ASCIIToUTF16("short_name")); - keyword.set_keyword(ASCIIToUTF16("keyword")); - keyword.set_show_in_default_list(true); - keyword.set_safe_for_autoreplace(true); - SetID(1, &keyword); - keyword.SetSuggestionsURL("url2"); + TemplateURLData keyword; + keyword.short_name = ASCIIToUTF16("short_name"); + keyword.SetKeyword(ASCIIToUTF16("keyword")); keyword.SetURL("http://url/"); - keyword.set_favicon_url(GURL("http://favicon.url/")); - EXPECT_TRUE(keyword_table->AddKeyword(keyword)); - - keyword.set_originating_url(GURL("http://originating.url/")); - keyword.set_autogenerate_keyword(true); - EXPECT_EQ(ASCIIToUTF16("url"), keyword.keyword()); - keyword.add_input_encoding("Shift_JIS"); - keyword.SetPrepopulateId(5); - keyword.SetInstantURL("http://instant2/"); - EXPECT_TRUE(keyword_table->UpdateKeyword(keyword)); + keyword.suggestions_url = "url2"; + keyword.favicon_url = GURL("http://favicon.url/"); + keyword.show_in_default_list = true; + keyword.safe_for_autoreplace = true; + keyword.id = 1; + TemplateURL url(keyword); + EXPECT_TRUE(keyword_table->AddKeyword(url)); + + keyword.originating_url = GURL("http://originating.url/"); + keyword.SetAutogenerateKeyword(true); + keyword.instant_url = "http://instant2/"; + keyword.input_encodings.push_back("Shift_JIS"); + keyword.prepopulate_id = 5; + TemplateURL url2(keyword); + EXPECT_TRUE(keyword_table->UpdateKeyword(url2)); KeywordTable::Keywords keywords; EXPECT_TRUE(keyword_table->GetKeywords(&keywords)); EXPECT_EQ(1U, keywords.size()); - const TemplateURL* restored_keyword = keywords.front(); + const TemplateURLData& restored_keyword = keywords.front(); - EXPECT_EQ(keyword.short_name(), restored_keyword->short_name()); - EXPECT_EQ(keyword.originating_url(), restored_keyword->originating_url()); - EXPECT_EQ(keyword.keyword(), restored_keyword->keyword()); + EXPECT_EQ(keyword.short_name, restored_keyword.short_name); + EXPECT_EQ(url2.keyword(), TemplateURL(restored_keyword).keyword()); EXPECT_EQ(keyword.autogenerate_keyword(), - restored_keyword->autogenerate_keyword()); - EXPECT_EQ(keyword.show_in_default_list(), - restored_keyword->show_in_default_list()); - EXPECT_EQ(keyword.safe_for_autoreplace(), - restored_keyword->safe_for_autoreplace()); - EXPECT_EQ(keyword.input_encodings(), restored_keyword->input_encodings()); - EXPECT_EQ(keyword.id(), restored_keyword->id()); - EXPECT_EQ(keyword.prepopulate_id(), restored_keyword->prepopulate_id()); - EXPECT_EQ(keyword.suggestions_url(), restored_keyword->suggestions_url()); - EXPECT_EQ(keyword.favicon_url(), restored_keyword->favicon_url()); - EXPECT_EQ(keyword.instant_url(), restored_keyword->instant_url()); - - STLDeleteElements(&keywords); + restored_keyword.autogenerate_keyword()); + EXPECT_EQ(keyword.suggestions_url, restored_keyword.suggestions_url); + EXPECT_EQ(keyword.instant_url, restored_keyword.instant_url); + EXPECT_EQ(keyword.favicon_url, restored_keyword.favicon_url); + EXPECT_EQ(keyword.originating_url, restored_keyword.originating_url); + EXPECT_EQ(keyword.show_in_default_list, + restored_keyword.show_in_default_list); + EXPECT_EQ(keyword.safe_for_autoreplace, + restored_keyword.safe_for_autoreplace); + EXPECT_EQ(keyword.input_encodings, restored_keyword.input_encodings); + EXPECT_EQ(keyword.id, restored_keyword.id); + EXPECT_EQ(keyword.prepopulate_id, restored_keyword.prepopulate_id); } TEST_F(KeywordTableTest, KeywordWithNoFavicon) { @@ -387,25 +383,24 @@ TEST_F(KeywordTableTest, KeywordWithNoFavicon) { ASSERT_EQ(sql::INIT_OK, db.Init(file_)); KeywordTable* keyword_table = db.GetKeywordTable(); - TemplateURL keyword; - keyword.set_short_name(ASCIIToUTF16("short_name")); - keyword.set_keyword(ASCIIToUTF16("keyword")); - keyword.set_safe_for_autoreplace(true); - SetID(-100, &keyword); + TemplateURLData keyword; + keyword.short_name = ASCIIToUTF16("short_name"); + keyword.SetKeyword(ASCIIToUTF16("keyword")); keyword.SetURL("http://url/"); - EXPECT_TRUE(keyword_table->AddKeyword(keyword)); + keyword.safe_for_autoreplace = true; + keyword.id = -100; + TemplateURL url(keyword); + EXPECT_TRUE(keyword_table->AddKeyword(url)); KeywordTable::Keywords keywords; EXPECT_TRUE(keyword_table->GetKeywords(&keywords)); EXPECT_EQ(1U, keywords.size()); - const TemplateURL* restored_keyword = keywords.front(); - - EXPECT_EQ(keyword.short_name(), restored_keyword->short_name()); - EXPECT_EQ(keyword.keyword(), restored_keyword->keyword()); - EXPECT_EQ(keyword.safe_for_autoreplace(), - restored_keyword->safe_for_autoreplace()); - EXPECT_EQ(keyword.id(), restored_keyword->id()); - EXPECT_EQ(keyword.favicon_url(), restored_keyword->favicon_url()); - - STLDeleteElements(&keywords); + const TemplateURLData& restored_keyword = keywords.front(); + + EXPECT_EQ(keyword.short_name, restored_keyword.short_name); + EXPECT_EQ(url.keyword(), TemplateURL(restored_keyword).keyword()); + EXPECT_EQ(keyword.favicon_url, restored_keyword.favicon_url); + EXPECT_EQ(keyword.safe_for_autoreplace, + restored_keyword.safe_for_autoreplace); + EXPECT_EQ(keyword.id, restored_keyword.id); } diff --git a/chrome/browser/webdata/web_data_service.cc b/chrome/browser/webdata/web_data_service.cc index e52e167..c13e635 100644 --- a/chrome/browser/webdata/web_data_service.cc +++ b/chrome/browser/webdata/web_data_service.cc @@ -72,7 +72,9 @@ WDAppImagesResult::~WDAppImagesResult() {} WDKeywordsResult::WDKeywordsResult() : default_search_provider_id(0), - builtin_keyword_version(0) { + builtin_keyword_version(0), + backup_valid(false), + did_default_search_provider_change(false) { } WDKeywordsResult::~WDKeywordsResult() {} @@ -152,14 +154,14 @@ void WebDataService::AddKeyword(const TemplateURL& url) { // the TemplateURL for use on another keyword. url.EnsureKeyword(); GenericRequest<TemplateURL>* request = - new GenericRequest<TemplateURL>(this, GetNextRequestHandle(), NULL, url); + new GenericRequest<TemplateURL>(this, GetNextRequestHandle(), NULL, url); RegisterRequest(request); ScheduleTask(FROM_HERE, Bind(&WebDataService::AddKeywordImpl, this, request)); } -void WebDataService::RemoveKeyword(const TemplateURL& url) { - GenericRequest<TemplateURLID>* request = new GenericRequest<TemplateURLID>( - this, GetNextRequestHandle(), NULL, url.id()); +void WebDataService::RemoveKeyword(TemplateURLID id) { + GenericRequest<TemplateURLID>* request = + new GenericRequest<TemplateURLID>(this, GetNextRequestHandle(), NULL, id); RegisterRequest(request); ScheduleTask(FROM_HERE, Bind(&WebDataService::RemoveKeywordImpl, this, request)); @@ -828,8 +830,7 @@ void WebDataService::AddKeywordImpl(GenericRequest<TemplateURL>* request) { request->RequestComplete(); } -void WebDataService::RemoveKeywordImpl( - GenericRequest<TemplateURLID>* request) { +void WebDataService::RemoveKeywordImpl(GenericRequest<TemplateURLID>* request) { InitializeDatabaseIfNecessary(); if (db_ && !request->IsCancelled(NULL)) { DCHECK(request->arg()); @@ -862,10 +863,9 @@ void WebDataService::GetKeywordsImpl(WebDataRequest* request) { db_->GetKeywordTable()->GetBuiltinKeywordVersion(); result.did_default_search_provider_change = db_->GetKeywordTable()->DidDefaultSearchProviderChange(); - result.default_search_provider_backup = - result.did_default_search_provider_change ? - db_->GetKeywordTable()->GetDefaultSearchProviderBackup() : - NULL; + result.backup_valid = result.did_default_search_provider_change && + db_->GetKeywordTable()->GetDefaultSearchProviderBackup( + &result.default_search_provider_backup); request->SetResult( new WDResult<WDKeywordsResult>(KEYWORDS_RESULT, result)); } diff --git a/chrome/browser/webdata/web_data_service.h b/chrome/browser/webdata/web_data_service.h index 54f22fb..4e2777a 100644 --- a/chrome/browser/webdata/web_data_service.h +++ b/chrome/browser/webdata/web_data_service.h @@ -20,6 +20,7 @@ #include "base/message_loop_helpers.h" #include "base/memory/ref_counted.h" #include "base/synchronization/lock.h" +#include "chrome/browser/search_engines/template_url.h" #include "chrome/browser/search_engines/template_url_id.h" #include "chrome/browser/webdata/keyword_table.h" #include "content/public/browser/browser_thread.h" @@ -38,7 +39,6 @@ struct IE7PasswordInfo; class MessageLoop; class Profile; class SkBitmap; -class TemplateURL; class WebDatabase; namespace base { @@ -119,8 +119,9 @@ struct WDKeywordsResult { int64 default_search_provider_id; // Version of the built-in keywords. A value of 0 indicates a first run. int builtin_keyword_version; - // Backup of the default search provider. NULL if the backup is invalid. - TemplateURL* default_search_provider_backup; + // Backup of the default search provider, and whether the backup is valid. + bool backup_valid; + TemplateURLData default_search_provider_backup; // Indicates if default search provider has been changed by something // other than user's action in the browser. bool did_default_search_provider_change; @@ -335,10 +336,9 @@ class WebDataService // Many of the keyword related methods do not return a handle. This is because // the caller (TemplateURLService) does not need to know when the request is // done. - void AddKeyword(const TemplateURL& url); - - void RemoveKeyword(const TemplateURL& url); + void AddKeyword(const TemplateURL& url); + void RemoveKeyword(TemplateURLID id); void UpdateKeyword(const TemplateURL& url); // Fetches the keywords. @@ -599,7 +599,7 @@ class WebDataService content::BrowserThread::UI>; friend class base::DeleteHelper<WebDataService>; - typedef GenericRequest2<std::vector<const TemplateURL*>, + typedef GenericRequest2<std::vector<const TemplateURLData*>, KeywordTable::Keywords> SetKeywordsRequest; // Invoked on the main thread if initializing the db fails. diff --git a/chrome/browser/webdata/web_data_service_unittest.cc b/chrome/browser/webdata/web_data_service_unittest.cc index 9eb7bb6..1c19008 100644 --- a/chrome/browser/webdata/web_data_service_unittest.cc +++ b/chrome/browser/webdata/web_data_service_unittest.cc @@ -795,5 +795,5 @@ TEST_F(WebDataServiceTest, DidDefaultSearchProviderChangeOnNewProfile) { WaitUntilCalled(); ASSERT_TRUE(consumer.load_succeeded); EXPECT_FALSE(consumer.keywords_result.did_default_search_provider_change); - EXPECT_EQ(NULL, consumer.keywords_result.default_search_provider_backup); + EXPECT_FALSE(consumer.keywords_result.backup_valid); } |