Skip to content

Commit

Permalink
Address Web Discovery feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
DJAndries committed Oct 25, 2024
1 parent 5715814 commit ba4c64a
Show file tree
Hide file tree
Showing 40 changed files with 455 additions and 310 deletions.
2 changes: 1 addition & 1 deletion android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import("//brave/components/ai_chat/core/common/buildflags/buildflags.gni")
import("//brave/components/p3a/buildflags.gni")
import("//brave/components/web_discovery/common/buildflags/buildflags.gni")
import("//brave/components/web_discovery/buildflags/buildflags.gni")
import("//brave/components/webcompat_reporter/buildflags/buildflags.gni")
import("//build/config/android/rules.gni")

Expand Down
2 changes: 1 addition & 1 deletion browser/brave_local_state_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#include "brave/components/p3a/star_randomness_meta.h"
#include "brave/components/skus/browser/skus_utils.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "build/build_config.h"
#include "chrome/common/pref_names.h"
#include "components/metrics/metrics_pref_names.h"
Expand Down
2 changes: 1 addition & 1 deletion browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
#include "brave/components/search_engines/brave_prepopulated_engines.h"
#include "brave/components/speedreader/common/buildflags/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "build/build_config.h"
#include "chrome/browser/prefetch/pref_names.h"
#include "chrome/browser/prefs/session_startup_pref.h"
Expand Down
2 changes: 1 addition & 1 deletion browser/browser_context_keyed_service_factories.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
#include "brave/components/request_otr/common/buildflags/buildflags.h"
#include "brave/components/speedreader/common/buildflags/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"

#if BUILDFLAG(ENABLE_BRAVE_VPN)
#include "brave/browser/brave_vpn/brave_vpn_service_factory.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "brave/components/request_otr/common/pref_names.h"
#include "brave/components/speedreader/common/buildflags/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/extensions/api/settings_private/prefs_util.h"
Expand Down
2 changes: 1 addition & 1 deletion browser/profiles/brave_profile_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "brave/components/ntp_background_images/common/pref_names.h"
#include "brave/components/request_otr/common/buildflags/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/profiles/profile_attributes_entry.h"
Expand Down
2 changes: 1 addition & 1 deletion browser/resources/settings/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import("//brave/build/config.gni")
import("//brave/components/brave_vpn/common/buildflags/buildflags.gni")
import("//brave/components/brave_wayback_machine/buildflags/buildflags.gni")
import("//brave/components/tor/buildflags/buildflags.gni")
import("//brave/components/web_discovery/common/buildflags/buildflags.gni")
import("//brave/components/web_discovery/buildflags/buildflags.gni")
import("//brave/resources/brave_grit.gni")
import("//chrome/common/features.gni")
import("//extensions/buildflags/buildflags.gni")
Expand Down
2 changes: 1 addition & 1 deletion browser/search_engines/search_engine_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ void SearchEngineTracker::RecordWebDiscoveryEnabledP3A() {
#endif
#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
if (base::FeatureList::IsEnabled(
web_discovery::features::kWebDiscoveryNative)) {
web_discovery::features::kBraveWebDiscoveryNative)) {
enabled =
profile_prefs_->GetBoolean(web_discovery::kWebDiscoveryNativeEnabled);
}
Expand Down
2 changes: 1 addition & 1 deletion browser/search_engines/search_engine_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include "base/memory/raw_ptr.h"
#include "base/scoped_observation.h"
#include "brave/components/time_period_storage/weekly_event_storage.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/prefs/pref_change_registrar.h"
Expand Down
6 changes: 3 additions & 3 deletions browser/sources.gni
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import("//brave/components/brave_webtorrent/browser/buildflags/buildflags.gni")
import("//brave/components/commander/common/buildflags/buildflags.gni")
import("//brave/components/greaselion/browser/buildflags/buildflags.gni")
import("//brave/components/tor/buildflags/buildflags.gni")
import("//brave/components/web_discovery/common/buildflags/buildflags.gni")
import("//brave/components/web_discovery/buildflags/buildflags.gni")
import("//extensions/buildflags/buildflags.gni")

