Skip to content

Commit

Permalink
[ads] General code health
Browse files Browse the repository at this point in the history
  • Loading branch information
tmancey committed Oct 15, 2024
1 parent 31d5aeb commit b4153a2
Show file tree
Hide file tree
Showing 16 changed files with 49 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "base/test/metrics/histogram_tester.h"
#include "brave/browser/brave_browser_process.h"
#include "brave/components/brave_ads/core/public/prefs/pref_names.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include "brave/components/brave_ads/browser/application_state/background_helper.h"

#include "brave/browser/brave_ads/application_state/background_helper/background_helper_holder.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"

namespace brave_ads {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include "base/no_destructor.h"
#include "brave/components/brave_ads/browser/application_state/background_helper.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"

#if BUILDFLAG(IS_ANDROID)
#include "brave/browser/brave_ads/application_state/background_helper/background_helper_android.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include "base/functional/bind.h"
#include "base/no_destructor.h"
#include "brave/browser/brave_ads/application_state/notification_helper/notification_helper_impl.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/notifications/notification_platform_bridge.h"
#include "chrome/browser/profiles/profile.h"
Expand Down
28 changes: 14 additions & 14 deletions browser/brave_ads/tabs/ads_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "components/prefs/pref_service.h"
#include "components/sessions/content/session_tab_helper.h"
#include "components/sessions/core/session_id.h"
#include "content/public/browser/media_player_id.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"
Expand Down Expand Up @@ -51,20 +52,18 @@ std::string MediaPlayerUuid(const content::MediaPlayerId& id) {

} // namespace

AdsTabHelper::AdsTabHelper(content::WebContents* web_contents)
AdsTabHelper::AdsTabHelper(content::WebContents* const web_contents)
: content::WebContentsObserver(web_contents),
content::WebContentsUserData<AdsTabHelper>(*web_contents),
session_id_(sessions::SessionTabHelper::IdForTab(web_contents)) {
if (!session_id_.is_valid()) {
// Invalid session id instance.
return;
}

Profile* const profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
ads_service_ = AdsServiceFactory::GetForProfile(profile);
if (!ads_service_) {
// No-op if the ads service is unavailable.
return;
}

Expand All @@ -84,7 +83,7 @@ AdsTabHelper::~AdsTabHelper() {
#endif
}

void AdsTabHelper::SetAdsServiceForTesting(AdsService* ads_service) {
void AdsTabHelper::SetAdsServiceForTesting(AdsService* const ads_service) {
CHECK_IS_TEST();

ads_service_ = ads_service;
Expand All @@ -108,6 +107,7 @@ bool AdsTabHelper::UserHasOptedInToNotificationAds() const {
}

bool AdsTabHelper::IsVisible() const {
// The web contents must be visible and the browser must be active.
return is_web_contents_visible_ && is_browser_active_.value_or(false);
}

Expand Down Expand Up @@ -142,15 +142,15 @@ void AdsTabHelper::MaybeSetBrowserIsNoLongerActive() {
}

bool AdsTabHelper::IsNewNavigation(
content::NavigationHandle* navigation_handle) {
content::NavigationHandle* const navigation_handle) {
CHECK(navigation_handle);

return ui::PageTransitionIsNewNavigation(
navigation_handle->GetPageTransition());
}

std::optional<int> AdsTabHelper::HttpStatusCode(
content::NavigationHandle* navigation_handle) {
content::NavigationHandle* const navigation_handle) {
CHECK(navigation_handle);

if (const net::HttpResponseHeaders* const response_headers =
Expand Down Expand Up @@ -198,11 +198,10 @@ void AdsTabHelper::MaybeNotifyBrowserDidResignActive() {
}

void AdsTabHelper::MaybeNotifyUserGestureEventTriggered(
content::NavigationHandle* navigation_handle) {
content::NavigationHandle* const navigation_handle) {
CHECK(navigation_handle);

if (!ads_service_) {
// No-op if the ads service is unavailable.
return;
}

Expand All @@ -225,7 +224,6 @@ void AdsTabHelper::MaybeNotifyUserGestureEventTriggered(

void AdsTabHelper::MaybeNotifyTabDidChange() {
if (!ads_service_) {
// No-op if the ads service is unavailable.
return;
}

Expand Down Expand Up @@ -273,8 +271,7 @@ void AdsTabHelper::MaybeNotifyTabHtmlContentDidChange() {
// for Brave Rewards users. However, we must notify that the tab content has
// changed with empty HTML to ensure that regular conversions are processed.
return ads_service_->NotifyTabHtmlContentDidChange(
/*tab_id=*/session_id_.id(), redirect_chain_,
/*html=*/"");
/*tab_id=*/session_id_.id(), redirect_chain_, /*html=*/"");
}

// Only utilized for verifiable conversions, which requires the user to have
Expand Down Expand Up @@ -342,7 +339,12 @@ void AdsTabHelper::MaybeNotifyTabdidClose() {

void AdsTabHelper::DidStartNavigation(
content::NavigationHandle* navigation_handle) {
if (!ads_service_ || !navigation_handle->IsInPrimaryMainFrame()) {
if (!ads_service_) {
// No-op if the ads service is unavailable.
return;
}

if (!navigation_handle->IsInPrimaryMainFrame()) {
return;
}

Expand All @@ -360,7 +362,6 @@ void AdsTabHelper::DidStartNavigation(
void AdsTabHelper::DidFinishNavigation(
content::NavigationHandle* navigation_handle) {
if (!ads_service_) {
// No-op if the ads service is unavailable.
return;
}

Expand Down Expand Up @@ -400,7 +401,6 @@ void AdsTabHelper::DidFinishNavigation(
// have finished loading.
void AdsTabHelper::DocumentOnLoadCompletedInPrimaryMainFrame() {
if (!ads_service_) {
// No-op if the ads service is unavailable.
return;
}

Expand Down
17 changes: 10 additions & 7 deletions browser/brave_ads/tabs/ads_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/values.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"
#include "components/sessions/core/session_id.h"
#include "content/public/browser/media_player_id.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h"

Expand All @@ -27,6 +26,10 @@
class Browser;
class GURL;

namespace content {
struct MediaPlayerId;
} // namespace content

namespace brave_ads {

class AdsService;
Expand All @@ -37,15 +40,15 @@ class AdsTabHelper : public content::WebContentsObserver,
#endif
public content::WebContentsUserData<AdsTabHelper> {
public:
explicit AdsTabHelper(content::WebContents*);
explicit AdsTabHelper(content::WebContents* const);
~AdsTabHelper() override;

AdsTabHelper(const AdsTabHelper&) = delete;
AdsTabHelper& operator=(const AdsTabHelper&) = delete;

AdsService* ads_service() { return ads_service_; }

void SetAdsServiceForTesting(AdsService* ads_service);
void SetAdsServiceForTesting(AdsService* const ads_service);

private:
friend class content::WebContentsUserData<AdsTabHelper>;
Expand All @@ -60,12 +63,12 @@ class AdsTabHelper : public content::WebContentsObserver,

// Returns 'false' if the navigation was a back/forward navigation or a
// reload, otherwise 'true'.
bool IsNewNavigation(content::NavigationHandle* navigation_handle);
bool IsNewNavigation(content::NavigationHandle* const navigation_handle);

// NOTE: DO NOT use this method before the navigation commit as it will return
// null. It is safe to use from `WebContentsObserver::DidFinishNavigation()`.
std::optional<int> HttpStatusCode(
content::NavigationHandle* navigation_handle);
content::NavigationHandle* const navigation_handle);

bool IsErrorPage(int http_status_code) const;

Expand All @@ -77,7 +80,7 @@ class AdsTabHelper : public content::WebContentsObserver,
void MaybeNotifyBrowserDidResignActive();

void MaybeNotifyUserGestureEventTriggered(
content::NavigationHandle* navigation_handle);
content::NavigationHandle* const navigation_handle);

void MaybeNotifyTabDidChange();

Expand Down
2 changes: 1 addition & 1 deletion browser/ui/views/brave_ads/notification_ad_popup_widget.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include <utility>

#include "brave/components/brave_ads/browser/ad_units/notification_ad/custom_notification_ad_feature.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/native_theme/native_theme.h"
#include "ui/views/widget/widget_delegate.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#ifndef BRAVE_COMPONENTS_BRAVE_ADS_BROWSER_AD_UNITS_NOTIFICATION_AD_CUSTOM_NOTIFICATION_AD_CONSTANTS_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_BROWSER_AD_UNITS_NOTIFICATION_AD_CUSTOM_NOTIFICATION_AD_CONSTANTS_H_

#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"

namespace brave_ads {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include "base/feature_list.h"
#include "base/metrics/field_trial_params.h"
#include "brave/components/brave_ads/browser/ad_units/notification_ad/custom_notification_ad_constants.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"

namespace brave_ads {

Expand Down
2 changes: 1 addition & 1 deletion components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
#include "brave/components/l10n/common/prefs.h"
#include "brave/components/ntp_background_images/common/pref_names.h"
#include "brave/components/services/bat_ads/public/interfaces/bat_ads.mojom.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_context.h"
#include "mojo/public/cpp/bindings/callback_helpers.h"
Expand Down
3 changes: 1 addition & 2 deletions components/brave_ads/browser/ads_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class AdsTooltipsDelegate;
class BatAdsServiceFactory;
class Database;
class DeviceId;
struct NewTabPageAdInfo;
class ResourceComponent;
struct NewTabPageAdInfo;

class AdsServiceImpl final : public AdsService,
public bat_ads::mojom::BatAdsClient,
Expand Down Expand Up @@ -480,7 +480,6 @@ class AdsServiceImpl final : public AdsService,

mojo::Receiver<bat_ads::mojom::BatAdsObserver> bat_ads_observer_receiver_{
this};

mojo::Remote<bat_ads::mojom::BatAdsService> bat_ads_service_remote_;
mojo::AssociatedReceiver<bat_ads::mojom::BatAdsClient>
bat_ads_client_associated_receiver_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include "brave/components/brave_ads/core/internal/common/platform/platform_helper.h"

#include "base/check_is_test.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"
#if BUILDFLAG(IS_ANDROID)
#include "brave/components/brave_ads/core/internal/common/platform/platform_helper_android.h"
#elif BUILDFLAG(IS_IOS)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include "brave/components/brave_ads/core/public/ad_units/notification_ad/notification_ad_value_util.h"
#include "brave/components/brave_ads/core/public/ads_client/ads_client.h"
#include "brave/components/brave_ads/core/public/prefs/pref_names.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"

#if BUILDFLAG(IS_ANDROID)
#include "brave/components/brave_ads/core/internal/application_state/browser_util.h"
Expand Down
2 changes: 1 addition & 1 deletion components/brave_ads/core/internal/flags/flag_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_FLAGS_FLAG_CONSTANTS_H_

#include "brave/components/brave_ads/core/mojom/brave_ads.mojom.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"

namespace brave_ads {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <cstdint>

#include "base/time/time.h"
#include "build/build_config.h" // IWYU pragma: keep
#include "build/build_config.h"

namespace brave_ads {

Expand Down
20 changes: 12 additions & 8 deletions ios/browser/api/ads/brave_ads.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1738,7 +1738,7 @@ - (void)notifyTabTextContentDidChange:(NSInteger)tabId
const std::vector<GURL> urls = [self GURLsWithNSURLs:redirectChain];

adsClientNotifier->NotifyTabTextContentDidChange(
(int32_t)tabId, urls, base::SysNSStringToUTF8(text));
static_cast<int32_t>(tabId), urls, base::SysNSStringToUTF8(text));
}

- (void)notifyTabHtmlContentDidChange:(NSInteger)tabId
Expand All @@ -1751,18 +1751,20 @@ - (void)notifyTabHtmlContentDidChange:(NSInteger)tabId
const std::vector<GURL> urls = [self GURLsWithNSURLs:redirectChain];

adsClientNotifier->NotifyTabHtmlContentDidChange(
(int32_t)tabId, urls, base::SysNSStringToUTF8(html));
static_cast<int32_t>(tabId), urls, base::SysNSStringToUTF8(html));
}

- (void)notifyTabDidStartPlayingMedia:(NSInteger)tabId {
if (adsClientNotifier != nil) {
adsClientNotifier->NotifyTabDidStartPlayingMedia((int32_t)tabId);
adsClientNotifier->NotifyTabDidStartPlayingMedia(
static_cast<int32_t>(tabId));
}
}

- (void)notifyTabDidStopPlayingMedia:(NSInteger)tabId {
if (adsClientNotifier != nil) {
adsClientNotifier->NotifyTabDidStopPlayingMedia((int32_t)tabId);
adsClientNotifier->NotifyTabDidStopPlayingMedia(
static_cast<int32_t>(tabId));
}
}

Expand All @@ -1779,8 +1781,9 @@ - (void)notifyTabDidChange:(NSInteger)tabId

const bool isVisible = isSelected && [self isBrowserActive];

adsClientNotifier->NotifyTabDidChange((int32_t)tabId, urls, isNewNavigation,
isRestoring, isVisible);
adsClientNotifier->NotifyTabDidChange(static_cast<int32_t>(tabId), urls,
isNewNavigation, isRestoring,
isVisible);
}

- (void)notifyTabDidLoad:(NSInteger)tabId
Expand All @@ -1789,12 +1792,13 @@ - (void)notifyTabDidLoad:(NSInteger)tabId
return;
}

adsClientNotifier->NotifyTabDidLoad((int32_t)tabId, (int32_t)httpStatusCode);
adsClientNotifier->NotifyTabDidLoad(static_cast<int32_t>(tabId),
static_cast<int32_t>(httpStatusCode));
}

- (void)notifyDidCloseTab:(NSInteger)tabId {
if (adsClientNotifier != nil) {
adsClientNotifier->NotifyDidCloseTab((int32_t)tabId);
adsClientNotifier->NotifyDidCloseTab(static_cast<int32_t>(tabId));
}
}

Expand Down

0 comments on commit b4153a2

Please sign in to comment.