Handle null callbacks in OnFaviconDownloaded

All callback are now optional for the IconFetcher.
(This fixes a regression and enables the proper background fetching of
large icons for new NTPs.)

BUG=698159,694270

Review-Url: https://codereview.chromium.org/2725293002
Cr-Commit-Position: refs/heads/master@{#454565}

components/ntp_tiles/icon_cacher.h

@@ -20,10 +20,10 @@ class IconCacher {
  public:
   virtual ~IconCacher() = default;
 
-  // Fetches the icon if necessary. It invokes |icon_available| if a new icon
-  // was fetched. This is the only required callback.
+  // Fetches the icon if necessary. If a new icon was fetched, the optional
+  // |icon_available| callback will be invoked.
   // If there are preliminary icons (e.g. provided by static resources), the
-  // |preliminary_icon_available| callback will be invoked additionally.
+  // optional |preliminary_icon_available| callback will be invoked in addition.
   // TODO(fhorschig): In case we keep these, make them OnceClosures.
   virtual void StartFetch(PopularSites::Site site,
                           const base::Closure& icon_available,

components/ntp_tiles/icon_cacher_impl.cc

@@ -49,7 +49,6 @@ void IconCacherImpl::StartFetch(
     PopularSites::Site site,
     const base::Closure& icon_available,
     const base::Closure& preliminary_icon_available) {
-  DCHECK(!icon_available.is_null());
   favicon::GetFaviconImageForPageURL(
       favicon_service_, site.url, IconType(site),
       base::Bind(&IconCacherImpl::OnGetFaviconImageForPageURLFinished,
@@ -85,7 +84,9 @@ void IconCacherImpl::OnFaviconDownloaded(PopularSites::Site site,
   }
 
   SaveIconForSite(site, fetched_image);
-  icon_available.Run();
+  if (icon_available) {
+    icon_available.Run();
+  }
 }
 
 void IconCacherImpl::SaveIconForSite(const PopularSites::Site& site,

components/ntp_tiles/icon_cacher_impl_unittest.cc

@@ -275,6 +275,16 @@ TEST_F(IconCacherTest, LargeNotCachedAndFetchFailed) {
   EXPECT_FALSE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));
 }
 
+TEST_F(IconCacherTest, HandlesEmptyCallbacksNicely) {
+  EXPECT_CALL(*image_fetcher_, SetDataUseServiceName(_));
+  EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(_));
+  ON_CALL(*image_fetcher_, StartOrQueueNetworkRequest(_, _, _))
+      .WillByDefault(PassFetch(128, 128));
+  IconCacherImpl cacher(&favicon_service_, std::move(image_fetcher_));
+  cacher.StartFetch(site_, base::Closure(), base::Closure());
+  WaitForTasksToFinish();
+}
+
 TEST_F(IconCacherTest, ProvidesDefaultIconAndSucceedsWithFetching) {
   // We are not interested which delegate function actually handles the call to
   // |GetNativeImageNamed| as long as we receive the right image.