Skip to content

Commit

Permalink
Merge pull request #24570 from brave/issues/39621
Browse files Browse the repository at this point in the history
[ads] Do not fetch geo and catalog on iOS when Brave Rewards/News disabled
  • Loading branch information
aseren committed Jul 12, 2024
1 parent 35fcd7f commit f39033a
Show file tree
Hide file tree
Showing 29 changed files with 201 additions and 118 deletions.
9 changes: 5 additions & 4 deletions browser/brave_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@

#include <string>

#include "brave/browser/new_tab/new_tab_shows_options.h"

#include "brave/browser/brave_shields/brave_shields_web_contents_observer.h"
#include "brave/browser/ethereum_remote_client/buildflags/buildflags.h"
#include "brave/browser/new_tab/new_tab_shows_options.h"
#include "brave/browser/search/ntp_utils.h"
#include "brave/browser/themes/brave_dark_mode_utils.h"
#include "brave/browser/translate/brave_translate_prefs_migration.h"
Expand All @@ -23,6 +22,8 @@
#include "brave/components/brave_news/browser/brave_news_controller.h"
#include "brave/components/brave_news/browser/brave_news_p3a.h"
#include "brave/components/brave_news/browser/brave_news_pref_manager.h"
#include "brave/components/brave_news/common/p3a_pref_names.h"
#include "brave/components/brave_news/common/pref_names.h"
#include "brave/components/brave_perf_predictor/browser/p3a_bandwidth_savings_tracker.h"
#include "brave/components/brave_perf_predictor/browser/perf_predictor_tab_helper.h"
#include "brave/components/brave_rewards/common/pref_names.h"
Expand Down Expand Up @@ -188,7 +189,7 @@ void RegisterProfilePrefsForMigration(

brave_rewards::RegisterProfilePrefsForMigration(registry);

brave_news::p3a::NewsMetrics::RegisterProfilePrefsForMigration(registry);
brave_news::p3a::prefs::RegisterProfileNewsMetricsPrefsForMigration(registry);

// Added May 2023
#if defined(TOOLKIT_VIEWS)
Expand Down Expand Up @@ -234,7 +235,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {

brave_shields::RegisterShieldsP3AProfilePrefs(registry);

brave_news::BraveNewsPrefManager::RegisterProfilePrefs(registry);
brave_news::prefs::RegisterProfilePrefs(registry);

// TODO(shong): Migrate this to local state also and guard in ENABLE_WIDEVINE.
// We don't need to display "don't ask widevine prompt option" in settings
Expand Down
4 changes: 3 additions & 1 deletion chromium_src/chrome/browser/prefs/browser_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "brave/components/ai_chat/core/common/buildflags/buildflags.h"
#include "brave/components/brave_ads/core/public/prefs/obsolete_pref_util.h"
#include "brave/components/brave_news/browser/brave_news_p3a.h"
#include "brave/components/brave_news/common/p3a_pref_names.h"
#include "brave/components/brave_news/common/pref_names.h"
#include "brave/components/brave_search_conversion/p3a.h"
#include "brave/components/brave_shields/content/browser/brave_shields_p3a.h"
#include "brave/components/brave_sync/brave_sync_prefs.h"
Expand Down Expand Up @@ -163,7 +165,7 @@ void MigrateObsoleteProfilePrefs(PrefService* profile_prefs,
profile_prefs->ClearPref(sidebar::kSidebarAlignmentChangedTemporarily);
#endif

brave_news::p3a::NewsMetrics::MigrateObsoleteProfilePrefs(profile_prefs);
brave_news::p3a::prefs::MigrateObsoleteProfileNewsMetricsPrefs(profile_prefs);

// Added 2023-09
ntp_background_images::ViewCounterService::MigrateObsoleteProfilePrefs(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import("//brave/components/ipfs/buildflags/buildflags.gni")
group("prefs") {
deps = [
"//brave/components/brave_ads/core",
"//brave/components/brave_news/common",
"//brave/components/brave_rewards/common",
"//brave/components/brave_shields/core/common",
"//brave/components/brave_sync:prefs",
Expand Down
1 change: 1 addition & 0 deletions chromium_src/ios/chrome/browser/shared/model/prefs/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
include_rules = [
"+brave/components/brave_ads/core/public",
"+brave/components/brave_news/common",
"+brave/components/brave_rewards/common",
"+brave/components/brave_sync",
"+brave/components/brave_wallet/browser",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "brave/components/ai_chat/core/browser/ai_chat_metrics.h"
#include "brave/components/ai_chat/core/common/pref_names.h"
#include "brave/components/brave_ads/core/public/prefs/pref_registry.h"
#include "brave/components/brave_news/common/pref_names.h"
#include "brave/components/brave_rewards/common/pref_registry.h"
#include "brave/components/brave_shields/core/common/pref_names.h"
#include "brave/components/brave_sync/brave_sync_prefs.h"
Expand Down Expand Up @@ -42,6 +43,7 @@ void BraveRegisterBrowserStatePrefs(
ipfs::IpfsService::RegisterProfilePrefs(registry);
#endif
ai_chat::prefs::RegisterProfilePrefs(registry);
brave_news::prefs::RegisterProfilePrefs(registry);
}

void BraveRegisterLocalStatePrefs(PrefRegistrySimple* registry) {
Expand Down
2 changes: 0 additions & 2 deletions components/brave_news/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ static_library("browser") {
"html_parsing.h",
"initialization_promise.cc",
"initialization_promise.h",
"locales_helper.cc",
"locales_helper.h",
"network.cc",
"network.h",
"publishers_controller.cc",
Expand Down
2 changes: 1 addition & 1 deletion components/brave_news/browser/brave_news_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@
#include "brave/components/brave_news/browser/channels_controller.h"
#include "brave/components/brave_news/browser/direct_feed_controller.h"
#include "brave/components/brave_news/browser/feed_v2_builder.h"
#include "brave/components/brave_news/browser/locales_helper.h"
#include "brave/components/brave_news/browser/network.h"
#include "brave/components/brave_news/browser/publishers_controller.h"
#include "brave/components/brave_news/browser/publishers_parsing.h"
#include "brave/components/brave_news/browser/suggestions_controller.h"
#include "brave/components/brave_news/browser/topics_fetcher.h"
#include "brave/components/brave_news/common/brave_news.mojom.h"
#include "brave/components/brave_news/common/features.h"
#include "brave/components/brave_news/common/locales_helper.h"
#include "brave/components/brave_private_cdn/private_cdn_helper.h"
#include "brave/components/brave_private_cdn/private_cdn_request_helper.h"
#include "components/favicon/core/favicon_service.h"
Expand Down
27 changes: 1 addition & 26 deletions components/brave_news/browser/brave_news_p3a.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
#include "base/logging.h"
#include "base/metrics/histogram_macros.h"
#include "brave/components/brave_news/browser/brave_news_pref_manager.h"
#include "brave/components/brave_news/common/pref_names.h"
#include "brave/components/brave_news/common/p3a_pref_names.h"
#include "brave/components/brave_rewards/common/pref_names.h"
#include "brave/components/p3a_utils/bucket.h"
#include "brave/components/p3a_utils/feature_usage.h"
#include "brave/components/time_period_storage/weekly_storage.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"

namespace brave_news::p3a {
Expand Down Expand Up @@ -308,30 +307,6 @@ void NewsMetrics::RecordWeeklySessionCount(bool is_add) {
total_session_count);
}

void NewsMetrics::RegisterProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterListPref(prefs::kBraveNewsWeeklySessionCount);
registry->RegisterListPref(prefs::kBraveNewsWeeklyDisplayAdViewedCount);
registry->RegisterListPref(prefs::kBraveNewsWeeklyAddedDirectFeedsCount);
registry->RegisterListPref(prefs::kBraveNewsTotalCardViews);
registry->RegisterListPref(prefs::kBraveNewsTotalCardVisits);
registry->RegisterListPref(prefs::kBraveNewsVisitDepthSum);
registry->RegisterListPref(prefs::kBraveNewsTotalSidebarFilterUsages);
p3a_utils::RegisterFeatureUsagePrefs(
registry, prefs::kBraveNewsFirstSessionTime,
prefs::kBraveNewsLastSessionTime, prefs::kBraveNewsUsedSecondDay, nullptr,
nullptr);
registry->RegisterBooleanPref(prefs::kBraveNewsWasEverEnabled, false);
}

void NewsMetrics::RegisterProfilePrefsForMigration(
PrefRegistrySimple* registry) {
// Reserved for future deprecated P3A-related prefs
}

void NewsMetrics::MigrateObsoleteProfilePrefs(PrefService* prefs) {
// Reserved for future deprecated P3A-related prefs
}

void NewsMetrics::OnConfigChanged() {
DVLOG(1) << __FUNCTION__;
if (was_enabled_ == pref_manager_->IsEnabled()) {
Expand Down
5 changes: 0 additions & 5 deletions components/brave_news/browser/brave_news_p3a.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "base/timer/wall_clock_timer.h"
#include "brave/components/brave_news/browser/brave_news_pref_manager.h"

class PrefRegistrySimple;
class PrefService;

namespace brave_news::p3a {
Expand Down Expand Up @@ -59,10 +58,6 @@ class NewsMetrics : public BraveNewsPrefManager::PrefObserver {
NewsMetrics(const NewsMetrics&) = delete;
NewsMetrics& operator=(const NewsMetrics&) = delete;

static void RegisterProfilePrefs(PrefRegistrySimple* registry);
static void RegisterProfilePrefsForMigration(PrefRegistrySimple* registry);
static void MigrateObsoleteProfilePrefs(PrefService* prefs);

void RecordAtInit();
void RecordAtSessionStart();

Expand Down
12 changes: 7 additions & 5 deletions components/brave_news/browser/brave_news_p3a_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@
#include "base/test/metrics/histogram_tester.h"
#include "brave/components/brave_news/browser/brave_news_controller.h"
#include "brave/components/brave_news/browser/brave_news_pref_manager.h"
#include "brave/components/brave_news/common/p3a_pref_names.h"
#include "brave/components/brave_news/common/pref_names.h"
#include "brave/components/brave_rewards/common/pref_names.h"
#include "brave/components/time_period_storage/weekly_storage.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/prefs/testing_pref_service.h"
#include "content/public/test/browser_task_environment.h"
Expand All @@ -29,7 +31,7 @@ class BraveNewsP3ATest : public testing::Test {
void SetUp() override {
task_environment_.AdvanceClock(base::Days(2));
PrefRegistrySimple* registry = pref_service_.registry();
BraveNewsPrefManager::RegisterProfilePrefs(registry);
::brave_news::prefs::RegisterProfilePrefs(registry);
registry->RegisterBooleanPref(brave_rewards::prefs::kEnabled, false);

pref_manager_ = std::make_unique<BraveNewsPrefManager>(pref_service_);
Expand Down Expand Up @@ -343,14 +345,14 @@ TEST_F(BraveNewsP3ATest, TestIsEnabled) {
PrefService* prefs = GetPrefs();

// should not record "disabled" if never opted in
prefs->SetBoolean(prefs::kNewTabPageShowToday, false);
prefs->SetBoolean(::brave_news::prefs::kNewTabPageShowToday, false);
histogram_tester_.ExpectTotalCount(kIsEnabledHistogramName, 0);

prefs->SetBoolean(prefs::kBraveNewsOptedIn, true);
prefs->SetBoolean(prefs::kNewTabPageShowToday, true);
prefs->SetBoolean(::brave_news::prefs::kBraveNewsOptedIn, true);
prefs->SetBoolean(::brave_news::prefs::kNewTabPageShowToday, true);
histogram_tester_.ExpectUniqueSample(kIsEnabledHistogramName, 1, 1);

prefs->SetBoolean(prefs::kNewTabPageShowToday, false);
prefs->SetBoolean(::brave_news::prefs::kNewTabPageShowToday, false);
histogram_tester_.ExpectBucketCount(kIsEnabledHistogramName, 0, 1);
}

Expand Down
16 changes: 0 additions & 16 deletions components/brave_news/browser/brave_news_pref_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@
#include "base/uuid.h"
#include "brave/components/brave_news/browser/brave_news_p3a.h"
#include "brave/components/brave_news/browser/channel_migrator.h"
#include "brave/components/brave_news/browser/locales_helper.h"
#include "brave/components/brave_news/common/brave_news.mojom.h"
#include "brave/components/brave_news/common/pref_names.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_observer.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"

Expand Down Expand Up @@ -154,20 +152,6 @@ SubscriptionsDiff BraveNewsSubscriptions::DiffChannels(
return result;
}

// static
void BraveNewsPrefManager::RegisterProfilePrefs(PrefRegistrySimple* registry) {
registry->RegisterBooleanPref(prefs::kShouldShowToolbarButton, true);
registry->RegisterBooleanPref(prefs::kNewTabPageShowToday,
IsUserInDefaultEnabledLocale());
registry->RegisterBooleanPref(prefs::kBraveNewsOptedIn, false);
registry->RegisterDictionaryPref(prefs::kBraveNewsSources);
registry->RegisterDictionaryPref(prefs::kBraveNewsChannels);
registry->RegisterDictionaryPref(prefs::kBraveNewsDirectFeeds);
registry->RegisterBooleanPref(prefs::kBraveNewsOpenArticlesInNewTab, true);

p3a::NewsMetrics::RegisterProfilePrefs(registry);
}

BraveNewsPrefManager::BraveNewsPrefManager(PrefService& prefs) : prefs_(prefs) {
pref_change_registrar_.Init(&*prefs_);
pref_change_registrar_.Add(
Expand Down
3 changes: 0 additions & 3 deletions components/brave_news/browser/brave_news_pref_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "brave/components/brave_news/common/brave_news.mojom-forward.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_observer.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -91,8 +90,6 @@ class BraveNewsPrefManager {
virtual void OnChannelsChanged() {}
};

static void RegisterProfilePrefs(PrefRegistrySimple* registry);

explicit BraveNewsPrefManager(PrefService& prefs);
~BraveNewsPrefManager();

Expand Down
2 changes: 1 addition & 1 deletion components/brave_news/browser/feed_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@
#include "brave/components/brave_news/browser/combined_feed_parsing.h"
#include "brave/components/brave_news/browser/direct_feed_fetcher.h"
#include "brave/components/brave_news/browser/feed_controller.h"
#include "brave/components/brave_news/browser/locales_helper.h"
#include "brave/components/brave_news/browser/network.h"
#include "brave/components/brave_news/browser/publishers_controller.h"
#include "brave/components/brave_news/browser/urls.h"
#include "brave/components/brave_news/common/brave_news.mojom-forward.h"
#include "brave/components/brave_news/common/brave_news.mojom-shared.h"
#include "brave/components/brave_news/common/locales_helper.h"
#include "brave/components/brave_private_cdn/headers.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"

Expand Down
1 change: 0 additions & 1 deletion components/brave_news/browser/publishers_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "base/strings/strcat.h"
#include "brave/components/api_request_helper/api_request_helper.h"
#include "brave/components/brave_news/browser/brave_news_pref_manager.h"
#include "brave/components/brave_news/browser/locales_helper.h"
#include "brave/components/brave_news/browser/network.h"
#include "brave/components/brave_news/browser/publishers_parsing.h"
#include "brave/components/brave_news/browser/urls.h"
Expand Down
2 changes: 1 addition & 1 deletion components/brave_news/browser/publishers_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
#include "brave/components/api_request_helper/api_request_helper.h"
#include "brave/components/brave_news/browser/brave_news_pref_manager.h"
#include "brave/components/brave_news/common/brave_news.mojom.h"
#include "brave/components/brave_news/common/locales_helper.h"

namespace brave_news {

using GetPublishersCallback = mojom::BraveNewsController::GetPublishersCallback;
using Publishers = base::flat_map<std::string, mojom::PublisherPtr>;

class PublishersController {
public:
Expand Down
2 changes: 1 addition & 1 deletion components/brave_news/browser/publishers_parsing.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

#include "base/values.h"
#include "brave/components/brave_news/browser/brave_news_pref_manager.h"
#include "brave/components/brave_news/browser/publishers_controller.h"
#include "brave/components/brave_news/common/brave_news.mojom-forward.h"
#include "brave/components/brave_news/common/locales_helper.h"

namespace brave_news {

Expand Down
6 changes: 2 additions & 4 deletions components/brave_news/browser/publishers_parsing_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@

#include "brave/components/brave_news/browser/publishers_parsing.h"

#include <memory>
#include <optional>
#include <utility>
#include <vector>
#include <string>

#include "base/containers/flat_map.h"
#include "base/test/values_test_util.h"
#include "brave/components/brave_news/common/brave_news.mojom-forward.h"
#include "brave/components/brave_news/common/brave_news.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace brave_news {
Expand Down
1 change: 0 additions & 1 deletion components/brave_news/browser/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ source_set("brave_news_unit_tests") {
"//brave/components/brave_news/browser/feed_sampling_unittest.cc",
"//brave/components/brave_news/browser/html_parsing_unittest.cc",
"//brave/components/brave_news/browser/initialization_promise_unittest.cc",
"//brave/components/brave_news/browser/locales_helper_unittest.cc",
"//brave/components/brave_news/browser/publishers_controller_unittest.cc",
"//brave/components/brave_news/browser/publishers_parsing_unittest.cc",
"//brave/components/brave_news/browser/suggestions_controller_unittest.cc",
Expand Down
26 changes: 25 additions & 1 deletion components/brave_news/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,39 @@
import("//brave/build/config.gni")
import("//mojo/public/tools/bindings/mojom.gni")

source_set("unit_tests") {
testonly = true

sources = [ "locales_helper_unittest.cc" ]

deps = [
":common",
"//base",
"//testing/gtest",
]
}

static_library("common") {
sources = [
"features.cc",
"features.h",
"locales_helper.cc",
"locales_helper.h",
"p3a_pref_names.cc",
"p3a_pref_names.h",
"pref_names.cc",
"pref_names.h",
"switches.h",
]

deps = [ "//base" ]
public_deps = [ ":mojom" ]

deps = [
"//base",
"//brave/components/l10n/common",
"//brave/components/p3a_utils",
"//components/prefs",
]
}

mojom("mojom") {
Expand Down
Loading

0 comments on commit f39033a

Please sign in to comment.