Skip to content

Commit

Permalink
Updated Improved Search Suggestions default value
Browse files Browse the repository at this point in the history
fix brave/brave-browser#29517

We'll turn it on when Brave Search is default provider in specific countries.
  • Loading branch information
simonhong committed Apr 21, 2023
1 parent 6b8904a commit d843f6d
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 3 deletions.
2 changes: 0 additions & 2 deletions browser/brave_prefs_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,6 @@ IN_PROC_BROWSER_TEST_F(BraveProfilePrefsBrowserTest,
spellcheck::prefs::kSpellCheckUseSpellingService));
EXPECT_FALSE(chrome_test_utils::GetProfile(this)->GetPrefs()->GetBoolean(
prefs::kSafeBrowsingExtendedReportingOptInAllowed));
EXPECT_FALSE(chrome_test_utils::GetProfile(this)->GetPrefs()->GetBoolean(
prefs::kSearchSuggestEnabled));
EXPECT_EQ(chrome_test_utils::GetProfile(this)->GetPrefs()->GetInteger(
prefetch::prefs::kNetworkPredictionOptions),
static_cast<int>(prefetch::NetworkPredictionOptions::kDisabled));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,17 @@

#include "brave/browser/search_engines/normal_window_search_engine_provider_service.h"

#include <string>

#include "base/containers/contains.h"
#include "base/functional/bind.h"
#include "brave/browser/search_engines/search_engine_provider_util.h"
#include "brave/components/l10n/common/locale_util.h"
#include "brave/components/search_engines/brave_prepopulated_engines.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_service.h"
#include "components/search_engines/search_engines_pref_names.h"
#include "components/search_engines/template_url_service.h"

Expand All @@ -22,6 +29,8 @@ NormalWindowSearchEngineProviderService::
base::Unretained(this)));

auto* service = TemplateURLServiceFactory::GetForProfile(profile_);
observation_.Observe(service);

