Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Omnibox]: Hide OnDeviceHeadProvider suggestions when top suggestion are disabled #28094

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ public class BraveSearchEnginesPreferences extends BravePreferenceFragment
private static final String PREF_SEARCH_SUGGESTIONS = "search_suggestions";
private static final String PREF_SHOW_AUTOCOMPLETE_IN_ADDRESS_BAR =
"show_autocomplete_in_address_bar";
private static final String PREF_AUTOCOMPLETE_TOP_SITES = "autocomplete_top_sites";
private static final String PREF_AUTOCOMPLETE_TOP_SUGGESTIONS = "autocomplete_top_sites";
private static final String PREF_ADD_OPEN_SEARCH_ENGINES = "brave.other_search_engines_enabled";
private static final String PREF_SEND_WEB_DISCOVERY = "send_web_discovery";

private ChromeManagedPreferenceDelegate mManagedPreferenceDelegate;

private ChromeSwitchPreference mShowAutocompleteInAddressBar;
private ChromeSwitchPreference mSearchSuggestions;
private ChromeSwitchPreference mAutocompleteTopSites;
private ChromeSwitchPreference mAutocompleteTopSuggestions;
private ChromeSwitchPreference mAddOpenSearchEngines;
private ChromeSwitchPreference mSendWebDiscovery;

