Connect ContentSuggestionsMediator to the ContentService

ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm

Index: ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
index c87b1631ed1e5f2e9755a62e6b91d777c554fcac..802daab4b9a5ac2521d32fb2068d3a53baf5fea4 100644
--- a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
+++ b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
@@ -4,17 +4,54 @@
 
 #import "ios/chrome/browser/content_suggestions/content_suggestions_mediator.h"
 
+#include "base/mac/bind_objc_block.h"
 #include "base/memory/ptr_util.h"
+#include "base/optional.h"
+#include "base/strings/sys_string_conversions.h"
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/content_suggestion.h"
+#import "ios/chrome/browser/content_suggestions/content_suggestions_category_wrapper.h"
 #import "ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.h"
 #import "ios/chrome/browser/ui/content_suggestions/content_suggestion.h"
 #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_data_sink.h"
+#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_image_updater.h"
+#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_item.h"
+#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_section_information.h"
+#include "ui/gfx/image/image.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
+namespace {
+
+// Returns the ItemType for this |category|.
+ContentSuggestionsItemType TypeForCategory(ntp_snippets::Category category) {
+  if (category.IsKnownCategory(ntp_snippets::KnownCategories::ARTICLES))
+    return ContentSuggestionsItemTypeArticle;
+  return ContentSuggestionsItemTypeText;
+}
+
+// Returns the section ID for this |category|.
+ContentSuggestionsSectionID SectionIDForCategory(
+    ntp_snippets::Category category) {
+  if (category.IsKnownCategory(ntp_snippets::KnownCategories::BOOKMARKS))
+    return ContentSuggestionsSectionBookmarks;
+  if (category.IsKnownCategory(ntp_snippets::KnownCategories::ARTICLES))
+    return ContentSuggestionsSectionArticles;
+
+  return ContentSuggestionsSectionCount;
+}
+
+// Returns the section layout corresponding to the category |layout|.
+ContentSuggestionsSectionLayout SectionLayoutForLayout(
+    ntp_snippets::ContentSuggestionsCardLayout layout) {
+  // For now, only cards are relevant.
+  return ContentSuggestionsSectionLayoutCard;
+}
+
+}  // namespace
+
 @interface ContentSuggestionsMediator ()<ContentSuggestionsServiceObserver> {
   // Bridge for this class to become an observer of a ContentSuggestionsService.
   std::unique_ptr<ContentSuggestionsServiceBridge> _suggestionBridge;
@@ -22,6 +59,7 @@
 
 @property(nonatomic, assign)
     ntp_snippets::ContentSuggestionsService* contentService;
+@property(nonatomic, strong) NSMutableDictionary* sectionInformationByCategory;

[stkhapugin, 2017/02/14 16:34:18]

Needs comment. What are the keys, objects and how this needs to be updated. It's
also nice to use the modern generic syntax:

@property(nonatomic, strong) NSMutableDictionary<KeyType*, ValueType*>*
dictName;

Bonus points for nullability on this property. 

[gambard, 2017/02/15 10:17:56]

Done.

 
 // Converts the data in |category| to ContentSuggestion and adds them to the
 // |contentArray|.
@@ -30,7 +68,11 @@
 
 // Converts the |contentsuggestion| to a ContentSuggestion.

[stkhapugin, 2017/02/14 16:34:18]

Update the arg name with same capitalization.

[gambard, 2017/02/15 10:17:56]

Done.

 - (ContentSuggestion*)convertContentSuggestion:
-    (const ntp_snippets::ContentSuggestion&)contentsuggestion;
+    (const ntp_snippets::ContentSuggestion&)contentSuggestion;
+
+// Adds the section information for |category| in
+// self.sectionInformationByCategory.
+- (void)addSectionInformationForCategory:(ntp_snippets::Category)category;
 
 @end
 
@@ -38,6 +80,7 @@
 
 @synthesize contentService = _contentService;
 @synthesize dataSink = _dataSink;
+@synthesize sectionInformationByCategory = _sectionInformationByCategory;

[stkhapugin, 2017/02/14 16:34:18]

Is it necessary? I keep forgetting :(

[gambard, 2017/02/15 10:17:56]

Seems so :)

[marq (ping after 24h), 2017/02/16 13:14:35]

We have not yet turned on property autosynthesis. It has been discussed, and
feel free to bring it up again.

 
 - (instancetype)initWithContentService:
     (ntp_snippets::ContentSuggestionsService*)contentService {
@@ -46,6 +89,7 @@
     _suggestionBridge =
         base::MakeUnique<ContentSuggestionsServiceBridge>(self, contentService);
     _contentService = contentService;
+    _sectionInformationByCategory = [[NSMutableDictionary alloc] init];
   }
   return self;
 }
@@ -57,6 +101,15 @@
       self.contentService->GetCategories();
   NSMutableArray<ContentSuggestion*>* dataHolders = [NSMutableArray array];
   for (auto& category : categories) {
+    if (self.contentService->GetCategoryStatus(category) !=
+        ntp_snippets::CategoryStatus::AVAILABLE) {
+      continue;
+    }
+    if (![self.sectionInformationByCategory

[stkhapugin, 2017/02/14 16:34:18]

Use modern syntax: if (!self.sectionInformationByCategory[...])

Bonus points for introducing a convenience initializer in
ContentSUggestionsCategoryWrapper:

+ (ContentSuggestionsCategoryWrapper*)wrapperWithCategory:(...)category {
   return [[self alloc] initWithCategory:category]; // needs autorelease if not
built with ARC
}

[gambard, 2017/02/15 10:17:56]

Done.

+            objectForKey:[[ContentSuggestionsCategoryWrapper alloc]
+                             initWithCategory:category]]) {
+      [self addSectionInformationForCategory:category];
+    }
     [self addContentInCategory:category toArray:dataHolders];
   }
   return dataHolders;
