Skip to content

Commit

Permalink
Enable search suggestions by default (uplift to 1.70.x) (#25421)
Browse files Browse the repository at this point in the history
* Uplift of #25161 (squashed) to beta

* Merge pull request #25099 from brave/rename_to_show_search_suggestions

Renamed to `Show search suggestions` in search engine settings

* Merge pull request #25118 from brave/prevent_search_suggestions_from_clipboard

Prevent search suggestions from clipboard text

* Merge pull request #25243 from brave/prevent_search_suggestions_for_suspicious_query

Improve query check before sending to search suggestions server

---------

Co-authored-by: Simon Hong <shong@brave.com>
  • Loading branch information
brave-builds and simonhong committed Sep 6, 2024
1 parent 71f7279 commit ccdcbd7
Show file tree
Hide file tree
Showing 29 changed files with 2,435 additions and 10 deletions.
6 changes: 6 additions & 0 deletions app/brave_settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,12 @@
</message>

<!-- Settings / Search engine -->
<message name="IDS_SETTINGS_BRAVE_SEARCH_ENGINES_SEARCH_SUGGEST_LABEL" desc="The label of the option to enable/disable sending omnibox input to the user's default search engine to get additional suggestions.">
Show search suggestions
</message>
<message name="IDS_SETTINGS_BRAVE_SEARCH_ENGINES_SEARCH_SUGGEST_DESC" desc="The desc of the option to enable/disable sending omnibox input to the user's default search engine to get additional suggestions.">
When you type in the address bar or search box, Brave sends what you type to your default search engine to get better suggestions. This is not available in private windows.
</message>
<message name="IDS_SETTINGS_BRAVE_OTHER_SEARCH_ENGINES_LABEL" desc="The label for the settings switch controlling the addition of custom search engines">
Index other search engines
</message>
Expand Down
1 change: 1 addition & 0 deletions browser/autocomplete/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ brave_browser_autocomplete_deps = [
"//base",
"//brave/common",
"//brave/components/brave_webtorrent/browser/buildflags",
"//brave/components/omnibox/buildflags",
"//chrome/browser/profiles:profile",
]

Expand Down
5 changes: 5 additions & 0 deletions browser/brave_browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@
#if BUILDFLAG(IS_ANDROID)
#include "chrome/browser/flags/android/chrome_feature_list.h"
#else
#include "brave/browser/search_engines/search_engine_provider_util.h"
#include "brave/browser/ui/brave_browser_command_controller.h"
#include "chrome/browser/first_run/first_run.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_list.h"
#endif
Expand Down Expand Up @@ -171,6 +173,9 @@ void BraveBrowserProcessImpl::Init() {
// suppressed it from previous os.
local_state()->ClearPref(prefs::kSuppressUnsupportedOSWarning);
}

brave::PrepareSearchSuggestionsConfig(local_state(),
first_run::IsChromeFirstRun());
#endif
}

Expand Down
3 changes: 3 additions & 0 deletions browser/brave_local_state_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@

