Add application-services/ads-client #29737

Almaju · 2025-10-01T21:48:23Z

Replace existing HTTP requests using the Rust AdsClient in UnifiedAdsProvider from application-services.
For now this is behind a feature flag and only in developer while we iterate.
We will need to implement the cache in our Rust component.

issammani · 2025-10-02T15:50:13Z

Thanks @Almaju. I can take a look at the AS bits next week. Tagging @lmarceau since she did touch the ads code a while back and has context.

mobiletest-ci-bot · 2025-10-02T16:34:06Z

	Messages
📖	Project coverage: 38.83%

💪 Quality guardian

1 tests files modified. You're a champion of test coverage! 🚀

🥇 Perfect PR size

Smaller PRs are easier to review. Thanks for making life easy for reviewers! ✨

❌ Per-file test coverage gate

The following changed file(s) are below 35.0% coverage:

File	Coverage	Required
`MozillaRustComponents/Sources/MozillaRustComponentsWrapper/AdsClient/AdsClient.swift`	0.0%	35.0%

Client.app: Coverage: 37.28

File	Coverage
UnifiedAdsProvider.swift	89.66%	✅
UnifiedTile.swift	100.0%	✅
NimbusFlaggableFeature.swift	99.47%	✅
NimbusFeatureFlagLayer.swift	76.61%	✅

Generated by 🚫 Danger Swift against e0d37ad

firefox-ios/Client/Frontend/Home/TopSites/DataManagement/UnifiedAds/UnifiedAdsProvider.swift

firefox-ios/Storage/Rust/RustAdsClient.swift

firefox-ios/Client/Frontend/Home/UnifiedAds/UnifiedTile.swift

firefox-ios/Client/Frontend/Home/UnifiedAds/UnifiedAdsProvider.swift

mergify · 2025-10-15T20:56:56Z

This pull request has conflicts when rebasing. Could you fix it @Almaju? 🙏

mergify · 2025-10-28T21:27:06Z

This pull request has conflicts when rebasing. Could you fix it @Almaju? 🙏

mergify · 2025-11-06T09:17:49Z

This pull request has conflicts when rebasing. Could you fix it @Almaju? 🙏

github-actions · 2025-11-21T00:34:16Z

This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions!

lmarceau · 2025-11-21T19:05:01Z

Hello @Almaju! Realized this has been sitting for a while sorry about that. Do we have any blockers? I think this needs to be rebase to fix conflicts and we can move forward with it?

Almaju · 2025-11-25T18:45:22Z

Hi @lmarceau
Thanks for pinging!
We are finalizing how we want to use telemetry using glean and then we will be ready to move forward!

github-actions · 2025-12-10T00:36:24Z

This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions!

mergify · 2025-12-10T18:45:11Z

This pull request has conflicts when rebasing. Could you fix it @Almaju? 🙏

lmarceau · 2025-12-11T14:54:21Z

firefox-ios/Client/Frontend/Home/UnifiedAds/UnifiedAdsProvider.swift

