Skip to content

Commit

Permalink
Merge pull request #18021 from brave/issues/29657
Browse files Browse the repository at this point in the history
Clean-up Brave Ads manager classes.
  • Loading branch information
aseren authored Apr 18, 2023
2 parents 76ec8d3 + 8195270 commit 5c5bdd0
Show file tree
Hide file tree
Showing 157 changed files with 982 additions and 1,108 deletions.
81 changes: 34 additions & 47 deletions components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <utility>

#include "base/base64.h"
#include "base/base_switches.h"
#include "base/check.h"
#include "base/containers/circular_deque.h"
#include "base/containers/flat_map.h"
Expand All @@ -24,7 +23,6 @@
#include "base/no_destructor.h"
#include "base/notreached.h"
#include "base/ranges/algorithm.h"
#include "base/strings/strcat.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task/sequenced_task_runner.h"
Expand All @@ -46,9 +44,9 @@
#include "brave/components/brave_ads/common/features.h"
#include "brave/components/brave_ads/common/pref_names.h"
#include "brave/components/brave_ads/core/ad_constants.h"
#include "brave/components/brave_ads/core/ad_switches.h" // IWYU pragma: keep
#include "brave/components/brave_ads/core/ads_util.h"
#include "brave/components/brave_ads/core/database.h"
#include "brave/components/brave_ads/core/flags_util.h"
#include "brave/components/brave_ads/core/new_tab_page_ad_info.h"
#include "brave/components/brave_ads/core/new_tab_page_ad_value_util.h"
#include "brave/components/brave_ads/core/notification_ad_info.h"
Expand All @@ -60,7 +58,6 @@
#include "brave/components/brave_rewards/browser/rewards_service.h"
#include "brave/components/brave_rewards/common/mojom/ledger.mojom.h"
#include "brave/components/brave_rewards/common/pref_names.h"
#include "brave/components/brave_rewards/common/rewards_flags.h"
#include "brave/components/l10n/common/locale_util.h"
#include "brave/grit/brave_generated_resources.h"
#include "build/build_config.h"
Expand Down Expand Up @@ -111,10 +108,6 @@ constexpr int kHttpUpgradeRequiredStatusResponseCode = 426;

constexpr char kNotificationAdUrlPrefix[] = "https://www.brave.com/ads/?";

constexpr char kFeaturesValueSeparator[] = ",";

constexpr char kSwitchValueSeparator[] = "=";

BASE_FEATURE(kFeature, "NotificationAds", base::FEATURE_ENABLED_BY_DEFAULT);

int GetDataResourceId(const std::string& name) {
Expand Down Expand Up @@ -283,32 +276,6 @@ void OnURLResponseStarted(
}
}

std::vector<std::string> ExtraCommandLineSwitches() {
std::vector<std::string> command_line_switches;

const std::string& rewards_command_line_switch =
brave_rewards::RewardsFlags::GetCommandLineSwitchASCII();
if (!rewards_command_line_switch.empty()) {
command_line_switches.push_back(rewards_command_line_switch);
}

const base::CommandLine* command_line =
base::CommandLine::ForCurrentProcess();
const std::string enabled_features =
command_line->GetSwitchValueASCII(::switches::kEnableFeatures);
const std::string disabled_features =
command_line->GetSwitchValueASCII(::switches::kDisableFeatures);

if (!enabled_features.empty() || !disabled_features.empty()) {
const std::string affected_features = base::StrCat(
{enabled_features, kFeaturesValueSeparator, disabled_features});
command_line_switches.push_back(base::StrCat(
{switches::kFeaturesSwitch, kSwitchValueSeparator, enabled_features}));
}

return command_line_switches;
}

void OnResetState(const bool success) {
if (!success) {
VLOG(1) << "Failed to reset ads state";
Expand Down Expand Up @@ -445,24 +412,18 @@ void AdsServiceImpl::StartBatAdsService() {
bat_ads_service_.BindNewPipeAndPassReceiver(),
content::ServiceProcessHost::Options()
.WithDisplayName(IDS_SERVICE_BAT_ADS)
.WithExtraCommandLineSwitches(ExtraCommandLineSwitches())
.Pass());

bat_ads_service_.set_disconnect_handler(base::BindOnce(
&AdsServiceImpl::RestartBatAdsServiceAfterDelay, AsWeakPtr()));

DCHECK(bat_ads_service_.is_bound());

SetSysInfo();

SetBuildChannel();

bat_ads_service_->Create(
bat_ads_client_.BindNewEndpointAndPassRemote(),
bat_ads_.BindNewEndpointAndPassReceiver(),
bat_ads_client_notifier_.BindNewPipeAndPassReceiver(),
base::BindOnce(&AdsServiceImpl::InitializeBasePathDirectory,
AsWeakPtr()));
base::BindOnce(&AdsServiceImpl::OnBatAdsServiceCreated, AsWeakPtr()));
}

void AdsServiceImpl::RestartBatAdsServiceAfterDelay() {
Expand All @@ -483,7 +444,13 @@ void AdsServiceImpl::CancelRestartBatAdsService() {
}
}

