Use high resolution icons where possible for streamlined hosted app icons.

chrome/browser/favicon/favicon_downloader.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/favicon/favicon_downloader.h"
+
+#include "base/bind.h"
+#include "chrome/browser/favicon/favicon_tab_helper.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/common/favicon_url.h"
+#include "third_party/skia/include/core/SkBitmap.h"

[pkotwicz, 2013/12/03 19:06:49]

You should include "ui/gfx/size.h"

[calamity, 2013/12/04 05:31:48]

Done.

+
+FaviconDownloader::FaviconDownloader(content::WebContents* web_contents,
+                                     std::vector<GURL> extra_favicon_urls,
+                                     FaviconDownloaderCallback callback)
+    : content::WebContentsObserver(web_contents),
+      favicon_url_candidates_(NULL),
+      extra_favicon_urls_(extra_favicon_urls),
+      callback_(callback),
+      weak_ptr_factory_(this) {
+}
+
+FaviconDownloader::~FaviconDownloader() {
+}
+
+void FaviconDownloader::Start() {

[pkotwicz, 2013/12/03 19:06:49]

If |extra_favicon_urls_| is non empty, you can start downloading those even if
|favicon_url_candidates_| is still empty

[calamity, 2013/12/04 05:31:48]

Done.

+  PopulateFaviconURLsFromWebContents();
+  // If the candidates aren't loaded, icons will be fetched when
+  // DidUpdateFaviconURL() is called.
+  if (favicon_url_candidates_)
+    FetchIcons();
+}
+
+int FaviconDownloader::DownloadImage(const GURL& url) {
+  return web_contents()->DownloadImage(
+      url,
+      true,  // is_favicon
+      0,     // no max size
+      base::Bind(&FaviconDownloader::DidDownloadFavicon,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void FaviconDownloader::PopulateFaviconURLsFromWebContents() {
+  FaviconTabHelper* favicon_tab_helper =
+      web_contents() ? FaviconTabHelper::FromWebContents(web_contents()) : NULL;
+  favicon_url_candidates_ =
+      favicon_tab_helper ? favicon_tab_helper->GetFaviconURLs() : NULL;
+}
+
+void FaviconDownloader::FetchIcons() {
+  // Ensure we only download each URL once.
+  std::set<GURL> favicon_urls;
+  for (std::vector<content::FaviconURL>::const_iterator it =
+           favicon_url_candidates_->begin();
+       it != favicon_url_candidates_->end(); ++it) {
+    if (it->icon_type != content::FaviconURL::INVALID_ICON)
+      favicon_urls.insert(it->icon_url);
+  }
+
+  for (std::vector<GURL>::const_iterator it = extra_favicon_urls_.begin();
+       it != extra_favicon_urls_.end(); ++it)
+    favicon_urls.insert(*it);
+
+  // Download icons and put their ids into |in_progress_requests_|.
+  for (std::set<GURL>::const_iterator it = favicon_urls.begin();
+       it != favicon_urls.end(); ++it)
+    in_progress_requests_.insert(DownloadImage(*it));
+
+  // If no downloads were initiated, we can proceed directly to running the
+  // callback.
+  if (in_progress_requests_.empty())
+    callback_.Run(true, favicon_map_);
+}
+
+void FaviconDownloader::DidDownloadFavicon(
+    int id,
+    int http_status_code,
+    const GURL& image_url,
+    const std::vector<SkBitmap>& bitmaps,
+    const std::vector<gfx::Size>& original_bitmap_sizes) {
+  // Request canceled by DidNavigateMainFrame().

[pkotwicz, 2013/12/03 19:06:49]

How about: "Request may have been canceled by DidNavigateMainFrame()."

[calamity, 2013/12/04 05:31:48]

Done.

+  if (in_progress_requests_.erase(id) == 0)
+    return;
+
+  favicon_map_[image_url] = bitmaps;
+
+  // Once all requests have been resolved, perform post-download tasks.
+  if (in_progress_requests_.empty())
+    callback_.Run(true, favicon_map_);
+}
+
+// content::WebContentsObserver overrides:
+void FaviconDownloader::DidNavigateMainFrame(
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  // Clear all pending requests.
+  in_progress_requests_.clear();
+  favicon_map_.clear();
+  callback_.Run(false, favicon_map_);
+}
+
+void FaviconDownloader::DidUpdateFaviconURL(
+    int32 page_id,
+    const std::vector<content::FaviconURL>& candidates) {
+  // Only consider the first candidates we are given. This prevents pages that
+  // change their favicon from spamming us.
+  if (favicon_url_candidates_)
+    return;
+
+  favicon_url_candidates_ = &candidates;

[pkotwicz, 2013/12/03 19:06:49]

You need to make a copy here. |candidates| does not live past the end of
WebContentsImpl::OnUpdateFaviconURL()

However, as I mentioned in the comment in the .h file, I think that you can
replace |favicon_url_candidates_| with |got_favicon_urls_|.

[calamity, 2013/12/04 05:31:48]

Done.

+  FetchIcons();
+}