@@ -100,14 +153,59 @@
                      toArray:(NSMutableArray<ContentSuggestion*>*)contentArray {
   const std::vector<ntp_snippets::ContentSuggestion>& suggestions =
       self.contentService->GetSuggestionsForCategory(category);
+  ContentSuggestionsCategoryWrapper* categoryWrapper =
+      [[ContentSuggestionsCategoryWrapper alloc] initWithCategory:category];
   for (auto& contentSuggestion : suggestions) {
-    [contentArray addObject:[self convertContentSuggestion:contentSuggestion]];
+    ContentSuggestion* suggestion =
+        [self convertContentSuggestion:contentSuggestion];
+    suggestion.type = TypeForCategory(category);
+    suggestion.section = self.sectionInformationByCategory[categoryWrapper];
+    [contentArray addObject:suggestion];
   }
 }
 
 - (ContentSuggestion*)convertContentSuggestion:

[stkhapugin, 2017/02/14 16:34:18]

This should be a method on ContentSuggestion (possibly you can have a category
on ContentSuggestion that initializes it with ntp_snippets::ContentSuggestion.

[gambard, 2017/02/15 10:17:56]

ContentSuggestion should not know about ntp_snippets::*.
I am moving it to a C function. It will allow easier refactoring if needed.

[stkhapugin, 2017/02/15 15:35:42]

A common Objecitve-C pattern is:

@class ModelClass;

@interface ViewClass : ... 

- (instancetype)initWithUIThingie:(Thingie*)thingie
otherThingie:(Thingie*)thingie;

@end

// Possibly in a different file

@interface ViewClass (InitWitModelClass)

- (instancetype)initWithModel:(ModelClass*)model;

@end

@implementation ViewClass(InitWithModelCLass)

- (instancetype)initWithModel:(ModelClass*)model {
  Thingie* firstUIThingieFromModel = helper(model);
  Thingie* secondUIThingieFromModel = other_helper(model);

return [self initWithUIThingie:firstUIThingieFromModel
otherThingie:secondUIThingieFromModel];
}

@end



This way you have a clear separation between UI and model code, but you keep the
code on the boundary in a separate file that can be included anywhere such
convertion is necessary. 

This pattern is used all the time for UICollectionViewCell to be configured with
a model object. Your case is really similar. But you don't have to do this. 

[gambard, 2017/02/16 10:06:29]

Acknowledged.

-    (const ntp_snippets::ContentSuggestion&)contentsuggestion {
-  return [[ContentSuggestion alloc] init];
+    (const ntp_snippets::ContentSuggestion&)contentSuggestion {
+  ContentSuggestionsImageUpdater* imageUpdater =
+      [[ContentSuggestionsImageUpdater alloc] init];
+
+  __weak ContentSuggestionsImageUpdater* weakImageUpdater = imageUpdater;
+  self.contentService->FetchSuggestionImage(
+      contentSuggestion.id(), base::BindBlockArc(^(const gfx::Image& image) {
+        [weakImageUpdater receivedImage:image.CopyUIImage()];
+      }));
+
+  ContentSuggestion* suggestion = [[ContentSuggestion alloc] init];
+  suggestion.title = base::SysUTF16ToNSString(contentSuggestion.title());
+  suggestion.text = base::SysUTF16ToNSString(contentSuggestion.snippet_text());
+  suggestion.url = contentSuggestion.url();
+  suggestion.imageUpdater = imageUpdater;
+
+  return suggestion;
+}
+
+- (void)addSectionInformationForCategory:(ntp_snippets::Category)category {
+  base::Optional<ntp_snippets::CategoryInfo> categoryInfo =
+      self.contentService->GetCategoryInfo(category);
+  ContentSuggestionsSectionInformation* sectionInfo =
+      [[ContentSuggestionsSectionInformation alloc]
+          initWithID:SectionIDForCategory(category)];
+  if (categoryInfo) {
+    sectionInfo.layout = SectionLayoutForLayout(categoryInfo->card_layout());

[stkhapugin, 2017/02/14 16:34:18]

All this setup of ContentSuggestionsSectionInformation with
ntp_snippets::CategoryInfo really belongs to
ContentSuggestionsSectionInformation methods. Seems like your code will benefit
from:

+
(ContentSuggestionsSectionInformation*)sectionInformationWithCategory:(ntp_snippets::Category)category
categoryInfo(base::Optional<...>)sectionInfo {
 // move code form this method there
}

[gambard, 2017/02/15 10:17:56]

Same. UI code should not know about ntp_snippets::*.
I am moving the creation code to a C function.

+    if (categoryInfo->show_if_empty()) {
+      NSString* title =
+          base::SysUTF16ToNSString(categoryInfo->no_suggestions_message());
+      sectionInfo.emptyCell = [[ContentSuggestionsItem alloc] initWithType:0
+                                                                     title:title
+                                                                  subtitle:nil];
+    }
+    sectionInfo.title = base::SysUTF16ToNSString(categoryInfo->title());
+  }
+
+  [self.sectionInformationByCategory
+      setObject:sectionInfo
+         forKey:[[ContentSuggestionsCategoryWrapper alloc]

[stkhapugin, 2017/02/14 16:34:18]

Use modern syntax:

self.sectionInformationByCategory[key] = value;

[gambard, 2017/02/15 10:17:56]

Done.

+                    initWithCategory:category]];
 }
 
 @end