From 90e40272f56596e9c190ee0cf9cbb4279b23e388 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Tue, 15 Oct 2019 16:17:18 -0700 Subject: [PATCH] Fix default search logic for Tor window and guest window Fixes https://github.com/brave/brave-browser/issues/6476 Specifically, fixes the engine when a previously set engine is no longer available. For example, in DE the engine used to be DDG (501), but was changed in the prepopulated list to DDG_DE(516). The value in the preferences will not be able to be used to get a valid engine (and cause a crash). This change checks if the currently selected engine is available and if not resets the value to the currently preset default. --- ...t_window_search_engine_provider_service.cc | 15 ++++-- ...r_window_search_engine_provider_service.cc | 50 ++++++++++++------- ...or_window_search_engine_provider_service.h | 6 ++- 3 files changed, 48 insertions(+), 23 deletions(-) diff --git a/browser/search_engines/guest_window_search_engine_provider_service.cc b/browser/search_engines/guest_window_search_engine_provider_service.cc index cb5aafeb8c0e..586512dbf426 100644 --- a/browser/search_engines/guest_window_search_engine_provider_service.cc +++ b/browser/search_engines/guest_window_search_engine_provider_service.cc @@ -38,13 +38,18 @@ void GuestWindowSearchEngineProviderService::OnTemplateURLServiceChanged() { if (ignore_template_url_service_changing_) return; - // The purpose of below code is togging alternative prefs + // The purpose of below code is toggling alternative prefs // when user changes from ddg to different search engine provider // (or vice versa) from settings ui. - const bool is_ddg_is_set = - otr_template_url_service_->GetDefaultSearchProvider()-> - data().prepopulate_id == - TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO; + bool is_ddg_is_set = false; + switch (otr_template_url_service_->GetDefaultSearchProvider() + ->data() + .prepopulate_id) { + case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO: + case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_DE: + case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_AU_NZ_IE: + is_ddg_is_set = true; + } if (UseAlternativeSearchEngineProvider() || is_ddg_is_set) brave::ToggleUseAlternativeSearchEngineProvider(otr_profile_); diff --git a/browser/search_engines/tor_window_search_engine_provider_service.cc b/browser/search_engines/tor_window_search_engine_provider_service.cc index 7f916af8255a..26b1f534adc5 100644 --- a/browser/search_engines/tor_window_search_engine_provider_service.cc +++ b/browser/search_engines/tor_window_search_engine_provider_service.cc @@ -25,9 +25,7 @@ TorWindowSearchEngineProviderService(Profile* otr_profile) // Configure previously used provider because effective tor profile is // off the recored profile. - auto provider_data = - TemplateURLPrepopulateData::GetPrepopulatedEngine( - otr_profile->GetPrefs(), GetInitialSearchEngineProvider()); + auto provider_data = GetInitialSearchEngineProvider(otr_profile->GetPrefs()); TemplateURL provider_url(*provider_data); otr_template_url_service_->SetUserSelectedDefaultSearchProvider( &provider_url); @@ -48,23 +46,41 @@ void TorWindowSearchEngineProviderService::OnTemplateURLServiceChanged() { data().prepopulate_id); } -int TorWindowSearchEngineProviderService:: -GetInitialSearchEngineProvider() const { +std::unique_ptr +TorWindowSearchEngineProviderService::GetInitialSearchEngineProvider( + PrefService* prefs) const { + std::unique_ptr provider_data = nullptr; int initial_id = alternative_search_engine_provider_in_tor_.GetValue(); - - bool region_for_qwant = - TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch( - otr_profile_->GetPrefs())->prepopulate_id == - TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_QWANT; + if (initial_id != + TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_INVALID) { + provider_data = std::move( + TemplateURLPrepopulateData::GetPrepopulatedEngine(prefs, initial_id)); + } // If this is first run, |initial_id| is invalid. Then, use qwant or ddg - // depends on default prepopulate data. - if (initial_id == - TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_INVALID) { - initial_id = region_for_qwant ? - TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_QWANT : - TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO; + // depends on default prepopulate data. If not, check that the initial_id + // returned data. + if (!provider_data) { + initial_id = TemplateURLPrepopulateData::GetPrepopulatedDefaultSearch( + otr_profile_->GetPrefs()) + ->prepopulate_id; + switch (initial_id) { + case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_QWANT: + case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO: + case TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO_DE: + case TemplateURLPrepopulateData:: + PREPOPULATED_ENGINE_ID_DUCKDUCKGO_AU_NZ_IE: + break; + + default: + initial_id = + TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_DUCKDUCKGO; + break; + } + provider_data = std::move( + TemplateURLPrepopulateData::GetPrepopulatedEngine(prefs, initial_id)); } - return initial_id; + DCHECK(provider_data); + return provider_data; } diff --git a/browser/search_engines/tor_window_search_engine_provider_service.h b/browser/search_engines/tor_window_search_engine_provider_service.h index 4f6cb7c8e56b..ffb35de9f0ac 100644 --- a/browser/search_engines/tor_window_search_engine_provider_service.h +++ b/browser/search_engines/tor_window_search_engine_provider_service.h @@ -9,6 +9,9 @@ #include "components/prefs/pref_member.h" #include "components/search_engines/template_url_service_observer.h" +class PrefService; +struct TemplateURLData; + // The purpose of this service for tor is making user changed search engine // provider persist across the sessions. // Also, BraveProfileManager::SetNonPersonalProfilePrefs() overrides for it. @@ -23,7 +26,8 @@ class TorWindowSearchEngineProviderService // TemplateURLServiceObserver overrides: void OnTemplateURLServiceChanged() override; - int GetInitialSearchEngineProvider() const; + std::unique_ptr GetInitialSearchEngineProvider( + PrefService* prefs) const; IntegerPrefMember alternative_search_engine_provider_in_tor_;