+                        if featureFlags.isFeatureEnabled(.adsClient, checking: .buildOnly) {
+            fetchTilesWithAdsClient(completion: completion)
+        } else {


Indentation seems off here 👀 If you're using Xcode, I am a fan of the keyboard shortcut Ctrl + i to automatically indent code (you need to select the block of code you want to auto-indent, then do the shortcut).

Oh indeed nice! ✅

lmarceau · 2025-12-11T14:55:13Z

firefox-ios/Client/Frontend/Home/UnifiedAds/UnifiedAdsProvider.swift

    ///   - timestamp: The timestamp to retrieve from cache, useful for tests. Default is Date.now()
    ///   - completion: Returns an array of Tiles, can be empty
-    func fetchTiles(timestamp: Timestamp, completion: @escaping @Sendable (UnifiedTileResult) -> Void)
+    func fetchTiles(timestamp: Shared.Timestamp, completion: @escaping @Sendable (UnifiedTileResult) -> Void)


nit; It's not necessary to add the package on Timestamp. Same comment to lines 23 and 32.

Suggested change

func fetchTiles(timestamp: Shared.Timestamp, completion: @escaping @Sendable (UnifiedTileResult) -> Void)

func fetchTiles(timestamp: Timestamp, completion: @escaping @Sendable (UnifiedTileResult) -> Void)

It says it is ambiguous because of two candidates:

public typealias Timestamp = Int64 in MozillaRustComponents/Sources/MozillaRustComponentsWrapper/Generated/tabs.swift

public typealias Timestamp = UInt64 in BrowserKit/Sources/Shared/Date/TimeConstants.swift

lmarceau · 2025-12-11T14:56:05Z

firefox-ios/Client/Frontend/Home/UnifiedAds/UnifiedAdsProvider.swift

+        self.logger.log("Fetching tiles with ads client", level: .info, category: .homepage)
+        let mozAdRequests = [
+            MozAdsPlacementRequest(placementId: TileOrder.position1.rawValue, iabContent: nil),
+            MozAdsPlacementRequest(placementId: TileOrder.position2.rawValue, iabContent: nil)
+        ]
+        do {
+            let mozAdsImages = try self.adsClient.requestImageAds(mozAdRequests: mozAdRequests, options: nil)
+            let unifiedTiles: [UnifiedTile] = mozAdsImages.map { name, mozAdsImage in
+                UnifiedTile.from(name: name, mozAdsImage: mozAdsImage)
+            }
+            self.logger.log("Ads client request successful", level: .info, category: .homepage)
+            completion(.success(unifiedTiles))
+        } catch let error {
+            self.logger.log("Ads client request failed: \(error)", level: .warning, category: .homepage)
+            completion(.failure(Error.noDataAvailable))


nit; it's not needed to use self. in this case. We prefer to keep self. usage where it's needed like in closures

Suggested change

self.logger.log("Fetching tiles with ads client", level: .info, category: .homepage)

let mozAdRequests = [

MozAdsPlacementRequest(placementId: TileOrder.position1.rawValue, iabContent: nil),

MozAdsPlacementRequest(placementId: TileOrder.position2.rawValue, iabContent: nil)

]

do {

let mozAdsImages = try self.adsClient.requestImageAds(mozAdRequests: mozAdRequests, options: nil)

let unifiedTiles: [UnifiedTile] = mozAdsImages.map { name, mozAdsImage in

UnifiedTile.from(name: name, mozAdsImage: mozAdsImage)

}

self.logger.log("Ads client request successful", level: .info, category: .homepage)

completion(.success(unifiedTiles))

} catch let error {

self.logger.log("Ads client request failed: \(error)", level: .warning, category: .homepage)

completion(.failure(Error.noDataAvailable))

logger.log("Fetching tiles with ads client", level: .info, category: .homepage)

let mozAdRequests = [

MozAdsPlacementRequest(placementId: TileOrder.position1.rawValue, iabContent: nil),

MozAdsPlacementRequest(placementId: TileOrder.position2.rawValue, iabContent: nil)

]

do {

let mozAdsImages = try self.adsClient.requestImageAds(mozAdRequests: mozAdRequests, options: nil)

let unifiedTiles: [UnifiedTile] = mozAdsImages.map { name, mozAdsImage in

UnifiedTile.from(name: name, mozAdsImage: mozAdsImage)

}

logger.log("Ads client request successful", level: .info, category: .homepage)

completion(.success(unifiedTiles))

} catch let error {

logger.log("Ads client request failed: \(error)", level: .warning, category: .homepage)

completion(.failure(Error.noDataAvailable))

Oh okay I did not know that about Swift, this is done ✅

issammani · 2025-12-11T15:03:14Z

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/Generated/Metrics/Metrics.swift

        public static let info = BuildInfo(buildDate: DateComponents(calendar: Calendar.current, timeZone: TimeZone(abbreviation: "UTC"), year: 2025, month: 12, day: 9, hour: 5, minute: 18, second: 29))
    }

+    enum AdsClient {


These changes should come from the AS. I suppose this will come in once we have merged: mozilla/application-services#7111

Absolutely, I did the rebase ✅

issammani · 2025-12-11T15:03:34Z

MozillaRustComponents/Sources/MozillaRustComponentsWrapper/Generated/ads_client.swift


 // Public interface members begin here.
-
+// Magic number for the Rust proxy to call using the same mechanism as every other method,


Same here. If you rebase you shouldn't need these changes. Anything under Generated should come directly from uniffi generated files.

Rebased ✅

issammani · 2025-12-16T16:17:03Z

@Almaju merged #31310. You shouldn't need the changes in the Generated/ files anymore

github-actions · 2025-12-31T00:37:18Z

This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions!

mergify · 2025-12-31T00:37:55Z

This pull request has conflicts when rebasing. Could you fix it @Almaju? 🙏

Almaju · 2026-01-05T18:07:13Z

@lmarceau @issammani happy new year! This should be ready for final review! 🥳

Almaju requested a review from luc-lisi October 1, 2025 21:48

Almaju requested a review from a team as a code owner October 1, 2025 21:48

Almaju requested a review from FilippoZazzeroni October 1, 2025 21:48

Almaju force-pushed the add-ads-client branch from ccf0de7 to b0142ec Compare October 2, 2025 15:43

issammani requested review from issammani and lmarceau and removed request for FilippoZazzeroni October 2, 2025 15:49

Almaju force-pushed the add-ads-client branch from b0142ec to 0a79333 Compare October 2, 2025 16:23

Almaju force-pushed the add-ads-client branch from 7a785bf to 015031a Compare October 2, 2025 20:47

lmarceau reviewed Oct 2, 2025

View reviewed changes

lmarceau self-requested a review October 8, 2025 00:58

lmarceau reviewed Oct 9, 2025

View reviewed changes

firefox-ios/Client/Frontend/Home/UnifiedAds/UnifiedAdsProvider.swift Show resolved Hide resolved

Almaju force-pushed the add-ads-client branch from ad0128f to 3ccaa74 Compare October 15, 2025 21:02

Almaju force-pushed the add-ads-client branch 3 times, most recently from 541f314 to c6c2e74 Compare October 28, 2025 21:26

Almaju force-pushed the add-ads-client branch 2 times, most recently from f210bd4 to 0257558 Compare October 30, 2025 21:16

github-actions bot added the stale Stalebot use this label to stale issues and PRs label Nov 21, 2025

lmarceau self-requested a review November 21, 2025 19:05

github-actions bot removed the stale Stalebot use this label to stale issues and PRs label Nov 22, 2025

Almaju force-pushed the add-ads-client branch from 0257558 to 398d25e Compare November 25, 2025 22:09

github-actions bot added the stale Stalebot use this label to stale issues and PRs label Dec 10, 2025

Almaju force-pushed the add-ads-client branch 2 times, most recently from 9d91fd6 to 1154a62 Compare December 10, 2025 19:41

github-actions bot removed the stale Stalebot use this label to stale issues and PRs label Dec 11, 2025

lmarceau reviewed Dec 11, 2025

View reviewed changes

issammani reviewed Dec 11, 2025

View reviewed changes

github-actions bot added the stale Stalebot use this label to stale issues and PRs label Dec 31, 2025

github-actions bot removed the stale Stalebot use this label to stale issues and PRs label Jan 1, 2026

Almaju force-pushed the add-ads-client branch 2 times, most recently from 84df90b to c8a2ac6 Compare January 5, 2026 18:01

feat: add ads-client experiment

e0d37ad

Almaju force-pushed the add-ads-client branch from c8a2ac6 to e0d37ad Compare January 5, 2026 18:04

Almaju requested review from issammani and lmarceau January 5, 2026 18:07

	func fetchTiles(timestamp: Shared.Timestamp, completion: @escaping @Sendable (UnifiedTileResult) -> Void)
	func fetchTiles(timestamp: Timestamp, completion: @escaping @Sendable (UnifiedTileResult) -> Void)


		// Public interface members begin here.

		// Magic number for the Rust proxy to call using the same mechanism as every other method,

Add application-services/ads-client #29737

Are you sure you want to change the base?

Add application-services/ads-client #29737

Conversation

Almaju commented Oct 1, 2025

Uh oh!

issammani commented Oct 2, 2025

Uh oh!

mobiletest-ci-bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💪 Quality guardian

🥇 Perfect PR size

❌ Per-file test coverage gate

Client.app: Coverage: 37.28

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mergify bot commented Oct 15, 2025

Uh oh!

mergify bot commented Oct 28, 2025

Uh oh!

mergify bot commented Nov 6, 2025

Uh oh!

github-actions bot commented Nov 21, 2025

Uh oh!

lmarceau commented Nov 21, 2025

Uh oh!

Almaju commented Nov 25, 2025

Uh oh!

github-actions bot commented Dec 10, 2025

Uh oh!

mergify bot commented Dec 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

issammani commented Dec 16, 2025

Uh oh!

github-actions bot commented Dec 31, 2025

Uh oh!

mergify bot commented Dec 31, 2025

Uh oh!

Almaju commented Jan 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mobiletest-ci-bot commented Oct 2, 2025 •

edited

Loading