Commit 570fe6f2 authored by sczs's avatar sczs Committed by Commit Bot

[ios] Adds DiscoverFeedVC as ContentSuggestions ChildVC

In order to properly handle the life cycle of the DiscoverFeedVC the
following changes are made:

- Makes the DiscoverVC a ChildVC of the ContentSuggestionsVC and removes
it on dealloc.
- ContentSuggestionsCoordinator now owns the DiscoverFeedVC.
- Adds isDiscoverItem method in ContentSuggestionsCollectionUpdater.

Without these changes the DiscoverFeedVC is not properly dealloc-ed
and might lead to some crashes.

Bug: 1085419
Change-Id: If4b7c1e2ada6876c036527ac5eefdae020339327
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2258367
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781415}
parent b2ca069f
......@@ -19,7 +19,7 @@
: CollectionViewItem <SuggestedContent>
// Contains the Discover feed coming from Discover provider.
@property(strong, nonatomic) UIViewController* discoverFeed;
@property(nonatomic, weak) UIViewController* discoverFeed;
@end
......
......@@ -13,7 +13,7 @@
@interface ContentSuggestionsDiscoverItem ()
// Contains a reference to the last configured cell containing the feed.
@property(strong, nonatomic) ContentSuggestionsDiscoverCell* lastConfiguredCell;
@property(nonatomic, weak) ContentSuggestionsDiscoverCell* lastConfiguredCell;
@end
......@@ -46,7 +46,7 @@
@interface ContentSuggestionsDiscoverCell ()
// The Discover feed which acts as the cell's content.
@property(strong, nonatomic) UIViewController* discoverFeed;
@property(nonatomic, weak) UIViewController* discoverFeed;
@end
......
......@@ -84,6 +84,9 @@ addSuggestionsToModel:
// items.
- (BOOL)isContentSuggestionsSection:(NSInteger)section;
// Returns whether |itemType| is a Discover ItemType.
- (BOOL)isDiscoverItem:(NSInteger)itemType;
// Dismisses the |item| from the model. Does not change the UI.
- (void)dismissItem:(CollectionViewItem<SuggestedContent>*)item;
......
......@@ -580,6 +580,10 @@ addSuggestionsToModel:(NSArray<CSCollectionViewItem*>*)suggestions
sectionIdentifierForSection:section]);
}
- (BOOL)isDiscoverItem:(NSInteger)itemType {
return itemType == ItemTypeDiscover;
}
- (void)dismissItem:(CSCollectionViewItem*)item {
[self.dataSource dismissSuggestion:item.suggestionIdentifier];
}
......
......@@ -69,6 +69,7 @@
ContentSuggestionsHeaderSynchronizer* headerCollectionInteractionHandler;
@property(nonatomic, strong) ContentSuggestionsMetricsRecorder* metricsRecorder;
@property(nonatomic, strong) NTPHomeMediator* NTPMediator;
@property(nonatomic, strong) UIViewController* discoverFeedViewController;
// Redefined as readwrite.
@property(nonatomic, strong, readwrite)
......@@ -156,7 +157,7 @@
ReadingListModelFactory::GetForBrowserState(
self.browser->GetBrowserState());
UIViewController* discoverFeed =
self.discoverFeedViewController =
ios::GetChromeBrowserProvider()
->GetDiscoverFeedProvider()
->NewFeedViewController(static_cast<id<ApplicationCommands>>(
......@@ -169,7 +170,7 @@
mostVisitedSite:std::move(mostVisitedFactory)
readingListModel:readingListModel
prefService:prefs
discoverFeed:discoverFeed];
discoverFeed:self.discoverFeedViewController];
self.contentSuggestionsMediator.commandHandler = self.NTPMediator;
self.contentSuggestionsMediator.headerProvider = self.headerController;
self.contentSuggestionsMediator.contentArticlesExpanded =
......
......@@ -11,6 +11,7 @@
#import "ios/chrome/browser/ui/collection_view/cells/collection_view_item.h"
#import "ios/chrome/browser/ui/collection_view/collection_view_model.h"
#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_cell.h"
#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_discover_item.h"
#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_most_visited_cell.h"
#import "ios/chrome/browser/ui/content_suggestions/cells/suggested_content.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h"
......@@ -56,6 +57,10 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
// The overscroll actions controller managing accelerators over the toolbar.
@property(nonatomic, strong)
OverscrollActionsController* overscrollActionsController;
// The DiscoverFeedVC that might be displayed by this VC.
@property(nonatomic, weak) UIViewController* discoverFeedVC;
@end
@implementation ContentSuggestionsViewController
......@@ -83,6 +88,9 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
}
- (void)dealloc {
[self.discoverFeedVC willMoveToParentViewController:nil];
[self.discoverFeedVC.view removeFromSuperview];
[self.discoverFeedVC removeFromParentViewController];
[self.overscrollActionsController invalidate];
}
......@@ -366,6 +374,19 @@ NSString* const kContentSuggestionsMostVisitedAccessibilityIdentifierPrefix =
CSCollectionViewItem* item =
[self.collectionViewModel itemAtIndexPath:indexPath];
if ([self.collectionUpdater
isDiscoverItem:[self.collectionViewModel
itemTypeForIndexPath:indexPath]]) {
ContentSuggestionsDiscoverItem* discoverFeedItem =
static_cast<ContentSuggestionsDiscoverItem*>(item);
self.discoverFeedVC = discoverFeedItem.discoverFeed;
[self addChildViewController:self.discoverFeedVC];
UICollectionViewCell* cell = [super collectionView:collectionView
cellForItemAtIndexPath:indexPath];
[self.discoverFeedVC didMoveToParentViewController:self];
return cell;
}
if ([self.collectionUpdater isContentSuggestionsSection:indexPath.section] &&
[self.collectionUpdater contentSuggestionTypeForItem:item] !=
ContentSuggestionTypeEmpty &&
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment