Skip to content

Commit

Permalink
[ios] Move ContentSuggestions commands to ContentSuggestionsMediator
Browse files Browse the repository at this point in the history
Refactors NTP code so that ContentSuggestionsCommands and
ContentSuggestionsGestureCommands are sent to ContentSuggestionsMediator.
This not only makes sense, but will allow for logging Content Suggestion
metrics split by Start/NTP surfaces to be encapsulated in one place.

Change-Id: Ic1fe5e5bf8eb5229caa77dcaa6ee2aaad134831e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3563430
Reviewed-by: Adam Arcaro <adamta@google.com>
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Cr-Commit-Position: refs/heads/main@{#989497}
  • Loading branch information
Chris Lu authored and Chromium LUCI CQ committed Apr 6, 2022
1 parent 662ce67 commit b8ce303
Show file tree
Hide file tree
Showing 10 changed files with 482 additions and 352 deletions.
8 changes: 8 additions & 0 deletions ios/chrome/browser/ui/content_suggestions/BUILD.gn
Expand Up @@ -227,6 +227,7 @@ source_set("unit_tests") {
"content_suggestions_category_wrapper_unittest.mm",
"content_suggestions_collection_utils_unittest.mm",
"content_suggestions_header_synchronizer_unittest.mm",
"content_suggestions_mediator_unittest.mm",
"ntp_home_mediator_unittest.mm",
]
deps = [
Expand All @@ -235,13 +236,20 @@ source_set("unit_tests") {
":content_suggestions_ui_util",
"//base",
"//base/test:test_support",
"//components/favicon/core",
"//components/favicon/core/test:test_support",
"//components/ntp_snippets",
"//components/ntp_tiles",
"//components/reading_list/core",
"//components/signin/public/identity_manager",
"//components/sync_preferences:test_support",
"//ios/chrome/browser",
"//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/favicon",
"//ios/chrome/browser/main:public",
"//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/ntp_snippets",
"//ios/chrome/browser/reading_list",
"//ios/chrome/browser/search_engines",
"//ios/chrome/browser/signin",
"//ios/chrome/browser/signin:test_support",
Expand Down
Expand Up @@ -23,15 +23,6 @@
- (void)hideMostRecentTab;
// Handles the actions following a tap on the promo.
- (void)handlePromoTapped;
// Handles the actions following a tap on the "Manage Activity" item in the
// Discover feed menu.
- (void)handleFeedManageActivityTapped;
// Handles the actions following a tap on the "Manage Interests" item in the
// Discover feed menu.
- (void)handleFeedManageInterestsTapped;
// Handles the actions following a tap on the "Learn More" item in the Discover
// feed menu.
- (void)handleFeedLearnMoreTapped;

@end

Expand Down
Expand Up @@ -198,13 +198,18 @@ - (void)start {
mostVisitedSite:std::move(mostVisitedFactory)
readingListModel:readingListModel
prefService:prefs
isGoogleDefaultSearchProvider:isGoogleDefaultSearchProvider];
self.contentSuggestionsMediator.commandHandler = self.ntpMediator;
self.contentSuggestionsMediator.headerProvider = self.headerController;
isGoogleDefaultSearchProvider:isGoogleDefaultSearchProvider
browser:self.browser];
self.contentSuggestionsMediator.discoverFeedDelegate =
self.discoverFeedDelegate;
// TODO(crbug.com/1045047): Use HandlerForProtocol after commands protocol
// clean up.
self.contentSuggestionsMediator.dispatcher =
static_cast<id<ApplicationCommands, BrowserCommands, OmniboxCommands,
SnackbarCommands>>(self.browser->GetCommandDispatcher());
self.contentSuggestionsMediator.webStateList =
self.browser->GetWebStateList();
self.contentSuggestionsMediator.webState = self.webState;
[self configureStartSurfaceIfNeeded];

if (!IsContentSuggestionsHeaderMigrationEnabled()) {
Expand All @@ -216,14 +221,15 @@ - (void)start {
self.contentSuggestionsViewController =
[[ContentSuggestionsViewController alloc] init];
self.contentSuggestionsViewController.suggestionCommandHandler =
self.ntpMediator;
self.contentSuggestionsMediator;
self.contentSuggestionsViewController.audience = self;
self.contentSuggestionsViewController.menuProvider = self;
} else {
self.collectionViewController =
[[ContentSuggestionsCollectionViewController alloc]
initWithStyle:CollectionViewControllerStyleDefault];
self.collectionViewController.suggestionCommandHandler = self.ntpMediator;
self.collectionViewController.suggestionCommandHandler =
self.contentSuggestionsMediator;
self.collectionViewController.audience = self;
self.collectionViewController.contentSuggestionsEnabled =
self.contentSuggestionsEnabled;
Expand All @@ -243,11 +249,6 @@ - (void)start {
if (!IsContentSuggestionsHeaderMigrationEnabled()) {
self.ntpMediator.consumer = self.headerController;
}
// TODO(crbug.com/1045047): Use HandlerForProtocol after commands protocol
// clean up.
self.ntpMediator.dispatcher =
static_cast<id<ApplicationCommands, BrowserCommands, OmniboxCommands,
SnackbarCommands>>(self.browser->GetCommandDispatcher());
// IsContentSuggestionsUIViewControllerMigrationEnabled() doesn't need to set
// the suggestionsViewController since it won't be retrieving an item's index
// from the CollectionView model.
Expand Down Expand Up @@ -324,6 +325,13 @@ - (UICollectionViewController*)contentSuggestionsCollectionViewController {
return self.collectionViewController;
}

#pragma mark - Setters

- (void)setWebState:(web::WebState*)webState {
_webState = webState;
self.contentSuggestionsMediator.webState = webState;
}

#pragma mark - ContentSuggestionsViewControllerAudience

- (void)promoShown {
Expand All @@ -334,6 +342,9 @@ - (void)promoShown {
}

- (void)viewDidDisappear {
// Start no longer showing
self.contentSuggestionsMediator.showingStartSurface = NO;
// Update DiscoverFeedService to NO
if (ShouldShowReturnToMostRecentTabForStartSurface()) {
[self.contentSuggestionsMediator hideRecentTabTile];
}
Expand Down Expand Up @@ -444,7 +455,7 @@ - (UIContextMenuConfiguration*)contextMenuConfigurationForItem:
toView:nil];

[menuElements addObject:[actionFactory actionToOpenInNewTabWithBlock:^{
[weakSelf.ntpMediator
[weakSelf.contentSuggestionsMediator
openNewTabWithMostVisitedItem:item
incognito:NO
atIndex:index
Expand All @@ -453,10 +464,11 @@ - (UIContextMenuConfiguration*)contextMenuConfigurationForItem:

UIAction* incognitoAction =
[actionFactory actionToOpenInNewIncognitoTabWithBlock:^{
[weakSelf.ntpMediator openNewTabWithMostVisitedItem:item
incognito:YES
atIndex:index
fromPoint:centerPoint];
[weakSelf.contentSuggestionsMediator
openNewTabWithMostVisitedItem:item
incognito:YES
atIndex:index
fromPoint:centerPoint];
}];

if (IsIncognitoModeDisabled(
Expand Down Expand Up @@ -485,7 +497,8 @@ - (UIContextMenuConfiguration*)contextMenuConfigurationForItem:
}]];

[menuElements addObject:[actionFactory actionToRemoveWithBlock:^{
[weakSelf.ntpMediator removeMostVisited:item];
[weakSelf.contentSuggestionsMediator
removeMostVisited:item];
}]];

return [UIMenu menuWithTitle:@"" children:menuElements];
Expand All @@ -504,6 +517,8 @@ - (void)configureStartSurfaceIfNeeded {
if (!scene.modifytVisibleNTPForStartSurface)
return;

// Update Mediator property to signal the NTP is currently showing Start.
self.contentSuggestionsMediator.showingStartSurface = YES;
if (ShouldShowReturnToMostRecentTabForStartSurface()) {
base::RecordAction(
base::UserMetricsAction("IOS.StartSurface.ShowReturnToRecentTabTile"));
Expand Down
Expand Up @@ -10,7 +10,9 @@
#include <memory>

#include "components/prefs/pref_service.h"
#import "ios/chrome/browser/ui/content_suggestions/cells/content_suggestions_gesture_commands.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_consumer.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h"
#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_consumer.h"
#import "ios/chrome/browser/ui/start_surface/start_surface_recent_tab_removal_observer_bridge.h"

Expand All @@ -26,20 +28,23 @@ namespace user_prefs {
class PrefRegistrySyncable;
} // namespace user_prefs

@protocol ContentSuggestionsCommands;
@protocol ApplicationCommands;
class Browser;
@protocol BrowserCommands;
@protocol ContentSuggestionsCollectionConsumer;
@protocol ContentSuggestionsGestureCommands;
@protocol ContentSuggestionsHeaderProvider;
@protocol DiscoverFeedDelegate;
class GURL;
class LargeIconCache;
class NotificationPromoWhatsNew;
class ReadingListModel;
@protocol SnackbarCommands;
class WebStateList;

// Mediator for ContentSuggestions.
@interface ContentSuggestionsMediator
: NSObject <StartSurfaceRecentTabObserving>
: NSObject <ContentSuggestionsCommands,
ContentSuggestionsGestureCommands,
StartSurfaceRecentTabObserving>

// Default initializer.
- (instancetype)
Expand All @@ -50,20 +55,23 @@ class WebStateList;
readingListModel:(ReadingListModel*)readingListModel
prefService:(PrefService*)prefService
isGoogleDefaultSearchProvider:(BOOL)isGoogleDefaultSearchProvider
NS_DESIGNATED_INITIALIZER;
browser:(Browser*)browser NS_DESIGNATED_INITIALIZER;

- (instancetype)init NS_UNAVAILABLE;

// Registers the feature preferences.
+ (void)registerBrowserStatePrefs:(user_prefs::PrefRegistrySyncable*)registry;

// Dispatcher.
@property(nonatomic, weak)
id<ApplicationCommands, BrowserCommands, SnackbarCommands>
dispatcher;

// Command handler for the mediator.
@property(nonatomic, weak)
id<ContentSuggestionsCommands, ContentSuggestionsGestureCommands>
commandHandler;

@property(nonatomic, weak) id<ContentSuggestionsHeaderProvider> headerProvider;

// Delegate used to communicate to communicate events to the DiscoverFeed.
@property(nonatomic, weak) id<DiscoverFeedDelegate> discoverFeedDelegate;

Expand All @@ -72,9 +80,15 @@ class WebStateList;
collectionConsumer;
@property(nonatomic, weak) id<ContentSuggestionsConsumer> consumer;

// YES if the Start Surface is being shown.
@property(nonatomic, assign) BOOL showingStartSurface;

// WebStateList associated with this mediator.
@property(nonatomic, assign) WebStateList* webStateList;

// The web state associated with this NTP.
@property(nonatomic, assign) web::WebState* webState;

// Disconnects the mediator.
- (void)disconnect;

Expand Down

0 comments on commit b8ce303

Please sign in to comment.