diff options
Diffstat (limited to 'chrome/browser/search_engines')
| -rw-r--r-- | chrome/browser/search_engines/template_url.h | 7 | ||||
| -rw-r--r-- | chrome/browser/search_engines/template_url_model.cc | 174 | ||||
| -rw-r--r-- | chrome/browser/search_engines/template_url_model.h | 12 | ||||
| -rw-r--r-- | chrome/browser/search_engines/template_url_model_unittest.cc | 247 | ||||
| -rw-r--r-- | chrome/browser/search_engines/util.cc | 170 | ||||
| -rw-r--r-- | chrome/browser/search_engines/util.h | 24 |
6 files changed, 455 insertions, 179 deletions
diff --git a/chrome/browser/search_engines/template_url.h b/chrome/browser/search_engines/template_url.h index 3373d42..1cee283 100644 --- a/chrome/browser/search_engines/template_url.h +++ b/chrome/browser/search_engines/template_url.h @@ -15,6 +15,8 @@ #include "googleurl/src/gurl.h" class TemplateURL; +class WebDataService; +struct WDKeywordsResult; // TemplateURL represents the relevant portions of the Open Search Description // Document (http://www.opensearch.org/Specifications/OpenSearch). @@ -440,6 +442,11 @@ 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 WebDatabaseTest; friend class WebDatabase; friend class TemplateURLModel; diff --git a/chrome/browser/search_engines/template_url_model.cc b/chrome/browser/search_engines/template_url_model.cc index 986d811..d6081b3 100644 --- a/chrome/browser/search_engines/template_url_model.cc +++ b/chrome/browser/search_engines/template_url_model.cc @@ -19,6 +19,7 @@ #include "chrome/browser/profile.h" #include "chrome/browser/rlz/rlz.h" #include "chrome/browser/search_engines/template_url_prepopulate_data.h" +#include "chrome/browser/search_engines/util.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/env_vars.h" #include "chrome/common/extensions/extension.h" @@ -466,8 +467,8 @@ void TemplateURLModel::Load() { } void TemplateURLModel::OnWebDataServiceRequestDone( - WebDataService::Handle h, - const WDTypedResult* result) { + WebDataService::Handle h, + const WDTypedResult* result) { // Reset the load_handle so that we don't try and cancel the load in // the destructor. load_handle_ = 0; @@ -481,57 +482,47 @@ void TemplateURLModel::OnWebDataServiceRequestDone( return; } - DCHECK(result->GetType() == KEYWORDS_RESULT); - - WDKeywordsResult keyword_result = reinterpret_cast< - const WDResult<WDKeywordsResult>*>(result)->GetValue(); - // prefs_default_search_provider_ is only needed before we've finished // loading. Now that we've loaded we can nuke it. prefs_default_search_provider_.reset(); - // Compiler won't implicitly convert std::vector<TemplateURL*> to - // std::vector<const TemplateURL*>, and reinterpret_cast is unsafe, - // so we just copy it. - std::vector<const TemplateURL*> template_urls(keyword_result.keywords.begin(), - keyword_result.keywords.end()); - - const int resource_keyword_version = - TemplateURLPrepopulateData::GetDataVersion(GetPrefs()); - if (keyword_result.builtin_keyword_version != resource_keyword_version) { - // There should never be duplicate TemplateURLs. We had a bug such that - // duplicate TemplateURLs existed for one locale. As such we invoke - // RemoveDuplicatePrepopulateIDs to nuke the duplicates. - RemoveDuplicatePrepopulateIDs(&template_urls); + std::vector<TemplateURL*> template_urls; + int new_resource_keyword_version = 0; + const TemplateURL* default_search_provider = NULL; + GetSearchProvidersUsingKeywordResult(*result, + service_.get(), + GetPrefs(), + &template_urls, + &default_search_provider, + &new_resource_keyword_version); + + // If the default search provider existed previously, then just + // set the member variable. Otherwise, we'll set it using the method + // to ensure that it is saved properly after its id is set. + if (default_search_provider && default_search_provider->id() != 0) { + default_search_provider_ = default_search_provider; + default_search_provider = NULL; } SetTemplateURLs(template_urls); - if (keyword_result.default_search_provider_id) { - // See if we can find the default search provider. - for (TemplateURLVector::iterator i = template_urls_.begin(); - i != template_urls_.end(); ++i) { - if ((*i)->id() == keyword_result.default_search_provider_id) { - default_search_provider_ = *i; - break; - } - } - } - - if (keyword_result.builtin_keyword_version != resource_keyword_version) { - MergeEnginesFromPrepopulateData(); - service_->SetBuiltinKeywordVersion(resource_keyword_version); + if (default_search_provider) { + // Note that this saves the default search provider to prefs. + SetDefaultSearchProvider(default_search_provider); + } else { + // Always save the default search provider to prefs. That way we don't + // have to worry about it being out of sync. + if (default_search_provider_) + SaveDefaultSearchProviderToPrefs(default_search_provider_); } - // Always save the default search provider to prefs. That way we don't have to - // worry about it being out of sync. - if (default_search_provider_) - SaveDefaultSearchProviderToPrefs(default_search_provider_); - // Index any visits that occurred before we finished loading. for (size_t i = 0; i < visits_to_add_.size(); ++i) UpdateKeywordSearchTermsForURL(visits_to_add_[i]); visits_to_add_.clear(); + if (new_resource_keyword_version && service_.get()) + service_->SetBuiltinKeywordVersion(new_resource_keyword_version); + loaded_ = true; FOR_EACH_OBSERVER(TemplateURLModelObserver, model_observers_, @@ -540,28 +531,6 @@ void TemplateURLModel::OnWebDataServiceRequestDone( NotifyLoaded(); } -void TemplateURLModel::RemoveDuplicatePrepopulateIDs( - std::vector<const TemplateURL*>* urls) { - std::set<int> ids; - for (std::vector<const TemplateURL*>::iterator i = urls->begin(); - i != urls->end(); ) { - int prepopulate_id = (*i)->prepopulate_id(); - if (prepopulate_id) { - if (ids.find(prepopulate_id) != ids.end()) { - if (service_.get()) - service_->RemoveKeyword(**i); - delete *i; - i = urls->erase(i); - } else { - ids.insert(prepopulate_id); - ++i; - } - } else { - ++i; - } - } -} - std::wstring TemplateURLModel::GetKeywordShortName(const std::wstring& keyword, bool* is_extension_keyword) { const TemplateURL* template_url = GetTemplateURLForKeyword(keyword); @@ -705,15 +674,29 @@ void TemplateURLModel::AddToMaps(const TemplateURL* template_url) { host_to_urls_map_[url.host()].insert(template_url); } -void TemplateURLModel::SetTemplateURLs( - const std::vector<const TemplateURL*>& urls) { +void TemplateURLModel::SetTemplateURLs(const std::vector<TemplateURL*>& urls) { // Add mappings for the new items. - for (TemplateURLVector::const_iterator i = urls.begin(); i != urls.end(); + + // First, add the items that already have id's, so that the next_id_ + // gets properly set. + for (std::vector<TemplateURL*>::const_iterator i = urls.begin(); + i != urls.end(); ++i) { + if ((*i)->id() == 0) + continue; next_id_ = std::max(next_id_, (*i)->id()); AddToMaps(*i); template_urls_.push_back(*i); } + + // Next add the new items that don't have id's. + for (std::vector<TemplateURL*>::const_iterator i = urls.begin(); + i != urls.end(); + ++i) { + if ((*i)->id() != 0) + continue; + Add(*i); + } } void TemplateURLModel::NotifyLoaded() { @@ -731,69 +714,6 @@ void TemplateURLModel::NotifyLoaded() { pending_extension_ids_.clear(); } -void TemplateURLModel::MergeEnginesFromPrepopulateData() { - // Build a map from prepopulate id to TemplateURL of existing urls. - typedef std::map<int, const TemplateURL*> IDMap; - IDMap id_to_turl; - for (TemplateURLVector::const_iterator i(template_urls_.begin()); - i != template_urls_.end(); ++i) { - int prepopulate_id = (*i)->prepopulate_id(); - if (prepopulate_id > 0) - id_to_turl[prepopulate_id] = *i; - } - - std::vector<TemplateURL*> loaded_urls; - size_t default_search_index; - TemplateURLPrepopulateData::GetPrepopulatedEngines(GetPrefs(), - &loaded_urls, - &default_search_index); - - std::set<int> updated_ids; - for (size_t i = 0; i < loaded_urls.size(); ++i) { - // We take ownership of |t_url|. - scoped_ptr<TemplateURL> t_url(loaded_urls[i]); - int t_url_id = t_url->prepopulate_id(); - if (!t_url_id || updated_ids.count(t_url_id)) { - // Prepopulate engines need a unique id. - NOTREACHED(); - continue; - } - - IDMap::iterator existing_url_iter(id_to_turl.find(t_url_id)); - if (existing_url_iter != id_to_turl.end()) { - const TemplateURL* existing_url = existing_url_iter->second; - if (!existing_url->safe_for_autoreplace()) { - // User edited the entry, preserve the keyword and description. - t_url->set_safe_for_autoreplace(false); - t_url->set_keyword(existing_url->keyword()); - t_url->set_autogenerate_keyword( - existing_url->autogenerate_keyword()); - t_url->set_short_name(existing_url->short_name()); - } - Update(existing_url, *t_url); - id_to_turl.erase(existing_url_iter); - } else { - Add(t_url.release()); - } - if (i == default_search_index && !default_search_provider_) - SetDefaultSearchProvider(loaded_urls[i]); - - updated_ids.insert(t_url_id); - } - - // 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. - for (IDMap::iterator i(id_to_turl.begin()); i != id_to_turl.end(); ++i) { - const TemplateURL* template_url = i->second; - // We use default_search_provider_ instead of GetDefaultSearchProvider() - // because we're running before |loaded_| is set, and calling - // GetDefaultSearchProvider() will erroneously try to read the prefs. - if ((template_url->safe_for_autoreplace()) && - (template_url != default_search_provider_)) - Remove(template_url); - } -} - void TemplateURLModel::SaveDefaultSearchProviderToPrefs( const TemplateURL* t_url) { PrefService* prefs = GetPrefs(); diff --git a/chrome/browser/search_engines/template_url_model.h b/chrome/browser/search_engines/template_url_model.h index 6559bb0d..cf4e62c 100644 --- a/chrome/browser/search_engines/template_url_model.h +++ b/chrome/browser/search_engines/template_url_model.h @@ -193,12 +193,6 @@ class TemplateURLModel : public WebDataServiceConsumer, virtual void OnWebDataServiceRequestDone(WebDataService::Handle h, const WDTypedResult* result); - // Removes (and deletes) TemplateURLs from |urls| that have duplicate - // prepopulate ids. Duplicate prepopulate ids are not allowed, but due to a - // bug it was possible get dups. This step is only called when the version - // number changes. - void RemoveDuplicatePrepopulateIDs(std::vector<const TemplateURL*>* urls); - // Returns the locale-direction-adjusted short name for the given keyword. // Also sets the out param to indicate whether the keyword belongs to an // extension. @@ -266,7 +260,7 @@ class TemplateURLModel : public WebDataServiceConsumer, // Sets the keywords. This is used once the keywords have been loaded. // This does NOT notify the delegate or the database. - void SetTemplateURLs(const std::vector<const TemplateURL*>& urls); + void SetTemplateURLs(const std::vector<TemplateURL*>& urls); void DeleteGeneratedKeywordsMatchingHost(const std::wstring& host); @@ -274,10 +268,6 @@ class TemplateURLModel : public WebDataServiceConsumer, // notification. void NotifyLoaded(); - // Loads engines from prepopulate data and merges them in with the existing - // engines. This is invoked when the version of the prepopulate data changes. - void MergeEnginesFromPrepopulateData(); - // Saves enough of url to preferences so that it can be loaded from // preferences on start up. void SaveDefaultSearchProviderToPrefs(const TemplateURL* url); diff --git a/chrome/browser/search_engines/template_url_model_unittest.cc b/chrome/browser/search_engines/template_url_model_unittest.cc index 5e174e3..a60e0ba 100644 --- a/chrome/browser/search_engines/template_url_model_unittest.cc +++ b/chrome/browser/search_engines/template_url_model_unittest.cc @@ -5,12 +5,14 @@ #include "base/callback.h" #include "base/file_util.h" #include "base/path_service.h" +#include "base/stl_util-inl.h" #include "base/string_util.h" #include "base/thread.h" #include "chrome/browser/chrome_thread.h" #include "chrome/browser/history/history.h" #include "chrome/browser/history/history_notifications.h" #include "chrome/browser/search_engines/template_url_model.h" +#include "chrome/browser/search_engines/template_url_prepopulate_data.h" #include "chrome/browser/webdata/web_database.h" #include "chrome/test/testing_profile.h" #include "testing/gtest/include/gtest/gtest.h" @@ -18,6 +20,21 @@ using base::Time; using base::TimeDelta; +// Create an URL that appears to have been prepopulated, but won't be in the +// current data. The caller owns the returned TemplateURL*. +static TemplateURL* CreatePreloadedTemplateURL() { + TemplateURL* t_url = new TemplateURL(); + t_url->SetURL("http://www.unittest.com/", 0, 0); + t_url->set_keyword(L"unittest"); + t_url->set_short_name(L"unittest"); + t_url->set_safe_for_autoreplace(true); + GURL favicon_url("http://favicon.url"); + t_url->SetFavIconURL(favicon_url); + t_url->set_date_created(Time::FromTimeT(100)); + t_url->set_prepopulate_id(999999); + return t_url; +} + // A Task used to coordinate when the database has finished processing // requests. See note in BlockTillServiceProcessesRequests for details. // @@ -173,7 +190,14 @@ class TemplateURLModelTest : public testing::Test, model_->Load(); BlockTillServiceProcessesRequests(); VerifyObserverCount(1); - changed_count_ = 0; + } + + // Makes the model believe it has been loaded (without actually doing the + // load). Since this avoids setting the built-in keyword version, the next + // load will do a merge from prepopulated data. + void ChangeModelToLoadState() { + model_->service_ = profile_->GetWebDataService(Profile::EXPLICIT_ACCESS); + model_->loaded_ = true; } // Creates a new TemplateURLModel. @@ -206,6 +230,12 @@ class TemplateURLModelTest : public testing::Test, TemplateURLRef::google_base_url_ = new std::string(base_url); } + // Verifies the behavior of when a preloaded url later gets changed. + // Since the input is the offset from the default, when one passes in + // 0, it tests the default. Passing in a number > 0 will verify what + // happens when a preloaded url that is not the default gets updated. + void TestLoadUpdatingPreloadedUrl(size_t index_offset_from_default); + MessageLoopForUI message_loop_; // Needed to make the DeleteOnUIThread trait of WebDataService work // properly. @@ -215,6 +245,59 @@ class TemplateURLModelTest : public testing::Test, int changed_count_; }; +void TemplateURLModelTest::TestLoadUpdatingPreloadedUrl( + size_t index_offset_from_default) { + // Create a TemplateURL with the same prepopulated id as + // a real prepopulated item. + TemplateURL* t_url = CreatePreloadedTemplateURL(); + t_url->set_safe_for_autoreplace(false); + std::vector<TemplateURL*> prepopulated_urls; + size_t default_search_provider_index = 0; + TemplateURLPrepopulateData::GetPrepopulatedEngines( + profile_->GetPrefs(), + &prepopulated_urls, + &default_search_provider_index); + ASSERT_LT(index_offset_from_default, prepopulated_urls.size()); + size_t prepopulated_index = + (default_search_provider_index + index_offset_from_default) % + prepopulated_urls.size(); + t_url->set_prepopulate_id( + prepopulated_urls[prepopulated_index]->prepopulate_id()); + + std::wstring original_url = t_url->url()->DisplayURL(); + std::wstring prepopulated_url = + prepopulated_urls[prepopulated_index]->url()->DisplayURL(); + ASSERT_STRNE(prepopulated_url.c_str(), original_url.c_str()); + STLDeleteElements(&prepopulated_urls); + prepopulated_urls.clear(); + + // Then add it to the model and save it all. + ChangeModelToLoadState(); + model_->Add(t_url); + const TemplateURL* keyword_url = + model_->GetTemplateURLForKeyword(L"unittest"); + ASSERT_EQ(t_url, keyword_url); + ASSERT_STREQ(original_url.c_str(), keyword_url->url()->DisplayURL().c_str()); + BlockTillServiceProcessesRequests(); + + // Now reload the model and verify that the merge updates the url. + ResetModel(true); + keyword_url = model_->GetTemplateURLForKeyword(L"unittest"); + ASSERT_TRUE(keyword_url != NULL); + ASSERT_STREQ(prepopulated_url.c_str(), + keyword_url->url()->DisplayURL().c_str()); + + // Wait for any saves to finish. + BlockTillServiceProcessesRequests(); + + // Reload the model to verify that change was saved correctly. + ResetModel(true); + keyword_url = model_->GetTemplateURLForKeyword(L"unittest"); + ASSERT_TRUE(keyword_url != NULL); + ASSERT_STREQ(prepopulated_url.c_str(), + keyword_url->url()->DisplayURL().c_str()); +} + TEST_F(TemplateURLModelTest, Load) { VerifyLoad(); } @@ -711,52 +794,134 @@ TEST_F(TemplateURLModelTest, GenerateVisitOnKeyword) { PageTransition::StripQualifier(callback.visits[0].transition)); } -// Make sure that MergeEnginesFromPrepopulateData() deletes prepopulated engines -// that no longer exist in the prepopulate data. -TEST_F(TemplateURLModelTest, MergeDeletesUnusedProviders) { - VerifyLoad(); +// Make sure that the load routine deletes prepopulated engines that no longer +// exist in the prepopulate data. +TEST_F(TemplateURLModelTest, LoadDeletesUnusedProvider) { + // Create a preloaded template url. Add it to a loaded model and wait for the + // saves to finish. + TemplateURL* t_url = CreatePreloadedTemplateURL(); + ChangeModelToLoadState(); + model_->Add(t_url); + ASSERT_TRUE(model_->GetTemplateURLForKeyword(L"unittest") != NULL); + BlockTillServiceProcessesRequests(); - // Create an URL that appears to have been prepopulated but won't be in the - // current data. - TemplateURL* t_url = new TemplateURL(); - t_url->SetURL("http://www.unittest.com/", 0, 0); - t_url->set_keyword(L"unittest"); - t_url->set_short_name(L"unittest"); - t_url->set_safe_for_autoreplace(true); - GURL favicon_url("http://favicon.url"); - t_url->SetFavIconURL(favicon_url); - t_url->set_date_created(Time::FromTimeT(100)); - t_url->set_prepopulate_id(999999); + // Ensure that merging clears this engine. + ResetModel(true); + ASSERT_TRUE(model_->GetTemplateURLForKeyword(L"unittest") == NULL); - // Make a few copies now, since as we add each to the model it takes ownership - // of them and deletes them when finished. - TemplateURL* t_url2 = new TemplateURL(*t_url); - TemplateURL* t_url3 = new TemplateURL(*t_url); + // Wait for any saves to finish. + BlockTillServiceProcessesRequests(); - // Ensure that merging clears this engine. - model_->Add(t_url); - EXPECT_EQ(t_url, model_->GetTemplateURLForKeyword(L"unittest")); - model_->MergeEnginesFromPrepopulateData(); + // Reload the model to verify that the database was updated as a result of the + // merge. + ResetModel(true); ASSERT_TRUE(model_->GetTemplateURLForKeyword(L"unittest") == NULL); +} + +// Make sure that load routine doesn't delete prepopulated engines that no +// longer exist in the prepopulate data if it has been modified by the user. +TEST_F(TemplateURLModelTest, LoadRetainsModifiedProvider) { + // Create a preloaded template url and add it to a loaded model. + TemplateURL* t_url = CreatePreloadedTemplateURL(); + t_url->set_safe_for_autoreplace(false); + ChangeModelToLoadState(); + model_->Add(t_url); + + // Do the copy after t_url is added so that the id is set. + TemplateURL copy_t_url = *t_url; + ASSERT_EQ(t_url, model_->GetTemplateURLForKeyword(L"unittest")); + + // Wait for any saves to finish. + BlockTillServiceProcessesRequests(); // Ensure that merging won't clear it if the user has edited it. - t_url2->set_safe_for_autoreplace(false); - model_->Add(t_url2); - ASSERT_EQ(t_url2, model_->GetTemplateURLForKeyword(L"unittest")); - model_->MergeEnginesFromPrepopulateData(); - ASSERT_FALSE(model_->GetTemplateURLForKeyword(L"unittest") == NULL); - model_->Remove(t_url2); - - // Ensure that merging won't clear it if it's the default engine. - model_->Add(t_url3); - ASSERT_EQ(t_url3, model_->GetTemplateURLForKeyword(L"unittest")); - model_->SetDefaultSearchProvider(t_url3); - ASSERT_EQ(t_url3, model_->GetDefaultSearchProvider()); - model_->MergeEnginesFromPrepopulateData(); - ASSERT_EQ(t_url3, model_->GetTemplateURLForKeyword(L"unittest")); - ASSERT_EQ(t_url3, model_->GetDefaultSearchProvider()); - // Don't remove |t_url3|; we'd need to make it non-default first, and why - // bother when the model shutdown will clean it up for us. + ResetModel(true); + const TemplateURL* url_for_unittest = + model_->GetTemplateURLForKeyword(L"unittest"); + ASSERT_TRUE(url_for_unittest != NULL); + AssertEquals(copy_t_url, *url_for_unittest); + + // Wait for any saves to finish. + BlockTillServiceProcessesRequests(); + + // Reload the model to verify that save/reload retains the item. + ResetModel(true); + ASSERT_TRUE(model_->GetTemplateURLForKeyword(L"unittest") != NULL); +} + +// Make sure that load routine doesn't delete +// prepopulated engines that no longer exist in the prepopulate data if +// it has been modified by the user. +TEST_F(TemplateURLModelTest, LoadSavesPrepopulatedDefaultSearchProvider) { + VerifyLoad(); + // Verify that the default search provider is set to something. + ASSERT_TRUE(model_->GetDefaultSearchProvider() != NULL); + TemplateURL default_url = *model_->GetDefaultSearchProvider(); + + // Wait for any saves to finish. + BlockTillServiceProcessesRequests(); + + // Reload the model and check that the default search provider + // was properly saved. + ResetModel(true); + ASSERT_TRUE(model_->GetDefaultSearchProvider() != NULL); + AssertEquals(default_url, *model_->GetDefaultSearchProvider()); +} + +// Make sure that the load routine doesn't delete +// prepopulated engines that no longer exist in the prepopulate data if +// it is the default search provider. +TEST_F(TemplateURLModelTest, LoadRetainsDefaultProvider) { + // Set the default search provider to a preloaded template url which + // is not in the current set of preloaded template urls and save + // the result. + TemplateURL* t_url = CreatePreloadedTemplateURL(); + ChangeModelToLoadState(); + model_->Add(t_url); + 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; + + ASSERT_EQ(t_url, model_->GetTemplateURLForKeyword(L"unittest")); + ASSERT_EQ(t_url, model_->GetDefaultSearchProvider()); + BlockTillServiceProcessesRequests(); + + // Ensure that merging won't clear the prepopulated template url + // which is no longer present if it's the default engine. + ResetModel(true); + { + const TemplateURL* keyword_url = + model_->GetTemplateURLForKeyword(L"unittest"); + ASSERT_TRUE(keyword_url != NULL); + AssertEquals(copy_t_url, *keyword_url); + ASSERT_EQ(keyword_url, model_->GetDefaultSearchProvider()); + } + + // Wait for any saves to finish. + BlockTillServiceProcessesRequests(); + + // Reload the model to verify that the update was saved. + ResetModel(true); + { + const TemplateURL* keyword_url = + model_->GetTemplateURLForKeyword(L"unittest"); + ASSERT_TRUE(keyword_url != NULL); + AssertEquals(copy_t_url, *keyword_url); + ASSERT_EQ(keyword_url, model_->GetDefaultSearchProvider()); + } +} + +// Make sure that the load routine updates the url of a preexisting +// default search engine provider and that the result is saved correctly. +TEST_F(TemplateURLModelTest, LoadUpdatesDefaultSearchUrl) { + TestLoadUpdatingPreloadedUrl(0); +} + +// Make sure that the load routine updates the url of a preexisting +// non-default search engine provider and that the result is saved correctly. +TEST_F(TemplateURLModelTest, LoadUpdatesSearchUrl) { + TestLoadUpdatingPreloadedUrl(1); } // Simulates failing to load the webdb and makes sure the default search diff --git a/chrome/browser/search_engines/util.cc b/chrome/browser/search_engines/util.cc index 28e1c93..f27643e 100644 --- a/chrome/browser/search_engines/util.cc +++ b/chrome/browser/search_engines/util.cc @@ -4,8 +4,16 @@ #include "chrome/browser/search_engines/util.h" +#include <set> +#include <vector> + +#include "base/logging.h" #include "base/utf_string_conversions.h" +#include "chrome/browser/chrome_thread.h" +#include "chrome/browser/search_engines/template_url.h" #include "chrome/browser/search_engines/template_url_model.h" +#include "chrome/browser/search_engines/template_url_prepopulate_data.h" +#include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profile.h" string16 GetDefaultSearchEngineName(Profile* profile) { @@ -23,3 +31,165 @@ string16 GetDefaultSearchEngineName(Profile* profile) { } return WideToUTF16(default_provider->short_name()); } + +// Removes (and deletes) TemplateURLs from |urls| that have duplicate +// prepopulate ids. Duplicate prepopulate ids are not allowed, but due to a +// bug it was possible get dups. This step is only called when the version +// number changes. Only pass in a non-NULL value for |service| if the removed +// items should be removed from the DB. +static void RemoveDuplicatePrepopulateIDs( + std::vector<TemplateURL*>* template_urls, + WebDataService* service) { + DCHECK(template_urls); + DCHECK(service == NULL || ChromeThread::CurrentlyOn(ChromeThread::UI)); + + std::set<int> ids; + for (std::vector<TemplateURL*>::iterator i = template_urls->begin(); + i != template_urls->end(); ) { + int prepopulate_id = (*i)->prepopulate_id(); + if (prepopulate_id) { + if (ids.find(prepopulate_id) != ids.end()) { + if (service) + service->RemoveKeyword(**i); + delete *i; + i = template_urls->erase(i); + } else { + ids.insert(prepopulate_id); + ++i; + } + } else { + ++i; + } + } +} + +// Loads engines from prepopulate data and merges them in with the existing +// engines. This is invoked when the version of the prepopulate data changes. +void MergeEnginesFromPrepopulateData( + PrefService* prefs, + WebDataService* service, + std::vector<TemplateURL*>* template_urls, + const TemplateURL** default_search_provider) { + DCHECK(service == NULL || ChromeThread::CurrentlyOn(ChromeThread::UI)); + DCHECK(template_urls); + DCHECK(default_search_provider); + + // Build a map from prepopulate id to TemplateURL of existing urls. + typedef std::map<int, TemplateURL*> IDMap; + IDMap id_to_turl; + for (std::vector<TemplateURL*>::iterator i(template_urls->begin()); + i != template_urls->end(); ++i) { + int prepopulate_id = (*i)->prepopulate_id(); + if (prepopulate_id > 0) + id_to_turl[prepopulate_id] = *i; + } + + std::vector<TemplateURL*> loaded_urls; + size_t default_search_index; + TemplateURLPrepopulateData::GetPrepopulatedEngines(prefs, + &loaded_urls, + &default_search_index); + + std::set<int> updated_ids; + for (size_t i = 0; i < loaded_urls.size(); ++i) { + // We take ownership of |t_url|. + scoped_ptr<TemplateURL> t_url(loaded_urls[i]); + const int t_url_id = t_url->prepopulate_id(); + if (!t_url_id || updated_ids.count(t_url_id)) { + // Prepopulate engines need a unique id. + NOTREACHED(); + continue; + } + + TemplateURL* current_t_url = t_url.get(); + IDMap::iterator existing_url_iter(id_to_turl.find(t_url_id)); + if (existing_url_iter != id_to_turl.end()) { + current_t_url = existing_url_iter->second; + if (!current_t_url->safe_for_autoreplace()) { + // User edited the entry, preserve the keyword and description. + t_url->set_safe_for_autoreplace(false); + t_url->set_keyword(current_t_url->keyword()); + t_url->set_autogenerate_keyword( + current_t_url->autogenerate_keyword()); + t_url->set_short_name(current_t_url->short_name()); + } + t_url->set_id(current_t_url->id()); + + *current_t_url = *t_url; + if (service) + service->UpdateKeyword(*current_t_url); + id_to_turl.erase(existing_url_iter); + } else { + template_urls->push_back(t_url.release()); + } + if (i == default_search_index && !*default_search_provider) + *default_search_provider = current_t_url; + + updated_ids.insert(t_url_id); + } + + // 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. + 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 = find(template_urls->begin(), + template_urls->end(), + template_url); + DCHECK(i != template_urls->end()); + template_urls->erase(i); + if (service) + service->RemoveKeyword(*template_url); + delete template_url; + } + } +} + +void GetSearchProvidersUsingKeywordResult( + const WDTypedResult& result, + WebDataService* service, + PrefService* prefs, + std::vector<TemplateURL*>* template_urls, + const TemplateURL** default_search_provider, + int* new_resource_keyword_version) { + DCHECK(service == NULL || ChromeThread::CurrentlyOn(ChromeThread::UI)); + DCHECK(template_urls); + DCHECK(template_urls->empty()); + DCHECK(default_search_provider); + DCHECK(*default_search_provider == NULL); + DCHECK(result.GetType() == KEYWORDS_RESULT); + DCHECK(new_resource_keyword_version); + + *new_resource_keyword_version = 0; + WDKeywordsResult keyword_result = reinterpret_cast< + const WDResult<WDKeywordsResult>*>(&result)->GetValue(); + + template_urls->swap(keyword_result.keywords); + + const int resource_keyword_version = + TemplateURLPrepopulateData::GetDataVersion(prefs); + if (keyword_result.builtin_keyword_version != resource_keyword_version) { + // There should never be duplicate TemplateURLs. We had a bug such that + // duplicate TemplateURLs existed for one locale. As such we invoke + // RemoveDuplicatePrepopulateIDs to nuke the duplicates. + RemoveDuplicatePrepopulateIDs(template_urls, service); + } + + if (keyword_result.default_search_provider_id) { + // See if we can find the default search provider. + for (std::vector<TemplateURL*>::iterator i = template_urls->begin(); + i != template_urls->end(); ++i) { + if ((*i)->id() == keyword_result.default_search_provider_id) { + *default_search_provider = *i; + break; + } + } + } + + if (keyword_result.builtin_keyword_version != resource_keyword_version) { + MergeEnginesFromPrepopulateData(prefs, service, template_urls, + default_search_provider); + *new_resource_keyword_version = resource_keyword_version; + } +} diff --git a/chrome/browser/search_engines/util.h b/chrome/browser/search_engines/util.h index 1391e6d..e517ba6 100644 --- a/chrome/browser/search_engines/util.h +++ b/chrome/browser/search_engines/util.h @@ -7,13 +7,37 @@ #pragma once // This file contains utility functions for search engine functionality. +#include <vector> #include "base/string16.h" +class PrefService; class Profile; +class TemplateURL; +class WDTypedResult; +class WebDataService; // Returns the short name of the default search engine, or the empty string if // none is set. |profile| may be NULL. string16 GetDefaultSearchEngineName(Profile* profile); +// Processes the results of WebDataService::GetKeywords, combining it with +// prepopulated search providers to result in: +// * a set of template_urls (search providers). The caller owns the +// TemplateURL* returned in template_urls. +// * the default search provider (and if *default_search_provider is not NULL, +// it is contained in template_urls). +// * whether there is a new resource keyword version (and the value). +// |*new_resource_keyword_version| is set to 0 if no new value. Otherwise, +// it is the new value. +// Only pass in a non-NULL value for service if the WebDataService should be +// updated. +void GetSearchProvidersUsingKeywordResult( + const WDTypedResult& result, + WebDataService* service, + PrefService* prefs, + std::vector<TemplateURL*>* template_urls, + const TemplateURL** default_search_provider, + int* new_resource_keyword_version); + #endif // CHROME_BROWSER_SEARCH_ENGINES_UTIL_H_ |