if (service->loaded()) {
PrepareInitialPrivateSearchProvider();
return;
Expand All @@ -40,6 +49,7 @@ NormalWindowSearchEngineProviderService::
void NormalWindowSearchEngineProviderService::Shutdown() {
template_url_service_subscription_ = {};
private_search_provider_guid_.Destroy();
observation_.Reset();
}

void NormalWindowSearchEngineProviderService::OnTemplateURLServiceLoaded(
Expand All @@ -56,3 +66,36 @@ void NormalWindowSearchEngineProviderService::
void NormalWindowSearchEngineProviderService::OnPreferenceChanged() {
brave::UpdateDefaultPrivateSearchProviderData(profile_);
}

void NormalWindowSearchEngineProviderService::OnTemplateURLServiceChanged() {
UpdateSearchSuggestionsDefaultValue();
}

void NormalWindowSearchEngineProviderService::
UpdateSearchSuggestionsDefaultValue() {
// As we want to have different default value for search suggestions option,
// we should update whenever default provider is changed.
auto* service = TemplateURLServiceFactory::GetForProfile(profile_);
auto* template_url = service->GetDefaultSearchProvider();
if (!template_url) {
return;
}

const std::string default_country_code =
brave_l10n::GetDefaultISOCountryCodeString();
constexpr std::array<const char*, 11> kSupportedCountries = {
"IN", "CA", "DE", "FR", "GB", "US", "AT", "ES", "MX", "BR", "AR"};
const bool search_suggestions_default_value =
(template_url->prepopulate_id() ==
TemplateURLPrepopulateData::PREPOPULATED_ENGINE_ID_BRAVE) &&
base::Contains(kSupportedCountries, default_country_code);

profile_->GetPrefs()->SetDefaultPrefValue(
prefs::kSearchSuggestEnabled,
base::Value(search_suggestions_default_value));
}

void NormalWindowSearchEngineProviderService::
OnTemplateURLServiceShuttingDown() {
observation_.Reset();
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@

#include "base/callback_list.h"
#include "base/memory/raw_ptr.h"
#include "base/scoped_observation.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/prefs/pref_member.h"
#include "components/search_engines/template_url_service_observer.h"

class Profile;
class TemplateURLService;

// Set default prefs for private search provider as it's stored in normal
// profile. And update TemplateURLData for private search provider whenever
Expand All @@ -21,7 +24,9 @@ class Profile;
// In this situation, previous default provider should be default one with
// new list. To do that, cached TemplateURLData can be added to
// TemplateURLService.
class NormalWindowSearchEngineProviderService : public KeyedService {
class NormalWindowSearchEngineProviderService
: public KeyedService,
public TemplateURLServiceObserver {
public:
explicit NormalWindowSearchEngineProviderService(Profile* profile);
~NormalWindowSearchEngineProviderService() override;
Expand All @@ -35,13 +40,20 @@ class NormalWindowSearchEngineProviderService : public KeyedService {
// KeyedService overrides:
void Shutdown() override;

// TemplateURLServiceObserver overrides:
void OnTemplateURLServiceChanged() override;
void OnTemplateURLServiceShuttingDown() override;

void OnTemplateURLServiceLoaded(Profile* profile);
void PrepareInitialPrivateSearchProvider();
void OnPreferenceChanged();
void UpdateSearchSuggestionsDefaultValue();

raw_ptr<Profile> profile_ = nullptr;
StringPrefMember private_search_provider_guid_;
base::CallbackListSubscription template_url_service_subscription_;
base::ScopedObservation<TemplateURLService, TemplateURLServiceObserver>
observation_{this};
};

#endif // BRAVE_BROWSER_SEARCH_ENGINES_NORMAL_WINDOW_SEARCH_ENGINE_PROVIDER_SERVICE_H_
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "brave/browser/search_engines/search_engine_provider_util.h"
#include "brave/browser/ui/browser_commands.h"
#include "brave/components/constants/pref_names.h"
#include "brave/components/l10n/common/test/scoped_default_locale.h"
#include "brave/components/search_engines/brave_prepopulated_engines.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "build/build_config.h"
Expand All @@ -23,11 +24,13 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/search_test_utils.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/country_codes/country_codes.h"
#include "components/prefs/pref_service.h"
#include "components/search_engines/search_engines_pref_names.h"
#include "components/search_engines/search_engines_test_util.h"
#include "components/search_engines/template_url_data_util.h"
Expand Down Expand Up @@ -249,6 +252,89 @@ IN_PROC_BROWSER_TEST_F(SearchEngineProviderServiceTest,
#endif
}

IN_PROC_BROWSER_TEST_F(SearchEngineProviderServiceTest,
DefaultSearchSuggestEnabledTest) {
auto* prefs = browser()->profile()->GetPrefs();
auto* service =
TemplateURLServiceFactory::GetForProfile(browser()->profile());
auto* search_suggest_pref =
prefs->FindPreference(prefs::kSearchSuggestEnabled);
auto brave_search_data = TemplateURLDataFromPrepopulatedEngine(
TemplateURLPrepopulateData::brave_search);
auto brave_template_url = std::make_unique<TemplateURL>(*brave_search_data);

auto bing_search_data = TemplateURLDataFromPrepopulatedEngine(
TemplateURLPrepopulateData::brave_bing);
auto bing_template_url = std::make_unique<TemplateURL>(*bing_search_data);

// Test with supported country for search suggestions on by default for Brave
// Search.
{
brave_l10n::test::ScopedDefaultLocale test_locale("en_US");
EXPECT_TRUE(search_suggest_pref->IsDefaultValue());

// Set Brave Search and check default value is true.
service->SetUserSelectedDefaultSearchProvider(brave_template_url.get());
EXPECT_TRUE(prefs->GetBoolean(prefs::kSearchSuggestEnabled));
EXPECT_TRUE(
prefs->GetDefaultPrefValue(prefs::kSearchSuggestEnabled)->GetBool());

// Set Bing and check default value is false.
service->SetUserSelectedDefaultSearchProvider(bing_template_url.get());
EXPECT_FALSE(prefs->GetBoolean(prefs::kSearchSuggestEnabled));
EXPECT_FALSE(
prefs->GetDefaultPrefValue(prefs::kSearchSuggestEnabled)->GetBool());

// Set Brave Search again and check default value is true.
service->SetUserSelectedDefaultSearchProvider(brave_template_url.get());
EXPECT_TRUE(prefs->GetBoolean(prefs::kSearchSuggestEnabled));
EXPECT_TRUE(
prefs->GetDefaultPrefValue(prefs::kSearchSuggestEnabled)->GetBool());

// Set on explicitely.
prefs->SetBoolean(prefs::kSearchSuggestEnabled, true);

// Set Bing and check default value changed to false but current value is
// not changed after explicitely turned it on above.
service->SetUserSelectedDefaultSearchProvider(bing_template_url.get());
EXPECT_TRUE(prefs->GetBoolean(prefs::kSearchSuggestEnabled));
EXPECT_FALSE(
prefs->GetDefaultPrefValue(prefs::kSearchSuggestEnabled)->GetBool());
}

// Clear to test for non supported country.
prefs->ClearPref(prefs::kSearchSuggestEnabled);

// Test witn non-supported country and default is always off regardless of
// providers.
{
brave_l10n::test::ScopedDefaultLocale test_locale("ko_KR");
EXPECT_TRUE(search_suggest_pref->IsDefaultValue());

// Set Brave Search and check default value and current value are false.
service->SetUserSelectedDefaultSearchProvider(brave_template_url.get());
EXPECT_FALSE(prefs->GetBoolean(prefs::kSearchSuggestEnabled));
EXPECT_FALSE(
prefs->GetDefaultPrefValue(prefs::kSearchSuggestEnabled)->GetBool());

// Set Bing and check both are still false.
service->SetUserSelectedDefaultSearchProvider(bing_template_url.get());
EXPECT_FALSE(prefs->GetBoolean(prefs::kSearchSuggestEnabled));
EXPECT_FALSE(
prefs->GetDefaultPrefValue(prefs::kSearchSuggestEnabled)->GetBool());

// Set on explicitely.
prefs->SetBoolean(prefs::kSearchSuggestEnabled, true);

// Set Brave Search and check default value is still false but current value
// is true.
service->SetUserSelectedDefaultSearchProvider(brave_template_url.get());
EXPECT_TRUE(prefs->GetBoolean(prefs::kSearchSuggestEnabled));
EXPECT_FALSE(
prefs->GetDefaultPrefValue(prefs::kSearchSuggestEnabled)->GetBool());
}
}

#if BUILDFLAG(ENABLE_EXTENSIONS)

namespace extensions {
Expand Down

0 comments on commit d843f6d

Please sign in to comment.