diff options
Diffstat (limited to 'chromium/components/search_engines')
29 files changed, 245 insertions, 111 deletions
diff --git a/chromium/components/search_engines/BUILD.gn b/chromium/components/search_engines/BUILD.gn index c2b09f07178..6cd8400300b 100644 --- a/chromium/components/search_engines/BUILD.gn +++ b/chromium/components/search_engines/BUILD.gn @@ -170,6 +170,7 @@ source_set("unit_tests") { "//components/prefs", "//components/sync:test_support", "//components/sync_preferences:test_support", + "//components/variations:test_support", "//components/webdata/common", "//net:net", "//services/network:test_support", diff --git a/chromium/components/search_engines/default_search_manager.cc b/chromium/components/search_engines/default_search_manager.cc index ce9d8859626..8fa9c57b381 100644 --- a/chromium/components/search_engines/default_search_manager.cc +++ b/chromium/components/search_engines/default_search_manager.cc @@ -77,6 +77,9 @@ const char DefaultSearchManager::kCreatedByPolicy[] = "created_by_policy"; const char DefaultSearchManager::kDisabledByPolicy[] = "disabled_by_policy"; const char DefaultSearchManager::kCreatedFromPlayAPI[] = "created_from_play_api"; +const char DefaultSearchManager::kPreconnectToSearchUrl[] = + "preconnect_to_search_url"; +const char DefaultSearchManager::kIsActive[] = "is_active"; DefaultSearchManager::DefaultSearchManager( PrefService* pref_service, diff --git a/chromium/components/search_engines/default_search_manager.h b/chromium/components/search_engines/default_search_manager.h index 8c3f4000010..86996ac647b 100644 --- a/chromium/components/search_engines/default_search_manager.h +++ b/chromium/components/search_engines/default_search_manager.h @@ -61,6 +61,8 @@ class DefaultSearchManager { static const char kCreatedByPolicy[]; static const char kDisabledByPolicy[]; static const char kCreatedFromPlayAPI[]; + static const char kPreconnectToSearchUrl[]; + static const char kIsActive[]; enum Source { // Default search engine chosen either from prepopulated engines set for diff --git a/chromium/components/search_engines/default_search_manager_unittest.cc b/chromium/components/search_engines/default_search_manager_unittest.cc index ef0c6d03822..1adb2a53df6 100644 --- a/chromium/components/search_engines/default_search_manager_unittest.cc +++ b/chromium/components/search_engines/default_search_manager_unittest.cc @@ -23,6 +23,7 @@ #include "components/search_engines/template_url_data_util.h" #include "components/search_engines/template_url_prepopulate_data.h" #include "components/sync_preferences/testing_pref_service_syncable.h" +#include "components/variations/scoped_variations_ids_provider.h" #include "testing/gtest/include/gtest/gtest.h" namespace { @@ -42,9 +43,9 @@ void SetOverrides(sync_preferences::TestingPrefServiceSyncable* prefs, entry->SetString("encoding", "UTF-8"); entry->SetInteger("id", 1001); entry->SetString("suggest_url", "http://foo.com/suggest?q={searchTerms}"); - auto alternate_urls = std::make_unique<base::ListValue>(); - alternate_urls->AppendString("http://foo.com/alternate?q={searchTerms}"); - entry->Set("alternate_urls", std::move(alternate_urls)); + base::ListValue alternate_urls; + alternate_urls.AppendString("http://foo.com/alternate?q={searchTerms}"); + entry->SetKey("alternate_urls", std::move(alternate_urls)); overrides->Append(std::move(entry)); entry = std::make_unique<base::DictionaryValue>(); @@ -94,6 +95,8 @@ class DefaultSearchManagerTest : public testing::Test { } private: + variations::ScopedVariationsIdsProvider scoped_variations_ids_provider_{ + variations::VariationsIdsProvider::Mode::kUseSignedInState}; std::unique_ptr<sync_preferences::TestingPrefServiceSyncable> pref_service_; DISALLOW_COPY_AND_ASSIGN(DefaultSearchManagerTest); diff --git a/chromium/components/search_engines/default_search_policy_handler.cc b/chromium/components/search_engines/default_search_policy_handler.cc index 69529f93493..ce1df13737c 100644 --- a/chromium/components/search_engines/default_search_policy_handler.cc +++ b/chromium/components/search_engines/default_search_policy_handler.cc @@ -45,13 +45,12 @@ void SetStringInPref(const PolicyMap& policies, const char* key, base::DictionaryValue* dict) { DCHECK(dict); - const base::Value* policy_value = policies.GetValue(policy_name); std::string str; - if (policy_value) { - bool is_string = policy_value->GetAsString(&str); - DCHECK(is_string); + if (const base::Value* policy_value = policies.GetValue(policy_name)) { + DCHECK(policy_value->is_string()); + str = policy_value->GetString(); } - dict->SetString(key, str); + dict->SetStringKey(key, str); } void SetBooleanInPref(const PolicyMap& policies, @@ -59,12 +58,12 @@ void SetBooleanInPref(const PolicyMap& policies, const char* key, base::DictionaryValue* dict) { DCHECK(dict); - const base::Value* policy_value = policies.GetValue(policy_name); bool bool_value = false; - if (policy_value) { - DCHECK(policy_value->GetAsBoolean(&bool_value)); + if (const base::Value* policy_value = policies.GetValue(policy_name)) { + DCHECK(policy_value->is_bool()); + bool_value = policy_value->GetBool(); } - dict->SetBoolean(key, bool_value); + dict->SetBoolPath(key, bool_value); } } // namespace @@ -272,9 +271,8 @@ bool DefaultSearchPolicyHandler::DefaultSearchProviderIsDisabled( const PolicyMap& policies) { const base::Value* provider_enabled = policies.GetValue(key::kDefaultSearchProviderEnabled); - bool enabled = true; - return provider_enabled && provider_enabled->GetAsBoolean(&enabled) && - !enabled; + return provider_enabled && provider_enabled->is_bool() && + !provider_enabled->GetBool(); } bool DefaultSearchPolicyHandler::DefaultSearchProviderPolicyIsSet( @@ -287,8 +285,11 @@ bool DefaultSearchPolicyHandler::DefaultSearchURLIsValid( const base::Value** url_value, std::string* url_string) { *url_value = policies.GetValue(key::kDefaultSearchProviderSearchURL); - if (!*url_value || !(*url_value)->GetAsString(url_string) || - url_string->empty()) + if (!*url_value || !(*url_value)->is_string()) + return false; + + *url_string = (*url_value)->GetString(); + if (url_string->empty()) return false; TemplateURLData data; data.SetURL(*url_string); diff --git a/chromium/components/search_engines/default_search_policy_handler.h b/chromium/components/search_engines/default_search_policy_handler.h index 7d12000b0f1..b236b80e29e 100644 --- a/chromium/components/search_engines/default_search_policy_handler.h +++ b/chromium/components/search_engines/default_search_policy_handler.h @@ -6,7 +6,6 @@ #define COMPONENTS_SEARCH_ENGINES_DEFAULT_SEARCH_POLICY_HANDLER_H_ #include <memory> -#include <vector> #include "base/macros.h" #include "components/policy/core/browser/configuration_policy_handler.h" diff --git a/chromium/components/search_engines/keyword_table.cc b/chromium/components/search_engines/keyword_table.cc index 097582a2331..cb82ffd1dc7 100644 --- a/chromium/components/search_engines/keyword_table.cc +++ b/chromium/components/search_engines/keyword_table.cc @@ -104,6 +104,10 @@ const std::string ColumnsForVersion(int version, bool concatenated) { // Column added in version 82. columns.push_back("created_from_play_api"); } + if (version >= 97) { + // Column added in version 97 + columns.push_back("is_active"); + } return base::JoinString(columns, std::string(concatenated ? " || " : ", ")); } @@ -159,6 +163,7 @@ void BindURLToStatement(const TemplateURLData& data, s->BindInt64(starting_column + 20, data.last_visited.since_origin().InMicroseconds()); s->BindBool(starting_column + 21, data.created_from_play_api); + s->BindInt(starting_column + 22, static_cast<int>(data.is_active)); } WebDatabaseTable::TypeKey GetKey() { @@ -209,7 +214,8 @@ bool KeywordTable::CreateTablesIfNecessary() { "image_url_post_params VARCHAR," "new_tab_url VARCHAR," "last_visited INTEGER DEFAULT 0, " - "created_from_play_api INTEGER DEFAULT 0)"); + "created_from_play_api INTEGER DEFAULT 0, " + "is_active INTEGER DEFAULT 0)"); } bool KeywordTable::IsSyncable() { @@ -239,6 +245,8 @@ bool KeywordTable::MigrateToVersion(int version, return MigrateToVersion77IncreaseTimePrecision(); case 82: return MigrateToVersion82AddCreatedFromPlayApiColumn(); + case 97: + return MigrateToVersion97AddIsActiveColumn(); } return true; @@ -453,8 +461,13 @@ bool KeywordTable::MigrateToVersion82AddCreatedFromPlayApiColumn() { "0"); } +bool KeywordTable::MigrateToVersion97AddIsActiveColumn() { + return db_->Execute( + "ALTER TABLE keywords ADD COLUMN is_active INTEGER DEFAULT 0"); +} + // static -bool KeywordTable::GetKeywordDataFromStatement(const sql::Statement& s, +bool KeywordTable::GetKeywordDataFromStatement(sql::Statement& s, TemplateURLData* data) { DCHECK(data); @@ -488,6 +501,7 @@ bool KeywordTable::GetKeywordDataFromStatement(const sql::Statement& s, data->usage_count = s.ColumnInt(8); data->prepopulate_id = s.ColumnInt(11); data->sync_guid = s.ColumnString(14); + data->is_active = static_cast<TemplateURLData::ActiveStatus>(s.ColumnInt(23)); data->alternate_urls.clear(); absl::optional<base::Value> value(base::JSONReader::Read(s.ColumnString(15))); @@ -510,7 +524,7 @@ bool KeywordTable::AddKeyword(const TemplateURLData& data) { DCHECK(data.id); std::string query("INSERT INTO keywords (" + GetKeywordColumns() + ") " - "VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"); + "VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"); sql::Statement s(db_->GetCachedStatement(SQL_FROM_HERE, query.c_str())); BindURLToStatement(data, &s, 0, 1); @@ -536,8 +550,8 @@ bool KeywordTable::UpdateKeyword(const TemplateURLData& data) { "created_by_policy=?, last_modified=?, sync_guid=?, alternate_urls=?, " "image_url=?, search_url_post_params=?, suggest_url_post_params=?, " "image_url_post_params=?, new_tab_url=?, last_visited=?, " - "created_from_play_api=? WHERE id=?")); - BindURLToStatement(data, &s, 22, 0); // "22" binds id() as the last item. + "created_from_play_api=?, is_active=? WHERE id=?")); + BindURLToStatement(data, &s, 23, 0); // "23" binds id() as the last item. return s.Run(); } diff --git a/chromium/components/search_engines/keyword_table.h b/chromium/components/search_engines/keyword_table.h index 951e529e467..d564fcbcc1e 100644 --- a/chromium/components/search_engines/keyword_table.h +++ b/chromium/components/search_engines/keyword_table.h @@ -36,7 +36,8 @@ class Statement; // keyword // favicon_url // url -// safe_for_autoreplace +// safe_for_autoreplace This is set to false for any entry that was manually +// added or edited by the user. // originating_url // date_created This column was added after we allowed keywords. // Keywords created before we started tracking @@ -68,6 +69,8 @@ class Statement; // version 69. // created_from_play_api See TemplateURLData::created_from_play_api. This was // added in version 82. +// is_active See TemplateURLData::is_active. This was added +// in version 97. // // This class also manages some fields in the |meta| table: // @@ -132,6 +135,7 @@ class KeywordTable : public WebDatabaseTable { bool MigrateToVersion76RemoveInstantColumns(); bool MigrateToVersion77IncreaseTimePrecision(); bool MigrateToVersion82AddCreatedFromPlayApiColumn(); + bool MigrateToVersion97AddIsActiveColumn(); private: friend class KeywordTableTest; @@ -143,7 +147,7 @@ class KeywordTable : public WebDatabaseTable { // Fills |data| with the data in |s|. Returns false if we couldn't fill // |data| for some reason, e.g. |s| tried to set one of the fields to an // illegal value. - static bool GetKeywordDataFromStatement(const sql::Statement& s, + static bool GetKeywordDataFromStatement(sql::Statement& s, TemplateURLData* data); // Adds a new keyword, updating the id field on success. diff --git a/chromium/components/search_engines/keyword_table_unittest.cc b/chromium/components/search_engines/keyword_table_unittest.cc index ef3f1dc709d..b23570625af 100644 --- a/chromium/components/search_engines/keyword_table_unittest.cc +++ b/chromium/components/search_engines/keyword_table_unittest.cc @@ -138,6 +138,7 @@ TEST_F(KeywordTableTest, Keywords) { restored_keyword.created_from_play_api); EXPECT_EQ(keyword.usage_count, restored_keyword.usage_count); EXPECT_EQ(keyword.prepopulate_id, restored_keyword.prepopulate_id); + EXPECT_EQ(keyword.is_active, restored_keyword.is_active); RemoveKeyword(restored_keyword.id); @@ -174,6 +175,7 @@ TEST_F(KeywordTableTest, UpdateKeyword) { EXPECT_EQ(keyword.prepopulate_id, restored_keyword.prepopulate_id); EXPECT_EQ(keyword.created_from_play_api, restored_keyword.created_from_play_api); + EXPECT_EQ(keyword.is_active, restored_keyword.is_active); } TEST_F(KeywordTableTest, KeywordWithNoFavicon) { diff --git a/chromium/components/search_engines/prepopulated_engines.json b/chromium/components/search_engines/prepopulated_engines.json index 269a4897ffc..3197f270725 100644 --- a/chromium/components/search_engines/prepopulated_engines.json +++ b/chromium/components/search_engines/prepopulated_engines.json @@ -28,7 +28,7 @@ // Increment this if you change the data in ways that mean users with // existing data should get a new version. Otherwise, existing data may // continue to be used and updates made here will not always appear. - "kCurrentDataVersion": 125 + "kCurrentDataVersion": 126 }, // The following engines are included in country lists and are added to the @@ -129,6 +129,7 @@ "{google:baseURL}s?q={searchTerms}" ], "type": "SEARCH_ENGINE_GOOGLE", + "preconnect_to_search_url" : "ALLOWED", "id": 1 }, diff --git a/chromium/components/search_engines/prepopulated_engines_schema.json b/chromium/components/search_engines/prepopulated_engines_schema.json index c46435e37b6..9d6a797fb86 100644 --- a/chromium/components/search_engines/prepopulated_engines_schema.json +++ b/chromium/components/search_engines/prepopulated_engines_schema.json @@ -65,6 +65,10 @@ "default": "SEARCH_ENGINE_OTHER", "optional": true }, + // Whether a connection to search_url should regularly be established when + // the engine is set as the "default search engine". "ALLOWED" is the only + // value that will enable the pre-connections. + { "field": "preconnect_to_search_url", "type": "string", "optional": true }, // Unique id for this prepopulate engine (corresponds to // TemplateURL::prepopulate_id). This ID must be greater than zero and must // remain the same for a particular site regardless of how the url changes; diff --git a/chromium/components/search_engines/search_terms_data.cc b/chromium/components/search_engines/search_terms_data.cc index b5d6ed69b10..52eca7441d8 100644 --- a/chromium/components/search_engines/search_terms_data.cc +++ b/chromium/components/search_engines/search_terms_data.cc @@ -20,8 +20,12 @@ std::string SearchTermsData::GoogleBaseURLValue() const { std::string SearchTermsData::GoogleBaseSearchByImageURLValue() const { const std::string kGoogleHomepageURLPath = std::string("searchbyimage/"); + // If both LensStandalone and LensRegionSearch features are enabled, + // LensStandalone parameters will take precedence even if the values differ. if (base::FeatureList::IsEnabled(lens::features::kLensStandalone)) { - return lens::features::GetHomepageURL(); + return lens::features::GetHomepageURLForImageSearch(); + } else if (base::FeatureList::IsEnabled(lens::features::kLensRegionSearch)) { + return lens::features::GetHomepageURLForRegionSearch(); } return google_util::kGoogleHomepageURL + kGoogleHomepageURLPath; } @@ -54,7 +58,7 @@ std::string SearchTermsData::GetSearchClient() const { return std::string(); } -std::string SearchTermsData::GetSuggestClient(bool from_ntp) const { +std::string SearchTermsData::GetSuggestClient() const { return std::string(); } diff --git a/chromium/components/search_engines/search_terms_data.h b/chromium/components/search_engines/search_terms_data.h index fbb3520a7aa..7ac4858ccc6 100644 --- a/chromium/components/search_engines/search_terms_data.h +++ b/chromium/components/search_engines/search_terms_data.h @@ -44,9 +44,8 @@ class SearchTermsData { // The suggest client parameter ("client") passed with Google suggest // requests. See GetSuggestRequestIdentifier() for more details. - // |from_ntp| is true if the search is made from a non-searchbox NTP surface. // This implementation returns the empty string. - virtual std::string GetSuggestClient(bool from_ntp) const; + virtual std::string GetSuggestClient() const; // The suggest request identifier parameter ("gs_ri") passed with Google // suggest requests. Along with suggestclient (See GetSuggestClient()), diff --git a/chromium/components/search_engines/template_url.cc b/chromium/components/search_engines/template_url.cc index 528adf36251..4332a76585e 100644 --- a/chromium/components/search_engines/template_url.cc +++ b/chromium/components/search_engines/template_url.cc @@ -1150,7 +1150,7 @@ std::string TemplateURLRef::HandleReplacements( // prefetch to allow the search server to treat the requests based on // source. "cs" represents Chrome Suggestions as the source. Adding a // new source should be supported by the Search engine. - HandleReplacement(std::string(), "pf=cs&", *i, &url); + HandleReplacement("pf", "cs", *i, &url); } break; } @@ -1195,11 +1195,8 @@ std::string TemplateURLRef::HandleReplacements( } case GOOGLE_SUGGEST_CLIENT: - HandleReplacement( - std::string(), - search_terms_data.GetSuggestClient( - search_terms_args.request_source == NON_SEARCHBOX_NTP), - *i, &url); + HandleReplacement(std::string(), search_terms_data.GetSuggestClient(), + *i, &url); break; case GOOGLE_SUGGEST_REQUEST_ID: diff --git a/chromium/components/search_engines/template_url.h b/chromium/components/search_engines/template_url.h index 4c667b0c287..9c681d89a71 100644 --- a/chromium/components/search_engines/template_url.h +++ b/chromium/components/search_engines/template_url.h @@ -68,11 +68,11 @@ class TemplateURLRef { // the |post_data|. See http://tools.ietf.org/html/rfc2046 for the details. typedef std::pair<std::string, std::string> PostContent; - // Enumeration of the known search or suggest request sources. + // Enumeration of the known search or suggest request sources. These values + // are not persisted or used in histograms; thus can be freely changed. enum RequestSource { SEARCHBOX, // Omnibox or the NTP realbox. The default. CROS_APP_LIST, // Chrome OS app list search box. - NON_SEARCHBOX_NTP, // Non-searchbox NTP surfaces. }; // This struct encapsulates arguments passed to @@ -723,6 +723,8 @@ class TemplateURL { const std::string& sync_guid() const { return data_.sync_guid; } + TemplateURLData::ActiveStatus is_active() const { return data_.is_active; } + const std::vector<TemplateURLRef>& url_refs() const { return url_refs_; } const TemplateURLRef& url_ref() const { // Sanity check for https://crbug.com/781703. diff --git a/chromium/components/search_engines/template_url_data.cc b/chromium/components/search_engines/template_url_data.cc index 9e38747bf73..877ab23b9c6 100644 --- a/chromium/components/search_engines/template_url_data.cc +++ b/chromium/components/search_engines/template_url_data.cc @@ -45,6 +45,7 @@ TemplateURLData::TemplateURLData() usage_count(0), prepopulate_id(0), sync_guid(base::GenerateGUID()), + is_active(ActiveStatus::kTrue), keyword_(u"dummy"), url_("x") {} @@ -68,6 +69,7 @@ TemplateURLData::TemplateURLData(const std::u16string& name, base::StringPiece favicon_url, base::StringPiece encoding, const base::ListValue& alternate_urls_list, + bool preconnect_to_search_url, int prepopulate_id) : suggestions_url(suggest_url), image_url(image_url), @@ -87,7 +89,8 @@ TemplateURLData::TemplateURLData(const std::u16string& name, created_from_play_api(false), usage_count(0), prepopulate_id(prepopulate_id), - sync_guid(GenerateGUID(prepopulate_id)) { + sync_guid(GenerateGUID(prepopulate_id)), + preconnect_to_search_url(preconnect_to_search_url) { SetShortName(name); SetKeyword(keyword); SetURL(std::string(search_url)); diff --git a/chromium/components/search_engines/template_url_data.h b/chromium/components/search_engines/template_url_data.h index 352aec22a7e..cb7a5361518 100644 --- a/chromium/components/search_engines/template_url_data.h +++ b/chromium/components/search_engines/template_url_data.h @@ -45,6 +45,7 @@ struct TemplateURLData { base::StringPiece favicon_url, base::StringPiece encoding, const base::ListValue& alternate_urls_list, + bool preconnect_to_search_url, int prepopulate_id); ~TemplateURLData(); @@ -153,6 +154,21 @@ struct TemplateURLData { // search terms from a URL. std::vector<std::string> alternate_urls; + // Whether a connection to |url_| should regularly be established when this is + // set as the "default search engine". + bool preconnect_to_search_url = false; + + enum class ActiveStatus { + kUnspecified = 0, // The default value when a search engine is auto-added. + kTrue, // Search engine is active. + kFalse, // SE has been manually deactivated by a user. + }; + + // Whether this entry is "active". Active entries can be invoked by keyword + // via the omnibox. Inactive search engines do nothing until they have been + // activated. A search engine is inactive if it's unspecified or false. + ActiveStatus is_active{ActiveStatus::kUnspecified}; + private: // Private so we can enforce using the setters and thus enforce that these // fields are never empty. diff --git a/chromium/components/search_engines/template_url_data_unittest.cc b/chromium/components/search_engines/template_url_data_unittest.cc index 869a9c0d517..7ae19582278 100644 --- a/chromium/components/search_engines/template_url_data_unittest.cc +++ b/chromium/components/search_engines/template_url_data_unittest.cc @@ -15,7 +15,7 @@ TEST(TemplateURLDataTest, Trim) { base::StringPiece(), base::StringPiece(), base::StringPiece(), base::StringPiece(), base::StringPiece(), base::StringPiece(), base::StringPiece(), base::StringPiece(), base::StringPiece(), - base::StringPiece(), base::StringPiece(), base::ListValue(), 0); + base::StringPiece(), base::StringPiece(), base::ListValue(), false, 0); EXPECT_EQ(u"shortname", data.short_name()); EXPECT_EQ(u"keyword", data.keyword()); diff --git a/chromium/components/search_engines/template_url_data_util.cc b/chromium/components/search_engines/template_url_data_util.cc index 1f4dd5d70cd..840b0515eed 100644 --- a/chromium/components/search_engines/template_url_data_util.cc +++ b/chromium/components/search_engines/template_url_data_util.cc @@ -102,9 +102,8 @@ std::unique_ptr<TemplateURLData> TemplateURLDataFromDictionary( const base::ListValue* alternate_urls = nullptr; if (dict.GetList(DefaultSearchManager::kAlternateURLs, &alternate_urls)) { for (const auto& it : alternate_urls->GetList()) { - std::string alternate_url; - if (it.GetAsString(&alternate_url)) - result->alternate_urls.push_back(std::move(alternate_url)); + if (it.is_string()) + result->alternate_urls.push_back(it.GetString()); } } @@ -112,8 +111,8 @@ std::unique_ptr<TemplateURLData> TemplateURLDataFromDictionary( if (dict.GetList(DefaultSearchManager::kInputEncodings, &encodings)) { for (const auto& it : encodings->GetList()) { std::string encoding; - if (it.GetAsString(&encoding)) - result->input_encodings.push_back(std::move(encoding)); + if (it.is_string()) + result->input_encodings.push_back(it.GetString()); } } @@ -121,6 +120,8 @@ std::unique_ptr<TemplateURLData> TemplateURLDataFromDictionary( &result->created_by_policy); dict.GetBoolean(DefaultSearchManager::kCreatedFromPlayAPI, &result->created_from_play_api); + dict.GetBoolean(DefaultSearchManager::kPreconnectToSearchUrl, + &result->preconnect_to_search_url); return result; } @@ -169,22 +170,24 @@ std::unique_ptr<base::DictionaryValue> TemplateURLDataToDictionary( base::NumberToString(data.last_visited.ToInternalValue())); url_dict->SetInteger(DefaultSearchManager::kUsageCount, data.usage_count); - auto alternate_urls = std::make_unique<base::ListValue>(); + base::ListValue alternate_urls; for (const auto& alternate_url : data.alternate_urls) - alternate_urls->AppendString(alternate_url); + alternate_urls.AppendString(alternate_url); - url_dict->Set(DefaultSearchManager::kAlternateURLs, - std::move(alternate_urls)); + url_dict->SetKey(DefaultSearchManager::kAlternateURLs, + std::move(alternate_urls)); - auto encodings = std::make_unique<base::ListValue>(); + base::ListValue encodings; for (const auto& input_encoding : data.input_encodings) - encodings->AppendString(input_encoding); - url_dict->Set(DefaultSearchManager::kInputEncodings, std::move(encodings)); + encodings.AppendString(input_encoding); + url_dict->SetKey(DefaultSearchManager::kInputEncodings, std::move(encodings)); url_dict->SetBoolean(DefaultSearchManager::kCreatedByPolicy, data.created_by_policy); url_dict->SetBoolean(DefaultSearchManager::kCreatedFromPlayAPI, data.created_from_play_api); + url_dict->SetBoolean(DefaultSearchManager::kPreconnectToSearchUrl, + data.preconnect_to_search_url); return url_dict; } @@ -206,7 +209,8 @@ std::unique_ptr<TemplateURLData> TemplateURLDataFromPrepopulatedEngine( ToStringPiece(engine.suggest_url_post_params), ToStringPiece(engine.image_url_post_params), ToStringPiece(engine.favicon_url), ToStringPiece(engine.encoding), - alternate_urls, engine.id); + alternate_urls, + ToStringPiece(engine.preconnect_to_search_url) == "ALLOWED", engine.id); } std::unique_ptr<TemplateURLData> TemplateURLDataFromOverrideDictionary( @@ -234,6 +238,7 @@ std::unique_ptr<TemplateURLData> TemplateURLDataFromOverrideDictionary( std::string search_url_post_params; std::string suggest_url_post_params; std::string image_url_post_params; + std::string preconnect_to_search_url; base::ListValue empty_list; const base::ListValue* alternate_urls = &empty_list; engine.GetString("suggest_url", &suggest_url); @@ -246,11 +251,12 @@ std::unique_ptr<TemplateURLData> TemplateURLDataFromOverrideDictionary( engine.GetString("suggest_url_post_params", &suggest_url_post_params); engine.GetString("image_url_post_params", &image_url_post_params); engine.GetList("alternate_urls", &alternate_urls); + engine.GetString("preconnect_to_search_url", &preconnect_to_search_url); return std::make_unique<TemplateURLData>( name, keyword, search_url, suggest_url, image_url, new_tab_url, contextual_search_url, logo_url, doodle_url, search_url_post_params, suggest_url_post_params, image_url_post_params, favicon_url, encoding, - *alternate_urls, id); + *alternate_urls, preconnect_to_search_url.compare("ALLOWED") == 0, id); } return nullptr; } diff --git a/chromium/components/search_engines/template_url_fetcher.cc b/chromium/components/search_engines/template_url_fetcher.cc index 775366d5194..c234b44a582 100644 --- a/chromium/components/search_engines/template_url_fetcher.cc +++ b/chromium/components/search_engines/template_url_fetcher.cc @@ -216,6 +216,12 @@ void TemplateURLFetcher::RequestDelegate::AddSearchProvider() { // Mark the keyword as replaceable so it can be removed if necessary. // Add() will automatically remove conflicting keyword replaceable engines. data.safe_for_autoreplace = true; + + // Autogenerated keywords are kUnspecified active status by default. When the + // active search engine feature flag is enabled, kUnspecified keywords are + // inactive and cannot be triggered in the omnibox until they are activated. + data.is_active = TemplateURLData::ActiveStatus::kUnspecified; + model->Add(std::make_unique<TemplateURL>(data)); fetcher_->RequestCompleted(this); diff --git a/chromium/components/search_engines/template_url_parser.cc b/chromium/components/search_engines/template_url_parser.cc index dbf40a50d2e..cb12dbcd771 100644 --- a/chromium/components/search_engines/template_url_parser.cc +++ b/chromium/components/search_engines/template_url_parser.cc @@ -177,7 +177,7 @@ void SafeTemplateURLParser::OnXmlParseComplete( // to access nodes by tag name in GetChildElementsByTag(). if (const base::Value* namespaces = root.FindDictKey(data_decoder::mojom::XmlParser::kNamespacesKey)) { - for (const auto& item : namespaces->DictItems()) { + for (auto item : namespaces->DictItems()) { namespaces_.push_back(item.first); } } diff --git a/chromium/components/search_engines/template_url_prepopulate_data.cc b/chromium/components/search_engines/template_url_prepopulate_data.cc index 150b548b368..f7495f041b7 100644 --- a/chromium/components/search_engines/template_url_prepopulate_data.cc +++ b/chromium/components/search_engines/template_url_prepopulate_data.cc @@ -4,8 +4,8 @@ #include "components/search_engines/template_url_prepopulate_data.h" +#include "base/cxx17_backports.h" #include "base/logging.h" -#include "base/stl_util.h" #include "build/build_config.h" #include "components/country_codes/country_codes.h" #include "components/pref_registry/pref_registry_syncable.h" diff --git a/chromium/components/search_engines/template_url_prepopulate_data_unittest.cc b/chromium/components/search_engines/template_url_prepopulate_data_unittest.cc index dfd75f6f30f..7bdeed491ce 100644 --- a/chromium/components/search_engines/template_url_prepopulate_data_unittest.cc +++ b/chromium/components/search_engines/template_url_prepopulate_data_unittest.cc @@ -124,17 +124,18 @@ TEST_F(TemplateURLPrepopulateDataTest, UniqueIDs) { TEST_F(TemplateURLPrepopulateDataTest, ProvidersFromPrefs) { prefs_.SetUserPref(prefs::kSearchProviderOverridesVersion, std::make_unique<base::Value>(1)); - auto overrides = std::make_unique<base::ListValue>(); - std::unique_ptr<base::DictionaryValue> entry(new base::DictionaryValue); + base::Value overrides(base::Value::Type::LIST); + base::Value entry(base::Value::Type::DICTIONARY); // Set only the minimal required settings for a search provider configuration. - entry->SetString("name", "foo"); - entry->SetString("keyword", "fook"); - entry->SetString("search_url", "http://foo.com/s?q={searchTerms}"); - entry->SetString("favicon_url", "http://foi.com/favicon.ico"); - entry->SetString("encoding", "UTF-8"); - entry->SetInteger("id", 1001); - overrides->Append(entry->CreateDeepCopy()); - prefs_.SetUserPref(prefs::kSearchProviderOverrides, std::move(overrides)); + entry.SetStringKey("name", "foo"); + entry.SetStringKey("keyword", "fook"); + entry.SetStringKey("search_url", "http://foo.com/s?q={searchTerms}"); + entry.SetStringKey("favicon_url", "http://foi.com/favicon.ico"); + entry.SetStringKey("encoding", "UTF-8"); + entry.SetIntKey("id", 1001); + overrides.Append(entry.Clone()); + prefs_.SetUserPref(prefs::kSearchProviderOverrides, + base::Value::ToUniquePtrValue(std::move(overrides))); int version = TemplateURLPrepopulateData::GetDataVersion(&prefs_); EXPECT_EQ(1, version); @@ -158,13 +159,14 @@ TEST_F(TemplateURLPrepopulateDataTest, ProvidersFromPrefs) { EXPECT_TRUE(t_urls[0]->last_modified.is_null()); // Test the optional settings too. - entry->SetString("suggest_url", "http://foo.com/suggest?q={searchTerms}"); - auto alternate_urls = std::make_unique<base::ListValue>(); - alternate_urls->AppendString("http://foo.com/alternate?q={searchTerms}"); - entry->Set("alternate_urls", std::move(alternate_urls)); - overrides = std::make_unique<base::ListValue>(); - overrides->Append(entry->CreateDeepCopy()); - prefs_.SetUserPref(prefs::kSearchProviderOverrides, std::move(overrides)); + entry.SetStringKey("suggest_url", "http://foo.com/suggest?q={searchTerms}"); + base::Value alternate_urls(base::Value::Type::LIST); + alternate_urls.Append("http://foo.com/alternate?q={searchTerms}"); + entry.SetKey("alternate_urls", std::move(alternate_urls)); + overrides = base::Value(base::Value::Type::LIST); + overrides.Append(entry.Clone()); + prefs_.SetUserPref(prefs::kSearchProviderOverrides, + base::Value::ToUniquePtrValue(std::move(overrides))); t_urls = TemplateURLPrepopulateData::GetPrepopulatedEngines( &prefs_, &default_index); @@ -183,19 +185,20 @@ TEST_F(TemplateURLPrepopulateDataTest, ProvidersFromPrefs) { // Test that subsequent providers are loaded even if an intermediate // provider has an incomplete configuration. - overrides = std::make_unique<base::ListValue>(); - overrides->Append(entry->CreateDeepCopy()); - entry->SetInteger("id", 1002); - entry->SetString("name", "bar"); - entry->SetString("keyword", "bark"); - entry->SetString("encoding", std::string()); - overrides->Append(entry->CreateDeepCopy()); - entry->SetInteger("id", 1003); - entry->SetString("name", "baz"); - entry->SetString("keyword", "bazk"); - entry->SetString("encoding", "UTF-8"); - overrides->Append(entry->CreateDeepCopy()); - prefs_.SetUserPref(prefs::kSearchProviderOverrides, std::move(overrides)); + overrides = base::Value(base::Value::Type::LIST); + overrides.Append(entry.Clone()); + entry.SetIntKey("id", 1002); + entry.SetStringKey("name", "bar"); + entry.SetStringKey("keyword", "bark"); + entry.SetStringKey("encoding", std::string()); + overrides.Append(entry.Clone()); + entry.SetIntKey("id", 1003); + entry.SetStringKey("name", "baz"); + entry.SetStringKey("keyword", "bazk"); + entry.SetStringKey("encoding", "UTF-8"); + overrides.Append(entry.Clone()); + prefs_.SetUserPref(prefs::kSearchProviderOverrides, + base::Value::ToUniquePtrValue(std::move(overrides))); t_urls = TemplateURLPrepopulateData::GetPrepopulatedEngines(&prefs_, @@ -206,17 +209,18 @@ TEST_F(TemplateURLPrepopulateDataTest, ProvidersFromPrefs) { TEST_F(TemplateURLPrepopulateDataTest, ClearProvidersFromPrefs) { prefs_.SetUserPref(prefs::kSearchProviderOverridesVersion, std::make_unique<base::Value>(1)); - auto overrides = std::make_unique<base::ListValue>(); - std::unique_ptr<base::DictionaryValue> entry(new base::DictionaryValue); + base::Value overrides(base::Value::Type::LIST); + base::Value entry(base::Value::Type::DICTIONARY); // Set only the minimal required settings for a search provider configuration. - entry->SetString("name", "foo"); - entry->SetString("keyword", "fook"); - entry->SetString("search_url", "http://foo.com/s?q={searchTerms}"); - entry->SetString("favicon_url", "http://foi.com/favicon.ico"); - entry->SetString("encoding", "UTF-8"); - entry->SetInteger("id", 1001); - overrides->Append(std::move(entry)); - prefs_.SetUserPref(prefs::kSearchProviderOverrides, std::move(overrides)); + entry.SetStringKey("name", "foo"); + entry.SetStringKey("keyword", "fook"); + entry.SetStringKey("search_url", "http://foo.com/s?q={searchTerms}"); + entry.SetStringKey("favicon_url", "http://foi.com/favicon.ico"); + entry.SetStringKey("encoding", "UTF-8"); + entry.SetIntKey("id", 1001); + overrides.Append(std::move(entry)); + prefs_.SetUserPref(prefs::kSearchProviderOverrides, + base::Value::ToUniquePtrValue(std::move(overrides))); int version = TemplateURLPrepopulateData::GetDataVersion(&prefs_); EXPECT_EQ(1, version); diff --git a/chromium/components/search_engines/template_url_service.cc b/chromium/components/search_engines/template_url_service.cc index 93866e85c6f..9f592cce67e 100644 --- a/chromium/components/search_engines/template_url_service.cc +++ b/chromium/components/search_engines/template_url_service.cc @@ -32,8 +32,8 @@ #include "components/sync/model/sync_change.h" #include "components/sync/model/sync_change_processor.h" #include "components/sync/model/sync_error_factory.h" +#include "components/sync/protocol/entity_specifics.pb.h" #include "components/sync/protocol/search_engine_specifics.pb.h" -#include "components/sync/protocol/sync.pb.h" #include "components/url_formatter/url_fixer.h" #include "components/variations/variations_associated_data.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" @@ -646,6 +646,8 @@ void TemplateURLService::ResetTemplateURL(TemplateURL* url, } data.safe_for_autoreplace = false; data.last_modified = clock_->Now(); + data.is_active = TemplateURLData::ActiveStatus::kTrue; + Update(url, TemplateURL(data)); } @@ -674,6 +676,7 @@ TemplateURL* TemplateURLService::CreatePlayAPISearchEngine( // Play API engines are created by explicit user gesture, and should not be // auto-replaceable by an auto-generated engine as the user browses. data.safe_for_autoreplace = false; + data.is_active = TemplateURLData::ActiveStatus::kTrue; // The Play API search engine is not guaranteed to be the best engine for // |keyword|, if there are user-defined, extension, or policy engines. @@ -908,6 +911,7 @@ void TemplateURLService::OnWebDataServiceRequestDone( { PatchMissingSyncGUIDs(template_urls.get()); + MaybeSetIsActiveSearchEngines(template_urls.get()); SetTemplateURLs(std::move(template_urls)); // This initializes provider_map_ which should be done before @@ -1299,6 +1303,39 @@ void TemplateURLService::ClearSessionToken() { } // static +TemplateURLData::ActiveStatus TemplateURLService::ActiveStatusFromSync( + sync_pb::SearchEngineSpecifics_ActiveStatus is_active) { + switch (is_active) { + case sync_pb::SearchEngineSpecifics_ActiveStatus:: + SearchEngineSpecifics_ActiveStatus_ACTIVE_STATUS_UNSPECIFIED: + return TemplateURLData::ActiveStatus::kUnspecified; + case sync_pb::SearchEngineSpecifics_ActiveStatus:: + SearchEngineSpecifics_ActiveStatus_ACTIVE_STATUS_TRUE: + return TemplateURLData::ActiveStatus::kTrue; + case sync_pb::SearchEngineSpecifics_ActiveStatus:: + SearchEngineSpecifics_ActiveStatus_ACTIVE_STATUS_FALSE: + return TemplateURLData::ActiveStatus::kFalse; + } +} + +// static +sync_pb::SearchEngineSpecifics_ActiveStatus +TemplateURLService::ActiveStatusToSync( + TemplateURLData::ActiveStatus is_active) { + switch (is_active) { + case TemplateURLData::ActiveStatus::kUnspecified: + return sync_pb::SearchEngineSpecifics_ActiveStatus:: + SearchEngineSpecifics_ActiveStatus_ACTIVE_STATUS_UNSPECIFIED; + case TemplateURLData::ActiveStatus::kTrue: + return sync_pb::SearchEngineSpecifics_ActiveStatus:: + SearchEngineSpecifics_ActiveStatus_ACTIVE_STATUS_TRUE; + case TemplateURLData::ActiveStatus::kFalse: + return sync_pb::SearchEngineSpecifics_ActiveStatus:: + SearchEngineSpecifics_ActiveStatus_ACTIVE_STATUS_FALSE; + } +} + +// static syncer::SyncData TemplateURLService::CreateSyncDataFromTemplateURL( const TemplateURL& turl) { sync_pb::EntitySpecifics specifics; @@ -1330,6 +1367,7 @@ syncer::SyncData TemplateURLService::CreateSyncDataFromTemplateURL( se_specifics->set_sync_guid(turl.sync_guid()); for (size_t i = 0; i < turl.alternate_urls().size(); ++i) se_specifics->add_alternate_urls(turl.alternate_urls()[i]); + se_specifics->set_is_active(ActiveStatusToSync(turl.is_active())); return syncer::SyncData::CreateLocalData(se_specifics->sync_guid(), se_specifics->keyword(), @@ -1401,6 +1439,7 @@ TemplateURLService::CreateTemplateURLFromTemplateURLAndSyncData( data.alternate_urls.clear(); for (int i = 0; i < specifics.alternate_urls_size(); ++i) data.alternate_urls.push_back(specifics.alternate_urls(i)); + data.is_active = ActiveStatusFromSync(specifics.is_active()); std::unique_ptr<TemplateURL> turl(new TemplateURL(data)); // If this TemplateURL matches a built-in prepopulated template URL, it's @@ -2114,6 +2153,22 @@ void TemplateURLService::OnSyncedDefaultSearchProviderGUIDChanged() { default_search_manager_.SetUserSelectedDefaultSearchEngine(turl->data()); } +void TemplateURLService::MaybeSetIsActiveSearchEngines( + OwnedTemplateURLVector* template_urls) { + DCHECK(template_urls); + for (auto& turl : *template_urls) { + DCHECK(turl); + // An turl is "active" if it has ever been used or manually added/modified. + // |safe_for_autoreplace| is false if the entry has been modified. + if (turl->is_active() == TemplateURLData::ActiveStatus::kUnspecified && + (!turl->safe_for_autoreplace() || turl->usage_count() > 0)) { + turl->data_.is_active = TemplateURLData::ActiveStatus::kTrue; + if (web_data_service_) + web_data_service_->UpdateKeyword(turl->data()); + } + } +} + template <typename Container> void TemplateURLService::AddMatchingKeywordsHelper( const Container& keyword_to_turl_and_length, diff --git a/chromium/components/search_engines/template_url_service.h b/chromium/components/search_engines/template_url_service.h index 7c4a49d74ab..2bd3101c835 100644 --- a/chromium/components/search_engines/template_url_service.h +++ b/chromium/components/search_engines/template_url_service.h @@ -31,6 +31,7 @@ #include "components/search_engines/template_url.h" #include "components/sync/model/sync_change.h" #include "components/sync/model/syncable_service.h" +#include "components/sync/protocol/search_engine_specifics.pb.h" #include "components/webdata/common/web_data_service_consumer.h" #if defined(OS_ANDROID) #include "base/android/scoped_java_ref.h" @@ -410,6 +411,16 @@ class TemplateURLService : public WebDataServiceConsumer, // data. void ClearSessionToken(); + // Explicitly converts from ActiveStatus enum in sync protos to enum in + // TemplateURLData. + static TemplateURLData::ActiveStatus ActiveStatusFromSync( + sync_pb::SearchEngineSpecifics_ActiveStatus is_active); + + // Explicitly converts from ActiveStatus enum in TemplateURLData to enum in + // sync protos. + static sync_pb::SearchEngineSpecifics_ActiveStatus ActiveStatusToSync( + TemplateURLData::ActiveStatus is_active); + // Returns a SyncData with a sync representation of the search engine data // from |turl|. static syncer::SyncData CreateSyncDataFromTemplateURL( @@ -636,6 +647,11 @@ class TemplateURLService : public WebDataServiceConsumer, void OnSyncedDefaultSearchProviderGUIDChanged(); + // Goes through a vector of TemplateURLs and sets is_active to true if it was + // not previously set (currently kUnspecified) and has been interacted with + // by the user. + void MaybeSetIsActiveSearchEngines(OwnedTemplateURLVector* template_urls); + // Adds to |matches| all TemplateURLs stored in |keyword_to_turl_and_length| // whose keywords begin with |prefix|, sorted shortest-keyword-first. If // |supports_replacement_only| is true, only TemplateURLs that support diff --git a/chromium/components/search_engines/template_url_service_util_unittest.cc b/chromium/components/search_engines/template_url_service_util_unittest.cc index d18f7680b4f..e8e822efe1d 100644 --- a/chromium/components/search_engines/template_url_service_util_unittest.cc +++ b/chromium/components/search_engines/template_url_service_util_unittest.cc @@ -26,7 +26,8 @@ std::unique_ptr<TemplateURLData> CreatePrepopulateTemplateURLData( "" /* contextual_search_url */, "" /* logo_url */, "" /* doodle_url */, "" /* search_url_post_params */, "" /* suggest_url_post_params */, "" /* image_url_post_params */, "" /* favicon_url */, "UTF-8", - base::ListValue() /* alternate_urls_list */, prepopulate_id); + base::ListValue() /* alternate_urls_list */, + false /* preconnect_to_search_url */, prepopulate_id); } // Creates a TemplateURL with default values except for the prepopulate ID, diff --git a/chromium/components/search_engines/template_url_unittest.cc b/chromium/components/search_engines/template_url_unittest.cc index 2cbb292a5bf..cf14fd08b8b 100644 --- a/chromium/components/search_engines/template_url_unittest.cc +++ b/chromium/components/search_engines/template_url_unittest.cc @@ -7,8 +7,8 @@ #include "base/base64.h" #include "base/base_paths.h" #include "base/command_line.h" +#include "base/cxx17_backports.h" #include "base/i18n/case_conversion.h" -#include "base/stl_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" @@ -1150,15 +1150,6 @@ TEST_F(TemplateURLTest, SuggestClient) { url.url_ref().ReplaceSearchTerms(search_terms_args, search_terms_data_)); ASSERT_TRUE(result_2.is_valid()); EXPECT_EQ("http://google.com/?client=suggest_client", result_2.spec()); - - // Check that the URL is correct when a suggest request is made from a - // non-searchbox NTP surface. - search_terms_args.request_source = TemplateURLRef::NON_SEARCHBOX_NTP; - GURL result_3( - url.url_ref().ReplaceSearchTerms(search_terms_args, search_terms_data_)); - ASSERT_TRUE(result_3.is_valid()); - EXPECT_EQ("http://google.com/?client=suggest_client_from_ntp", - result_3.spec()); } TEST_F(TemplateURLTest, GetURLNoSuggestionsURL) { diff --git a/chromium/components/search_engines/testing_search_terms_data.cc b/chromium/components/search_engines/testing_search_terms_data.cc index 91002b4fa0e..3ebf3e13e12 100644 --- a/chromium/components/search_engines/testing_search_terms_data.cc +++ b/chromium/components/search_engines/testing_search_terms_data.cc @@ -27,8 +27,8 @@ std::string TestingSearchTermsData::GetSearchClient() const { return search_client_; } -std::string TestingSearchTermsData::GetSuggestClient(bool from_ntp) const { - return from_ntp ? suggest_client_ + "_from_ntp" : suggest_client_; +std::string TestingSearchTermsData::GetSuggestClient() const { + return suggest_client_; } std::string TestingSearchTermsData::GoogleImageSearchSource() const { diff --git a/chromium/components/search_engines/testing_search_terms_data.h b/chromium/components/search_engines/testing_search_terms_data.h index fa3895e4af1..9563cef4be8 100644 --- a/chromium/components/search_engines/testing_search_terms_data.h +++ b/chromium/components/search_engines/testing_search_terms_data.h @@ -16,7 +16,7 @@ class TestingSearchTermsData : public SearchTermsData { std::string GoogleBaseURLValue() const override; std::u16string GetRlzParameterValue(bool from_app_list) const override; std::string GetSearchClient() const override; - std::string GetSuggestClient(bool from_ntp) const override; + std::string GetSuggestClient() const override; std::string GoogleImageSearchSource() const override; // Estimates dynamic memory usage. |