Fixes

browser/brave_ads/ads_service_delegate.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "components/search_engines/search_engines_pref_names.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_service.h"
 
@@ -151,14 +152,9 @@ base::Value::Dict AdsServiceDelegate::GetVirtualPrefs() const {
     return {};
   }
 
-  // TODO(aseren): Is there a way we can return the synced GUID rather than a
-  // short name?
-  auto virtual_prefs = base::Value::Dict().Set(
-      "default_search_provider.synced_guid",
+  return base::Value::Dict().Set(
+      prefs::kDefaultSearchProviderGUID,
       base::UTF16ToUTF8(default_search_provider->short_name()));
-  VLOG(0) << "FOOBAR.VIRTUAL_PREFS: " << virtual_prefs;
-
-  return virtual_prefs;
 }
 
 }  // namespace brave_ads

components/brave_ads/core/internal/ads_client/ads_client_pref_provider.cc

@@ -5,16 +5,12 @@
 
 #include "brave/components/brave_ads/core/internal/ads_client/ads_client_pref_provider.h"
 
-#include <optional>
-
 #include "brave/components/brave_ads/core/internal/ads_client/ads_client_util.h"
 #include "brave/components/brave_ads/core/public/ads_client/ads_client.h"
 
 namespace brave_ads {
 
-AdsClientPrefProvider::AdsClientPrefProvider() {
-  virtual_prefs_ = GetAdsClient()->GetVirtualPrefs();
-}
+AdsClientPrefProvider::AdsClientPrefProvider() = default;
 
 AdsClientPrefProvider::~AdsClientPrefProvider() = default;
 
@@ -60,33 +56,17 @@ bool AdsClientPrefProvider::HasLocalStatePrefPath(
 
 std::optional<base::Value> AdsClientPrefProvider::GetVirtualPref(
     const std::string& virtual_pref_path) const {
-  if (virtual_pref_path.starts_with("[virtual]:")) {
-    const std::string pref_path =
-        virtual_pref_path.substr(/*pos=*/std::size("[virtual]:"));
-
-    if (const base::Value* const value = virtual_prefs_.Find(pref_path)) {
-      return value->Clone();
-    }
-  }
-
-  if (virtual_pref_path.starts_with("[virtual_default]:")) {
+  if (virtual_pref_path.starts_with(kVirtualPrefPathPrefix)) {
     const std::string pref_path =
-        virtual_pref_path.substr(/*pos=*/std::size("[virtual_default]:"));
-
-    if (HasProfilePrefPath(pref_path)) {
-      return GetProfilePref(pref_path);
-    }
-
-    if (HasLocalStatePrefPath(pref_path)) {
-      return GetLocalStatePref(pref_path);
-    }
+        virtual_pref_path.substr(/*pos=*/std::strlen(kVirtualPrefPathPrefix));
 
-    if (const base::Value* const value = virtual_prefs_.Find(pref_path)) {
+    const base::Value::Dict virtual_prefs = GetAdsClient()->GetVirtualPrefs();
+    if (const base::Value* const value = virtual_prefs_->Find(pref_path)) {
       return value->Clone();
     }
   }
 
-  // Unknown virtual pref path.
+  // The virtual preference does not exist.
   return std::nullopt;
 }
 

components/brave_ads/core/internal/prefs/pref_provider.cc

@@ -53,33 +53,16 @@ bool PrefProvider::HasLocalStatePrefPath(const std::string& pref_path) const {
 
 std::optional<base::Value> PrefProvider::GetVirtualPref(
     const std::string& virtual_pref_path) const {
-  if (virtual_pref_path.starts_with("[virtual]:")) {
+  if (virtual_pref_path.starts_with(kVirtualPrefPathPrefix)) {
     const std::string pref_path =
-        virtual_pref_path.substr(/*pos=*/std::size("[virtual]:"));
+        virtual_pref_path.substr(/*pos=*/std::strlen(kVirtualPrefPathPrefix));
 
     if (const base::Value* const value = virtual_prefs_.Find(pref_path)) {
       return value->Clone();
     }
   }
 
-  if (virtual_pref_path.starts_with("[virtual_default]:")) {
-    const std::string pref_path =
-        virtual_pref_path.substr(/*pos=*/std::size("[virtual_default]:"));
-
-    if (HasProfilePrefPath(pref_path)) {
-      return GetProfilePref(pref_path);
-    }
-
-    if (HasLocalStatePrefPath(pref_path)) {
-      return GetLocalStatePref(pref_path);
-    }
-
-    if (const base::Value* const value = virtual_prefs_.Find(pref_path)) {
-      return value->Clone();
-    }
-  }
-
-  // Unknown virtual pref path.
+  // The virtual preference does not exist.
   return std::nullopt;
 }
 

components/brave_ads/core/internal/serving/new_tab_page_ad_serving_condition_matcher_util_internal.cc

@@ -38,9 +38,8 @@ std::optional<base::Value> MaybeGetRootPrefValue(
     const std::string& pref_path) {
   CHECK(pref_provider);
 
-  if (std::optional<base::Value> pref_value =
-          pref_provider->GetVirtualPref(pref_path)) {
-    return pref_value;
+  if (pref_path.starts_with(kVirtualPrefPathPrefix)) {
+    return pref_provider->GetVirtualPref(pref_path);
   }
 
   if (std::optional<base::Value> pref_value =

components/brave_ads/core/internal/serving/new_tab_page_ad_serving_condition_matcher_util_internal_unittest.cc

@@ -290,6 +290,33 @@ TEST_F(BraveAdsNewTabPageAdServingConditionMatcherUtilInternalTest,
   EXPECT_FALSE(MatchRegex("foo.baz.bar", "bar.*.foo"));
 }
 
+TEST_F(BraveAdsNewTabPageAdServingConditionMatcherUtilInternalTest,
+       GetVirtualPrefValue) {
+  // Arrange
+  test::RegisterProfileIntegerPref("matrix", 303);
+
+  ON_CALL(ads_client_mock_, GetVirtualPrefs).WillByDefault([]() {
+    return base::Value::Dict().Set("matrix", 101);
+  });
+
+  // Act & Assert
+  EXPECT_EQ(base::Value(101),
+            MaybeGetPrefValue(&pref_provider_, "[virtual]:matrix"));
+}
+
+TEST_F(BraveAdsNewTabPageAdServingConditionMatcherUtilInternalTest,
+       DoNotGetUnknownVirtualPrefValue) {
+  // Arrange
+  test::RegisterProfileIntegerPref("matrix", 303);
+
+  ON_CALL(ads_client_mock_, GetVirtualPrefs).WillByDefault([]() {
+    return base::Value::Dict().Set("inverse.matrices", 101);
+  });
+
+  // Act & Assert
+  EXPECT_FALSE(MaybeGetPrefValue(&pref_provider_, "[virtual]:matrix"));
+}
+
 TEST_F(BraveAdsNewTabPageAdServingConditionMatcherUtilInternalTest,
        GetBooleanProfilePrefValue) {
   // Arrange

components/brave_ads/core/public/ads_client/ads_client.h

@@ -57,9 +57,6 @@ class ADS_EXPORT AdsClient {
   // Close the notification ad for the specified `placement_id`.
   virtual void CloseNotificationAd(const std::string& placement_id) = 0;
 
-  // Returns a default search engine.
-  virtual base::Value::Dict GetVirtualPrefs() const = 0;
-
   // Cache an ad event for the specified instance `id`, `mojom_ad_type`,
   // `mojom_confirmation_type` and `time`.
   virtual void CacheAdEventForInstanceId(
@@ -164,6 +161,9 @@ class ADS_EXPORT AdsClient {
   // preference `path`.
   virtual bool HasLocalStatePrefPath(const std::string& path) const = 0;
 
+  // Returns a default search engine.
+  virtual base::Value::Dict GetVirtualPrefs() const = 0;
+
   // Log a `message` to `file` and the console log with `line` and
   // `verbose_level`.
   virtual void Log(const char* file,

components/brave_ads/core/public/prefs/pref_provider_interface.h

@@ -13,6 +13,8 @@
 
 namespace brave_ads {
 
+inline constexpr char kVirtualPrefPathPrefix[] = "[virtual]:";
+
 class PrefProviderInterface {
  public:
   virtual ~PrefProviderInterface() = default;

components/ntp_background_images/browser/view_counter_service.cc

@@ -274,6 +274,16 @@ ViewCounterService::GetNextBrandedWallpaperWhichMatchesConditions() {
   const auto initial_branded_wallpaper_index =
       model_.GetCurrentBrandedImageIndex();
 
+  base::Value::Dict virtual_prefs;
+  if (ads_service_) {
+    const brave_ads::AdsService::Delegate* const delegate =
+        ads_service_->GetDelegate();
+    CHECK(delegate);
+    virtual_prefs = delegate->GetVirtualPrefs();
+  }
+  const brave_ads::PrefProvider pref_provider(prefs_, local_state_prefs_,
+                                              std::move(virtual_prefs));
+
   do {
     std::optional<base::Value::Dict> branded_wallpaper =
         GetCurrentBrandedWallpaperFromModel();
@@ -289,15 +299,6 @@ ViewCounterService::GetNextBrandedWallpaperWhichMatchesConditions() {
       return branded_wallpaper;
     }
 
-    base::Value::Dict virtual_prefs;
-    if (ads_service_) {
-      brave_ads::AdsService::Delegate* delegate = ads_service_->GetDelegate();
-      CHECK(delegate);
-      virtual_prefs = delegate->GetVirtualPrefs();
-    }
-
-    const brave_ads::PrefProvider pref_provider(prefs_, local_state_prefs_,
-                                                std::move(virtual_prefs));
     if (brave_ads::MatchConditions(&pref_provider, *condition_matchers)) {
       // The branded wallpaper matches the conditions, so we can return it.
       return branded_wallpaper;

components/services/bat_ads/bat_ads_client_mojo_bridge.cc

@@ -112,16 +112,6 @@ void BatAdsClientMojoBridge::CloseNotificationAd(
   }
 }
 
-base::Value::Dict BatAdsClientMojoBridge::GetVirtualPrefs() const {
-  if (!bat_ads_client_associated_remote_.is_bound()) {
-    return {};
-  }
-
-  base::Value::Dict virtual_prefs;
-  bat_ads_client_associated_remote_->GetVirtualPrefs(&virtual_prefs);
-  return virtual_prefs;
-}
-
 void BatAdsClientMojoBridge::CacheAdEventForInstanceId(
     const std::string& id,
     const brave_ads::mojom::AdType mojom_ad_type,
@@ -405,6 +395,18 @@ bool BatAdsClientMojoBridge::HasLocalStatePrefPath(
   return value;
 }
 
+base::Value::Dict BatAdsClientMojoBridge::GetVirtualPrefs() const {
+  if (!bat_ads_client_associated_remote_.is_bound()) {
+    return {};
+  }
+
+  base::Value::Dict virtual_prefs;
+  bat_ads_client_associated_remote_->GetVirtualPrefs(&virtual_prefs);
+  return virtual_prefs;
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
 std::optional<base::Value> BatAdsClientMojoBridge::CachedProfilePrefValue(
     const std::string& path) const {
   if (!cached_profile_prefs_.contains(path)) {

components/services/bat_ads/bat_ads_client_mojo_bridge.h

@@ -62,8 +62,6 @@ class BatAdsClientMojoBridge : public brave_ads::AdsClient {
   void ShowNotificationAd(const brave_ads::NotificationAdInfo& ad) override;
   void CloseNotificationAd(const std::string& placement_id) override;
 
-  base::Value::Dict GetVirtualPrefs() const override;
-
   void CacheAdEventForInstanceId(
       const std::string& id,
       brave_ads::mojom::AdType mojom_ad_type,
@@ -115,6 +113,8 @@ class BatAdsClientMojoBridge : public brave_ads::AdsClient {
   void ClearLocalStatePref(const std::string& path) override;
   bool HasLocalStatePrefPath(const std::string& path) const override;
 
+  base::Value::Dict GetVirtualPrefs() const override;
+
   void Log(const char* file,
            int line,
            int verbose_level,

ios/browser/api/ads/ads_client_bridge.h

@@ -27,7 +27,6 @@
 - (void)notifyPendingObservers;
 - (bool)isNetworkConnectionAvailable;
 - (bool)canShowNotificationAds;
-- (base::Value::Dict)getVirtualPrefs;
 - (void)loadResourceComponent:(const std::string&)id
                       version:(const int)version
                      callback:(brave_ads::LoadFileCallback)callback;
@@ -73,6 +72,7 @@
 - (std::optional<base::Value>)getLocalStatePref:(const std::string&)path;
 - (void)clearLocalStatePref:(const std::string&)path;
 - (bool)hasLocalStatePrefPath:(const std::string&)path;
+- (base::Value::Dict)getVirtualPrefs;
 - (void)recordP2AEvents:(const std::vector<std::string>&)events;
 
 @end

ios/browser/api/ads/ads_client_ios.mm

@@ -74,10 +74,6 @@
                                 time:time];
 }
 
-base::Value::Dict AdsClientIOS::GetVirtualPrefs() const {
-  return [bridge_ getVirtualPrefs];
-}
-
 std::vector<base::Time> AdsClientIOS::GetCachedAdEvents(
     const brave_ads::mojom::AdType mojom_ad_type,
     const brave_ads::mojom::ConfirmationType mojom_confirmation_type) const {
@@ -189,6 +185,10 @@
   return [bridge_ hasLocalStatePrefPath:path];
 }
 
+base::Value::Dict AdsClientIOS::GetVirtualPrefs() const {
+  return [bridge_ getVirtualPrefs];
+}
+
 void AdsClientIOS::RecordP2AEvents(const std::vector<std::string>& events) {
   [bridge_ recordP2AEvents:events];
 }

ios/browser/api/ads/brave_ads.mm

@@ -1288,10 +1288,6 @@ - (void)closeNotificationAd:(const std::string&)placement_id {
       closeNotificationAd:base::SysUTF8ToNSString(placement_id)];
 }
 
-- (base::Value::Dict)getVirtualPrefs {
-  return {};
-}
-
 - (void)cacheAdEventForInstanceId:(const std::string&)id
                            adType:(const brave_ads::mojom::AdType)mojom_ad_type
                  confirmationType:(const brave_ads::mojom::ConfirmationType)
@@ -1499,6 +1495,11 @@ - (bool)hasLocalStatePrefPath:(const std::string&)path {
   return self.localStatePrefService->HasPrefPath(path);
 }
 
+- (base::Value::Dict)getVirtualPrefs {
+  // Intentionally empty.
+  return {};
+}
+
 - (void)log:(const char*)file
             line:(const int)line
     verboseLevel:(const int)verbose_level