Favicon for TileSaver are downloaded in ContentSuggestions

ContentSuggestions is enabled to 100%, the GoogleLandingMediator won't
be used anymore.
Saving the favicons for the Content widget should be done from the
ContentSuggestionsMediator.

Bug: 764720
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I31667a896cfdbe378d0efa6b67df5c8e9189d4d7
Reviewed-on: https://chromium-review.googlesource.com/776798Reviewed-by: Elodie Banel <lod@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517447}

ios/chrome/browser/content_suggestions/BUILD.gn

@@ -63,6 +63,7 @@ source_set("content_suggestions") {
     "//ios/chrome/browser/ui/reading_list",
     "//ios/chrome/browser/ui/toolbar",
     "//ios/chrome/browser/web_state_list",
+    "//ios/chrome/common/app_group",
     "//ios/public/provider/chrome/browser",
     "//ios/public/provider/chrome/browser/images",
     "//ios/public/provider/chrome/browser/voice",

ios/chrome/browser/content_suggestions/content_suggestions_favicon_mediator.h

@@ -20,6 +20,7 @@ class ContentSuggestionsService;
 @protocol ContentSuggestionsDataSink;
 @class ContentSuggestionsItem;
 @class ContentSuggestionsMostVisitedItem;
+@class FaviconAttributesProvider;
 class LargeIconCache;
 
 // Mediator handling the fetching of the favicon for all ContentSuggestions
@@ -40,6 +41,10 @@ initWithContentService:
 // The data sink which should be notified of the changes in the items.
 @property(nonatomic, weak, nullable) id<ContentSuggestionsDataSink> dataSink;
 
+// FaviconAttributesProvider to fetch the favicon for the most visited tiles.
+@property(nonatomic, nullable, strong, readonly)
+    FaviconAttributesProvider* mostVisitedAttributesProvider;
+
 // Sets the |mostVisitedData| used to log the impression of the tiles.
 - (void)setMostVisitedDataForLogging:
     (const ntp_tiles::NTPTilesVector&)mostVisitedData;

ios/chrome/browser/content_suggestions/content_suggestions_favicon_mediator.mm

@@ -45,13 +45,14 @@ const CGFloat kMostVisitedFaviconMinimalSize = 32;
 @property(nonatomic, assign)
     ntp_snippets::ContentSuggestionsService* contentService;
 
-// FaviconAttributesProvider to fetch the favicon for the most visited tiles.
-@property(nonatomic, nullable, strong)
-    FaviconAttributesProvider* mostVisitedAttributesProvider;
 // FaviconAttributesProvider to fetch the favicon for the suggestions.
 @property(nonatomic, nullable, strong)
     FaviconAttributesProvider* suggestionsAttributesProvider;
 
+// Redefined as readwrite
+@property(nonatomic, nullable, strong, readwrite)
+    FaviconAttributesProvider* mostVisitedAttributesProvider;
+
 @end
 
 @implementation ContentSuggestionsFaviconMediator

ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm

@@ -32,6 +32,8 @@
 #import "ios/chrome/browser/ui/content_suggestions/identifier/content_suggestion_identifier.h"
 #import "ios/chrome/browser/ui/content_suggestions/identifier/content_suggestions_section_information.h"
 #import "ios/chrome/browser/ui/ntp/notification_promo_whats_new.h"
+#include "ios/chrome/browser/ui/ntp/ntp_tile_saver.h"
+#include "ios/chrome/common/app_group/app_group_constants.h"
 #include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
 #include "ios/public/provider/chrome/browser/images/branded_image_provider.h"
 #include "ios/public/provider/chrome/browser/images/whats_new_icon.h"
@@ -439,6 +441,11 @@ initWithContentService:(ntp_snippets::ContentSuggestionsService*)contentService
 
 - (void)onMostVisitedURLsAvailable:
     (const ntp_tiles::NTPTilesVector&)mostVisited {
+  // This is used by the content widget.
+  ntp_tile_saver::SaveMostVisitedToDisk(
+      mostVisited, self.faviconMediator.mostVisitedAttributesProvider,
+      app_group::ContentWidgetFaviconsFolder());
+
   self.freshMostVisitedItems = [NSMutableArray array];
   for (const ntp_tiles::NTPTile& tile : mostVisited) {
     ContentSuggestionsMostVisitedItem* item =
@@ -463,6 +470,11 @@ initWithContentService:(ntp_snippets::ContentSuggestionsService*)contentService
 }
 
 - (void)onIconMadeAvailable:(const GURL&)siteURL {
+  // This is used by the content widget.
+  ntp_tile_saver::UpdateSingleFavicon(
+      siteURL, self.faviconMediator.mostVisitedAttributesProvider,
+      app_group::ContentWidgetFaviconsFolder());
+
   for (ContentSuggestionsMostVisitedItem* item in self.mostVisitedItems) {
     if (item.URL == siteURL) {
       [self.faviconMediator fetchFaviconForMostVisited:item];

ios/chrome/browser/ui/ntp/BUILD.gn

@@ -278,6 +278,7 @@ source_set("unit_tests") {
     "//ios/chrome/browser/ui",
     "//ios/chrome/browser/ui/commands",
     "//ios/chrome/browser/ui/content_suggestions:content_suggestions_constant",
+    "//ios/chrome/browser/ui/favicon",
     "//ios/chrome/browser/ui/favicon:favicon_ui",
     "//ios/chrome/browser/web_state_list:test_support",
     "//ios/chrome/browser/web_state_list:web_state_list",

ios/chrome/browser/ui/ntp/google_landing_mediator.mm

@@ -253,10 +253,6 @@ void SearchEngineObserver::OnTemplateURLServiceChanged() {
 #pragma mark - MostVisitedSitesObserving
 
 - (void)onMostVisitedURLsAvailable:(const ntp_tiles::NTPTilesVector&)data {
-  // This is used by the content widget.
-  ntp_tile_saver::SaveMostVisitedToDisk(
-      data, self, app_group::ContentWidgetFaviconsFolder());
-
   if (_mostVisitedData.size() > 0) {
     // If some content is already displayed to the user, do not update it to
     // prevent updating the all the tiles without any action from the user.
@@ -275,8 +271,6 @@ void SearchEngineObserver::OnTemplateURLServiceChanged() {
 }
 
 - (void)onIconMadeAvailable:(const GURL&)siteUrl {
-  ntp_tile_saver::UpdateSingleFavicon(siteUrl, self,
-                                      app_group::ContentWidgetFaviconsFolder());
   for (size_t i = 0; i < _mostVisitedData.size(); ++i) {
     const ntp_tiles::NTPTile& ntpTile = _mostVisitedData[i];
     if (ntpTile.url == siteUrl) {

ios/chrome/browser/ui/ntp/ntp_tile_saver.h

@@ -9,7 +9,7 @@
 
 #include "components/ntp_tiles/ntp_tile.h"
 
-@protocol GoogleLandingDataSource;
+@class FaviconAttributesProvider;
 @class NTPTile;
 
 // These functions are used to save the ntp tiles (favicon and name) offline for
@@ -22,7 +22,7 @@ namespace ntp_tile_saver {
 // Saves the most visited sites to disk with icons in |favicons_folder|, using
 // |favicon_fetcher| to get the favicons.
 void SaveMostVisitedToDisk(const ntp_tiles::NTPTilesVector& most_visited_data,
-                           id<GoogleLandingDataSource> favicon_fetcher,
+                           FaviconAttributesProvider* favicon_provider,
                            NSURL* favicons_folder);
 
 // Read the current saved most visited sites from disk.
@@ -31,7 +31,7 @@ NSDictionary* ReadSavedMostVisited();
 // Fetches the updated favicon for a single site and saves it in
 // |favicons_folder|.
 void UpdateSingleFavicon(const GURL& site_url,
-                         id<GoogleLandingDataSource> favicon_fetcher,
+                         FaviconAttributesProvider* favicon_provider,
                          NSURL* favicons_folder);
 
 }  // namespace ntp_tile_saver

ios/chrome/browser/ui/ntp/ntp_tile_saver.mm

@@ -13,7 +13,7 @@
 #include "components/favicon/core/fallback_url_util.h"
 #include "components/ntp_tiles/ntp_tile.h"
 #import "ios/chrome/browser/ui/favicon/favicon_attributes.h"
-#import "ios/chrome/browser/ui/ntp/google_landing_data_source.h"
+#import "ios/chrome/browser/ui/favicon/favicon_attributes_provider.h"
 #import "ios/chrome/browser/ui/ntp/ntp_tile.h"
 #include "ios/chrome/common/app_group/app_group_constants.h"
 #import "net/base/mac/url_conversions.h"
@@ -49,9 +49,9 @@ void WriteSingleUpdatedTileToDisk(NTPTile* tile);
 // Empty the temporary favicon folder.
 void ClearTemporaryFaviconFolder();
 
-// Get the favicons using |favicon_fetcher| and writes them to disk.
+// Get the favicons using |favicon_provider| and writes them to disk.
 void GetFaviconsAndSave(const ntp_tiles::NTPTilesVector& most_visited_data,
-                        id<GoogleLandingDataSource> favicon_fetcher,
+                        FaviconAttributesProvider* favicon_provider,
                         NSURL* favicons_folder);
 
 }  // namespace ntp_tile_saver
@@ -125,7 +125,7 @@ NSString* GetFaviconFileName(const GURL& url) {
 }
 
 void GetFaviconsAndSave(const ntp_tiles::NTPTilesVector& most_visited_data,
-                        id<GoogleLandingDataSource> favicon_fetcher,
+                        FaviconAttributesProvider* favicon_provider,
                         NSURL* favicons_folder) {
   NSMutableDictionary<NSURL*, NTPTile*>* tiles =
       [[NSMutableDictionary alloc] init];
@@ -196,15 +196,13 @@ void GetFaviconsAndSave(const ntp_tiles::NTPTilesVector& most_visited_data,
     // The cache is not used here as it is simply a way to have a synchronous
     // immediate return. In this case it is unnecessary.
 
-    [favicon_fetcher getFaviconForPageURL:gurl
-                                     size:48
-                                 useCache:NO
-                                 callback:faviconAttributesBlock];
+    [favicon_provider fetchFaviconAttributesForURL:gurl
+                                        completion:faviconAttributesBlock];
   }
 }
 
 void SaveMostVisitedToDisk(const ntp_tiles::NTPTilesVector& most_visited_data,
-                           id<GoogleLandingDataSource> favicon_fetcher,
+                           FaviconAttributesProvider* favicon_provider,
                            NSURL* favicons_folder) {
   if (favicons_folder == nil) {
     return;
@@ -215,7 +213,7 @@ void SaveMostVisitedToDisk(const ntp_tiles::NTPTilesVector& most_visited_data,
       base::BindOnce(&ClearTemporaryFaviconFolder),
       base::Bind(base::BindBlockArc(
                      ^(const ntp_tiles::NTPTilesVector& most_visited_data) {
-                       GetFaviconsAndSave(most_visited_data, favicon_fetcher,
+                       GetFaviconsAndSave(most_visited_data, favicon_provider,
                                           favicons_folder);
                      }),
                  most_visited_data));
@@ -245,7 +243,7 @@ NSDictionary* ReadSavedMostVisited() {
 }
 
 void UpdateSingleFavicon(const GURL& site_url,
-                         id<GoogleLandingDataSource> favicon_fetcher,
+                         FaviconAttributesProvider* favicon_provider,
                          NSURL* favicons_folder) {
   NSDictionary* tiles = ReadSavedMostVisited();
 
@@ -319,9 +317,7 @@ void UpdateSingleFavicon(const GURL& site_url,
 
   // The cache is not used here as it is simply a way to have a synchronous
   // immediate return. In this case it is unnecessary.
-  [favicon_fetcher getFaviconForPageURL:site_url
-                                   size:48
-                               useCache:NO
-                               callback:faviconAttributesBlock];
+  [favicon_provider fetchFaviconAttributesForURL:site_url
+                                      completion:faviconAttributesBlock];
 }
 }  // namespace ntp_tile_saver

ios/chrome/browser/ui/ntp/ntp_tile_saver_unittest.mm

@@ -9,7 +9,7 @@
 #include "base/test/scoped_task_environment.h"
 #import "components/ntp_tiles/ntp_tile.h"
 #import "ios/chrome/browser/ui/favicon/favicon_attributes.h"
-#import "ios/chrome/browser/ui/ntp/google_landing_data_source.h"
+#import "ios/chrome/browser/ui/favicon/favicon_attributes_provider.h"
 #import "ios/chrome/browser/ui/ntp/ntp_tile.h"
 #import "ios/chrome/test/block_cleanup_test.h"
 #import "net/base/mac/url_conversions.h"
@@ -53,23 +53,21 @@ class NTPTileSaverControllerTest : public BlockCleanupTest {
                          std::set<GURL> imageURLs,
                          std::set<GURL> fallbackURLs) {
     OCMStub([[mock ignoringNonObjectArgs]
-                getFaviconForPageURL:GURL()
-                                size:0
-                            useCache:NO
-                            callback:[OCMArg isNotNil]])
+                fetchFaviconAttributesForURL:GURL()
+                                  completion:[OCMArg isNotNil]])
         .andDo(^(NSInvocation* invocation) {
           GURL* urltest;
           [invocation getArgument:&urltest atIndex:2];
           if (imageURLs.find(GURL(*urltest)) != imageURLs.end()) {
             __unsafe_unretained void (^callback)(id);
-            [invocation getArgument:&callback atIndex:5];
+            [invocation getArgument:&callback atIndex:3];
             UIGraphicsBeginImageContext(CGSizeMake(10, 10));
             UIImage* image = UIGraphicsGetImageFromCurrentImageContext();
             UIGraphicsEndImageContext();
             callback([FaviconAttributes attributesWithImage:image]);
           } else if (fallbackURLs.find(GURL(*urltest)) != fallbackURLs.end()) {
             __unsafe_unretained void (^callback)(id);
-            [invocation getArgument:&callback atIndex:5];
+            [invocation getArgument:&callback atIndex:3];
             callback([FaviconAttributes
                 attributesWithMonogram:@"C"
                              textColor:UIColor.whiteColor
@@ -121,7 +119,7 @@ TEST_F(NTPTileSaverControllerTest, SaveMostVisitedToDisk) {
   fallbackTile.title = base::ASCIIToUTF16("Title");
   fallbackTile.url = GURL("http://fallback.com");
 
-  id mockFaviconFetcher = OCMProtocolMock(@protocol(GoogleLandingDataSource));
+  id mockFaviconFetcher = OCMClassMock([FaviconAttributesProvider class]);
   setupMockCallback(mockFaviconFetcher, {imageTile.url}, {fallbackTile.url});
 
   ntp_tiles::NTPTilesVector tiles = {
@@ -165,7 +163,7 @@ TEST_F(NTPTileSaverControllerTest, UpdateSingleFaviconFallback) {
   fallbackTile.title = base::ASCIIToUTF16("Title");
   fallbackTile.url = GURL("http://fallback.com");
 
-  id mockFaviconFetcher = OCMProtocolMock(@protocol(GoogleLandingDataSource));
+  id mockFaviconFetcher = OCMClassMock([FaviconAttributesProvider class]);
   setupMockCallback(mockFaviconFetcher, {imageTile1.url, imageTile2.url},
                     {fallbackTile.url});
 
@@ -198,7 +196,7 @@ TEST_F(NTPTileSaverControllerTest, UpdateSingleFaviconFallback) {
   verifyWithFallback(fallbackSavedTile, fallbackTitle, fallbackURL);
 
   // Mock returning a fallback value for the first image tile.
-  id mockFaviconFetcher2 = OCMProtocolMock(@protocol(GoogleLandingDataSource));
+  id mockFaviconFetcher2 = OCMClassMock([FaviconAttributesProvider class]);
   setupMockCallback(mockFaviconFetcher2, {imageTile2.url},
                     {imageTile1.url, fallbackTile.url});
   ntp_tile_saver::UpdateSingleFavicon(imageTile1.url, mockFaviconFetcher2,