brave_chrome_browser_visibility = [
Expand Down Expand Up @@ -215,8 +215,7 @@ brave_chrome_browser_deps = [
"//brave/components/speedreader/common/buildflags",
"//brave/components/tor/buildflags",
"//brave/components/version_info",
"//brave/components/web_discovery/common",
"//brave/components/web_discovery/common/buildflags",
"//brave/components/web_discovery/buildflags",
"//brave/components/webcompat/content/browser",
"//brave/components/webcompat/core/common",
"//brave/services/network/public/cpp",
Expand Down Expand Up @@ -365,6 +364,7 @@ if (enable_web_discovery_native) {
brave_chrome_browser_deps += [
"//brave/browser/web_discovery",
"//brave/components/web_discovery/browser",
"//brave/components/web_discovery/common",
]
}

Expand Down
8 changes: 3 additions & 5 deletions browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import("//brave/components/request_otr/common/buildflags/buildflags.gni")
import("//brave/components/speedreader/common/buildflags/buildflags.gni")
import("//brave/components/text_recognition/common/buildflags/buildflags.gni")
import("//brave/components/tor/buildflags/buildflags.gni")
import("//brave/components/web_discovery/common/buildflags/buildflags.gni")
import("//brave/components/web_discovery/buildflags/buildflags.gni")
import("//build/config/features.gni")
import("//chrome/common/features.gni")
import("//components/gcm_driver/config.gni")
Expand Down Expand Up @@ -835,6 +835,7 @@ source_set("ui") {
"//brave/components/tor/buildflags",
"//brave/components/url_sanitizer/browser",
"//brave/components/vector_icons",
"//brave/components/web_discovery/buildflags",
"//brave/components/webui",
"//chrome/app:command_ids",
"//chrome/app/vector_icons:vector_icons",
Expand Down Expand Up @@ -903,10 +904,7 @@ source_set("ui") {
}

if (enable_web_discovery_native) {
deps += [
"//brave/components/web_discovery/common",
"//brave/components/web_discovery/common/buildflags",
]
deps += [ "//brave/components/web_discovery/common" ]
}

if (is_linux) {
Expand Down
14 changes: 9 additions & 5 deletions browser/ui/webui/brave_settings_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@
#include "brave/components/speedreader/common/buildflags/buildflags.h"
#include "brave/components/tor/buildflags/buildflags.h"
#include "brave/components/version_info/version_info.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/common/features.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "build/build_config.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/settings/metrics_reporting_handler.h"
Expand Down Expand Up @@ -93,6 +92,10 @@
#include "brave/components/playlist/common/features.h"
#endif

#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
#include "brave/components/web_discovery/common/features.h"
#endif

using ntp_background_images::ViewCounterServiceFactory;

BraveSettingsUI::BraveSettingsUI(content::WebUI* web_ui) : SettingsUI(web_ui) {
Expand Down Expand Up @@ -195,9 +198,10 @@ void BraveSettingsUI::AddResources(content::WebUIDataSource* html_source,

html_source->AddBoolean("enable_extensions", BUILDFLAG(ENABLE_EXTENSIONS));
#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
html_source->AddBoolean("isWebDiscoveryNativeEnabled",
base::FeatureList::IsEnabled(
web_discovery::features::kWebDiscoveryNative));
html_source->AddBoolean(
"isWebDiscoveryNativeEnabled",
base::FeatureList::IsEnabled(
web_discovery::features::kBraveWebDiscoveryNative));
#endif

html_source->AddBoolean("extensionsManifestV2Feature",
Expand Down
4 changes: 2 additions & 2 deletions browser/web_discovery/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
# 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/.

import("//brave/components/web_discovery/common/buildflags/buildflags.gni")
import("//brave/components/web_discovery/buildflags/buildflags.gni")
import("//extensions/buildflags/buildflags.gni")

if (enable_web_discovery_native) {
source_set("web_discovery") {
static_library("web_discovery") {
sources = [
"web_discovery_service_factory.cc",
"web_discovery_service_factory.h",
Expand Down
4 changes: 2 additions & 2 deletions browser/web_discovery/web_discovery_cta_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "brave/components/constants/pref_names.h"
#include "brave/components/constants/url_constants.h"
#include "brave/components/search_engines/brave_prepopulated_engines.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/search_engines/template_url.h"
Expand Down Expand Up @@ -96,7 +96,7 @@ bool ShouldShowWebDiscoveryInfoBar(TemplateURLService* service,
const char* enabled_pref_name = kWebDiscoveryExtensionEnabled;
#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
if (base::FeatureList::IsEnabled(
web_discovery::features::kWebDiscoveryNative)) {
web_discovery::features::kBraveWebDiscoveryNative)) {
enabled_pref_name = web_discovery::kWebDiscoveryNativeEnabled;
}
#endif
Expand Down
4 changes: 2 additions & 2 deletions browser/web_discovery/web_discovery_infobar_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include "brave/browser/web_discovery/web_discovery_cta_util.h"
#include "brave/components/constants/pref_names.h"
#include "brave/components/web_discovery/common/buildflags/buildflags.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "components/infobars/core/infobar.h"
#include "components/prefs/pref_service.h"

Expand Down Expand Up @@ -51,7 +51,7 @@ void WebDiscoveryInfoBarDelegate::EnableWebDiscovery() {
const char* pref_name = kWebDiscoveryExtensionEnabled;
#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
if (base::FeatureList::IsEnabled(
web_discovery::features::kWebDiscoveryNative)) {
web_discovery::features::kBraveWebDiscoveryNative)) {
pref_name = web_discovery::kWebDiscoveryNativeEnabled;
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion browser/web_discovery/web_discovery_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ KeyedService* WebDiscoveryServiceFactory::BuildServiceInstanceFor(

content::BrowserContext* WebDiscoveryServiceFactory::GetBrowserContextToUse(
content::BrowserContext* context) const {
if (!base::FeatureList::IsEnabled(features::kWebDiscoveryNative)) {
if (!base::FeatureList::IsEnabled(features::kBraveWebDiscoveryNative)) {
return nullptr;
}
// Prevents creation of service instance for incognito/OTR profiles
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
namespace web_discovery {

TEST(WebDiscoveryServiceFactoryTest, PrivateNotCreated) {
base::test::ScopedFeatureList scoped_features(
features::kBraveWebDiscoveryNative);
content::BrowserTaskEnvironment task_environment;
base::test::ScopedFeatureList scoped_features(features::kWebDiscoveryNative);
auto* browser_process = TestingBrowserProcess::GetGlobal();
TestingProfileManager profile_manager(browser_process);
ASSERT_TRUE(profile_manager.SetUp());
Expand Down
2 changes: 1 addition & 1 deletion chromium_src/chrome/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ include_rules = [
"+brave/components/url_sanitizer",
"+brave/components/vector_icons",
"+brave/components/version_info",
"+brave/components/web_discovery/common",
"+brave/components/web_discovery",
"+brave/components/webcompat",
"+brave/net",
"+brave/services/network/public",
Expand Down
15 changes: 12 additions & 3 deletions chromium_src/chrome/browser/flags/android/chrome_feature_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "brave/components/playlist/common/features.h"
#include "brave/components/request_otr/common/features.h"
#include "brave/components/speedreader/common/features.h"
#include "brave/components/web_discovery/common/features.h"
#include "brave/components/web_discovery/buildflags/buildflags.h"
#include "brave/components/webcompat/core/common/features.h"
#include "net/base/features.h"
#include "third_party/blink/public/common/features.h"
Expand All @@ -30,9 +30,18 @@
#define BRAVE_AI_CHAT_FLAG
#endif

#if BUILDFLAG(ENABLE_WEB_DISCOVERY_NATIVE)
#include "brave/components/web_discovery/common/features.h"
#define BRAVE_WEB_DISCOVERY_FLAG \
&web_discovery::features::kBraveWebDiscoveryNative,
#else
#define BRAVE_WEB_DISCOVERY_FLAG
#endif

// clang-format off
#define kForceWebContentsDarkMode kForceWebContentsDarkMode, \
BRAVE_AI_CHAT_FLAG \
BRAVE_WEB_DISCOVERY_FLAG \
&brave_rewards::features::kBraveRewards, \
&brave_search_conversion::features::kOmniboxBanner, \
&brave_vpn::features::kBraveVPNLinkSubscriptionAndroidUI, \
Expand All @@ -50,14 +59,14 @@
&google_sign_in_permission::features::kBraveGoogleSignInPermission, \
&net::features::kBraveForgetFirstPartyStorage, \
&brave_shields::features::kBraveShowStrictFingerprintingMode, \
&brave_shields::features::kBraveLocalhostAccessPermission, \
&web_discovery::features::kWebDiscoveryNative
&brave_shields::features::kBraveLocalhostAccessPermission

// clang-format on

#include "src/chrome/browser/flags/android/chrome_feature_list.cc"
#undef kForceWebContentsDarkMode
#undef BRAVE_AI_CHAT_FLAG
#undef BRAVE_WEB_DISCOVERY_FLAG

namespace chrome {
namespace android {
Expand Down
4 changes: 1 addition & 3 deletions components/web_discovery/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@
# 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/.

import("//brave/components/web_discovery/common/buildflags/buildflags.gni")
import("//brave/components/web_discovery/buildflags/buildflags.gni")

assert(enable_web_discovery_native)

static_library("browser") {
sources = [
"credential_manager.cc",
"credential_manager.h",
"credential_signer.cc",
"credential_signer.h",
"patterns.cc",
"patterns.h",
Expand All @@ -33,7 +32,6 @@ static_library("browser") {
"//brave/components/web_discovery/common",
"//components/keyed_service/core",
"//components/prefs",
"//content/public/browser",
"//crypto",
"//extensions/buildflags",
"//net",
Expand Down
1 change: 1 addition & 0 deletions components/web_discovery/browser/DEPS
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
include_rules = [
"-content",
"+services/network/public",
"+extensions/buildflags/buildflags.h",
"+services/service_manager/public/cpp",
Expand Down
Loading

0 comments on commit ba4c64a

Please sign in to comment.