Suggested Articles can be dismissed

ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm

Index: ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm
index 5104b68a259a490fc94e029c46cc2b7493508d32..983923e53f7c3f20271223c3b01ebe8643d3ccc6 100644
--- a/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm
+++ b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm
@@ -6,9 +6,12 @@
 
 #include "base/mac/scoped_nsobject.h"
 #include "base/strings/sys_string_conversions.h"
+#include "components/ntp_snippets/content_suggestions_service.h"
+#include "components/ntp_snippets/remote/remote_suggestions_scheduler.h"
 #import "ios/chrome/browser/content_suggestions/content_suggestions_mediator.h"
 #include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h"
 #import "ios/chrome/browser/ui/alert_coordinator/action_sheet_coordinator.h"
+#import "ios/chrome/browser/ui/content_suggestions/content_suggestion_identifier.h"
 #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.h"
 #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h"
 #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h"
@@ -22,16 +25,21 @@
 #error "This file requires ARC support."
 #endif
 
-@interface ContentSuggestionsCoordinator ()<ContentSuggestionsCommands> {
-  ContentSuggestionsMediator* _contentSuggestionsMediator;
-}
+@interface ContentSuggestionsCoordinator ()<ContentSuggestionsCommands>
 
 @property(nonatomic, strong) AlertCoordinator* alertCoordinator;
 @property(nonatomic, strong) UINavigationController* navigationController;
 @property(nonatomic, strong)
     ContentSuggestionsViewController* suggestionsViewController;
+@property(nonatomic, strong)
+    ContentSuggestionsMediator* contentSuggestionsMediator;
 
+// Opens the |URL| in a new tab |incognito| or not.
 - (void)openNewTabWithURL:(const GURL&)URL incognito:(BOOL)incognito;
+// Dismisses the |article|, removing it from the content service, and dismisses
+// the item at |indexPath| in the view controller.
+- (void)dismissArticle:(ContentSuggestionsArticleItem*)article
+           atIndexPath:(NSIndexPath*)indexPath;
 
 @end
 
@@ -43,6 +51,7 @@
 @synthesize suggestionsViewController = _suggestionsViewController;
 @synthesize URLLoader = _URLLoader;
 @synthesize visible = _visible;
+@synthesize contentSuggestionsMediator = _contentSuggestionsMediator;
 
 - (void)start {
   if (self.visible || !self.browserState) {
@@ -53,13 +62,17 @@
 
   _visible = YES;
 
-  _contentSuggestionsMediator = [[ContentSuggestionsMediator alloc]
-      initWithContentService:IOSChromeContentSuggestionsServiceFactory::
-                                 GetForBrowserState(self.browserState)];
+  ntp_snippets::ContentSuggestionsService* contentSuggestionsService =
+      IOSChromeContentSuggestionsServiceFactory::GetForBrowserState(
+          self.browserState);
+  contentSuggestionsService->remote_suggestions_scheduler()->OnNTPOpened();
+
+  self.contentSuggestionsMediator = [[ContentSuggestionsMediator alloc]
+      initWithContentService:contentSuggestionsService];
 
   self.suggestionsViewController = [[ContentSuggestionsViewController alloc]
       initWithStyle:CollectionViewControllerStyleDefault
-         dataSource:_contentSuggestionsMediator];
+         dataSource:self.contentSuggestionsMediator];
 
   self.suggestionsViewController.suggestionCommandHandler = self;
   _navigationController = [[UINavigationController alloc]
@@ -108,7 +121,8 @@
 }
 
 - (void)displayContextMenuForArticle:(ContentSuggestionsArticleItem*)articleItem
-                             atPoint:(CGPoint)touchLocation {
+                             atPoint:(CGPoint)touchLocation
+                         atIndexPath:(NSIndexPath*)indexPath {
   NSString* urlString = base::SysUTF8ToNSString(articleItem.articleURL.spec());
   self.alertCoordinator = [[ActionSheetCoordinator alloc]
       initWithBaseViewController:self.navigationController
@@ -120,6 +134,7 @@
 
   __weak ContentSuggestionsCoordinator* weakSelf = self;
   GURL articleURL = articleItem.articleURL;
+  __weak ContentSuggestionsArticleItem* weakArticle = articleItem;

[lpromero, 2017/03/06 10:29:00]

What is the issue you are avoiding by using a weak pointer? If the coordinator
is still alive but the article has been destroyed? For example by the backend?
In that case, what happens when dismissArticle:atIndexPath: is called with a nil
item and an index path?

[gambard, 2017/03/06 12:16:00]

The idea is to prevent deleting an item twice: if it has been destroyed by the
backend and already removed from the collection view, we would remove twice the
item at its index path.
I forgot the guard in |dismissArticle:atIndexPath| in this patch.

[lpromero, 2017/03/06 13:02:30]

One issue with this: in  a ref counted environment, you can't guarantee when an
item is destroyed. I think you should not rely on this. Since articles have an
identifier, couldn't you use this instead to prevent attempt to double delete?
Other possibility: how come double deletes can happen at this stage?

[gambard, 2017/03/06 17:25:33]

I know but some guards is better than none. With the interactions as they are
today, the item cannot be dismissed twice. But once the content suggestions
service is updated in the background, it might change.

 
   NSString* openInNewTabTitle =
       l10n_util::GetNSString(IDS_IOS_CONTENT_CONTEXT_OPENLINKNEWTAB);
@@ -146,7 +161,8 @@
   [self.alertCoordinator addItemWithTitle:deleteTitle
                                    action:^{
                                      // TODO(crbug.com/691979): Add metrics.
-                                     [weakSelf removeEntry];
+                                     [weakSelf dismissArticle:weakArticle
+                                                  atIndexPath:indexPath];
                                    }
                                     style:UIAlertActionStyleDefault];
 
@@ -173,8 +189,12 @@
   [self stop];
 }
 
-- (void)removeEntry {
+- (void)dismissArticle:(ContentSuggestionsArticleItem*)article
+           atIndexPath:(NSIndexPath*)indexPath {
   // TODO(crbug.com/691979): Add metrics.
+  [self.contentSuggestionsMediator
+      dismissSuggestion:article.suggestionIdentifier];
+  [self.suggestionsViewController dismissEntryAtIndexPath:indexPath];
 }
 
 @end