Skip to content

Commit f959dd4

Browse files
committed
refactor(feed): decouple ad injection and simplify decorator service
- Replace ad injection with stateless ad placeholder injection - Remove public `injectAds` method and related ad loading responsibilities - Update class documentation
1 parent c7a4d2e commit f959dd4

File tree

1 file changed

+35
-35
lines changed

1 file changed

+35
-35
lines changed

lib/shared/services/feed_decorator_service.dart

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,22 @@ class _DecoratorCandidate {
5151
}
5252

5353
/// A service responsible for decorating a primary list of feed content (e.g.,
54-
/// headlines) with secondary items like in-feed calls-to-action and ads.
54+
/// headlines) with secondary items like in-feed calls-to-action and ad placeholders.
5555
///
5656
/// This service implements a multi-stage pipeline to ensure that the most
5757
/// relevant and timely items are injected in a logical and non-intrusive way.
58+
///
59+
/// **Ad Injection Architecture:**
60+
/// In the current architecture, the responsibility for loading and managing
61+
/// native ads is completely decoupled from this service and the BLoC layer.
62+
/// Instead of injecting fully loaded `AdFeedItem` objects, this service now
63+
/// only injects stateless `AdPlaceholder` markers into the feed.
64+
///
65+
/// The actual ad loading and lifecycle management (including caching and
66+
/// disposal) is handled by the `AdLoaderWidget` in the UI layer. This
67+
/// simplifies this service's role and prevents the BLoC from holding onto
68+
/// stateful native ad objects, which was the root cause of previous crashes
69+
/// and performance issues during scrolling.
5870
class FeedDecoratorService {
5971
/// Creates a [FeedDecoratorService].
6072
///
@@ -148,9 +160,9 @@ class FeedDecoratorService {
148160
}
149161
}
150162

151-
// --- Step 2: Ad Injection ---
152-
// Inject ads into the list that may or may not already contain a decorator.
153-
final finalFeed = await _injectAds(
163+
// --- Step 2: Ad Placeholder Injection ---
164+
// Inject ad placeholders into the list that may or may not already contain a decorator.
165+
final finalFeed = await injectAdPlaceholders(
154166
feedItems: feedWithDecorators,
155167
user: user,
156168
adConfig: remoteConfig.adConfig,
@@ -165,26 +177,6 @@ class FeedDecoratorService {
165177
);
166178
}
167179

168-
// The public `injectAds` method has been removed.
169-
//
170-
// Rationale for removal:
171-
// In the new architecture, the responsibility for loading and managing
172-
// native ads has been completely decoupled from the FeedDecoratorService
173-
// and the BLoC. Instead of injecting fully loaded `AdFeedItem` objects,
174-
// this service now only injects stateless `AdPlaceholder` markers.
175-
//
176-
// The actual ad loading and lifecycle management (including caching and
177-
// disposal) is now handled by the new `AdLoaderWidget` in the UI layer.
178-
// This simplifies the FeedDecoratorService's role and prevents the BLoC
179-
// from holding onto stateful native ad objects, which was the root cause
180-
// of previous crashes and performance issues during scrolling.
181-
//
182-
// Therefore, the `_injectAds` private method is now only called internally
183-
// by `decorateFeed` to place placeholders, and there is no longer a need
184-
// for a public method to inject already-loaded ads for pagination.
185-
// Pagination now simply adds more content, and the `AdLoaderWidget`
186-
// handles ad loading on demand.
187-
188180
/// Determines the single highest-priority feed decorator that is currently
189181
/// due to be shown to the user.
190182
///
@@ -353,22 +345,30 @@ class FeedDecoratorService {
353345
}
354346
}
355347

356-
/// Injects ads into a list of [FeedItem]s based on frequency rules.
348+
/// Injects stateless [AdPlaceholder] markers into a list of [FeedItem]s
349+
/// based on configured ad frequency rules.
357350
///
358-
/// This method ensures that ads are placed according to the `adPlacementInterval`
359-
/// (initial buffer before the first ad) and `adFrequency` (subsequent ad spacing).
360-
/// It correctly accounts for content items only, ignoring previously injected ads
361-
/// when calculating placement.
351+
/// This method ensures that ad placeholders are placed according to the
352+
/// `adPlacementInterval` (initial buffer before the first ad) and
353+
/// `adFrequency` (subsequent ad spacing). It correctly accounts for
354+
/// content items only, ignoring previously injected decorators when
355+
/// calculating placement.
362356
///
363-
/// [feedItems]: The list of feed items (headlines, decorators) to inject ads into.
357+
/// [feedItems]: The list of feed items (headlines, other decorators)
358+
/// to inject ad placeholders into.
364359
/// [user]: The current authenticated user, used to determine ad configuration.
365360
/// [adConfig]: The remote configuration for ad display rules.
366-
/// [processedContentItemCount]: The count of *content items* (non-ad, non-decorator)
367-
/// that have already been processed in previous feed loads/pages. This is
368-
/// crucial for maintaining correct ad placement across pagination.
361+
/// [imageStyle]: The desired image style for the ad, used to determine
362+
/// the placeholder's template type.
363+
/// [adThemeStyle]: The current theme style for ads, passed through to the
364+
/// AdLoaderWidget for consistent styling.
365+
/// [processedContentItemCount]: The count of *content items* (non-ad,
366+
/// non-decorator) that have already been processed in previous feed
367+
/// loads/pages. This is crucial for maintaining correct ad placement
368+
/// across pagination.
369369
///
370-
/// Returns a new list of [FeedItem] objects, interspersed with ads.
371-
Future<List<FeedItem>> _injectAds({
370+
/// Returns a new list of [FeedItem] objects, interspersed with ad placeholders.
371+
Future<List<FeedItem>> injectAdPlaceholders({
372372
required List<FeedItem> feedItems,
373373
required User? user,
374374
required AdConfig adConfig,

0 commit comments

Comments
 (0)