Ensure PopularSite icon availability in ntp_tiles.

components/ntp_tiles/icon_cacher.cc

+// Copyright 2015 The Chromium Authors. All rights reserved.

[Marc Treib, 2016/10/12 09:11:28]

2016

[sfiera, 2016/10/13 09:06:45]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/ntp_tiles/icon_cacher.h"
+
+#include "base/callback.h"

[Marc Treib, 2016/10/12 09:11:28]

No need to repeat all the includes from the header

[sfiera, 2016/10/13 09:06:45]

Took this as an opportunity to trim the includes in the header.

+#include "base/memory/weak_ptr.h"
+#include "components/favicon/core/favicon_service.h"
+#include "components/favicon/core/favicon_util.h"
+#include "components/favicon_base/favicon_types.h"
+#include "components/favicon_base/favicon_util.h"
+#include "components/image_fetcher/image_fetcher.h"
+#include "components/ntp_tiles/popular_sites.h"
+#include "ui/gfx/image/image_skia.h"  // DO_NOT_SUBMIT

[Marc Treib, 2016/10/12 09:11:28]

:)

[sfiera, 2016/10/13 09:06:45]

:X

+#include "url/gurl.h"
+
+namespace ntp_tiles {
+
+IconCacher::IconCacher(
+    favicon::FaviconService* favicon_service,
+    std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher)
+    : favicon_service_(favicon_service),
+      image_fetcher_(std::move(image_fetcher)),
+      weak_ptr_factory_(this) {
+  image_fetcher_->SetDataUseServiceName(
+      data_use_measurement::DataUseUserData::NTP_TILES);

[Marc Treib, 2016/10/12 09:11:28]

:)
There's a TODO to add just this in popular_sites.cc, you can remove that now!

[sfiera, 2016/10/13 09:06:45]

Done.

+}
+
+IconCacher::~IconCacher() = default;
+
+void IconCacher::StartFetch(PopularSites::Site site,
+                            base::Callback<void(bool)> done) {
+  favicon::GetFaviconImageForPageURL(
+      favicon_service_, site.url, IconType(site),
+      base::Bind(&IconCacher::OnGetFaviconImageForPageURLFinished,
+                 weak_ptr_factory_.GetWeakPtr(), std::move(site), done),

[Marc Treib, 2016/10/12 09:11:28]

WeakPtr isn't necessary I think: the CancelableTaskTracker should make sure
we're not called back if we get destroyed

[sfiera, 2016/10/13 09:06:45]

Done.

+      &tracker_);
+}
+
+favicon_base::IconType IconCacher::IconType(const PopularSites::Site& site) {
+  return site.large_icon_url.is_valid() ? favicon_base::TOUCH_ICON
+                                        : favicon_base::FAVICON;
+}
+
+const GURL& IconCacher::IconURL(const PopularSites::Site& site) {
+  return site.large_icon_url.is_valid() ? site.large_icon_url
+                                        : site.favicon_url;
+}
+
+void IconCacher::OnGetFaviconImageForPageURLFinished(
+    PopularSites::Site site,
+    base::Callback<void(bool)> done,
+    const favicon_base::FaviconImageResult& result) {
+  if (!result.image.IsEmpty()) {
+    done.Run(false);
+    return;
+  }
+
+  image_fetcher_->StartOrQueueNetworkRequest(
+      "", IconURL(site),

[Marc Treib, 2016/10/12 09:11:28]

nit: s/""/std::string()/

[sfiera, 2016/10/13 09:06:45]

Done.

+      base::Bind(&IconCacher::OnFaviconDownloaded,
+                 weak_ptr_factory_.GetWeakPtr(), site, done));

[Marc Treib, 2016/10/12 09:11:28]

WeakPtr isn't necessary I think (we own the ImageFetcher)

[sfiera, 2016/10/13 09:06:45]

Done.

+}
+
+void IconCacher::OnFaviconDownloaded(PopularSites::Site site,
+                                     base::Callback<void(bool)> done,
+                                     const std::string& id,
+                                     const gfx::Image& fetched_image) {
+  if (fetched_image.IsEmpty()) {
+    done.Run(false);
+    return;
+  }
+
+  gfx::Image image = fetched_image;
+  favicon_base::SetFaviconColorSpace(&image);
+  favicon_service_->SetFavicons(site.url, IconURL(site), IconType(site), image);
+  done.Run(true);
+}
+
+}  // namespace ntp_tiles