diff options
Diffstat (limited to 'chromium/components/search_engines')
17 files changed, 112 insertions, 565 deletions
diff --git a/chromium/components/search_engines/BUILD.gn b/chromium/components/search_engines/BUILD.gn index 09343edebb7..b4779bccdf1 100644 --- a/chromium/components/search_engines/BUILD.gn +++ b/chromium/components/search_engines/BUILD.gn @@ -11,10 +11,6 @@ static_library("search_engines") { "default_search_manager.h", "default_search_pref_migration.cc", "default_search_pref_migration.h", - "desktop_search_redirection_infobar_delegate.cc", - "desktop_search_redirection_infobar_delegate.h", - "desktop_search_utils.cc", - "desktop_search_utils.h", "keyword_table.cc", "keyword_table.h", "keyword_web_data_service.cc", @@ -119,7 +115,6 @@ source_set("unit_tests") { sources = [ "default_search_manager_unittest.cc", "default_search_pref_migration_unittest.cc", - "desktop_search_utils_unittest.cc", "keyword_table_unittest.cc", "search_engine_data_type_controller_unittest.cc", "search_host_to_urls_map_unittest.cc", diff --git a/chromium/components/search_engines/default_search_pref_test_util.cc b/chromium/components/search_engines/default_search_pref_test_util.cc index 3fdf4807725..193ab8f1452 100644 --- a/chromium/components/search_engines/default_search_pref_test_util.cc +++ b/chromium/components/search_engines/default_search_pref_test_util.cc @@ -49,7 +49,7 @@ DefaultSearchPrefTestUtil::CreateDefaultSearchPreferenceValue( std::unique_ptr<base::ListValue> alternate_url_list(new base::ListValue()); if (!alternate_url.empty()) - alternate_url_list->Append(new base::StringValue(alternate_url)); + alternate_url_list->AppendString(alternate_url); value->Set(DefaultSearchManager::kAlternateURLs, alternate_url_list.release()); return value; diff --git a/chromium/components/search_engines/desktop_search_redirection_infobar_delegate.cc b/chromium/components/search_engines/desktop_search_redirection_infobar_delegate.cc deleted file mode 100644 index d1a44280b86..00000000000 --- a/chromium/components/search_engines/desktop_search_redirection_infobar_delegate.cc +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "components/search_engines/desktop_search_redirection_infobar_delegate.h" - -#include <memory> -#include <vector> - -#include "base/logging.h" -#include "base/metrics/histogram_macros.h" -#include "base/metrics/user_metrics.h" -#include "components/infobars/core/infobar.h" -#include "components/infobars/core/infobar_delegate.h" -#include "components/infobars/core/infobar_manager.h" -#include "components/prefs/pref_service.h" -#include "components/search_engines/desktop_search_utils.h" -#include "grit/components_strings.h" -#include "ui/base/l10n/l10n_util.h" -#include "ui/base/window_open_disposition.h" - -namespace { - -// Values for the Search.DesktopSearch.RedirectionInfobarCloseAction histogram. -enum DesktopSearchRedirectionInfobarCloseAction { - DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_MANAGE_SEARCH_SETTINGS = 0, - DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_DISMISS = 1, - DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_IGNORE = 2, - DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_MAX -}; - -void RecordDesktopSearchInfobarCloseActionHistogram( - DesktopSearchRedirectionInfobarCloseAction action) { - DCHECK_LT(action, DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_MAX); - UMA_HISTOGRAM_ENUMERATION( - "Search.DesktopSearch.RedirectionInfobarCloseAction", action, - DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_MAX); -} - -} // namespace - -void DesktopSearchRedirectionInfobarDelegate::Show( - infobars::InfoBarManager* infobar_manager, - const base::string16& default_search_engine_name, - const base::Closure& manage_search_settings_callback, - PrefService* pref_service) { - DCHECK(infobar_manager); - infobar_manager->AddInfoBar(infobar_manager->CreateConfirmInfoBar( - std::unique_ptr<ConfirmInfoBarDelegate>( - new DesktopSearchRedirectionInfobarDelegate( - default_search_engine_name, manage_search_settings_callback)))); - pref_service->SetBoolean(prefs::kDesktopSearchRedirectionInfobarShownPref, - true); - base::RecordAction( - base::UserMetricsAction("DesktopSearchRedirectionInfoBar_Shown")); -} - -DesktopSearchRedirectionInfobarDelegate:: - DesktopSearchRedirectionInfobarDelegate( - const base::string16& default_search_engine_name, - const base::Closure& manage_search_settings_callback) - : default_search_engine_name_(default_search_engine_name), - manage_search_settings_callback_(manage_search_settings_callback), - closed_by_user_(false) {} - -DesktopSearchRedirectionInfobarDelegate:: - ~DesktopSearchRedirectionInfobarDelegate() { - if (!closed_by_user_) { - base::RecordAction( - base::UserMetricsAction("DesktopSearchRedirectionInfoBar_Ignore")); - RecordDesktopSearchInfobarCloseActionHistogram( - DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_IGNORE); - } -} - -infobars::InfoBarDelegate::InfoBarIdentifier -DesktopSearchRedirectionInfobarDelegate::GetIdentifier() const { - return DESKTOP_SEARCH_REDIRECTION_INFOBAR_DELEGATE; -} - -void DesktopSearchRedirectionInfobarDelegate::InfoBarDismissed() { - base::RecordAction( - base::UserMetricsAction("DesktopSearchRedirectionInfoBar_Dismiss")); - RecordDesktopSearchInfobarCloseActionHistogram( - DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_DISMISS); - closed_by_user_ = true; -} - -base::string16 DesktopSearchRedirectionInfobarDelegate::GetMessageText() const { - return l10n_util::GetStringFUTF16( - IDS_DESKTOP_SEARCH_REDIRECTION_INFOBAR_MESSAGE, - default_search_engine_name_); -} - -int DesktopSearchRedirectionInfobarDelegate::GetButtons() const { - return BUTTON_OK; -} - -base::string16 DesktopSearchRedirectionInfobarDelegate::GetButtonLabel( - InfoBarButton button) const { - return l10n_util::GetStringUTF16( - IDS_DESKTOP_SEARCH_REDIRECTION_INFOBAR_BUTTON); -} - -bool DesktopSearchRedirectionInfobarDelegate::Accept() { - base::RecordAction(base::UserMetricsAction( - "DesktopSearchRedirectionInfoBar_ManageSearchSettings")); - manage_search_settings_callback_.Run(); - - // Close the infobar. - RecordDesktopSearchInfobarCloseActionHistogram( - DESKTOP_SEARCH_REDIRECTION_INFOBAR_CLOSE_ACTION_MANAGE_SEARCH_SETTINGS); - closed_by_user_ = true; - return true; -} diff --git a/chromium/components/search_engines/desktop_search_redirection_infobar_delegate.h b/chromium/components/search_engines/desktop_search_redirection_infobar_delegate.h deleted file mode 100644 index bccb77a7608..00000000000 --- a/chromium/components/search_engines/desktop_search_redirection_infobar_delegate.h +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_SEARCH_ENGINES_DESKTOP_SEARCH_INFOBAR_DELEGATE_H_ -#define COMPONENTS_SEARCH_ENGINES_DESKTOP_SEARCH_INFOBAR_DELEGATE_H_ - -#include "base/callback.h" -#include "base/macros.h" -#include "base/strings/string16.h" -#include "components/infobars/core/confirm_infobar_delegate.h" - -class PrefService; - -namespace infobars { -class InfoBarManager; -} // namespace infobars - -// Informs the user that a desktop search has been redirected to the default -// search engine. -class DesktopSearchRedirectionInfobarDelegate : public ConfirmInfoBarDelegate { - public: - // Creates a search redirection infobar and delegate and adds the infobar to - // |infobar_manager|. Records in |pref_service| that such an infobar was - // shown. |default_search_engine_name| is the name of the default search - // engine. |manage_search_settings_callback| should open the search settings - // page. - static void Show(infobars::InfoBarManager* infobar_manager, - const base::string16& default_search_engine_name, - const base::Closure& manage_search_settings_callback, - PrefService* pref_service); - - private: - DesktopSearchRedirectionInfobarDelegate( - const base::string16& default_search_engine_name, - const base::Closure& manage_search_settings_callback); - ~DesktopSearchRedirectionInfobarDelegate() override; - - // ConfirmInfoBarDelegate: - infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override; - void InfoBarDismissed() override; - base::string16 GetMessageText() const override; - int GetButtons() const override; - base::string16 GetButtonLabel(InfoBarButton button) const override; - bool Accept() override; - - base::string16 default_search_engine_name_; - base::Closure manage_search_settings_callback_; - - // True when the infobar has been closed explicitly by the user. - bool closed_by_user_; - - DISALLOW_COPY_AND_ASSIGN(DesktopSearchRedirectionInfobarDelegate); -}; - -#endif // COMPONENTS_SEARCH_ENGINES_DESKTOP_SEARCH_INFOBAR_DELEGATE_H_ diff --git a/chromium/components/search_engines/desktop_search_utils.cc b/chromium/components/search_engines/desktop_search_utils.cc deleted file mode 100644 index e42c35e2f11..00000000000 --- a/chromium/components/search_engines/desktop_search_utils.cc +++ /dev/null @@ -1,138 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "components/search_engines/desktop_search_utils.h" - -#include <memory> -#include <string> - -#include "base/metrics/histogram_macros.h" -#include "base/metrics/user_metrics.h" -#include "base/strings/string_util.h" -#include "components/pref_registry/pref_registry_syncable.h" -#include "components/prefs/pref_service.h" -#include "components/search_engines/prepopulated_engines.h" -#include "components/search_engines/template_url_prepopulate_data.h" -#include "components/search_engines/template_url_service.h" -#include "components/search_engines/util.h" -#include "net/base/url_util.h" - -namespace { - -// Values for the Search.DesktopSearch.URLAction histogram. -enum DesktopSearchURLAction { - DESKTOP_SEARCH_URL_ACTION_NO_REDIRECTION_FEATURE_DISABLED = 0, - DESKTOP_SEARCH_URL_ACTION_NO_REDIRECTION_DEFAULT_SEARCH_IS_BING = 1, - DESKTOP_SEARCH_URL_ACTION_NO_REDIRECTION_INVALID_SEARCH_ENGINE = 2, - DESKTOP_SEARCH_URL_ACTION_REDIRECTION = 3, - DESKTOP_SEARCH_URL_ACTION_MAX -}; - -void RecordDesktopSearchURLAction(DesktopSearchURLAction action) { - DCHECK_LT(action, DESKTOP_SEARCH_URL_ACTION_MAX); - UMA_HISTOGRAM_ENUMERATION("Search.DesktopSearch.URLAction", action, - DESKTOP_SEARCH_URL_ACTION_MAX); -} - -// Detects whether a |url| comes from a desktop search. If so, puts the search -// terms in |search_terms| and returns true. -bool DetectDesktopSearch(const GURL& url, - const SearchTermsData& search_terms_data, - base::string16* search_terms) { - DCHECK(search_terms); - search_terms->clear(); - - std::unique_ptr<TemplateURLData> template_url_data = - TemplateURLPrepopulateData::MakeTemplateURLDataFromPrepopulatedEngine( - TemplateURLPrepopulateData::bing); - TemplateURL template_url(*template_url_data); - if (!template_url.ExtractSearchTermsFromURL(url, search_terms_data, - search_terms)) - return false; - - // Query parameter that tells the source of a Bing search URL, and values - // associated with desktop search. - const char kBingSourceQueryKey[] = "form"; - const char kBingSourceDesktopText[] = "WNSGPH"; - const char kBingSourceDesktopVoice[] = "WNSBOX"; - for (net::QueryIterator it(url); !it.IsAtEnd(); it.Advance()) { - // Use a case-insensitive comparison because the key is sometimes in capital - // letters. - if (base::EqualsCaseInsensitiveASCII(it.GetKey(), kBingSourceQueryKey)) { - const std::string source = it.GetValue(); - if (source == kBingSourceDesktopText || source == kBingSourceDesktopVoice) - return true; - } - } - - return false; -} - -} // namespace - -namespace prefs { -const char kDesktopSearchRedirectionInfobarShownPref[] = - "desktop_search_redirection_infobar_shown"; -} // namespace prefs - -const base::Feature kDesktopSearchRedirectionFeature{ - "DesktopSearchRedirection", base::FEATURE_DISABLED_BY_DEFAULT}; - -void RegisterDesktopSearchRedirectionPref( - user_prefs::PrefRegistrySyncable* registry) { - registry->RegisterBooleanPref( - prefs::kDesktopSearchRedirectionInfobarShownPref, false); -} - -bool ReplaceDesktopSearchURLWithDefaultSearchURLIfNeeded( - const PrefService* pref_service, - TemplateURLService* template_url_service, - GURL* url) { - DCHECK(pref_service); - DCHECK(template_url_service); - DCHECK(url); - - // Check if |url| is a desktop search. - base::string16 search_terms; - if (!DetectDesktopSearch(*url, template_url_service->search_terms_data(), - &search_terms)) - return false; - - // Record that the user searched from the desktop. - base::RecordAction(base::UserMetricsAction("DesktopSearch")); - - // Check if the redirection feature is enabled. - if (!base::FeatureList::IsEnabled(kDesktopSearchRedirectionFeature)) { - RecordDesktopSearchURLAction( - DESKTOP_SEARCH_URL_ACTION_NO_REDIRECTION_FEATURE_DISABLED); - return false; - } - - // Check if the default search engine is Bing. - const TemplateURL* default_search_engine = - template_url_service->GetDefaultSearchProvider(); - if (default_search_engine && - TemplateURLPrepopulateData::GetEngineType( - *default_search_engine, template_url_service->search_terms_data()) == - SEARCH_ENGINE_BING) { - RecordDesktopSearchURLAction( - DESKTOP_SEARCH_URL_ACTION_NO_REDIRECTION_DEFAULT_SEARCH_IS_BING); - return false; - } - - // Replace |url| by a default search engine URL. - GURL search_url( - GetDefaultSearchURLForSearchTerms(template_url_service, search_terms)); - if (!search_url.is_valid()) { - RecordDesktopSearchURLAction( - DESKTOP_SEARCH_URL_ACTION_NO_REDIRECTION_INVALID_SEARCH_ENGINE); - return false; - } - - RecordDesktopSearchURLAction(DESKTOP_SEARCH_URL_ACTION_REDIRECTION); - - url->Swap(&search_url); - return !pref_service->GetBoolean( - prefs::kDesktopSearchRedirectionInfobarShownPref); -} diff --git a/chromium/components/search_engines/desktop_search_utils.h b/chromium/components/search_engines/desktop_search_utils.h deleted file mode 100644 index 773ff7f56b8..00000000000 --- a/chromium/components/search_engines/desktop_search_utils.h +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_SEARCH_ENGINES_DESKTOP_SEARCH_UTILS_H_ -#define COMPONENTS_SEARCH_ENGINES_DESKTOP_SEARCH_UTILS_H_ - -#include "base/feature_list.h" -#include "base/strings/string16.h" -#include "url/gurl.h" - -class PrefService; -class TemplateURLService; - -namespace user_prefs { -class PrefRegistrySyncable; -} - -namespace prefs { -// Name of the preference keeping track of whether the desktop search -// redirection infobar has been shown. -extern const char kDesktopSearchRedirectionInfobarShownPref[]; -} - -// Desktop search redirection feature. This is exposed in the header file so -// that it can be referenced from about_flags.cc. -extern const base::Feature kDesktopSearchRedirectionFeature; - -// Registers the preference keeping track of whether the desktop search -// redirection infobar has been shown. -void RegisterDesktopSearchRedirectionPref( - user_prefs::PrefRegistrySyncable* registry); - -// Replaces |url| by a default search engine URL if: -// - |url| is a desktop search URL. -// - The desktop search redirection feature is enabled. -// - The default search engine is not Bing. -// Returns true if an infobar should be shown to tell the user about the -// redirection. -bool ReplaceDesktopSearchURLWithDefaultSearchURLIfNeeded( - const PrefService* pref_service, - TemplateURLService* template_url_service, - GURL* url); - -#endif // COMPONENTS_SEARCH_ENGINES_DESKTOP_SEARCH_UTILS_H_ diff --git a/chromium/components/search_engines/desktop_search_utils_unittest.cc b/chromium/components/search_engines/desktop_search_utils_unittest.cc deleted file mode 100644 index 56fa841e0b7..00000000000 --- a/chromium/components/search_engines/desktop_search_utils_unittest.cc +++ /dev/null @@ -1,151 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "components/search_engines/desktop_search_utils.h" - -#include <memory> -#include <string> - -#include "base/feature_list.h" -#include "base/macros.h" -#include "base/strings/string16.h" -#include "base/strings/utf_string_conversions.h" -#include "components/prefs/pref_service.h" -#include "components/search_engines/prepopulated_engines.h" -#include "components/search_engines/template_url.h" -#include "components/search_engines/template_url_prepopulate_data.h" -#include "components/search_engines/template_url_service.h" -#include "components/search_engines/util.h" -#include "components/syncable_prefs/testing_pref_service_syncable.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "url/gurl.h" - -namespace { - -struct ReplaceDesktopSearchURLTestData { - // Value of the |url| argument of - // ReplaceDesktopSearchURLWithDefaultSearchURLIfNeeded on input. - const char* input_arg; - - // Expected value of the |url| argument of - // ReplaceDesktopSearchURLWithDefaultSearchURLIfNeeded on output. - const char* expected_output_arg; -}; - -struct ShouldReplaceDesktopSearchURLTestData { - bool feature_enabled; - bool default_search_engine_is_bing; - const GURL* expected_output_arg; -}; - -} // namespace - -class DesktopSearchUtilsTest : public testing::Test { - public: - DesktopSearchUtilsTest() : template_url_service_(nullptr, 0) { - RegisterDesktopSearchRedirectionPref(prefs_.registry()); - } - - protected: - void SetFeatureEnabled(bool enabled) { - base::FeatureList::ClearInstanceForTesting(); - std::unique_ptr<base::FeatureList> feature_list(new base::FeatureList); - if (enabled) { - feature_list->InitializeFromCommandLine( - kDesktopSearchRedirectionFeature.name, std::string()); - } - base::FeatureList::SetInstance(std::move(feature_list)); - } - - void SetDefaultSearchEngine( - const TemplateURLPrepopulateData::PrepopulatedEngine - default_search_engine) { - std::unique_ptr<TemplateURLData> template_url_data = - TemplateURLPrepopulateData::MakeTemplateURLDataFromPrepopulatedEngine( - default_search_engine); - TemplateURL template_url(*template_url_data); - template_url_service_.SetUserSelectedDefaultSearchProvider(&template_url); - } - - TemplateURLService template_url_service_; - syncable_prefs::TestingPrefServiceSyncable prefs_; - - private: - DISALLOW_COPY_AND_ASSIGN(DesktopSearchUtilsTest); -}; - -// Checks that ReplaceDesktopSearchURLWithDefaultSearchURLIfNeeded correctly -// replaces the URL it receives when the desktop search redirection feature is -// enabled and the default search engine is not Bing. -TEST_F(DesktopSearchUtilsTest, ReplaceDesktopSearchURL) { - const std::string default_search_url_keyword( - GetDefaultSearchURLForSearchTerms(&template_url_service_, - base::WideToUTF16(L"keyword")) - .spec()); - const std::string default_search_url_special_chars( - GetDefaultSearchURLForSearchTerms(&template_url_service_, - base::WideToUTF16(L"\xE8 \xE9")) - .spec()); - - const ReplaceDesktopSearchURLTestData test_data[] = { - {"https://www.google.com/", "https://www.google.com/"}, - {"https://www.bing.com/search/", "https://www.bing.com/search/"}, - {"https://www.bing.com/search?q=keyword&form=QBLH", - "https://www.bing.com/search?q=keyword&form=QBLH"}, - {"https://www.bing.com/search?q=keyword&form=WNSGPH", - default_search_url_keyword.c_str()}, - {"https://www.bing.com/search?q=keyword&form=WNSBOX", - default_search_url_keyword.c_str()}, - {"https://www.bing.com/search?q=keyword&FORM=WNSGPH", - default_search_url_keyword.c_str()}, - {"https://www.bing.com/search?q=keyword&FORM=WNSBOX", - default_search_url_keyword.c_str()}, - {"https://www.bing.com/search?form=WNSGPH&q=keyword", - default_search_url_keyword.c_str()}, - {"https://www.bing.com/search?q=keyword&form=WNSGPH&other=stuff", - default_search_url_keyword.c_str()}, - {"https://www.bing.com/search?q=%C3%A8+%C3%A9&form=WNSGPH", - default_search_url_special_chars.c_str()}, - }; - - SetFeatureEnabled(true); - - for (const auto& test : test_data) { - GURL url(test.input_arg); - ReplaceDesktopSearchURLWithDefaultSearchURLIfNeeded( - &prefs_, &template_url_service_, &url); - EXPECT_EQ(test.expected_output_arg, url.spec()); - } -} - -// Checks that ReplaceDesktopSearchURLWithDefaultSearchURLIfNeeded doesn't -// change the URL it receives when the desktop search redirection feature is -// disabled or when the default search engine is Bing. -TEST_F(DesktopSearchUtilsTest, ShouldReplaceDesktopSearchURL) { - SetDefaultSearchEngine(TemplateURLPrepopulateData::google); - const GURL desktop_search_url( - "https://www.bing.com/search?q=keyword&form=WNSGPH"); - const GURL default_search_url(GetDefaultSearchURLForSearchTerms( - &template_url_service_, base::WideToUTF16(L"keyword"))); - - const ShouldReplaceDesktopSearchURLTestData test_data[] = { - {false, false, &desktop_search_url}, - {true, false, &default_search_url}, - {false, true, &desktop_search_url}, - {true, true, &desktop_search_url}, - }; - - for (const auto& test : test_data) { - SetFeatureEnabled(test.feature_enabled); - SetDefaultSearchEngine(test.default_search_engine_is_bing - ? TemplateURLPrepopulateData::bing - : TemplateURLPrepopulateData::google); - - // Check whether the desktop search URL is replaced. - GURL url(desktop_search_url); - ReplaceDesktopSearchURLWithDefaultSearchURLIfNeeded( - &prefs_, &template_url_service_, &url); - EXPECT_EQ(*test.expected_output_arg, url); - } -} diff --git a/chromium/components/search_engines/search_engine_data_type_controller_unittest.cc b/chromium/components/search_engines/search_engine_data_type_controller_unittest.cc index b025e60a18f..97930b8cd2d 100644 --- a/chromium/components/search_engines/search_engine_data_type_controller_unittest.cc +++ b/chromium/components/search_engines/search_engine_data_type_controller_unittest.cc @@ -84,7 +84,7 @@ class SyncSearchEngineDataTypeControllerTest search_engine_dtc_->StartAssociating( base::Bind(&sync_driver::StartCallbackMock::Run, base::Unretained(&start_callback_))); - base::MessageLoop::current()->RunUntilIdle(); + base::RunLoop().RunUntilIdle(); } base::MessageLoop message_loop_; diff --git a/chromium/components/search_engines/search_engine_type.h b/chromium/components/search_engines/search_engine_type.h index d49c0462527..a1872cc701e 100644 --- a/chromium/components/search_engines/search_engine_type.h +++ b/chromium/components/search_engines/search_engine_type.h @@ -10,6 +10,7 @@ // to disrupt UMA data already recorded. enum SearchEngineType { // Prepopulated engines. + SEARCH_ENGINE_UNKNOWN = -1, SEARCH_ENGINE_OTHER = 0, // At the top in case of future list changes. SEARCH_ENGINE_AOL, SEARCH_ENGINE_ASK, diff --git a/chromium/components/search_engines/template_url.cc b/chromium/components/search_engines/template_url.cc index 22e361ad561..a3be5cff8eb 100644 --- a/chromium/components/search_engines/template_url.cc +++ b/chromium/components/search_engines/template_url.cc @@ -25,6 +25,7 @@ #include "components/metrics/proto/omnibox_input_type.pb.h" #include "components/search_engines/search_engines_switches.h" #include "components/search_engines/search_terms_data.h" +#include "components/search_engines/template_url_prepopulate_data.h" #include "components/url_formatter/url_formatter.h" #include "google_apis/google_api_keys.h" #include "net/base/escape.h" @@ -1204,7 +1205,8 @@ TemplateURL::TemplateURL(const TemplateURLData& data) TemplateURLRef::INSTANT), image_url_ref_(this, TemplateURLRef::IMAGE), new_tab_url_ref_(this, TemplateURLRef::NEW_TAB), - contextual_search_url_ref_(this, TemplateURLRef::CONTEXTUAL_SEARCH) { + contextual_search_url_ref_(this, TemplateURLRef::CONTEXTUAL_SEARCH), + engine_type_(SEARCH_ENGINE_UNKNOWN) { ResizeURLRefVector(); SetPrepopulateId(data_.prepopulate_id); @@ -1332,6 +1334,17 @@ std::string TemplateURL::GetExtensionId() const { return extension_info_->extension_id; } +SearchEngineType TemplateURL::GetEngineType( + const SearchTermsData& search_terms_data) const { + if (engine_type_ == SEARCH_ENGINE_UNKNOWN) { + const GURL url = GenerateSearchURL(search_terms_data); + engine_type_ = url.is_valid() ? + TemplateURLPrepopulateData::GetEngineType(url) : SEARCH_ENGINE_OTHER; + DCHECK_NE(SEARCH_ENGINE_UNKNOWN, engine_type_); + } + return engine_type_; +} + bool TemplateURL::ExtractSearchTermsFromURL( const GURL& url, const SearchTermsData& search_terms_data, @@ -1469,6 +1482,7 @@ void TemplateURL::CopyFrom(const TemplateURL& other) { void TemplateURL::SetURL(const std::string& url) { data_.SetURL(url); + engine_type_ = SEARCH_ENGINE_UNKNOWN; url_ref_->InvalidateCachedValues(); } diff --git a/chromium/components/search_engines/template_url.h b/chromium/components/search_engines/template_url.h index 2acb1468518..16c79ea9f9d 100644 --- a/chromium/components/search_engines/template_url.h +++ b/chromium/components/search_engines/template_url.h @@ -17,6 +17,7 @@ #include "base/time/time.h" #include "components/metrics/proto/omnibox_event.pb.h" #include "components/metrics/proto/omnibox_input_type.pb.h" +#include "components/search_engines/search_engine_type.h" #include "components/search_engines/template_url_data.h" #include "components/search_engines/template_url_id.h" #include "ui/gfx/geometry/size.h" @@ -644,6 +645,11 @@ class TemplateURL { // OMNIBOX_API_EXTENSION. std::string GetExtensionId() const; + // Returns the type of this search engine, or SEARCH_ENGINE_OTHER if no + // engines match. + SearchEngineType GetEngineType( + const SearchTermsData& search_terms_data) const; + // Use the alternate URLs and the search URL to match the provided |url| // and extract |search_terms| from it. Returns false and an empty // |search_terms| if no search terms can be matched. The order in which the @@ -749,6 +755,9 @@ class TemplateURL { TemplateURLRef contextual_search_url_ref_; std::unique_ptr<AssociatedExtensionInfo> extension_info_; + // Caches the computed engine type across successive calls to GetEngineType(). + mutable SearchEngineType engine_type_; + // TODO(sky): Add date last parsed OSD file. DISALLOW_COPY_AND_ASSIGN(TemplateURL); diff --git a/chromium/components/search_engines/template_url_prepopulate_data.cc b/chromium/components/search_engines/template_url_prepopulate_data.cc index 33a72bbb0ab..114757dd66d 100644 --- a/chromium/components/search_engines/template_url_prepopulate_data.cc +++ b/chromium/components/search_engines/template_url_prepopulate_data.cc @@ -21,7 +21,7 @@ #include "components/prefs/pref_service.h" #include "components/search_engines/prepopulated_engines.h" #include "components/search_engines/search_engines_pref_names.h" -#include "components/search_engines/template_url.h" +#include "components/search_engines/template_url_data.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" #include "url/gurl.h" @@ -1125,6 +1125,11 @@ ScopedVector<TemplateURLData> GetPrepopulatedEngines( return t_urls; } +std::vector<const PrepopulatedEngine*> GetAllPrepopulatedEngines() { + return std::vector<const PrepopulatedEngine*>(std::begin(kAllEngines), + std::end(kAllEngines)); +} + std::unique_ptr<TemplateURLData> MakeTemplateURLDataFromPrepopulatedEngine( const PrepopulatedEngine& engine) { base::ListValue alternate_urls; @@ -1166,16 +1171,6 @@ std::unique_ptr<TemplateURLData> GetPrepopulatedDefaultSearch( return default_search_provider; } -SearchEngineType GetEngineType(const TemplateURL& url, - const SearchTermsData& search_terms_data) { - // By calling ReplaceSearchTerms, we ensure that even TemplateURLs whose URLs - // can't be directly inspected (e.g. due to containing {google:baseURL}) can - // be converted to GURLs we can look at. - GURL gurl(url.url_ref().ReplaceSearchTerms(TemplateURLRef::SearchTermsArgs( - base::ASCIIToUTF16("x")), search_terms_data)); - return gurl.is_valid() ? GetEngineType(gurl) : SEARCH_ENGINE_OTHER; -} - SearchEngineType GetEngineType(const GURL& url) { DCHECK(url.is_valid()); diff --git a/chromium/components/search_engines/template_url_prepopulate_data.h b/chromium/components/search_engines/template_url_prepopulate_data.h index 9d6787305b9..a3224f33adb 100644 --- a/chromium/components/search_engines/template_url_prepopulate_data.h +++ b/chromium/components/search_engines/template_url_prepopulate_data.h @@ -17,8 +17,6 @@ class GURL; class PrefService; -class SearchTermsData; -class TemplateURL; struct TemplateURLData; namespace user_prefs { @@ -45,6 +43,9 @@ ScopedVector<TemplateURLData> GetPrepopulatedEngines( PrefService* prefs, size_t* default_search_provider_index); +// Returns all prepopulated engines for all locales. Used only by tests. +std::vector<const PrepopulatedEngine*> GetAllPrepopulatedEngines(); + // Returns a TemplateURLData for the specified prepopulated engine. std::unique_ptr<TemplateURLData> MakeTemplateURLDataFromPrepopulatedEngine( const PrepopulatedEngine& engine); @@ -59,15 +60,6 @@ void ClearPrepopulatedEnginesInPrefs(PrefService* prefs); std::unique_ptr<TemplateURLData> GetPrepopulatedDefaultSearch( PrefService* prefs); -// Returns the type of the provided engine, or SEARCH_ENGINE_OTHER if no engines -// match. This checks the TLD+1 for the most part, but will report the type as -// SEARCH_ENGINE_GOOGLE for any hostname that causes -// google_util::IsGoogleHostname() to return true. -// -// NOTE: Must be called on the UI thread. -SearchEngineType GetEngineType(const TemplateURL& template_url, - const SearchTermsData& search_terms_data); - // Like the above, but takes a GURL which is expected to represent a search URL. // This may be called on any thread. SearchEngineType GetEngineType(const GURL& url); 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 368597b9ff3..acda60f229d 100644 --- a/chromium/components/search_engines/template_url_prepopulate_data_unittest.cc +++ b/chromium/components/search_engines/template_url_prepopulate_data_unittest.cc @@ -2,8 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "components/search_engines/template_url_prepopulate_data.h" + #include <stddef.h> +#include <memory> +#include <utility> + #include "base/command_line.h" #include "base/files/scoped_temp_dir.h" #include "base/macros.h" @@ -15,7 +20,6 @@ #include "components/search_engines/search_engines_pref_names.h" #include "components/search_engines/search_terms_data.h" #include "components/search_engines/template_url.h" -#include "components/search_engines/template_url_prepopulate_data.h" #include "components/search_engines/template_url_service.h" #include "testing/gtest/include/gtest/gtest.h" @@ -26,8 +30,7 @@ namespace { SearchEngineType GetEngineType(const std::string& url) { TemplateURLData data; data.SetURL(url); - return TemplateURLPrepopulateData::GetEngineType(TemplateURL(data), - SearchTermsData()); + return TemplateURL(data).GetEngineType(SearchTermsData()); } std::string GetHostFromTemplateURLData(const TemplateURLData& data) { @@ -207,7 +210,7 @@ TEST_F(TemplateURLPrepopulateDataTest, ClearProvidersFromPrefs) { prefs_.SetUserPref(prefs::kSearchProviderOverridesVersion, new base::FundamentalValue(1)); base::ListValue* overrides = new base::ListValue; - base::DictionaryValue* entry(new base::DictionaryValue); + std::unique_ptr<base::DictionaryValue> entry(new base::DictionaryValue); // Set only the minimal required settings for a search provider configuration. entry->SetString("name", "foo"); entry->SetString("keyword", "fook"); @@ -215,7 +218,7 @@ TEST_F(TemplateURLPrepopulateDataTest, ClearProvidersFromPrefs) { entry->SetString("favicon_url", "http://foi.com/favicon.ico"); entry->SetString("encoding", "UTF-8"); entry->SetInteger("id", 1001); - overrides->Append(entry); + overrides->Append(std::move(entry)); prefs_.SetUserPref(prefs::kSearchProviderOverrides, overrides); int version = TemplateURLPrepopulateData::GetDataVersion(&prefs_); @@ -248,8 +251,7 @@ TEST_F(TemplateURLPrepopulateDataTest, ClearProvidersFromPrefs) { EXPECT_FALSE(t_urls[default_index]->contextual_search_url.empty()); EXPECT_FALSE(t_urls[default_index]->image_url_post_params.empty()); EXPECT_EQ(SEARCH_ENGINE_GOOGLE, - TemplateURLPrepopulateData::GetEngineType( - TemplateURL(*t_urls[default_index]), + TemplateURL(*t_urls[default_index]).GetEngineType( SearchTermsData())); } @@ -287,8 +289,7 @@ TEST_F(TemplateURLPrepopulateDataTest, ProvidersFromPrepopulated) { for (size_t i = 0; i < t_urls[default_index]->alternate_urls.size(); ++i) EXPECT_FALSE(t_urls[default_index]->alternate_urls[i].empty()); EXPECT_EQ(SEARCH_ENGINE_GOOGLE, - TemplateURLPrepopulateData::GetEngineType( - TemplateURL(*t_urls[default_index]), + TemplateURL(*t_urls[default_index]).GetEngineType( SearchTermsData())); EXPECT_FALSE(t_urls[default_index]->search_terms_replacement_key.empty()); } @@ -350,3 +351,16 @@ TEST_F(TemplateURLPrepopulateDataTest, GetEngineTypeAdvanced) { switches::kGoogleBaseURL, "http://www.foo.com/"); EXPECT_EQ(SEARCH_ENGINE_GOOGLE, GetEngineType(foo_url)); } + +TEST_F(TemplateURLPrepopulateDataTest, GetEngineTypeForAllPrepopulatedEngines) { + using PrepopulatedEngine = TemplateURLPrepopulateData::PrepopulatedEngine; + const std::vector<const PrepopulatedEngine*> all_engines = + TemplateURLPrepopulateData::GetAllPrepopulatedEngines(); + for (const PrepopulatedEngine* engine : all_engines) { + std::unique_ptr<TemplateURLData> data = + TemplateURLPrepopulateData::MakeTemplateURLDataFromPrepopulatedEngine( + *engine); + EXPECT_EQ(engine->type, + TemplateURL(*data).GetEngineType(SearchTermsData())); + } +} diff --git a/chromium/components/search_engines/template_url_service.cc b/chromium/components/search_engines/template_url_service.cc index f89aa3c10e6..064a37db4e6 100644 --- a/chromium/components/search_engines/template_url_service.cc +++ b/chromium/components/search_engines/template_url_service.cc @@ -131,18 +131,18 @@ bool IsFromSync(const TemplateURL* turl, const SyncDataMap& sync_data) { // underscores, which could occur as the result of conflict resolution. void LogDuplicatesHistogram( const TemplateURLService::TemplateURLVector& template_urls) { - std::map<std::string, int> duplicates; + std::map<base::string16, int> duplicates; for (TemplateURLService::TemplateURLVector::const_iterator it = template_urls.begin(); it != template_urls.end(); ++it) { - std::string keyword = base::UTF16ToASCII((*it)->keyword()); - base::TrimString(keyword, "_", &keyword); + base::string16 keyword = (*it)->keyword(); + base::TrimString(keyword, base::ASCIIToUTF16("_"), &keyword); duplicates[keyword]++; } // Count the keywords with duplicates. int num_dupes = 0; - for (std::map<std::string, int>::const_iterator it = duplicates.begin(); - it != duplicates.end(); ++it) { + for (std::map<base::string16, int>::const_iterator it = duplicates.begin(); + it != duplicates.end(); ++it) { if (it->second > 1) num_dupes++; } @@ -827,8 +827,7 @@ void TemplateURLService::OnWebDataServiceRequestDone( UMA_HISTOGRAM_ENUMERATION( "Search.DefaultSearchProviderType", - TemplateURLPrepopulateData::GetEngineType( - *default_search_provider_, search_terms_data()), + default_search_provider_->GetEngineType(search_terms_data()), SEARCH_ENGINE_MAX); if (rappor_service_) { @@ -2201,13 +2200,13 @@ base::string16 TemplateURLService::UniquifyKeyword(const TemplateURL& turl, return keyword_candidate; } -bool TemplateURLService::IsLocalTemplateURLBetter( - const TemplateURL* local_turl, - const TemplateURL* sync_turl) { +bool TemplateURLService::IsLocalTemplateURLBetter(const TemplateURL* local_turl, + const TemplateURL* sync_turl, + bool prefer_local_default) { DCHECK(GetTemplateURLForGUID(local_turl->sync_guid())); return local_turl->last_modified() > sync_turl->last_modified() || local_turl->created_by_policy() || - local_turl== GetDefaultSearchProvider(); + (prefer_local_default && local_turl == GetDefaultSearchProvider()); } void TemplateURLService::ResolveSyncKeywordConflict( @@ -2264,11 +2263,9 @@ void TemplateURLService::MergeInSyncTemplateURL( FindNonExtensionTemplateURLForKeyword(sync_turl->keyword()); bool should_add_sync_turl = true; - // If there was no TemplateURL in the local model that conflicts with - // |sync_turl|, skip the following preparation steps and just add |sync_turl| - // directly. Otherwise, modify |conflicting_turl| to make room for - // |sync_turl|. + // Resolve conflicts with local TemplateURLs. if (conflicting_turl) { + // Modify |conflicting_turl| to make room for |sync_turl|. if (IsFromSync(conflicting_turl, sync_data)) { // |conflicting_turl| is already known to Sync, so we're not allowed to // remove it. In this case, we want to uniquify the worse one and send an @@ -2306,6 +2303,43 @@ void TemplateURLService::MergeInSyncTemplateURL( // Remove the entry from the local data so it isn't pushed up to Sync. local_data->erase(guid); } + } else { + // Check for a turl with a conflicting prepopulate_id. This detects the case + // where the user changes a prepopulated engine's keyword on one client, + // then begins syncing on another client. We want to reflect this keyword + // change to that prepopulated URL on other clients instead of assuming that + // the modified TemplateURL is a new entity. + TemplateURL* conflicting_prepopulated_turl = + FindPrepopulatedTemplateURL(sync_turl->prepopulate_id()); + + // If we found a conflict, and the sync entity is better, apply the remote + // changes locally. We consider |sync_turl| better if it's been modified + // more recently and the local TemplateURL isn't yet known to sync. We will + // consider the sync entity better even if the local TemplateURL is the + // current default, since in this case being default does not necessarily + // mean the user explicitly set this TemplateURL as default. If we didn't do + // this, changes to the keywords of prepopulated default engines would never + // be applied to other clients. + // If we can't safely replace the local entry with the synced one, or merge + // the relevant changes in, we give up and leave both intact. + if (conflicting_prepopulated_turl && + !IsFromSync(conflicting_prepopulated_turl, sync_data) && + !IsLocalTemplateURLBetter(conflicting_prepopulated_turl, sync_turl, + false)) { + std::string guid = conflicting_prepopulated_turl->sync_guid(); + if (conflicting_prepopulated_turl == default_search_provider_) { + ApplyDefaultSearchChange(&sync_turl->data(), + DefaultSearchManager::FROM_USER); + merge_result->set_num_items_modified( + merge_result->num_items_modified() + 1); + } else { + Remove(conflicting_prepopulated_turl); + merge_result->set_num_items_deleted(merge_result->num_items_deleted() + + 1); + } + // Remove the local data so it isn't written to sync. + local_data->erase(guid); + } } if (should_add_sync_turl) { diff --git a/chromium/components/search_engines/template_url_service.h b/chromium/components/search_engines/template_url_service.h index 3f32b11e744..551b357ffc4 100644 --- a/chromium/components/search_engines/template_url_service.h +++ b/chromium/components/search_engines/template_url_service.h @@ -621,9 +621,11 @@ class TemplateURLService : public WebDataServiceConsumer, // of the following are true: // * |local_turl|'s last_modified timestamp is newer than sync_turl. // * |local_turl| is created by policy. - // * |local_turl| is the local default search provider. + // * |prefer_local_default| is true and |local_turl| is the local default + // search provider bool IsLocalTemplateURLBetter(const TemplateURL* local_turl, - const TemplateURL* sync_turl); + const TemplateURL* sync_turl, + bool prefer_local_default = true); // Given two synced TemplateURLs with a conflicting keyword, one of which // needs to be added to or updated in the local model (|unapplied_sync_turl|) diff --git a/chromium/components/search_engines/template_url_unittest.cc b/chromium/components/search_engines/template_url_unittest.cc index 6103835a45f..e0a413a5684 100644 --- a/chromium/components/search_engines/template_url_unittest.cc +++ b/chromium/components/search_engines/template_url_unittest.cc @@ -641,10 +641,6 @@ TEST_F(TemplateURLTest, ReplaceInputType) { metrics::OmniboxInputType::URL, "{google:baseURL}?{searchTerms}&{google:inputType}", "http://www.google.com/?foo&oit=3&" }, - { ASCIIToUTF16("foo"), - metrics::OmniboxInputType::FORCED_QUERY, - "{google:baseURL}?{searchTerms}&{google:inputType}", - "http://www.google.com/?foo&oit=5&" }, }; TemplateURLData data; data.input_encodings.push_back("UTF-8"); |