Expand Down Expand Up @@ -107,20 +107,20 @@ private void updateSearchEnginePreference() {
(ChromeSwitchPreference) findPreference(PREF_SHOW_AUTOCOMPLETE_IN_ADDRESS_BAR);
mShowAutocompleteInAddressBar.setOnPreferenceChangeListener(this);

mAutocompleteTopSites =
(ChromeSwitchPreference) findPreference(PREF_AUTOCOMPLETE_TOP_SITES);
mAutocompleteTopSites.setOnPreferenceChangeListener(this);
mAutocompleteTopSuggestions =
(ChromeSwitchPreference) findPreference(PREF_AUTOCOMPLETE_TOP_SUGGESTIONS);
mAutocompleteTopSuggestions.setOnPreferenceChangeListener(this);

boolean autocompleteEnabled =
UserPrefs.get(getProfile()).getBoolean(BravePref.AUTOCOMPLETE_ENABLED);
mSearchSuggestions.setVisible(autocompleteEnabled);
mAutocompleteTopSites.setVisible(autocompleteEnabled);
mAutocompleteTopSuggestions.setVisible(autocompleteEnabled);

mShowAutocompleteInAddressBar.setChecked(autocompleteEnabled);
mSearchSuggestions.setChecked(
UserPrefs.get(getProfile()).getBoolean(Pref.SEARCH_SUGGEST_ENABLED));
mAutocompleteTopSites.setChecked(
UserPrefs.get(getProfile()).getBoolean(BravePref.TOP_SITE_SUGGESTIONS_ENABLED));
mAutocompleteTopSuggestions.setChecked(
UserPrefs.get(getProfile()).getBoolean(BravePref.TOP_SUGGESTIONS_ENABLED));

mAddOpenSearchEngines =
(ChromeSwitchPreference) findPreference(PREF_ADD_OPEN_SEARCH_ENGINES);
Expand Down Expand Up @@ -154,12 +154,12 @@ public boolean onPreferenceChange(Preference preference, Object newValue) {
} else if (PREF_SHOW_AUTOCOMPLETE_IN_ADDRESS_BAR.equals(key)) {
boolean autocompleteEnabled = (boolean) newValue;
mSearchSuggestions.setVisible(autocompleteEnabled);
mAutocompleteTopSites.setVisible(autocompleteEnabled);
mAutocompleteTopSuggestions.setVisible(autocompleteEnabled);
UserPrefs.get(getProfile())
.setBoolean(BravePref.AUTOCOMPLETE_ENABLED, autocompleteEnabled);
} else if (PREF_AUTOCOMPLETE_TOP_SITES.equals(key)) {
} else if (PREF_AUTOCOMPLETE_TOP_SUGGESTIONS.equals(key)) {
UserPrefs.get(getProfile())
.setBoolean(BravePref.TOP_SITE_SUGGESTIONS_ENABLED, (boolean) newValue);
.setBoolean(BravePref.TOP_SUGGESTIONS_ENABLED, (boolean) newValue);
} else if (PREF_ADD_OPEN_SEARCH_ENGINES.equals(key)) {
UserPrefs.get(getProfile())
.setBoolean(BravePref.ADD_OPEN_SEARCH_ENGINES, (boolean) newValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
<org.chromium.components.browser_ui.settings.ChromeSwitchPreference
android:key="autocomplete_top_sites"
android:order="6"
android:title="@string/autocomplete_top_sites_title"
android:title="@string/autocomplete_top_suggestions_title"
android:persistent="false"/>
<org.chromium.components.browser_ui.settings.ChromeSwitchPreference
android:key="brave.other_search_engines_enabled"
Expand Down
4 changes: 2 additions & 2 deletions app/brave_settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_AUTOCOMPLETE_IN_ADDRESS_BAR" desc="The label for settings switch controlling the showing of autocomplete in address bar">
Show autocomplete suggestions in address bar
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_USE_AUTOCOMPLETE_TOP_SITES" desc="The label for settings checkbox controlling whether or not top sites show up in autocomplete">
Top sites
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_USE_AUTOCOMPLETE_TOP_SUGGESTIONS" desc="The label for settings checkbox controlling whether or not top suggestions show up in autocomplete">
Top Suggestions
</message>
<message name="IDS_SETTINGS_APPEARANCE_SETTINGS_USE_AUTOCOMPLETE_HISTORY" desc="The label for settings checkbox controlling whether or not browsing history shows up in autocomplete">
Browsing History
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ const PrefsUtil::TypedPrefMap& BravePrefsUtil::GetAllowlistedKeys() {
(*s_brave_allowlist)[kLocationBarIsWide] = settings_api::PrefType::kBoolean;
(*s_brave_allowlist)[omnibox::kAutocompleteEnabled] =
settings_api::PrefType::kBoolean;
(*s_brave_allowlist)[omnibox::kTopSiteSuggestionsEnabled] =
(*s_brave_allowlist)[omnibox::kTopSuggestionsEnabled] =
settings_api::PrefType::kBoolean;
(*s_brave_allowlist)[omnibox::kHistorySuggestionsEnabled] =
settings_api::PrefType::kBoolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
<settings-checkbox
class="cr-row list-item"
pref="{{prefs.brave.top_site_suggestions_enabled}}"
label="$i18n{appearanceSettingsUseTopSiteSuggestions}">
label="$i18n{appearanceSettingsUseTopSuggestions}">
</settings-checkbox>
<settings-checkbox
class="cr-row list-item"
Expand Down
4 changes: 2 additions & 2 deletions browser/ui/android/strings/android_brave_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -504,8 +504,8 @@ This file contains all "about" strings. It is set to NOT be translated, in tran
<message name="IDS_SHOW_SEARCH_SUGGESTIONS_TITLE" desc="Title for a switch in Settings that controls URL and search autocompletion and informs the user about the data shared by this feature.">
Show search suggestions
</message>
<message name="IDS_AUTOCOMPLETE_TOP_SITES_TITLE" desc="The label for settings switch controlling whether or not top sites show up in autocomplete">
Show top sites suggestions
<message name="IDS_AUTOCOMPLETE_TOP_SUGGESTIONS_TITLE" desc="The label for settings switch controlling whether or not top suggestions show up in autocomplete">
Show top suggestions
</message>
<message name="IDS_INDEX_OTHER_SEARCH_ENGINES_TITLE" desc="Title for the settings related to index other search engines">
Index other search engines
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
IDS_SETTINGS_NEVER_SHOW_BOOKMARK_BAR_DESC},
{"appearanceSettingsShowAutocompleteInAddressBar",
IDS_SETTINGS_APPEARANCE_SETTINGS_SHOW_AUTOCOMPLETE_IN_ADDRESS_BAR},
{"appearanceSettingsUseTopSiteSuggestions",
IDS_SETTINGS_APPEARANCE_SETTINGS_USE_AUTOCOMPLETE_TOP_SITES},
{"appearanceSettingsUseTopSuggestions",
IDS_SETTINGS_APPEARANCE_SETTINGS_USE_AUTOCOMPLETE_TOP_SUGGESTIONS},
{"appearanceSettingsUseHistorySuggestions",
IDS_SETTINGS_APPEARANCE_SETTINGS_USE_AUTOCOMPLETE_HISTORY},
{"appearanceSettingsUseBookmarkSuggestions",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "brave/components/omnibox/browser/brave_history_quick_provider.h"
#include "brave/components/omnibox/browser/brave_history_url_provider.h"
#include "brave/components/omnibox/browser/brave_local_history_zero_suggest_provider.h"
#include "brave/components/omnibox/browser/brave_on_device_head_provider.h"
#include "brave/components/omnibox/browser/brave_search_provider.h"
#include "brave/components/omnibox/browser/brave_shortcuts_provider.h"
#include "brave/components/omnibox/browser/leo_provider.h"
Expand Down Expand Up @@ -94,6 +95,7 @@ void MaybeShowLeoMatch(AutocompleteResult* result) {
#define LocalHistoryZeroSuggestProvider BraveLocalHistoryZeroSuggestProvider
#define BookmarkProvider BraveBookmarkProvider
#define ShortcutsProvider BraveShortcutsProvider
#define OnDeviceHeadProvider BraveOnDeviceHeadProvider
#define BRAVE_AUTOCOMPLETE_CONTROLLER_AUTOCOMPLETE_CONTROLLER \
MaybeAddCommanderProvider(providers_, this); \
MaybeAddLeoProvider(providers_, this); \
Expand All @@ -114,6 +116,7 @@ void MaybeShowLeoMatch(AutocompleteResult* result) {

#undef BRAVE_AUTOCOMPLETE_CONTROLLER_UPDATE_RESULT
#undef BRAVE_AUTOCOMPLETE_CONTROLLER_AUTOCOMPLETE_CONTROLLER
#undef OnDeviceHeadProvider
#undef ShortcutsProvider
#undef BookmarkProvider
#undef LocalHistoryZeroSuggestProvider
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) 2025 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_ON_DEVICE_HEAD_PROVIDER_H_
#define BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_ON_DEVICE_HEAD_PROVIDER_H_

#define OnDeviceHeadProviderTest \
OnDeviceHeadProviderTest; \
friend class BraveOnDeviceHeadProvider

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

#undef OnDeviceHeadProviderTest

#endif // BRAVE_CHROMIUM_SRC_COMPONENTS_OMNIBOX_BROWSER_ON_DEVICE_HEAD_PROVIDER_H_
1 change: 1 addition & 0 deletions components/omnibox/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ source_set("unit_tests") {
"//brave/components/omnibox/browser/on_device_head_provider_unittest.cc",
"//brave/components/omnibox/browser/topsites_provider_unittest.cc",
"brave_autocomplete_result_unittest.cc",
"brave_on_device_head_provider_unittest.cc",
"brave_search_suggestion_parser_unittest.cc",
"promotion_unittest.cc",
]
Expand Down
2 changes: 1 addition & 1 deletion components/omnibox/browser/brave_omnibox_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace omnibox {

void RegisterBraveProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterBooleanPref(kAutocompleteEnabled, true);
registry->RegisterBooleanPref(kTopSiteSuggestionsEnabled, true);
registry->RegisterBooleanPref(kTopSuggestionsEnabled, true);
registry->RegisterBooleanPref(kHistorySuggestionsEnabled, true);
registry->RegisterBooleanPref(kBookmarkSuggestionsEnabled, true);
registry->RegisterBooleanPref(kCommanderSuggestionsEnabled, true);
Expand Down
8 changes: 5 additions & 3 deletions components/omnibox/browser/brave_omnibox_prefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ namespace omnibox {
// the individual prefs listed below.
inline constexpr char kAutocompleteEnabled[] = "brave.autocomplete_enabled";

// Determines whether top sites show up in the omnibox results. See
// |TopsitesProvider|.
inline constexpr char kTopSiteSuggestionsEnabled[] =
// Determines whether top suggestions show up in the omnibox results. See
// TopSitesProvider and BraveOnDeviceHeadProvider.
// Note: Originally this only controlled TopSitesProvider, but now it also
// controls BraveOnDeviceHeadProvider, hence the pref name.
inline constexpr char kTopSuggestionsEnabled[] =
"brave.top_site_suggestions_enabled";

// Determines whether history suggestions show up in the omnibox results. This
Expand Down
30 changes: 30 additions & 0 deletions components/omnibox/browser/brave_on_device_head_provider.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) 2025 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 "brave/components/omnibox/browser/brave_on_device_head_provider.h"

#include "brave/components/omnibox/browser/brave_omnibox_prefs.h"
#include "components/omnibox/browser/autocomplete_provider_client.h"
#include "components/omnibox/browser/autocomplete_provider_listener.h"
#include "components/prefs/pref_service.h"

// static
BraveOnDeviceHeadProvider* BraveOnDeviceHeadProvider::Create(
AutocompleteProviderClient* client,
AutocompleteProviderListener* listener) {
return new BraveOnDeviceHeadProvider(client, listener);
}

void BraveOnDeviceHeadProvider::Start(const AutocompleteInput& input,
bool minimal_changes) {
auto* prefs = client_->GetPrefs();
if (!prefs || !prefs->GetBoolean(omnibox::kTopSuggestionsEnabled)) {
matches_.clear();
return;
}
OnDeviceHeadProvider::Start(input, minimal_changes);
}

BraveOnDeviceHeadProvider::~BraveOnDeviceHeadProvider() = default;
24 changes: 24 additions & 0 deletions components/omnibox/browser/brave_on_device_head_provider.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright (c) 2025 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_COMPONENTS_OMNIBOX_BROWSER_BRAVE_ON_DEVICE_HEAD_PROVIDER_H_
#define BRAVE_COMPONENTS_OMNIBOX_BROWSER_BRAVE_ON_DEVICE_HEAD_PROVIDER_H_

#include "components/omnibox/browser/on_device_head_provider.h"

class BraveOnDeviceHeadProvider : public OnDeviceHeadProvider {
public:
static BraveOnDeviceHeadProvider* Create(
AutocompleteProviderClient* client,
AutocompleteProviderListener* listener);

void Start(const AutocompleteInput& input, bool minimal_changes) override;

protected:
using OnDeviceHeadProvider::OnDeviceHeadProvider;
~BraveOnDeviceHeadProvider() override;
};

#endif // BRAVE_COMPONENTS_OMNIBOX_BROWSER_BRAVE_ON_DEVICE_HEAD_PROVIDER_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright (c) 2025 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 "brave/components/omnibox/browser/brave_on_device_head_provider.h"

#include <memory>
#include <string_view>

#include "base/memory/scoped_refptr.h"
#include "base/strings/utf_string_conversions.h"
#include "brave/components/omnibox/browser/brave_fake_autocomplete_provider_client.h"
#include "brave/components/omnibox/browser/brave_omnibox_prefs.h"
#include "components/omnibox/browser/autocomplete_input.h"
#include "components/omnibox/browser/autocomplete_provider_listener.h"
#include "components/omnibox/browser/test_scheme_classifier.h"
#include "components/prefs/pref_service.h"
#include "content/public/test/browser_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"

class BraveOnDeviceHeadProviderTest : public testing::Test,
public AutocompleteProviderListener {
public:
AutocompleteInput CreateAutocompleteInput(std::string_view text) {
AutocompleteInput input(base::UTF8ToUTF16(text),
metrics::OmniboxEventProto::OTHER, classifier_);
return input;
}

void SetUp() override {
provider_ =
base::WrapRefCounted(BraveOnDeviceHeadProvider::Create(&client_, this));
}

void OnProviderUpdate(bool updated_matches,
const AutocompleteProvider* provider) override {}

PrefService* prefs() { return client_.GetPrefs(); }

protected:
content::BrowserTaskEnvironment task_environment_;
TestSchemeClassifier classifier_;
BraveFakeAutocompleteProviderClient client_;
scoped_refptr<BraveOnDeviceHeadProvider> provider_;
};

TEST_F(BraveOnDeviceHeadProviderTest, SuggestionsDisabledNoResults) {
prefs()->SetBoolean(omnibox::kTopSuggestionsEnabled, false);
provider_->Start(CreateAutocompleteInput("Hello"), false);
EXPECT_TRUE(provider_->done());
EXPECT_TRUE(provider_->matches().empty());
}

TEST_F(BraveOnDeviceHeadProviderTest, SuggestionsEnabledRunsProvider) {
prefs()->SetBoolean(omnibox::kTopSuggestionsEnabled, true);
provider_->Start(CreateAutocompleteInput("Hello"), false);
EXPECT_FALSE(provider_->done());
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ class OmniboxAutocompleteUnitTest : public testing::Test {
};

TEST_F(OmniboxAutocompleteUnitTest, TopSiteSuggestionsEnabledTest) {
EXPECT_TRUE(prefs()->GetBoolean(omnibox::kTopSiteSuggestionsEnabled));
EXPECT_TRUE(prefs()->GetBoolean(omnibox::kTopSuggestionsEnabled));
}
2 changes: 1 addition & 1 deletion components/omnibox/browser/promotion_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class OmniboxPromotionTest : public testing::Test {
void SetUp() override {
RegisterPrefs(pref_service_.registry());
omnibox::RegisterBraveProfilePrefs(pref_service_.registry());
pref_service_.SetBoolean(omnibox::kTopSiteSuggestionsEnabled, false);
pref_service_.SetBoolean(omnibox::kTopSuggestionsEnabled, false);

scoped_default_locale_ =
std::make_unique<brave_l10n::test::ScopedDefaultLocale>("en_US");
Expand Down
2 changes: 2 additions & 0 deletions components/omnibox/browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ brave_components_omnibox_browser_sources = [
"//brave/components/omnibox/browser/brave_local_history_zero_suggest_provider.h",
"//brave/components/omnibox/browser/brave_omnibox_prefs.cc",
"//brave/components/omnibox/browser/brave_omnibox_prefs.h",
"//brave/components/omnibox/browser/brave_on_device_head_provider.cc",
"//brave/components/omnibox/browser/brave_on_device_head_provider.h",
"//brave/components/omnibox/browser/brave_search_provider.cc",
"//brave/components/omnibox/browser/brave_search_provider.h",
"//brave/components/omnibox/browser/brave_search_suggestion_parser.cc",
Expand Down
2 changes: 1 addition & 1 deletion components/omnibox/browser/topsites_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ void TopSitesProvider::Start(const AutocompleteInput& input,
bool minimal_changes) {
matches_.clear();
auto* prefs = client_->GetPrefs();
if (!prefs || !prefs->GetBoolean(omnibox::kTopSiteSuggestionsEnabled)) {
if (!prefs || !prefs->GetBoolean(omnibox::kTopSuggestionsEnabled)) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion components/omnibox/browser/topsites_provider_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ TEST_F(TopSitesProviderTest, SmokeTest) {
}

TEST_F(TopSitesProviderTest, NoMatchingWhenPrefIsOff) {
prefs()->SetBoolean(omnibox::kTopSiteSuggestionsEnabled, false);
prefs()->SetBoolean(omnibox::kTopSuggestionsEnabled, false);
provider_->Start(CreateAutocompleteInput("dex"), false);
EXPECT_TRUE(provider_->matches().empty());
}
Loading