void AdsServiceImpl::InitializeBasePathDirectory() {
void AdsServiceImpl::OnBatAdsServiceCreated() {
SetSysInfo();

SetBuildChannel();

SetFlags();

file_task_runner_->PostTaskAndReplyWithResult(
FROM_HERE,
base::BindOnce(&EnsureBaseDirectoryExistsOnFileTaskRunner, base_path_),
Expand Down Expand Up @@ -582,18 +549,38 @@ void AdsServiceImpl::ShutdownAndResetState() {
}

void AdsServiceImpl::SetSysInfo() {
DCHECK(bat_ads_service_.is_bound());
bat_ads_service_->SetSysInfo(sys_info_.Clone(), base::NullCallback());
if (bat_ads_.is_bound()) {
bat_ads_->SetSysInfo(sys_info_.Clone());
}
}

void AdsServiceImpl::SetBuildChannel() {
if (!bat_ads_.is_bound()) {
return;
}

mojom::BuildChannelInfoPtr build_channel = mojom::BuildChannelInfo::New();
build_channel->name = brave::GetChannelName();
build_channel->is_release = build_channel->name == "release";

DCHECK(bat_ads_service_.is_bound());
bat_ads_service_->SetBuildChannel(std::move(build_channel),
base::NullCallback());
bat_ads_->SetBuildChannel(std::move(build_channel));
}

void AdsServiceImpl::SetFlags() {
if (!bat_ads_.is_bound()) {
return;
}

mojom::FlagsPtr flags = BuildFlags();
DCHECK(flags);
#if BUILDFLAG(IS_ANDROID)
if (GetPrefService()->GetBoolean(
brave_rewards::prefs::kUseRewardsStagingServer)) {
flags->environment_type = mojom::EnvironmentType::kStaging;
}
#endif // BUILDFLAG(IS_ANDROID)

bat_ads_->SetFlags(std::move(flags));
}

void AdsServiceImpl::CleanUpOnFirstRun() {
Expand Down
4 changes: 2 additions & 2 deletions components/brave_ads/browser/ads_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class AdsServiceImpl : public AdsService,
void RestartBatAdsServiceAfterDelay();
void CancelRestartBatAdsService();

void InitializeBasePathDirectory();
void OnBatAdsServiceCreated();
void OnInitializeBasePathDirectory(bool success);
void Initialize();
void InitializeDatabase();
Expand All @@ -121,8 +121,8 @@ class AdsServiceImpl : public AdsService,
void ShutdownAndResetState();

void SetSysInfo();

void SetBuildChannel();
void SetFlags();

void CleanUpOnFirstRun();
void RemoveDeprecatedFiles() const;
Expand Down
11 changes: 11 additions & 0 deletions components/brave_ads/common/interfaces/ads.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ struct BuildChannelInfo {
string name;
};

enum EnvironmentType {
kStaging = 0,
kProduction
};

struct Flags {
bool should_debug;
bool did_override_from_command_line;
EnvironmentType environment_type;
};

struct StatementInfo {
double earnings_last_month;
double earnings_this_month;
Expand Down
4 changes: 1 addition & 3 deletions components/brave_ads/core/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ source_set("headers") {
"ad_content_value_util.h",
"ad_event_history.h",
"ad_info.h",
"ad_switches.h",
"ad_type.h",
"ads.h",
"ads_callback.h",
Expand All @@ -41,12 +40,12 @@ source_set("headers") {
"ads_client_notifier.h",
"ads_client_notifier_observer.h",
"ads_util.h",
"build_channel.h",
"category_content_action_types.h",
"category_content_info.h",
"confirmation_type.h",
"database.h",
"export.h",
"flags_util.h",
"history_filter_types.h",
"history_item_info.h",
"history_item_value_util.h",
Expand All @@ -67,7 +66,6 @@ source_set("headers") {
"promoted_content_ad_info.h",
"promoted_content_ad_value_util.h",
"supported_subdivisions.h",
"sys_info.h",
]

deps = [
Expand Down
17 changes: 0 additions & 17 deletions components/brave_ads/core/ad_switches.h

This file was deleted.

6 changes: 6 additions & 0 deletions components/brave_ads/core/ads.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ class ADS_EXPORT Ads {

static Ads* CreateInstance(AdsClient* ads_client);

virtual void SetSysInfo(mojom::SysInfoPtr sys_info) = 0;

virtual void SetBuildChannel(mojom::BuildChannelInfoPtr build_channel) = 0;

virtual void SetFlags(mojom::FlagsPtr flags) = 0;

// Called to initialize ads. The callback takes one argument - |bool| is set
// to |true| if successful otherwise |false|.
virtual void Initialize(InitializeCallback callback) = 0;
Expand Down
19 changes: 0 additions & 19 deletions components/brave_ads/core/build_channel.h

This file was deleted.

18 changes: 18 additions & 0 deletions components/brave_ads/core/flags_util.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* Copyright (c) 2023 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_BRAVE_ADS_CORE_FLAGS_UTIL_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_FLAGS_UTIL_H_

#include "brave/components/brave_ads/common/interfaces/ads.mojom-forward.h"

namespace brave_ads {

// Builds Flags basing on command line arguments and environment.
mojom::FlagsPtr BuildFlags();

} // namespace brave_ads

#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_FLAGS_UTIL_H_
15 changes: 6 additions & 9 deletions components/brave_ads/core/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ source_set("internal") {
"ad_content_value_util.cc",
"ad_event_history.cc",
"ad_info.cc",
"ad_switches.cc",
"ad_type.cc",
"ads.cc",
"ads/ad_events/ad_event_info.cc",
Expand Down Expand Up @@ -486,7 +485,6 @@ source_set("internal") {
"browser/browser_manager_observer.h",
"browser/browser_util.cc",
"browser/browser_util.h",
"build_channel.cc",
"catalog/campaign/catalog_campaign_info.cc",
"catalog/campaign/catalog_campaign_info.h",
"catalog/campaign/catalog_daypart_info.cc",
Expand Down Expand Up @@ -808,12 +806,8 @@ source_set("internal") {
"flags/did_override/did_override_features_from_command_line_util.h",
"flags/environment/environment_command_line_switch_parser_util.cc",
"flags/environment/environment_command_line_switch_parser_util.h",
"flags/environment/environment_types.h",
"flags/flag_manager.cc",
"flags/flag_manager.h",
"flags/flag_manager_constants.h",
"flags/flag_manager_util.cc",
"flags/flag_manager_util.h",
"flags/flag_constants.h",
"flags_util.cc",
"geographic/subdivision/get_subdivision_url_request_builder.cc",
"geographic/subdivision/get_subdivision_url_request_builder.h",
"geographic/subdivision/get_subdivision_url_request_builder_constants.h",
Expand All @@ -823,6 +817,10 @@ source_set("internal") {
"geographic/subdivision/subdivision_targeting_util.h",
"geographic/subdivision/supported_subdivision_codes.cc",
"geographic/subdivision/supported_subdivision_codes.h",
"global_state/global_state.cc",
"global_state/global_state.h",
"global_state/global_state_holder.cc",
"global_state/global_state_holder.h",
"history/ad_content_util.cc",
"history/ad_content_util.h",
"history/category_content_util.cc",
Expand Down Expand Up @@ -1084,7 +1082,6 @@ source_set("internal") {
"studies/studies_util.cc",
"studies/studies_util.h",
"supported_subdivisions.cc",
"sys_info.cc",
"tabs/tab_info.cc",
"tabs/tab_info.h",
"tabs/tab_manager.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "brave/components/brave_ads/core/internal/account/confirmations/confirmation_payload_json_writer.h"

#include "brave/components/brave_ads/core/internal/account/confirmations/confirmation_info.h"
#include "brave/components/brave_ads/core/internal/account/confirmations/confirmation_unittest_util.h"
#include "brave/components/brave_ads/core/internal/common/unittest/unittest_base.h"
#include "brave/components/brave_ads/core/internal/privacy/tokens/unblinded_tokens/unblinded_tokens_unittest_util.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include "brave/components/brave_ads/core/internal/common/unittest/unittest_time_util.h"
#include "brave/components/brave_ads/core/internal/conversions/conversion_queue_item_unittest_util.h"
#include "brave/components/brave_ads/core/internal/conversions/conversions_unittest_constants.h"
#include "brave/components/brave_ads/core/sys_info.h"
#include "brave/components/brave_ads/core/internal/global_state/global_state.h"
#include "third_party/re2/src/re2/re2.h"

// npm run test -- brave_unit_tests --filter=BatAds*
Expand All @@ -37,7 +37,8 @@ TEST_F(BatAdsConfirmationUserDataBuilderTest,

SetCatalogId(kCatalogId);

SysInfo().device_id =
auto& sys_info = GlobalState::GetInstance()->SysInfo();
sys_info.device_id =
"21b4677de1a9b4a197ab671a1481d3fcb24f826a4358a05aafbaee5a9a51b57e";

const base::Time time =
Expand Down Expand Up @@ -75,7 +76,8 @@ TEST_F(BatAdsConfirmationUserDataBuilderTest,

SetCatalogId(kCatalogId);

SysInfo().device_id =
auto& sys_info = GlobalState::GetInstance()->SysInfo();
sys_info.device_id =
"21b4677de1a9b4a197ab671a1481d3fcb24f826a4358a05aafbaee5a9a51b57e";

const base::Time time =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "brave/components/brave_ads/core/internal/account/transactions/transaction_info.h"
#include "brave/components/brave_ads/core/internal/account/transactions/transactions_unittest_util.h"
#include "brave/components/brave_ads/core/internal/common/unittest/unittest_base.h"
#include "brave/components/brave_ads/core/internal/deprecated/confirmations/confirmation_state_manager.h"
#include "brave/components/brave_ads/core/internal/privacy/tokens/token_generator_mock.h"
#include "brave/components/brave_ads/core/internal/privacy/tokens/token_generator_unittest_util.h"
#include "brave/components/brave_ads/core/internal/privacy/tokens/unblinded_payment_tokens/unblinded_payment_token_util.h"
Expand Down
Loading

0 comments on commit 5c5bdd0

Please sign in to comment.