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

Followup #3 to "Implement Search Result and New Tab Page ads attribution if ads are enabled or disabled" #18770

Merged
merged 6 commits into from
Jun 6, 2023
41 changes: 40 additions & 1 deletion browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "brave/browser/ethereum_remote_client/buildflags/buildflags.h"
#include "brave/browser/ethereum_remote_client/features.h"
#include "brave/browser/ui/tabs/features.h"
#include "brave/components/brave_ads/common/brave_ads_feature.h"
#include "brave/components/brave_ads/common/custom_notification_ad_feature.h"
#include "brave/components/brave_ads/common/notification_ad_feature.h"
#include "brave/components/brave_component_updater/browser/features.h"
Expand Down Expand Up @@ -682,6 +683,45 @@
FEATURE_VALUE_TYPE(brave_rewards::features:: \
kAllowUnsupportedWalletProvidersFeature), \
}, \
{ \
"brave-ads-should-launch-brave-ads-as-an-in-process-service", \
"Launch Brave Ads as an in-process service", \
"Launch Brave Ads as an in-process service removing the utility " \
"process.", \
kOsAll, \
FEATURE_VALUE_TYPE( \
brave_ads::kShouldLaunchBraveAdsAsAnInProcessServiceFeature), \
}, \
{ \
"brave-ads-should-always-run-brave-ads-service", \
"Should always run Brave Ads service", \
"Always run Brave Ads service to support triggering ad events when " \
"Brave Private Ads are disabled.", \
kOsAll, \
FEATURE_VALUE_TYPE( \
brave_ads::kShouldAlwaysRunBraveAdsServiceFeature), \
}, \
{ \
"brave-ads-should-always-trigger-new-tab-page-ad-events", \
"Should always trigger new tab page ad events", \
"Support triggering new tab page ad events if Brave Private Ads " \
"are disabled. Requires " \
"#brave-ads-should-always-run-brave-ads-service to be enabled.", \
kOsAll, \
FEATURE_VALUE_TYPE( \
brave_ads::kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature), \
}, \
{ \
"brave-ads-should-always-trigger-search-result-ad-events", \
"Should always trigger search result ad events", \
"Support triggering search result ad events if Brave Private Ads " \
"are disabled. Requires " \
"#brave-ads-should-always-run-brave-ads-service to be enabled.", \
kOsAll, \
FEATURE_VALUE_TYPE( \
brave_ads:: \
kShouldAlwaysTriggerBraveSearchResultAdEventsFeature), \
}, \
{ \
"brave-ads-custom-push-notifications-ads", \
"Enable Brave Ads custom push notifications", \
Expand Down Expand Up @@ -839,7 +879,6 @@
BRAVE_SHARED_PINNED_TABS \
BRAVE_AI_CHAT \
LAST_BRAVE_FEATURE_ENTRIES_ITEM // Keep it as the last item.

namespace flags_ui {
namespace {

Expand Down
5 changes: 2 additions & 3 deletions browser/brave_ads/services/bat_ads_service_factory_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ mojo::Remote<bat_ads::mojom::BatAdsService> BatAdsServiceFactoryImpl::Launch()
const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

return kShouldLaunchAsInProcessRunService.Get()
? LaunchInProcessBatAdsService()
: LaunchOutOfProcessBatAdsService();
return ShouldLaunchAsInProcessService() ? LaunchInProcessBatAdsService()
: LaunchOutOfProcessBatAdsService();
}

} // namespace brave_ads
6 changes: 3 additions & 3 deletions components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ AdsServiceImpl::AdsServiceImpl(

rewards_service_->AddObserver(this);

if (kShouldAlwaysRunService.Get()) {
if (ShouldAlwaysRunService()) {
RegisterResourceComponentsForDefaultLocale();
}
}
Expand Down Expand Up @@ -379,7 +379,7 @@ void AdsServiceImpl::GetDeviceIdCallback(std::string device_id) {
bool AdsServiceImpl::CanStartBatAdsService() const {
return IsSupportedRegion() &&
(UserHasOptedInToBravePrivateAds() || UserHasOptedInToBraveNews() ||
kShouldAlwaysRunService.Get());
ShouldAlwaysRunService());
}

void AdsServiceImpl::MaybeStartBatAdsService() {
Expand Down Expand Up @@ -547,7 +547,7 @@ void AdsServiceImpl::InitializeBatAdsCallback(const bool success) {

is_bat_ads_initialized_ = true;

if (!kShouldAlwaysRunService.Get()) {
if (!ShouldAlwaysRunService()) {
RegisterResourceComponentsForDefaultLocale();
}

Expand Down
35 changes: 32 additions & 3 deletions components/brave_ads/common/brave_ads_feature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,39 @@

namespace brave_ads {

BASE_FEATURE(kBraveAdsFeature, "BraveAds", base::FEATURE_ENABLED_BY_DEFAULT);
BASE_FEATURE(kShouldLaunchBraveAdsAsAnInProcessServiceFeature,
"ShouldLaunchBraveAdsAsInProcessService",
base::FEATURE_DISABLED_BY_DEFAULT);

bool IsBraveAdsFeatureEnabled() {
return base::FeatureList::IsEnabled(kBraveAdsFeature);
bool ShouldLaunchAsInProcessService() {
return base::FeatureList::IsEnabled(
kShouldLaunchBraveAdsAsAnInProcessServiceFeature);
}

BASE_FEATURE(kShouldAlwaysRunBraveAdsServiceFeature,
"ShouldAlwaysRunBraveAdsService",
base::FEATURE_DISABLED_BY_DEFAULT);

bool ShouldAlwaysRunService() {
return base::FeatureList::IsEnabled(kShouldAlwaysRunBraveAdsServiceFeature);
}

BASE_FEATURE(kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature,
"ShouldAlwaysTriggerBraveNewTabPageAdEvents",
base::FEATURE_DISABLED_BY_DEFAULT);

bool ShouldAlwaysTriggerNewTabPageAdEvents() {
return base::FeatureList::IsEnabled(
kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature);
}

BASE_FEATURE(kShouldAlwaysTriggerBraveSearchResultAdEventsFeature,
"ShouldAlwaysTriggerBraveSearchResultAdEvents",
base::FEATURE_DISABLED_BY_DEFAULT);

bool ShouldAlwaysTriggerSearchResultAdEvents() {
return base::FeatureList::IsEnabled(
kShouldAlwaysTriggerBraveSearchResultAdEventsFeature);
}

} // namespace brave_ads
29 changes: 14 additions & 15 deletions components/brave_ads/common/brave_ads_feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,33 @@
#define BRAVE_COMPONENTS_BRAVE_ADS_COMMON_BRAVE_ADS_FEATURE_H_

#include "base/feature_list.h"
#include "base/metrics/field_trial_params.h"

namespace brave_ads {

BASE_DECLARE_FEATURE(kBraveAdsFeature);

bool IsBraveAdsFeatureEnabled();

// Set to |true| to launch as an in process service.
constexpr base::FeatureParam<bool> kShouldLaunchAsInProcessRunService{
&kBraveAdsFeature, "should_launch_as_in_process_service", false};
BASE_DECLARE_FEATURE(kShouldLaunchBraveAdsAsAnInProcessServiceFeature);

bool ShouldLaunchAsInProcessService();

// Set to |true| to always run the ads service, even if Brave Private Ads are
// disabled.
constexpr base::FeatureParam<bool> kShouldAlwaysRunService{
&kBraveAdsFeature, "should_always_run_service", false};
BASE_DECLARE_FEATURE(kShouldAlwaysRunBraveAdsServiceFeature);

bool ShouldAlwaysRunService();

// Set to |true| to always trigger new tab page ad events even if Brave Private
// Ads are disabled. |kShouldAlwaysRunService| must be set to |true|, otherwise
// Ads are disabled. |ShouldAlwaysRunService| must be set to |true|, otherwise
// this feature param will be ignored.
constexpr base::FeatureParam<bool> kShouldAlwaysTriggerNewTabPageAdEvents{
&kBraveAdsFeature, "should_always_trigger_new_tab_page_ad_events", false};
BASE_DECLARE_FEATURE(kShouldAlwaysTriggerBraveNewTabPageAdEventsFeature);

bool ShouldAlwaysTriggerNewTabPageAdEvents();

// Set to |true| to always trigger search result ad events even if Brave Private
// Ads are disabled. |kShouldAlwaysRunService| must be set to |true|, otherwise
// Ads are disabled. |ShouldAlwaysRunService| must be set to |true|, otherwise
// this feature param will be ignored.
constexpr base::FeatureParam<bool> kShouldAlwaysTriggerSearchResultAdEvents{
&kBraveAdsFeature, "should_always_trigger_search_result_ad_events", false};
BASE_DECLARE_FEATURE(kShouldAlwaysTriggerBraveSearchResultAdEventsFeature);

bool ShouldAlwaysTriggerSearchResultAdEvents();

} // namespace brave_ads

Expand Down
Loading