Download large icon if available when creating secondary tiles on Windows 8.

chrome/browser/ui/metro_pin_tab_helper_win.cc

Index: chrome/browser/ui/metro_pin_tab_helper_win.cc
diff --git a/chrome/browser/ui/metro_pin_tab_helper_win.cc b/chrome/browser/ui/metro_pin_tab_helper_win.cc
index 489bdd6320783e1899e75eb799cfd5c7f2470f1b..687ef4fa86b2f62e55995113d2f0502600fd979d 100644
--- a/chrome/browser/ui/metro_pin_tab_helper_win.cc
+++ b/chrome/browser/ui/metro_pin_tab_helper_win.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/metro_pin_tab_helper_win.h"
 
+#include <set>
+
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/file_path.h"
@@ -16,11 +18,15 @@
 #include "base/utf_string_conversions.h"
 #include "base/win/metro.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
+#include "chrome/browser/favicon/favicon_util.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/icon_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "crypto/sha2.h"
+#include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/color_analysis.h"
@@ -75,19 +81,25 @@ bool CreateSiteSpecificLogo(const gfx::ImageSkia& image,
   if (image.isNull())
     return false;
 
-  *logo_path = logo_dir.Append(tile_id).ReplaceExtension(L".png");
-
-  // Use a canvas to paint the tile logo.
-  gfx::Canvas canvas(gfx::Size(kLogoWidth, kLogoHeight), ui::SCALE_FACTOR_100P,
-                     true);
+  // First paint the image onto an opaque background to get rid of transparency.
+  // White is used as it will be disregarded in the mean calculation because of
+  // lightness limit.
+  SkPaint paint;
+  paint.setColor(SK_ColorWHITE);
+  gfx::Canvas favicon_canvas(gfx::Size(image.width(), image.height()),
+                             ui::SCALE_FACTOR_100P, true);
+  favicon_canvas.DrawRect(gfx::Rect(0, 0, image.width(), image.height()),
+                          paint);
+  favicon_canvas.DrawImageInt(image, 0, 0);
 
   // Fill the tile logo with the average color from bitmap. To do this we need
   // to work out the 'average color' which is calculated using PNG encoded data
   // of the bitmap.
-  SkPaint paint;
   std::vector<unsigned char> icon_png;
-  if (!gfx::PNGCodec::EncodeBGRASkBitmap(*image.bitmap(), true, &icon_png))
+  if (!gfx::PNGCodec::EncodeBGRASkBitmap(
+          favicon_canvas.ExtractImageRep().sk_bitmap(), false, &icon_png)) {
     return false;
+  }
 
   scoped_refptr<base::RefCountedStaticMemory> icon_mem(
       new base::RefCountedStaticMemory(&icon_png.front(), icon_png.size()));
@@ -95,6 +107,8 @@ bool CreateSiteSpecificLogo(const gfx::ImageSkia& image,
   SkColor mean_color = color_utils::CalculateKMeanColorOfPNG(
       icon_mem, kColorMeanDarknessLimit, kColorMeanLightnessLimit, sampler);
   paint.setColor(mean_color);
+  gfx::Canvas canvas(gfx::Size(kLogoWidth, kLogoHeight), ui::SCALE_FACTOR_100P,
+                     true);
   canvas.DrawRect(gfx::Rect(0, 0, kLogoWidth, kLogoHeight), paint);
 
   // Now paint a faded square for the favicon to go in.
@@ -115,6 +129,7 @@ bool CreateSiteSpecificLogo(const gfx::ImageSkia& image,
   if (!gfx::PNGCodec::EncodeBGRASkBitmap(logo_bitmap, true, &logo_png))
     return false;
 
+  *logo_path = logo_dir.Append(tile_id).ReplaceExtension(L".png");
   return file_util::WriteFile(*logo_path,
                               reinterpret_cast<char*>(&logo_png[0]),
                               logo_png.size()) > 0;
@@ -142,31 +157,50 @@ bool GetPathToBackupLogo(const FilePath& logo_dir,
   return file_util::CopyFile(default_logo_path, *logo_path);
 }
 
-}  // namespace
-
-class MetroPinTabHelper::TaskRunner
-    : public base::RefCountedThreadSafe<TaskRunner> {
+// The PinPageTaskRunner class performs the necessary FILE thread actions to
+// pin a page, such as generating or copying the tile image file. When it
+// has performed these actions it will send the tile creation request to the
+// metro driver.
+class PinPageTaskRunner : public base::RefCountedThreadSafe<PinPageTaskRunner> {
  public:
-  TaskRunner() {}
+  // Creates a task runner for the pinning operation with the given details.
+  // |favicon| can be a null image (i.e. favicon.isNull() can be true), in
+  // which case the backup tile image will be used.
+  PinPageTaskRunner(string16& title, string16& url, gfx::ImageSkia& favicon);
 
-  void PinPageToStartScreen(const string16& title,
-                            const string16& url,
-                            const gfx::ImageSkia& image);
+  void Run();
+  void RunOnFileThread();
 
  private:
-  ~TaskRunner() {}
+  ~PinPageTaskRunner() {}
+
+  // Details of the page being pinned.
+  string16 title_;
+  string16 url_;
+  gfx::ImageSkia favicon_;
 
-  friend class base::RefCountedThreadSafe<TaskRunner>;
-  DISALLOW_COPY_AND_ASSIGN(TaskRunner);
+  friend class base::RefCountedThreadSafe<PinPageTaskRunner>;
+  DISALLOW_COPY_AND_ASSIGN(PinPageTaskRunner);
 };
 
-void MetroPinTabHelper::TaskRunner::PinPageToStartScreen(
-    const string16& title,
-    const string16& url,
-    const gfx::ImageSkia& image) {
+PinPageTaskRunner::PinPageTaskRunner(string16& title,
+                                     string16& url,
+                                     gfx::ImageSkia& favicon)
+    : title_(title), url_(url), favicon_(favicon) {}

[sky, 2012/10/29 15:06:41]

Since you wrapped the constructor each param on its own line.

[benwells, 2012/10/29 23:20:44]

Done.

+
+void PinPageTaskRunner::Run() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&PinPageTaskRunner::RunOnFileThread, this));
+}
+
+void PinPageTaskRunner::RunOnFileThread() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
 
-  string16 tile_id = GenerateTileId(url);
+  string16 tile_id = GenerateTileId(url_);
   FilePath logo_dir = GetTileImagesDir();
   if (logo_dir.empty()) {
     LOG(ERROR) << "Could not create directory to store tile image.";
@@ -174,7 +208,7 @@ void MetroPinTabHelper::TaskRunner::PinPageToStartScreen(
   }
 
   FilePath logo_path;
-  if (!CreateSiteSpecificLogo(image, tile_id, logo_dir, &logo_path) &&
+  if (!CreateSiteSpecificLogo(favicon_, tile_id, logo_dir, &logo_path) &&
       !GetPathToBackupLogo(logo_dir, &logo_path)) {
     LOG(ERROR) << "Count not get path to logo tile.";
     return;
@@ -194,15 +228,137 @@ void MetroPinTabHelper::TaskRunner::PinPageToStartScreen(
     return;
   }
 
-  VLOG(1) << __FUNCTION__ << " calling pin with title: " << title
-          << " and url: " << url;
-  metro_pin_to_start_screen(tile_id, title, url, logo_path);
+  metro_pin_to_start_screen(tile_id, title_, url_, logo_path);
+}
+
+}  // namespace
+
+class MetroPinTabHelper::FaviconDownloader {
+ public:
+  FaviconDownloader(MetroPinTabHelper* helper,
+                    const string16& title,
+                    const string16& url,
+                    const gfx::ImageSkia& history_image);
+  ~FaviconDownloader() {}
+
+  void Start(content::RenderViewHost* host,
+             const std::vector<FaviconURL>& candidates);
+
+  // Callback for when a favicon has been downloaded. The best bitmap so far
+  // will be stored in |best_candidate_|. If this is the last URL that was being
+  // downloaded, the page is pinned by calling PinPageToStartScreen on the FILE
+  // thread.
+  void OnDidDownloadFavicon(int id,
+                            const GURL& image_url,
+                            bool errored,
+                            int requested_size,
+                            const std::vector<SkBitmap>& bitmaps);
+
+ private:
+  // The tab helper that this downloader is operating for.
+  MetroPinTabHelper* helper_;
+
+  // Title and URL of the page being pinned.
+  string16 title_;

[sky, 2012/10/29 15:06:41]

const title_ and url_

[benwells, 2012/10/29 23:20:44]

Done.

+  string16 url_;
+
+  // The best candidate we have so far for the current pin operation.
+  gfx::ImageSkia best_candidate_;
+
+  // Outstanding favicon download requests.
+  std::set<int> in_progress_requests_;
+
+  DISALLOW_COPY_AND_ASSIGN(FaviconDownloader);
+};
+
+MetroPinTabHelper::FaviconDownloader::FaviconDownloader(
+    MetroPinTabHelper* helper,
+    const string16& title,
+    const string16& url,
+    const gfx::ImageSkia& history_image)
+        : helper_(helper),
+          title_(title),
+          url_(url),
+          best_candidate_(history_image) {}
+
+void MetroPinTabHelper::FaviconDownloader::Start(
+    content::RenderViewHost* host,
+    const std::vector<FaviconURL>& candidates) {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  // If there are no candidate URLs, progress straight to pinning.
+  if (candidates.empty()) {
+    scoped_refptr<PinPageTaskRunner> runner(
+        new PinPageTaskRunner(title_, url_, best_candidate_));
+    runner->Run();
+    helper_->FaviconDownloaderFinished();
+    return;
+  }
+
+  // Request all the candidates.
+  int image_size = 0; // Request the full sized image.
+  for (std::vector<FaviconURL>::const_iterator iter = candidates.begin();
+       iter != candidates.end();
+       ++iter) {
+    in_progress_requests_.insert(
+        FaviconUtil::DownloadFavicon(host, iter->icon_url, image_size));
+  }
+}
+
+void MetroPinTabHelper::FaviconDownloader::OnDidDownloadFavicon(
+    int id,
+    const GURL& image_url,
+    bool errored,
+    int requested_size,
+    const std::vector<SkBitmap>& bitmaps) {
+  const int kMaxIconSize = 32;
+
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+  std::set<int>::iterator iter = in_progress_requests_.find(id);
+  // Check that this request is one of ours.
+  if (iter == in_progress_requests_.end())
+    return;
+
+  in_progress_requests_.erase(iter);
+
+  // Process the bitmaps, keeping the one that is best so far.
+   if (!errored) {
+    for (std::vector<SkBitmap>::const_iterator iter = bitmaps.begin();
+         iter != bitmaps.end();
+         ++iter) {
+
+      // If we don't have a best candidate yet, this is better so just grab it.
+      if (best_candidate_.isNull()) {

[sky, 2012/10/29 15:06:41]

Might this result in an image bigger than kMaxIconSize?

[benwells, 2012/10/29 23:20:44]

Oh yes, good catch.

+        best_candidate_ = gfx::ImageSkia(*iter).DeepCopy();
+        continue;
+      }
+      // If the new bitmap is bigger than the best candidate, and not greater
+      // than the max size, grab it.
+      if (iter->height() > kMaxIconSize || iter->width() > kMaxIconSize)
+        continue;
+
+      if (iter->height() <= best_candidate_.height() ||
+          iter->width() <= best_candidate_.width()) {
+        continue;
+      }
+
+      best_candidate_ = gfx::ImageSkia(*iter).DeepCopy();
+    }
+   }
+
+  // If there are no more outstanding requests, pin the page on the FILE thread.
+  // Once this happens this downloader has done its job, so delete it.
+  if (in_progress_requests_.empty()) {
+    scoped_refptr<PinPageTaskRunner> runner(
+        new PinPageTaskRunner(title_, url_, best_candidate_));
+    runner->Run();
+    helper_->FaviconDownloaderFinished();
+  }
 }
 
 MetroPinTabHelper::MetroPinTabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
-      is_pinned_(false),
-      task_runner_(new TaskRunner) {}
+      is_pinned_(false) {}
 
 MetroPinTabHelper::~MetroPinTabHelper() {}
 
@@ -220,42 +376,58 @@ void MetroPinTabHelper::TogglePinnedToStartScreen() {
     return;
   }
 
-  // TODO(benwells): Handle downloading a larger favicon if there is one.
   GURL url = web_contents()->GetURL();
   string16 url_str = UTF8ToUTF16(url.spec());
   string16 title = web_contents()->GetTitle();
-  TabContents* tab_contents = TabContents::FromWebContents(web_contents());
-  DCHECK(tab_contents);
+  gfx::ImageSkia favicon;
   FaviconTabHelper* favicon_tab_helper = FaviconTabHelper::FromWebContents(
-      tab_contents->web_contents());
-  if (favicon_tab_helper->FaviconIsValid()) {
-    gfx::Image favicon = favicon_tab_helper->GetFavicon();
-    gfx::ImageSkia favicon_skia = favicon.AsImageSkia().DeepCopy();
-    content::BrowserThread::PostTask(
-        content::BrowserThread::FILE,
-        FROM_HERE,
-        base::Bind(&TaskRunner::PinPageToStartScreen,
-                   task_runner_,
-                   title,
-                   url_str,
-                   favicon_skia));
-    return;
-  }
-
-  content::BrowserThread::PostTask(
-      content::BrowserThread::FILE,
-      FROM_HERE,
-      base::Bind(&TaskRunner::PinPageToStartScreen,
-                 task_runner_,
-                 title,
-                 url_str,
-                 gfx::ImageSkia()));
+      web_contents());
+  if (favicon_tab_helper->FaviconIsValid())
+    favicon = favicon_tab_helper->GetFavicon().AsImageSkia().DeepCopy();
+
+  favicon_downloader_.reset(new FaviconDownloader(this, title, url_str,
+                                                  favicon));
+  favicon_downloader_->Start(web_contents()->GetRenderViewHost(),
+                             favicon_url_candidates_);
 }
 
 void MetroPinTabHelper::DidNavigateMainFrame(
     const content::LoadCommittedDetails& /*details*/,
     const content::FrameNavigateParams& /*params*/) {
   UpdatePinnedStateForCurrentURL();
+  // Cancel any outstanding pin operations once the user navigates away from
+  // the page.
+  if (favicon_downloader_.get())
+    favicon_downloader_.reset();

[sky, 2012/10/29 15:06:41]

Does this mean if the user pins something and the favicons are slow to download
the pin will fail? Is that expected?

[benwells, 2012/10/29 23:20:44]

I think it makes the most sense. The pin action comes up in a windows dialog
which feels quite separate from Chrome. Getting this coming up for a page you
were previously browsing would feel pretty strange.

I think this is an edge case, if it is flagged as a problem it should be easy to
change with this structure.

+  // Any candidate favicons we have are now out of date so clear them.
+  favicon_url_candidates_.clear();
+}
+
+bool MetroPinTabHelper::OnMessageReceived(const IPC::Message& message) {
+  bool message_handled = false;   // Allow other handlers to receive these.
+  IPC_BEGIN_MESSAGE_MAP(MetroPinTabHelper, message)
+    IPC_MESSAGE_HANDLER(IconHostMsg_UpdateFaviconURL, OnUpdateFaviconURL)
+    IPC_MESSAGE_HANDLER(IconHostMsg_DidDownloadFavicon, OnDidDownloadFavicon)
+    IPC_MESSAGE_UNHANDLED(message_handled = false)
+  IPC_END_MESSAGE_MAP()
+  return message_handled;
+}
+
+void MetroPinTabHelper::OnUpdateFaviconURL(
+    int32 page_id,
+    const std::vector<FaviconURL>& candidates) {
+  favicon_url_candidates_ = candidates;
+}
+
+void MetroPinTabHelper::OnDidDownloadFavicon(
+    int id,
+    const GURL& image_url,
+    bool errored,
+    int requested_size,
+    const std::vector<SkBitmap>& bitmaps) {
+  if (favicon_downloader_.get())
+    favicon_downloader_->OnDidDownloadFavicon(id, image_url, errored,
+                                              requested_size, bitmaps);
 }
 
 void MetroPinTabHelper::UpdatePinnedStateForCurrentURL() {
@@ -275,8 +447,6 @@ void MetroPinTabHelper::UpdatePinnedStateForCurrentURL() {
   GURL url = web_contents()->GetURL();
   string16 tile_id = GenerateTileId(UTF8ToUTF16(url.spec()));
   is_pinned_ = metro_is_pinned_to_start_screen(tile_id) != 0;
-  VLOG(1) << __FUNCTION__ << " with url " << UTF8ToUTF16(url.spec())
-          << " result: " << is_pinned_;
 }
 
 void MetroPinTabHelper::UnPinPageFromStartScreen() {
@@ -294,8 +464,10 @@ void MetroPinTabHelper::UnPinPageFromStartScreen() {
   }
 
   GURL url = web_contents()->GetURL();
-  VLOG(1) << __FUNCTION__ << " calling unpin with url: "
-          << UTF8ToUTF16(url.spec());
   string16 tile_id = GenerateTileId(UTF8ToUTF16(url.spec()));
   metro_un_pin_from_start_screen(tile_id);
 }
+
+void MetroPinTabHelper::FaviconDownloaderFinished() {
+  favicon_downloader_.reset();
+}