#if !BUILDFLAG(IS_ANDROID)
#include "brave/browser/p3a/p3a_core_metrics.h"
#include "brave/browser/search_engines/pref_names.h"
#include "brave/browser/ui/whats_new/whats_new_util.h"
#include "chrome/browser/first_run/first_run.h"
#endif // !BUILDFLAG(IS_ANDROID)
Expand Down Expand Up @@ -127,6 +128,8 @@ void RegisterLocalStatePrefs(PrefRegistrySimple* registry) {
BraveWindowTracker::RegisterPrefs(registry);
dark_mode::RegisterBraveDarkModeLocalStatePrefs(registry);
whats_new::RegisterLocalStatePrefs(registry);

registry->RegisterBooleanPref(kEnableSearchSuggestionsByDefault, false);
#endif

#if defined(TOOLKIT_VIEWS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
<settings-toggle-button id="searchSuggestToggle"
class="hr"
pref="{{prefs.search.suggest_enabled}}"
label="$i18n{searchSuggestPref}"
sub-label="$i18n{searchSuggestPrefDesc}">
label="$i18n{searchSuggestLabel}"
sub-label="$i18n{searchSuggestDesc}">
</settings-toggle-button>
</template>
<if expr="enable_extensions">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/functional/bind.h"
#include "brave/browser/search_engines/search_engine_provider_util.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "components/search_engines/search_engines_pref_names.h"
Expand All @@ -15,6 +16,8 @@
NormalWindowSearchEngineProviderService::
NormalWindowSearchEngineProviderService(Profile* profile)
: profile_(profile) {
brave::UpdateDefaultSearchSuggestionsPrefs(g_browser_process->local_state(),
profile_->GetPrefs());
private_search_provider_guid_.Init(
prefs::kSyncedDefaultPrivateSearchProviderGUID, profile_->GetPrefs(),
base::BindRepeating(
Expand Down
4 changes: 4 additions & 0 deletions browser/search_engines/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@ inline constexpr char kUseAlternativePrivateSearchEngineProvider[] =
inline constexpr char kShowAlternativePrivateSearchEngineProviderToggle[] =
"brave.show_alternate_private_search_engine_toggle";

// local state
inline constexpr char kEnableSearchSuggestionsByDefault[] =
"brave.enable_search_suggestions_by_default";

#endif // BRAVE_BROWSER_SEARCH_ENGINES_PREF_NAMES_H_
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include <tuple>
#include <vector>

#include "base/path_service.h"
#include "brave/browser/profile_resetter/brave_profile_resetter.h"
#include "brave/browser/profiles/brave_profile_manager.h"
Expand All @@ -12,6 +15,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 @@ -24,11 +28,14 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.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_engine_choice/search_engine_choice_service.h"
#include "components/search_engines/search_engines_pref_names.h"
#include "components/search_engines/search_engines_test_util.h"
Expand Down Expand Up @@ -260,6 +267,65 @@ IN_PROC_BROWSER_TEST_F(SearchEngineProviderServiceTest,
#endif
}

class SearchSuggestionsEnabledTest : public InProcessBrowserTest,
public ::testing::WithParamInterface<
std::tuple<std::string, bool, bool>> {
public:
SearchSuggestionsEnabledTest() : default_locale(GetLocale()) {}
~SearchSuggestionsEnabledTest() override = default;

void SetUpCommandLine(base::CommandLine* command_line) final {
if (IsNewUser()) {
command_line->AppendSwitch(switches::kForceFirstRun);
}
}

const std::string& GetLocale() const { return std::get<0>(GetParam()); }
bool IsNewUser() const { return std::get<1>(GetParam()); }
bool IsSearchSuggestionsEnabled() const { return std::get<2>(GetParam()); }

const brave_l10n::test::ScopedDefaultLocale default_locale;
};

IN_PROC_BROWSER_TEST_P(SearchSuggestionsEnabledTest,
DefaultSearchSuggestEnabledTest) {
auto* prefs = browser()->profile()->GetPrefs();
auto* service =
TemplateURLServiceFactory::GetForProfile(browser()->profile());
auto brave_search_data = TemplateURLDataFromPrepopulatedEngine(
TemplateURLPrepopulateData::brave_search);
TemplateURL brave_template_url(*brave_search_data);

auto bing_search_data = TemplateURLDataFromPrepopulatedEngine(
TemplateURLPrepopulateData::brave_bing);
TemplateURL bing_template_url(*bing_search_data);

EXPECT_EQ(IsSearchSuggestionsEnabled(),
prefs->GetBoolean(prefs::kSearchSuggestEnabled));

service->SetUserSelectedDefaultSearchProvider(&bing_template_url);
EXPECT_EQ(IsSearchSuggestionsEnabled(),
prefs->GetBoolean(prefs::kSearchSuggestEnabled));

service->SetUserSelectedDefaultSearchProvider(&brave_template_url);
EXPECT_EQ(IsSearchSuggestionsEnabled(),
prefs->GetBoolean(prefs::kSearchSuggestEnabled));
}

// Check suggestions is enabled with supported(US) or non-supported(KR) country
// per new user(or not). Only new user from supported country enables search
// suggestions.
INSTANTIATE_TEST_SUITE_P(
/*no prefix*/,
SearchSuggestionsEnabledTest,
testing::ValuesIn(std::vector<std::tuple<std::string /* locale */,
bool /* new user */,
bool /* suggestions enabled */>>{
{"en_US", true, true},
{"en_US", false, false},
{"ko_KR", true, false},
{"ko_KR", false, false}}));

#if BUILDFLAG(ENABLE_EXTENSIONS)

namespace extensions {
Expand Down
36 changes: 36 additions & 0 deletions browser/search_engines/search_engine_provider_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@
#include <string>
#include <vector>

#include "base/containers/contains.h"
#include "base/values.h"
#include "brave/browser/search_engines/pref_names.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_engine_choice/search_engine_choice_service_factory.h"
#include "chrome/browser/search_engines/template_url_service_factory.h"
#include "chrome/common/pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/search_engines/search_engines_pref_names.h"
Expand Down Expand Up @@ -174,4 +178,36 @@ void ResetDefaultPrivateSearchProvider(Profile* profile) {
PrepareDefaultPrivateSearchProviderDataIfNeeded(profile);
}

void PrepareSearchSuggestionsConfig(PrefService* local_state, bool first_run) {
if (!first_run) {
return;
}

const std::string default_country_code =
brave_l10n::GetDefaultISOCountryCodeString();
constexpr int kNumberOfTargetCountries = 12;
constexpr const char* kTargetCountriesForEnableSearchSuggestionsByDefault[] =
{"AR", "AT", "BR", "CA", "DE", "ES", "FR", "GB", "IN", "IT", "MX", "US"};
static_assert(
std::size(kTargetCountriesForEnableSearchSuggestionsByDefault) ==
kNumberOfTargetCountries);

const bool enable_search_suggestions_default_value =
base::Contains(kTargetCountriesForEnableSearchSuggestionsByDefault,
default_country_code);

local_state->SetBoolean(kEnableSearchSuggestionsByDefault,
enable_search_suggestions_default_value);
}

void UpdateDefaultSearchSuggestionsPrefs(PrefService* local_state,
PrefService* profile_prefs) {
if (!local_state->GetBoolean(kEnableSearchSuggestionsByDefault)) {
return;
}

profile_prefs->SetDefaultPrefValue(prefs::kSearchSuggestEnabled,
base::Value(true));
}

} // namespace brave
4 changes: 4 additions & 0 deletions browser/search_engines/search_engine_provider_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ void PrepareDefaultPrivateSearchProviderDataIfNeeded(Profile* profile);
void UpdateDefaultPrivateSearchProviderData(Profile* profile);
void ResetDefaultPrivateSearchProvider(Profile* profile);

void PrepareSearchSuggestionsConfig(PrefService* local_state, bool first_run);
void UpdateDefaultSearchSuggestionsPrefs(PrefService* local_state,
PrefService* profile_prefs);

} // namespace brave

#endif // BRAVE_BROWSER_SEARCH_ENGINES_SEARCH_ENGINE_PROVIDER_UTIL_H_
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
IDS_SETTINGS_BRAVE_SHIELDS_TWITTER_EMBEDDED_TWEETS_LABEL},
{"linkedInEmbedControlLabel",
IDS_SETTINGS_BRAVE_SHIELDS_LINKEDIN_EMBEDDED_POSTS_LABEL},
{"searchSuggestLabel",
IDS_SETTINGS_BRAVE_SEARCH_ENGINES_SEARCH_SUGGEST_LABEL},
{"searchSuggestDesc",
IDS_SETTINGS_BRAVE_SEARCH_ENGINES_SEARCH_SUGGEST_DESC},
{"otherSearchEnginesControlLabel",
IDS_SETTINGS_BRAVE_OTHER_SEARCH_ENGINES_LABEL},
{"otherSearchEnginesControlDesc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "brave/browser/misc_metrics/process_misc_metrics.h"
#include "brave/browser/ui/brave_browser.h"
#include "brave/browser/ui/sidebar/sidebar_controller.h"
#include "chrome/browser/ui/omnibox/clipboard_utils.h"
#endif // BUILDFLAG(!IS_ANDROID)

#if BUILDFLAG(ENABLE_COMMANDER)
Expand Down Expand Up @@ -98,4 +99,13 @@ bool ChromeAutocompleteProviderClient::IsLeoProviderEnabled() {
ai_chat::prefs::kBraveChatAutocompleteProviderEnabled);
#endif
}

#endif // BUILDFLAG(ENABLE_AI_CHAT)

std::u16string ChromeAutocompleteProviderClient::GetClipboardText() const {
#if !BUILDFLAG(IS_ANDROID)
return ::GetClipboardText(/*notify_if_restricted*/ false);
#else
return u"";
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@
bool IsLeoProviderEnabled
#endif // BUILDFLAG(ENABLE_AI_CHAT)

#define GetAcceptLanguages \
GetAcceptLanguages() const override; \
std::u16string GetClipboardText

#include "src/chrome/browser/autocomplete/chrome_autocomplete_provider_client.h" // IWYU pragma: export

#undef GetAcceptLanguages
#undef GetInMemoryDatabase
#undef GetAutocompleteClassifier

Expand Down
4 changes: 4 additions & 0 deletions chromium_src/chrome/browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ brave_chromium_src_chrome_browser_deps = [
"//components/version_info",
]

if (!is_android) {
brave_chromium_src_chrome_browser_deps += [ "//chrome/browser/ui" ]
}

if (enable_brave_vpn) {
brave_chromium_src_chrome_browser_deps +=
[ "//brave/components/brave_vpn/browser" ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright (c) 2024 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.

#include "src/components/omnibox/browser/autocomplete_provider_client.cc"

std::u16string AutocompleteProviderClient::GetClipboardText() const {
return u"";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) 2024 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at https://mozilla.org/MPL/2.0/.

#ifndef BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_AUTOCOMPLETE_PROVIDER_CLIENT_H_
#define BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_AUTOCOMPLETE_PROVIDER_CLIENT_H_

#define StartServiceWorker \
UnUsed() {} \
virtual std::u16string GetClipboardText() const; \
virtual void StartServiceWorker

#include "src/components/omnibox/browser/autocomplete_provider_client.h" // IWYU pragma: export

#undef StartServiceWorker

#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_AUTOCOMPLETE_PROVIDER_CLIENT_H_
3 changes: 3 additions & 0 deletions chromium_src/components/omnibox/browser/search_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
bool IsBraveRichSuggestion(bool is_keyword); \
virtual void DoHistoryQuery

#define IsQueryPotentiallyPrivate virtual IsQueryPotentiallyPrivate

#include "src/components/omnibox/browser/search_provider.h" // IWYU pragma: export

#undef IsQueryPotentiallyPrivate
#undef DoHistoryQuery

#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_SEARCH_PROVIDER_H_
7 changes: 7 additions & 0 deletions components/omnibox/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# You can obtain one at http://mozilla.org/MPL/2.0/.

import("//brave/components/commander/common/buildflags/buildflags.gni")
import("//brave/components/omnibox/buildflags/buildflags.gni")

source_set("unit_tests") {
testonly = true
Expand All @@ -30,6 +31,7 @@ source_set("unit_tests") {
"//brave/components/brave_search_conversion",
"//brave/components/constants",
"//brave/components/l10n/common:test_support",
"//brave/components/omnibox/buildflags",
"//brave/components/search_engines",
"//components/bookmarks/browser",
"//components/bookmarks/test",
Expand All @@ -50,9 +52,14 @@ source_set("unit_tests") {
"//chrome/browser/search_engines",
"//chrome/test:test_support",
"//content/test:test_support",
"//ui/base/clipboard:clipboard_test_support",
]
}

if (enable_strict_query_check_for_search_suggestions) {
deps += [ "//brave/components/omnibox/browser/search_suggestions" ]
}

if (enable_commander) {
sources +=
[ "//brave/components/omnibox/browser/commander_provider_unittest.cc" ]
Expand Down
Loading

0 comments on commit ccdcbd7

Please sign in to comment.