Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(367)

Unified Diff: components/ntp_tiles/icon_cacher.cc

Issue 2388783004: Ensure PopularSite icon availability in ntp_tiles. (Closed)
Patch Set: Fix comments, variable names. Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/ntp_tiles/icon_cacher.cc
diff --git a/components/ntp_tiles/icon_cacher.cc b/components/ntp_tiles/icon_cacher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b1b9c235aae78fca0ff0f7911afe2e9b48738f10
--- /dev/null
+++ b/components/ntp_tiles/icon_cacher.cc
@@ -0,0 +1,81 @@
+// 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 i
+#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_site
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 CancelableTas
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 ImageF
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

Powered by Google App Engine
This is Rietveld 408576698