Skip to content

Commit c7a4d2e

Browse files
committed
refactor(ads): remove injectAds method and decouple ad loading from FeedDecoratorService
- Remove public `injectAds` method to prevent injecting loaded AdFeedItems - FeedDecoratorService now only injects stateless AdPlaceholder markers - Actual ad loading and management is handled by new AdLoaderWidget in UI layer - This change simplifies FeedDecoratorService, improves performance, and prevents crashes related to stateful native ad objects
1 parent 01bc4ad commit c7a4d2e

File tree

1 file changed

+29
-31
lines changed

1 file changed

+29
-31
lines changed

lib/shared/services/feed_decorator_service.dart

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'dart:math';
33
import 'package:core/core.dart';
44
import 'package:data_repository/data_repository.dart';
55
import 'package:flutter_news_app_mobile_client_full_source_code/ads/ad_service.dart';
6+
import 'package:flutter_news_app_mobile_client_full_source_code/ads/models/ad_placeholder.dart'; // Import the new AdPlaceholder model
67
import 'package:flutter_news_app_mobile_client_full_source_code/ads/models/ad_theme_style.dart';
78
import 'package:flutter_news_app_mobile_client_full_source_code/router/routes.dart';
89
import 'package:uuid/uuid.dart';
@@ -164,29 +165,25 @@ class FeedDecoratorService {
164165
);
165166
}
166167

167-
/// Injects only [Ad] items into a list of [FeedItem]s.
168-
///
169-
/// This method is designed for pagination, where new content is added to an
170-
/// existing feed without re-evaluating or injecting new decorators.
171-
///
172-
/// Returns a new list of [FeedItem] objects, interspersed with ads.
173-
Future<List<FeedItem>> injectAds({
174-
required List<FeedItem> feedItems,
175-
required User? user,
176-
required AdConfig adConfig,
177-
required HeadlineImageStyle imageStyle,
178-
required AdThemeStyle adThemeStyle,
179-
int processedContentItemCount = 0,
180-
}) async {
181-
return _injectAds(
182-
feedItems: feedItems,
183-
user: user,
184-
adConfig: adConfig,
185-
processedContentItemCount: processedContentItemCount,
186-
imageStyle: imageStyle,
187-
adThemeStyle: adThemeStyle,
188-
);
189-
}
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.
190187

191188
/// Determines the single highest-priority feed decorator that is currently
192189
/// due to be shown to the user.
@@ -428,14 +425,15 @@ class FeedDecoratorService {
428425
// multiple of the ad frequency.
429426
if (currentContentItemCount >= adPlacementInterval &&
430427
(currentContentItemCount - adPlacementInterval) % adFrequency == 0) {
431-
// Request an ad from the AdService.
432-
final adToInject = await _adService.getAd(
433-
imageStyle: imageStyle,
434-
adThemeStyle: adThemeStyle,
435-
);
436-
if (adToInject != null) {
437-
result.add(adToInject);
438-
}
428+
// Instead of injecting a fully loaded ad, inject an AdPlaceholder.
429+
// This is a crucial change: the FeedDecoratorService no longer loads
430+
// the actual ad. It only marks a spot for an ad.
431+
//
432+
// The actual ad loading will be handled by a dedicated `AdLoaderWidget`
433+
// in the UI layer when this placeholder scrolls into view. This
434+
// decouples ad loading from the BLoC's state and allows for efficient
435+
// caching and disposal of native ad resources.
436+
result.add(AdPlaceholder(id: _uuid.v4()));
439437
}
440438
}
441439
return result;

0 commit comments

Comments
 (0)