From 98d1b2b5ff2fb4c8cfc5f8cb05393235585dfe4e Mon Sep 17 00:00:00 2001 From: Aleksei Seren <86598290+aseren@users.noreply.github.com> Date: Mon, 22 Sep 2025 09:21:26 -0500 Subject: [PATCH] Unify the Android Dynamic NTT for reuse on iOS (#31185) Unify the Android Rich NTT for reuse on iOS The current PR is pre-requisite of Dynamic NTT on iOS as it allows to reuse android `New Tab Takeover WebUI` on iOS. The PR removes dependency of android `New Tab Takeover WebUI` from `ViewCounterService` which is not available on iOS. It also removes `current_wallpaper` base::Value cache in ViewCounterService which allows to save memory and makes code cleaner. --- .../browser/ntp/BraveNewTabPageLayout.java | 56 +++++++++++-------- .../ntp/SponsoredRichMediaWebView.java | 26 ++++++++- .../android/new_tab_takeover_ui.cc | 34 ++++++++--- .../android/new_tab_takeover_ui.h | 13 +++-- .../android/new_tab_takeover_ui_config.cc | 9 ++- components/new_tab_takeover/App.tsx | 44 ++++++++++++--- .../mojom/new_tab_takeover.mojom | 9 ++- .../browser/ntp_sponsored_images_data.cc | 14 +++++ .../browser/ntp_sponsored_images_data.h | 3 + .../ntp_sponsored_images_data_unittest.cc | 32 +++++++++++ .../browser/view_counter_service.cc | 51 ----------------- .../browser/view_counter_service.h | 10 ---- .../browser/view_counter_service_unittest.cc | 34 +++-------- 13 files changed, 200 insertions(+), 135 deletions(-) diff --git a/android/java/org/chromium/chrome/browser/ntp/BraveNewTabPageLayout.java b/android/java/org/chromium/chrome/browser/ntp/BraveNewTabPageLayout.java index a9000e5b1d4..782259b8fe7 100644 --- a/android/java/org/chromium/chrome/browser/ntp/BraveNewTabPageLayout.java +++ b/android/java/org/chromium/chrome/browser/ntp/BraveNewTabPageLayout.java @@ -1255,19 +1255,22 @@ public class BraveNewTabPageLayout extends NewTabPageLayout boolean wasWallpaperShown = true; if (ntpImage instanceof Wallpaper && ((Wallpaper) ntpImage).isRichMedia()) { - setupSponsoredBackgroundContent(); - } else if (ntpImage instanceof Wallpaper - && NTPImageUtil.isReferralEnabled() - && Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - setBackgroundImage(ntpImage); - - } else if (UserPrefs.get(ProfileManager.getLastUsedRegularProfile()) - .getBoolean(BravePref.NEW_TAB_PAGE_SHOW_BACKGROUND_IMAGE) - && mSponsoredTab != null - && NTPImageUtil.shouldEnableNTPFeature()) { - setBackgroundImage(ntpImage); + setupSponsoredBackgroundContent((Wallpaper) ntpImage); } else { - wasWallpaperShown = false; + maybeResetSponsoredRichMediaBackground(); + + if (ntpImage instanceof Wallpaper + && NTPImageUtil.isReferralEnabled() + && Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + setBackgroundImage(ntpImage); + } else if (UserPrefs.get(ProfileManager.getLastUsedRegularProfile()) + .getBoolean(BravePref.NEW_TAB_PAGE_SHOW_BACKGROUND_IMAGE) + && mSponsoredTab != null + && NTPImageUtil.shouldEnableNTPFeature()) { + setBackgroundImage(ntpImage); + } else { + wasWallpaperShown = false; + } } if (wasWallpaperShown @@ -1280,19 +1283,28 @@ public class BraveNewTabPageLayout extends NewTabPageLayout } } - private void setupSponsoredBackgroundContent() { - if (mSponsoredRichMediaWebView != null) { + private void setupSponsoredBackgroundContent(Wallpaper wallpaper) { + if (mSponsoredRichMediaWebView == null) { + mSponsoredRichMediaWebView = + new SponsoredRichMediaWebView(mActivity, mWindowAndroid, mProfile); + + mBackgroundSponsoredRichMediaView = findViewById(R.id.bg_sponsored_rich_media_view); + mBackgroundSponsoredRichMediaView.setVisibility(View.VISIBLE); + mBackgroundSponsoredRichMediaView.addView(mSponsoredRichMediaWebView.getView()); + } + + mSponsoredRichMediaWebView.maybeLoadSponsoredRichMedia( + wallpaper.getWallpaperId(), wallpaper.getCreativeInstanceId()); + } + + private void maybeResetSponsoredRichMediaBackground() { + if (mBackgroundSponsoredRichMediaView == null || mSponsoredRichMediaWebView == null) { return; } - mSponsoredRichMediaWebView = - new SponsoredRichMediaWebView(mActivity, mWindowAndroid, mProfile); - - mBackgroundSponsoredRichMediaView = findViewById(R.id.bg_sponsored_rich_media_view); - mBackgroundSponsoredRichMediaView.setVisibility(View.VISIBLE); - mBackgroundSponsoredRichMediaView.addView(mSponsoredRichMediaWebView.getView()); - - mSponsoredRichMediaWebView.loadSponsoredRichMedia(); + mBackgroundSponsoredRichMediaView.setVisibility(View.GONE); + mBackgroundSponsoredRichMediaView.removeAllViews(); + mSponsoredRichMediaWebView = null; } private void setBackgroundImage(NTPImage ntpImage) { diff --git a/android/java/org/chromium/chrome/browser/ntp/SponsoredRichMediaWebView.java b/android/java/org/chromium/chrome/browser/ntp/SponsoredRichMediaWebView.java index 6f0857cfab0..66dea365c02 100644 --- a/android/java/org/chromium/chrome/browser/ntp/SponsoredRichMediaWebView.java +++ b/android/java/org/chromium/chrome/browser/ntp/SponsoredRichMediaWebView.java @@ -6,6 +6,7 @@ package org.chromium.chrome.browser.ntp; import android.app.Activity; +import android.net.Uri; import android.view.View; import android.view.ViewGroup; import android.widget.FrameLayout; @@ -23,11 +24,15 @@ import org.chromium.net.NetId; import org.chromium.ui.base.ViewAndroidDelegate; import org.chromium.ui.base.WindowAndroid; +import java.util.Objects; + public class SponsoredRichMediaWebView { private static final String NEW_TAB_TAKEOVER_URL = "chrome://new-tab-takeover"; private final WebContents mWebContents; private final ThinWebView mWebView; + private String mPlacementId; + private String mCreativeInstanceId; public SponsoredRichMediaWebView( Activity activity, WindowAndroid windowAndroid, Profile profile) { @@ -59,11 +64,28 @@ public class SponsoredRichMediaWebView { mWebView.attachWebContents(mWebContents, webContentView, null); } - public void loadSponsoredRichMedia() { - mWebContents.getNavigationController().loadUrl(new LoadUrlParams(NEW_TAB_TAKEOVER_URL)); + public void maybeLoadSponsoredRichMedia(String placementId, String creativeInstanceId) { + if (Objects.equals(mPlacementId, placementId) + && Objects.equals(mCreativeInstanceId, creativeInstanceId)) { + return; + } + + mPlacementId = placementId; + mCreativeInstanceId = creativeInstanceId; + + mWebContents + .getNavigationController() + .loadUrl(new LoadUrlParams(getNewTabTakeoverUrl(placementId, creativeInstanceId))); } public View getView() { return mWebView.getView(); } + + private String getNewTabTakeoverUrl(String placementId, String creativeInstanceId) { + Uri.Builder builder = Uri.parse(NEW_TAB_TAKEOVER_URL).buildUpon(); + builder.appendQueryParameter("placementId", placementId); + builder.appendQueryParameter("creativeInstanceId", creativeInstanceId); + return builder.build().toString(); + } } diff --git a/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.cc b/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.cc index b99a3daf821..f0c8c02ed79 100644 --- a/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.cc +++ b/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.cc @@ -10,13 +10,12 @@ #include #include -#include "base/values.h" #include "brave/browser/ui/webui/brave_webui_source.h" #include "brave/components/constants/webui_url_constants.h" #include "brave/components/new_tab_takeover/grit/new_tab_takeover_generated_map.h" +#include "brave/components/ntp_background_images/browser/ntp_background_images_service.h" +#include "brave/components/ntp_background_images/browser/ntp_sponsored_images_data.h" #include "brave/components/ntp_background_images/browser/ntp_sponsored_rich_media_ad_event_handler.h" -#include "brave/components/ntp_background_images/browser/url_constants.h" -#include "brave/components/ntp_background_images/browser/view_counter_service.h" #include "chrome/browser/ui/android/tab_model/tab_model.h" #include "chrome/browser/ui/android/tab_model/tab_model_list.h" #include "components/grit/brave_components_resources.h" @@ -43,11 +42,12 @@ content::WebContents* GetActiveWebContents() { NewTabTakeoverUI::NewTabTakeoverUI( content::WebUI* const web_ui, - ntp_background_images::ViewCounterService* view_counter_service, + ntp_background_images::NTPBackgroundImagesService& + ntp_background_images_service, std::unique_ptr rich_media_ad_event_handler) : ui::MojoWebUIController(web_ui), - view_counter_service_(view_counter_service), + ntp_background_images_service_(ntp_background_images_service), rich_media_ad_event_handler_(std::move(rich_media_ad_event_handler)) { content::WebUIDataSource* source = CreateAndAddWebUIDataSource( web_ui, kNewTabTakeoverHost, kNewTabTakeoverGenerated, @@ -84,10 +84,30 @@ void NewTabTakeoverUI::SetSponsoredRichMediaAdEventHandler( } void NewTabTakeoverUI::GetCurrentWallpaper( + const std::string& creative_instance_id, GetCurrentWallpaperCallback callback) { - if (view_counter_service_) { - view_counter_service_->GetCurrentBrandedWallpaper(std::move(callback)); + auto failed = [&callback]() { + std::move(callback).Run(/*url=*/std::nullopt, + /*should_metrics_fallback_to_p3a=*/false, + /*target_url=*/std::nullopt); + }; + + const ntp_background_images::NTPSponsoredImagesData* sponsored_images_data = + ntp_background_images_service_->GetSponsoredImagesData( + /*super_referral=*/false, /*supports_rich_media=*/true); + if (!sponsored_images_data) { + return failed(); } + + const ntp_background_images::Creative* creative = + sponsored_images_data->GetCreativeByInstanceId(creative_instance_id); + if (!creative) { + return failed(); + } + + std::move(callback).Run(creative->url, + creative->should_metrics_fallback_to_p3a, + GURL(creative->logo.destination_url)); } void NewTabTakeoverUI::NavigateToUrl(const GURL& url) { diff --git a/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.h b/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.h index 8f1ff510afd..5a0edfc10fd 100644 --- a/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.h +++ b/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.h @@ -10,6 +10,7 @@ #include #include "base/memory/raw_ptr.h" +#include "base/memory/raw_ref.h" #include "brave/components/new_tab_takeover/mojom/new_tab_takeover.mojom.h" #include "brave/components/ntp_background_images/browser/mojom/ntp_background_images.mojom.h" #include "mojo/public/cpp/bindings/pending_receiver.h" @@ -17,8 +18,8 @@ #include "ui/webui/mojo_web_ui_controller.h" namespace ntp_background_images { +class NTPBackgroundImagesService; class NTPSponsoredRichMediaAdEventHandler; -class ViewCounterService; } // namespace ntp_background_images // On desktop, we use a Web UI to display new tab pages. On Android, however, @@ -33,7 +34,8 @@ class NewTabTakeoverUI : public ui::MojoWebUIController, public: NewTabTakeoverUI( content::WebUI* const web_ui, - ntp_background_images::ViewCounterService* view_counter_service, + ntp_background_images::NTPBackgroundImagesService& + ntp_background_images_service, std::unique_ptr< ntp_background_images::NTPSponsoredRichMediaAdEventHandler> rich_media_ad_event_handler); @@ -53,14 +55,15 @@ class NewTabTakeoverUI : public ui::MojoWebUIController, mojo::PendingReceiver< ntp_background_images::mojom::SponsoredRichMediaAdEventHandler> event_handler) override; - void GetCurrentWallpaper(GetCurrentWallpaperCallback callback) override; + void GetCurrentWallpaper(const std::string& creative_instance_id, + GetCurrentWallpaperCallback callback) override; void NavigateToUrl(const GURL& url) override; mojo::Receiver new_tab_takeover_receiver_{this}; - raw_ptr - view_counter_service_; // Not owned. + const raw_ref + ntp_background_images_service_; // Not owned. std::unique_ptr rich_media_ad_event_handler_; diff --git a/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui_config.cc b/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui_config.cc index caf0874c559..a8d1276a82b 100644 --- a/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui_config.cc +++ b/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui_config.cc @@ -9,6 +9,7 @@ #include #include "brave/browser/brave_ads/ads_service_factory.h" +#include "brave/browser/brave_browser_process.h" #include "brave/browser/ntp_background/view_counter_service_factory.h" #include "brave/browser/ui/webui/new_tab_takeover/android/new_tab_takeover_ui.h" #include "brave/components/constants/webui_url_constants.h" @@ -39,6 +40,12 @@ NewTabTakeoverUIConfig::CreateWebUIController(content::WebUI* web_ui, ntp_background_images::NTPSponsoredRichMediaAdEventHandler>( brave_ads::AdsServiceFactory::GetForProfile(profile), ntp_p3a_helper); + ntp_background_images::NTPBackgroundImagesService* + ntp_background_images_service = + g_brave_browser_process->ntp_background_images_service(); + CHECK(ntp_background_images_service); + return std::make_unique( - web_ui, view_counter_service, std::move(rich_media_ad_event_handler)); + web_ui, *ntp_background_images_service, + std::move(rich_media_ad_event_handler)); } diff --git a/components/new_tab_takeover/App.tsx b/components/new_tab_takeover/App.tsx index cc595be0c79..9db0784ff87 100644 --- a/components/new_tab_takeover/App.tsx +++ b/components/new_tab_takeover/App.tsx @@ -13,27 +13,57 @@ import { SponsoredRichMediaBackgroundInfo, SponsoredRichMediaBackground } from '../brave_new_tab_ui/containers/newTab/sponsored_rich_media_background' +function sanitizeId(value: string | null): string | null { + if (!value || value.length === 0) { + return null; + } + + // Restrict input to alphanumeric characters and hyphens to prevent + // potential injections. + if (!/^[0-9a-fA-F-]+$/.test(value)) { + return null; + } + + return value; +} + +function useParametersFromQuery(): { placementId: string | null; + creativeInstanceId: string | null } { + return React.useMemo(() => { + const urlParams = new URLSearchParams(window.location.search); + const placementId = urlParams.get('placementId'); + const creativeInstanceId = urlParams.get('creativeInstanceId'); + + return { + placementId: sanitizeId(placementId), + creativeInstanceId: sanitizeId(creativeInstanceId) + }; + }, []); +} + export default function App(props: React.PropsWithChildren) { + const { placementId, creativeInstanceId } = useParametersFromQuery(); const [sponsoredRichMediaBackgroundInfo, setSponsoredRichMediaBackgroundInfo] = React.useState(null) const [sponsoredRichMediaAdEventHandler, setSponsoredRichMediaAdEventHandler] = React.useState(null) const [newTabTakeover, setNewTabTakeover] = React.useState(null) const [richMediaHasLoaded, setRichMediaHasLoaded] = React.useState(false) const getCurrentWallpaper = React.useCallback(async () => { - if (!newTabTakeover) { + if (!newTabTakeover || !placementId || !creativeInstanceId) { return } + try { - const response = await newTabTakeover.getCurrentWallpaper(); - if (!response || !response.url || !response.placementId || - !response.creativeInstanceId || !response.targetUrl) { + const response = + await newTabTakeover.getCurrentWallpaper(creativeInstanceId); + if (!response || !response.url || !response.targetUrl) { return } const sponsoredRichMediaBackgroundInfo: SponsoredRichMediaBackgroundInfo = { url: response.url.url, - placementId: response.placementId, - creativeInstanceId: response.creativeInstanceId, + placementId: placementId, + creativeInstanceId: creativeInstanceId, shouldMetricsFallbackToP3a: response.shouldMetricsFallbackToP3a, targetUrl: response.targetUrl.url } @@ -41,7 +71,7 @@ export default function App(props: React.PropsWithChildren) { } catch (error) { console.error('Failed to get last displayed branded wallpaper:', error); } - }, [newTabTakeover]); + }, [newTabTakeover, placementId, creativeInstanceId]); React.useEffect(() => { const newTabTakeover = NewTabTakeoverMojom.NewTabTakeover.getRemote(); diff --git a/components/new_tab_takeover/mojom/new_tab_takeover.mojom b/components/new_tab_takeover/mojom/new_tab_takeover.mojom index a7093272308..756d618bed8 100644 --- a/components/new_tab_takeover/mojom/new_tab_takeover.mojom +++ b/components/new_tab_takeover/mojom/new_tab_takeover.mojom @@ -12,11 +12,10 @@ interface NewTabTakeover { SetSponsoredRichMediaAdEventHandler( pending_receiver event_handler); - GetCurrentWallpaper() => (url.mojom.Url? url, - string? placement_id, - string? creative_instance_id, - bool should_metrics_fallback_to_p3a, - url.mojom.Url? target_url); + GetCurrentWallpaper(string creative_instance_id) => + (url.mojom.Url? url, + bool should_metrics_fallback_to_p3a, + url.mojom.Url? target_url); NavigateToUrl(url.mojom.Url url); }; diff --git a/components/ntp_background_images/browser/ntp_sponsored_images_data.cc b/components/ntp_background_images/browser/ntp_sponsored_images_data.cc index 453df5acd92..6126f82198f 100644 --- a/components/ntp_background_images/browser/ntp_sponsored_images_data.cc +++ b/components/ntp_background_images/browser/ntp_sponsored_images_data.cc @@ -463,6 +463,20 @@ bool NTPSponsoredImagesData::IsSuperReferral() const { return IsValid() && !theme_name.empty(); } +const Creative* NTPSponsoredImagesData::GetCreativeByInstanceId( + const std::string& creative_instance_id) const { + // TODO(https://github.com/brave/brave-browser/issues/49222): + // Use a map-based lookup for creatives to improve performance. + for (const Campaign& campaign : campaigns) { + for (const Creative& creative : campaign.creatives) { + if (creative.creative_instance_id == creative_instance_id) { + return &creative; + } + } + } + return nullptr; +} + std::optional NTPSponsoredImagesData::MaybeGetBackgroundAt( size_t campaign_index, size_t creative_index) const { diff --git a/components/ntp_background_images/browser/ntp_sponsored_images_data.h b/components/ntp_background_images/browser/ntp_sponsored_images_data.h index 9bca49af281..4bf962074ca 100644 --- a/components/ntp_background_images/browser/ntp_sponsored_images_data.h +++ b/components/ntp_background_images/browser/ntp_sponsored_images_data.h @@ -151,6 +151,9 @@ struct NTPSponsoredImagesData { bool IsSuperReferral() const; + const Creative* GetCreativeByInstanceId( + const std::string& creative_instance_id) const; + std::string url_prefix; std::optional grace_period; diff --git a/components/ntp_background_images/browser/ntp_sponsored_images_data_unittest.cc b/components/ntp_background_images/browser/ntp_sponsored_images_data_unittest.cc index 24cf4fc8971..02073a164aa 100644 --- a/components/ntp_background_images/browser/ntp_sponsored_images_data_unittest.cc +++ b/components/ntp_background_images/browser/ntp_sponsored_images_data_unittest.cc @@ -164,4 +164,36 @@ TEST(NTPSponsoredImagesDataTest, ParseSponsoredRichMediaCampaign) { EXPECT_THAT(creative.logo.image_url, testing::IsEmpty()); } +TEST(NTPSponsoredImagesDataTest, + GetCreativeByInstanceIdFromSponsoredImagesCampaign) { + base::Value::Dict dict = + base::test::ParseJsonDict(kTestSponsoredImagesCampaign); + base::FilePath installed_dir(FILE_PATH_LITERAL("ntp_sponsored_images_data")); + NTPSponsoredImagesData data(dict, installed_dir); + EXPECT_THAT(data.IsValid(), testing::IsTrue()); + + EXPECT_EQ( + data.GetCreativeByInstanceId("30244a36-561a-48f0-8d7a-780e9035c57a"), + &data.campaigns[0].creatives[0]); + EXPECT_EQ( + data.GetCreativeByInstanceId("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"), + nullptr); +} + +TEST(NTPSponsoredImagesDataTest, + GetCreativeByInstanceIdFromSponsoredRichMediaCampaign) { + base::Value::Dict dict = + base::test::ParseJsonDict(kTestSponsoredRichMediaCampaign); + base::FilePath installed_dir(FILE_PATH_LITERAL("ntp_sponsored_images_data")); + NTPSponsoredImagesData data(dict, installed_dir); + EXPECT_THAT(data.IsValid(), testing::IsTrue()); + + EXPECT_EQ( + data.GetCreativeByInstanceId("39d78863-327d-4b64-9952-cd0e5e330eb6"), + &data.campaigns[0].creatives[0]); + EXPECT_EQ( + data.GetCreativeByInstanceId("xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"), + nullptr); +} + } // namespace ntp_background_images diff --git a/components/ntp_background_images/browser/view_counter_service.cc b/components/ntp_background_images/browser/view_counter_service.cc index 094bdeddacb..71968677a41 100644 --- a/components/ntp_background_images/browser/view_counter_service.cc +++ b/components/ntp_background_images/browser/view_counter_service.cc @@ -187,7 +187,6 @@ std::optional ViewCounterService::GetCurrentWallpaperForDisplay() { if (ShouldShowSponsoredImages()) { if (std::optional dict = GetCurrentBrandedWallpaper()) { - current_wallpaper_ = dict->Clone(); return dict; } } @@ -237,56 +236,6 @@ ViewCounterService::GetCurrentBrandedWallpaper() const { return GetCurrentBrandedWallpaperFromAdsService(); } -void ViewCounterService::GetCurrentBrandedWallpaper( - GetCurrentBrandedWallpaperCallback callback) const { - auto failed = [&callback]() { - std::move(callback).Run(/*url=*/std::nullopt, - /*placement_id=*/std::nullopt, - /*creative_instance_id=*/std::nullopt, - /*should_metrics_fallback_to_p3a=*/false, - /*target_url=*/std::nullopt); - }; - - if (!current_wallpaper_) { - return failed(); - } - - const std::string* const url = - current_wallpaper_->FindString(ntp_background_images::kWallpaperURLKey); - if (!url) { - return failed(); - } - - const std::string* const creative_instance_id = - current_wallpaper_->FindString( - ntp_background_images::kCreativeInstanceIDKey); - if (!creative_instance_id) { - return failed(); - } - - const std::string* const placement_id = - current_wallpaper_->FindString(ntp_background_images::kWallpaperIDKey); - if (!placement_id) { - return failed(); - } - - const bool should_metrics_fallback_to_p3a = - current_wallpaper_ - ->FindBool( - ntp_background_images::kWallpaperShouldMetricsFallbackToP3aKey) - .value_or(false); - - const std::string* const target_url = - current_wallpaper_->FindStringByDottedPath( - ntp_background_images::kLogoDestinationURLPath); - if (!target_url) { - return failed(); - } - - std::move(callback).Run(GURL(*url), *placement_id, *creative_instance_id, - should_metrics_fallback_to_p3a, GURL(*target_url)); -} - std::optional ViewCounterService::GetCurrentBrandedWallpaperFromAdsService() const { DCHECK(ads_service_); diff --git a/components/ntp_background_images/browser/view_counter_service.h b/components/ntp_background_images/browser/view_counter_service.h index c449c820492..7b9277877b3 100644 --- a/components/ntp_background_images/browser/view_counter_service.h +++ b/components/ntp_background_images/browser/view_counter_service.h @@ -49,13 +49,6 @@ class WeeklyStorage; namespace ntp_background_images { -using GetCurrentBrandedWallpaperCallback = base::OnceCallback& url, - const std::optional& placement_id, - const std::optional& creative_instance_id, - bool should_metrics_fallback_to_p3a, - const std::optional& target_url)>; - class BraveNTPCustomBackgroundService; class NTPP3AHelper; @@ -101,8 +94,6 @@ class ViewCounterService : public KeyedService, std::optional GetCurrentWallpaperForDisplay(); std::optional GetCurrentWallpaper() const; std::optional GetCurrentBrandedWallpaper() const; - void GetCurrentBrandedWallpaper( - GetCurrentBrandedWallpaperCallback callback) const; std::optional GetCurrentBrandedWallpaperFromAdsService() const; std::optional GetCurrentBrandedWallpaperFromModel() const; @@ -219,7 +210,6 @@ class ViewCounterService : public KeyedService, PrefChangeRegistrar pref_change_registrar_; ViewCounterModel model_; base::WallClockTimer p3a_update_timer_; - std::optional current_wallpaper_; // Can be null if custom background is not supported. const raw_ptr custom_background_service_ = diff --git a/components/ntp_background_images/browser/view_counter_service_unittest.cc b/components/ntp_background_images/browser/view_counter_service_unittest.cc index cf726f0712d..2894a3208a8 100644 --- a/components/ntp_background_images/browser/view_counter_service_unittest.cc +++ b/components/ntp_background_images/browser/view_counter_service_unittest.cc @@ -13,7 +13,6 @@ #include "base/files/file_path.h" #include "base/memory/raw_ptr.h" #include "base/memory/scoped_refptr.h" -#include "base/test/mock_callback.h" #include "base/test/task_environment.h" #include "base/test/values_test_util.h" #include "base/time/time.h" @@ -375,7 +374,7 @@ class ViewCounterServiceTest : public testing::Test { return view_counter_service_->GetCurrentWallpaperForDisplay(); } - void VerifyGetCurrentBrandedWallpaperExpectation() { + void VerifyGetNewTabTakeoverWallpaperExpectation() { EXPECT_CALL(ads_service_mock_, PrefetchNewTabPageAd) .Times(GetInitialCountToBrandedWallpaper()); const brave_ads::NewTabPageAdInfo ad = BuildNewTabPageAd(); @@ -400,17 +399,9 @@ class ViewCounterServiceTest : public testing::Test { const std::string* target_url = wallpaper->FindStringByDottedPath(kLogoDestinationURLPath); ASSERT_TRUE(target_url); - - base::MockCallback callback; - EXPECT_CALL(callback, Run(::testing::Optional(GURL(*url)), - ::testing::Optional(*placement_id), - ::testing::Optional(*creative_instance_id), - /*should_metrics_fallback_to_p3a=*/false, - ::testing::Optional(GURL(*target_url)))); - view_counter_service_->GetCurrentBrandedWallpaper(callback.Get()); } - void VerifyDoNotGetCurrentBrandedWallpaperExpectation() { + void VerifyDoNotGetNewTabTakeoverWallpaperExpectation() { EXPECT_EQ(base::test::ParseJsonDict(R"JSON( { "author": "Brave", @@ -422,13 +413,6 @@ class ViewCounterServiceTest : public testing::Test { "wallpaperImageUrl": "chrome://background-wallpaper/wallpaper1.jpg" })JSON"), CycleThroughPageViewsAndMaybeGetNewTabTakeoverWallpaper()); - - base::MockCallback callback; - EXPECT_CALL( - callback, - Run(::testing::Eq(std::nullopt), ::testing::Eq(std::nullopt), - ::testing::Eq(std::nullopt), false, ::testing::Eq(std::nullopt))); - view_counter_service_->GetCurrentBrandedWallpaper(callback.Get()); } protected: @@ -755,7 +739,7 @@ TEST_F(ViewCounterServiceTest, EXPECT_CALL(ads_service_mock_, MaybeGetPrefetchedNewTabPageAd) .WillOnce(::testing::Return(ad)); EXPECT_CALL(ads_service_mock_, OnFailedToPrefetchNewTabPageAd); - VerifyDoNotGetCurrentBrandedWallpaperExpectation(); + VerifyDoNotGetNewTabTakeoverWallpaperExpectation(); } TEST_F(ViewCounterServiceTest, @@ -767,7 +751,7 @@ TEST_F(ViewCounterServiceTest, base::Minutes(1); task_environment_.AdvanceClock(base::Minutes(1)); - VerifyGetCurrentBrandedWallpaperExpectation(); + VerifyGetNewTabTakeoverWallpaperExpectation(); } TEST_F(ViewCounterServiceTest, @@ -782,7 +766,7 @@ TEST_F(ViewCounterServiceTest, EXPECT_CALL(ads_service_mock_, PrefetchNewTabPageAd).Times(0); EXPECT_CALL(ads_service_mock_, MaybeGetPrefetchedNewTabPageAd).Times(0); EXPECT_CALL(ads_service_mock_, OnFailedToPrefetchNewTabPageAd).Times(0); - VerifyDoNotGetCurrentBrandedWallpaperExpectation(); + VerifyDoNotGetNewTabTakeoverWallpaperExpectation(); } TEST_F(ViewCounterServiceTest, @@ -796,7 +780,7 @@ TEST_F(ViewCounterServiceTest, EXPECT_CALL(ads_service_mock_, PrefetchNewTabPageAd).Times(0); EXPECT_CALL(ads_service_mock_, MaybeGetPrefetchedNewTabPageAd).Times(0); EXPECT_CALL(ads_service_mock_, OnFailedToPrefetchNewTabPageAd).Times(0); - VerifyDoNotGetCurrentBrandedWallpaperExpectation(); + VerifyDoNotGetNewTabTakeoverWallpaperExpectation(); } TEST_F(ViewCounterServiceTest, @@ -808,7 +792,7 @@ TEST_F(ViewCounterServiceTest, base::Minutes(1); task_environment_.AdvanceClock(base::Minutes(1)); - VerifyGetCurrentBrandedWallpaperExpectation(); + VerifyGetNewTabTakeoverWallpaperExpectation(); } TEST_F(ViewCounterServiceTest, @@ -823,7 +807,7 @@ TEST_F(ViewCounterServiceTest, EXPECT_CALL(ads_service_mock_, PrefetchNewTabPageAd).Times(0); EXPECT_CALL(ads_service_mock_, MaybeGetPrefetchedNewTabPageAd).Times(0); EXPECT_CALL(ads_service_mock_, OnFailedToPrefetchNewTabPageAd).Times(0); - VerifyDoNotGetCurrentBrandedWallpaperExpectation(); + VerifyDoNotGetNewTabTakeoverWallpaperExpectation(); } TEST_F(ViewCounterServiceTest, @@ -837,7 +821,7 @@ TEST_F(ViewCounterServiceTest, EXPECT_CALL(ads_service_mock_, PrefetchNewTabPageAd).Times(0); EXPECT_CALL(ads_service_mock_, MaybeGetPrefetchedNewTabPageAd).Times(0); EXPECT_CALL(ads_service_mock_, OnFailedToPrefetchNewTabPageAd).Times(0); - VerifyDoNotGetCurrentBrandedWallpaperExpectation(); + VerifyDoNotGetNewTabTakeoverWallpaperExpectation(); } } // namespace ntp_background_images