Sync bookmark app icon urls and sizes, and download icons for new apps.

chrome/browser/extensions/bookmark_app_helper.cc

 // Copyright 2014 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/extensions/bookmark_app_helper.h"
 
 #include <cctype>
 
 #include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher.h"
+#include "chrome/browser/bitmap_fetcher/bitmap_fetcher_delegate.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/favicon_downloader.h"
 #include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/app_list_service.h"
 #include "chrome/browser/ui/app_list/app_list_util.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/image_loader.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "extensions/common/url_pattern.h"
 #include "grit/platform_locale_settings.h"
+#include "net/base/load_flags.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/url_request/url_request.h"
 #include "skia/ext/image_operations.h"
 #include "skia/ext/platform_canvas.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_analysis.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/image/canvas_image_source.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_family.h"
 
 #if defined(OS_MACOSX)
 #include "base/command_line.h"
 #include "chrome/browser/web_applications/web_app_mac.h"
 #include "chrome/common/chrome_switches.h"
 #endif
 
 #if defined(USE_ASH)
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #endif
 
 namespace {
 
+using extensions::BookmarkAppHelper;
+
 // Overlays a shortcut icon over the bottom left corner of a given image.
 class GeneratedIconImageSource : public gfx::CanvasImageSource {
  public:
   explicit GeneratedIconImageSource(char letter, SkColor color, int output_size)
       : gfx::CanvasImageSource(gfx::Size(output_size, output_size), false),
         letter_(letter),
         color_(color),
         output_size_(output_size) {}
   ~GeneratedIconImageSource() override {}
 
@@ skipping 63 matching lines @@
   // Generate container icons from smaller icons.
   const int kIconSizesToGenerate[] = {
       extension_misc::EXTENSION_ICON_SMALL,
       extension_misc::EXTENSION_ICON_MEDIUM,
       extension_misc::EXTENSION_ICON_LARGE,
   };
   return std::set<int>(kIconSizesToGenerate,
                        kIconSizesToGenerate + arraysize(kIconSizesToGenerate));
 }
 
-void GenerateIcons(std::set<int> generate_sizes,
-                   const GURL& app_url,
-                   SkColor generated_icon_color,
-                   std::map<int, SkBitmap>* bitmap_map) {
+void GenerateIcons(
+    std::set<int> generate_sizes,
+    const GURL& app_url,
+    SkColor generated_icon_color,
+    std::map<int, BookmarkAppHelper::BitmapAndSource>* bitmap_map) {
   // The letter that will be painted on the generated icon.
   char icon_letter = ' ';
   std::string domain_and_registry(
       net::registry_controlled_domains::GetDomainAndRegistry(
           app_url,
           net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
   if (!domain_and_registry.empty()) {
     icon_letter = domain_and_registry[0];
   } else if (!app_url.host().empty()) {
     icon_letter = app_url.host()[0];
   }
 
   // If no color has been specified, use a dark gray so it will stand out on the
   // black shelf.
   if (generated_icon_color == SK_ColorTRANSPARENT)
     generated_icon_color = SK_ColorDKGRAY;
 
   for (std::set<int>::const_iterator it = generate_sizes.begin();
        it != generate_sizes.end(); ++it) {
     extensions::BookmarkAppHelper::GenerateIcon(
         bitmap_map, *it, generated_icon_color, icon_letter);
     // Also generate the 2x resource for this size.
     extensions::BookmarkAppHelper::GenerateIcon(
         bitmap_map, *it * 2, generated_icon_color, icon_letter);
   }
 }
 
-void ReplaceWebAppIcons(std::map<int, SkBitmap> bitmap_map,
-                        WebApplicationInfo* web_app_info) {
+void ReplaceWebAppIcons(
+    std::map<int, BookmarkAppHelper::BitmapAndSource> bitmap_map,
+    WebApplicationInfo* web_app_info) {
   web_app_info->icons.clear();
 
   // Populate the icon data into the WebApplicationInfo we are using to
   // install the bookmark app.
-  for (std::map<int, SkBitmap>::const_iterator bitmap_map_it =
-           bitmap_map.begin();
-       bitmap_map_it != bitmap_map.end(); ++bitmap_map_it) {
+  for (const auto& pair : bitmap_map) {
     WebApplicationInfo::IconInfo icon_info;
-    icon_info.data = bitmap_map_it->second;
+    icon_info.data = pair.second.bitmap;
+    icon_info.url = pair.second.source_url;
     icon_info.width = icon_info.data.width();
     icon_info.height = icon_info.data.height();
     web_app_info->icons.push_back(icon_info);
   }
 }
 
+void UpdateWebAppInfoIcons(

[calamity, 2015/04/24 05:05:40]

This function is mostly about generating icons if needed right?

GenerateWebAppInfoIconsIfNeeded() perhaps?

[benwells, 2015/04/27 09:54:54]

It does a bit more than that, it resizes as well. I've renamed it and moved it
to be a public static function (for testing).

+    std::vector<BookmarkAppHelper::BitmapAndSource> downloaded_icons,
+    WebApplicationInfo* web_app_info) {
+  // Add the downloaded icons. Extensions only allow certain icon sizes. First
+  // populate icons that match the allowed sizes exactly and then downscale
+  // remaining icons to the closest allowed size that doesn't yet have an icon.
+  std::set<int> allowed_sizes(extension_misc::kExtensionIconSizes,
+                              extension_misc::kExtensionIconSizes +
+                                  extension_misc::kNumExtensionIconSizes);
+
+  // If there are icons that don't match the accepted icon sizes, find the
+  // closest bigger icon to the accepted sizes and resize the icon to it. An
+  // icon will be resized and used for at most one size.
+  std::map<int, BookmarkAppHelper::BitmapAndSource> resized_bitmaps(
+      extensions::BookmarkAppHelper::ConstrainBitmapsToSizes(downloaded_icons,
+                                                             allowed_sizes));
+
+  // Determine the color that will be used for the icon's background. For this
+  // the dominant color of the first icon found is used.
+  if (resized_bitmaps.size()) {
+    color_utils::GridSampler sampler;
+    web_app_info->generated_icon_color =
+        color_utils::CalculateKMeanColorOfBitmap(
+            resized_bitmaps.begin()->second.bitmap);
+  }
+
+  std::set<int> generate_sizes;
+  for (int size : SizesToGenerate()) {
+    if (resized_bitmaps.find(size) == resized_bitmaps.end())
+      generate_sizes.insert(size);
+  }
+  GenerateIcons(generate_sizes, web_app_info->app_url,
+                web_app_info->generated_icon_color, &resized_bitmaps);
+
+  ReplaceWebAppIcons(resized_bitmaps, web_app_info);
+}
+
+// Class to handle installing a bookmark app. Handles downloading and decoding
+// the icons.
+class BookmarkAppInstaller : public base::RefCounted<BookmarkAppInstaller>,
+                             public chrome::BitmapFetcherDelegate {
+ public:
+  BookmarkAppInstaller(ExtensionService* service,
+                       const WebApplicationInfo& web_app_info)
+      : service_(service), web_app_info_(web_app_info) {}
+
+  void Run() {
+    for (const auto& icon : web_app_info_.icons) {
+      if (icon.url.is_valid())
+        urls_to_download_.push_back(icon.url);

[calamity, 2015/04/24 05:05:40]

Are these URLs deduped anywhere? Since you can get multiple icons from the same
URL coming in from sync, you might redownload the same image multiple times.

[benwells, 2015/04/27 09:54:54]

Yeah, I thought about this. The bitmapfetcher only ever returns one image per
file. I don't think the extra downloads are really a worry but instead of having
all the different images extracted the same one will be used each time.

I'm thinking either leave it like it is for now and later update the bitmap
fetcher thing to support icos properly, or maybe just ignore .icos totally.

[calamity, 2015/04/28 06:56:35]

Acknowledged. I'm happy to just leave it as is for now. Ignoring icos sounds
like a good option.

+    }
+
+    if (urls_to_download_.size()) {
+      DownloadNextImage();
+
+      // Matched in OnFetchComplete.
+      AddRef();
+      return;
+    }
+
+    FinishInstallation();
+  }
+
+ private:
+  friend class base::RefCounted<BookmarkAppInstaller>;
+  ~BookmarkAppInstaller() override {}
+
+  // BitmapFetcherDelegate:
+  void OnFetchComplete(const GURL& url, const SkBitmap* bitmap) override {
+    if (bitmap && !bitmap->empty() && bitmap->width() == bitmap->height()) {
+      BookmarkAppHelper::BitmapAndSource bitmap_and_source;
+      bitmap_and_source.source_url = url;
+      bitmap_and_source.bitmap = *bitmap;
+      downloaded_bitmaps_.push_back(bitmap_and_source);
+    }
+
+    if (urls_to_download_.size()) {
+      DownloadNextImage();
+      return;
+    }
+
+    FinishInstallation();
+    Release();
+  }
+
+  void DownloadNextImage() {
+    DCHECK(urls_to_download_.size());
+
+    bitmap_fetcher_.reset(
+        new chrome::BitmapFetcher(urls_to_download_.back(), this));
+    urls_to_download_.pop_back();
+    bitmap_fetcher_->Start(
+        service_->profile()->GetRequestContext(), std::string(),
+        net::URLRequest::CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE,
+        net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DO_NOT_SEND_COOKIES);
+  }
+
+  void FinishInstallation() {
+    UpdateWebAppInfoIcons(downloaded_bitmaps_, &web_app_info_);
+    scoped_refptr<extensions::CrxInstaller> installer(
+        extensions::CrxInstaller::CreateSilent(service_));
+    installer->set_error_on_unsupported_requirements(true);
+    installer->InstallWebApp(web_app_info_);
+  }
+
+  ExtensionService* service_;
+  WebApplicationInfo web_app_info_;
+
+  scoped_ptr<chrome::BitmapFetcher> bitmap_fetcher_;
+  std::vector<GURL> urls_to_download_;
+  std::vector<BookmarkAppHelper::BitmapAndSource> downloaded_bitmaps_;
+};
+
 }  // namespace
 
 namespace extensions {
 
 // static
 void BookmarkAppHelper::UpdateWebAppInfoFromManifest(
     const content::Manifest& manifest,
     WebApplicationInfo* web_app_info) {
   if (!manifest.short_name.is_null())
     web_app_info->title = manifest.short_name.string();
@@ skipping 13 matching lines @@
     for (const auto& icon : manifest.icons) {
       // TODO(benwells): Take the declared icon density and sizes into account.
       WebApplicationInfo::IconInfo info;
       info.url = icon.src;
       web_app_info->icons.push_back(info);
     }
   }
 }
 
 // static
-void BookmarkAppHelper::GenerateIcon(std::map<int, SkBitmap>* bitmaps,
-                                     int output_size,
-                                     SkColor color,
-                                     char letter) {
+std::map<int, BookmarkAppHelper::BitmapAndSource>
+BookmarkAppHelper::ConstrainBitmapsToSizes(

[calamity, 2015/04/24 05:05:40]

This reverts a deletion of this code right? I think there were tests that got
deleted too. Should add those back.

[benwells, 2015/04/27 09:54:54]

Yep, done. Here is the original CL if you're interested...
https://codereview.chromium.org/782693002

+    const std::vector<BookmarkAppHelper::BitmapAndSource>& bitmaps,
+    const std::set<int>& sizes) {
+  std::map<int, BitmapAndSource> output_bitmaps;
+  std::map<int, BitmapAndSource> ordered_bitmaps;
+  for (std::vector<BitmapAndSource>::const_iterator it = bitmaps.begin();
+       it != bitmaps.end(); ++it) {
+    DCHECK(it->bitmap.width() == it->bitmap.height());
+    ordered_bitmaps[it->bitmap.width()] = *it;
+  }
+
+  std::set<int>::const_iterator sizes_it = sizes.begin();
+  std::map<int, BitmapAndSource>::const_iterator bitmaps_it =
+      ordered_bitmaps.begin();
+  while (sizes_it != sizes.end() && bitmaps_it != ordered_bitmaps.end()) {
+    int size = *sizes_it;
+    // Find the closest not-smaller bitmap.
+    bitmaps_it = ordered_bitmaps.lower_bound(size);
+    ++sizes_it;
+    // Ensure the bitmap is valid and smaller than the next allowed size.
+    if (bitmaps_it != ordered_bitmaps.end() &&
+        (sizes_it == sizes.end() ||
+         bitmaps_it->second.bitmap.width() < *sizes_it)) {
+      output_bitmaps[size] = bitmaps_it->second;
+      // Resize the bitmap if it does not exactly match the desired size.
+      if (output_bitmaps[size].bitmap.width() != size) {
+        output_bitmaps[size].bitmap = skia::ImageOperations::Resize(
+            output_bitmaps[size].bitmap, skia::ImageOperations::RESIZE_LANCZOS3,
+            size, size);
+      }
+    }
+  }
+  return output_bitmaps;
+}
+
+// static
+void BookmarkAppHelper::GenerateIcon(
+    std::map<int, BookmarkAppHelper::BitmapAndSource>* bitmaps,
+    int output_size,
+    SkColor color,
+    char letter) {
   // Do nothing if there is already an icon of |output_size|.
   if (bitmaps->count(output_size))
     return;
 
   gfx::ImageSkia icon_image(
       new GeneratedIconImageSource(letter, color, output_size),
       gfx::Size(output_size, output_size));
-  icon_image.bitmap()->deepCopyTo(&(*bitmaps)[output_size]);
+  icon_image.bitmap()->deepCopyTo(&(*bitmaps)[output_size].bitmap);
+}
+
+BookmarkAppHelper::BitmapAndSource::BitmapAndSource() {
+}
+
+BookmarkAppHelper::BitmapAndSource::~BitmapAndSource() {
 }
 
 BookmarkAppHelper::BookmarkAppHelper(Profile* profile,
                                      WebApplicationInfo web_app_info,
                                      content::WebContents* contents)
     : profile_(profile),
       contents_(contents),
       web_app_info_(web_app_info),
       crx_installer_(extensions::CrxInstaller::CreateSilent(
           ExtensionSystem::Get(profile)->extension_service())) {
@@ skipping 54 matching lines @@
     bool success,
     const std::map<GURL, std::vector<SkBitmap> >& bitmaps) {
   // The tab has navigated away during the icon download. Cancel the bookmark
   // app creation.
   if (!success) {
     favicon_downloader_.reset();
     callback_.Run(nullptr, web_app_info_);
     return;
   }
 
-  std::vector<SkBitmap> downloaded_icons;
+  std::vector<BitmapAndSource> downloaded_icons;
   for (FaviconDownloader::FaviconMap::const_iterator map_it = bitmaps.begin();
        map_it != bitmaps.end();
        ++map_it) {
     for (std::vector<SkBitmap>::const_iterator bitmap_it =
              map_it->second.begin();
          bitmap_it != map_it->second.end();
          ++bitmap_it) {
       if (bitmap_it->empty() || bitmap_it->width() != bitmap_it->height())
         continue;
 
-      downloaded_icons.push_back(*bitmap_it);
+      BitmapAndSource bitmap_and_source;
+      bitmap_and_source.source_url = map_it->first;
+      bitmap_and_source.bitmap = *bitmap_it;
+      downloaded_icons.push_back(bitmap_and_source);
     }
   }
 
   // Add all existing icons from WebApplicationInfo.
   for (std::vector<WebApplicationInfo::IconInfo>::const_iterator it =
            web_app_info_.icons.begin();
        it != web_app_info_.icons.end();
        ++it) {
     const SkBitmap& icon = it->data;
-    if (!icon.drawsNothing() && icon.width() == icon.height())
-      downloaded_icons.push_back(icon);
+    if (!icon.drawsNothing() && icon.width() == icon.height()) {
+      BitmapAndSource bitmap_and_source;
+      bitmap_and_source.bitmap = icon;

[calamity, 2015/04/24 05:05:40]

bitmap_and_source.source_url = it->url?

or is there a reason for this not to be done.

[benwells, 2015/04/27 09:54:54]

I didn't think they would have a URL in this case... but it doesn't hurt to copy
it over in case they do. Done, and I added a convenience constructor to make
this patter simpler.

+      downloaded_icons.push_back(bitmap_and_source);
+    }
   }
 
-  // Add the downloaded icons. Extensions only allow certain icon sizes. First
-  // populate icons that match the allowed sizes exactly and then downscale
-  // remaining icons to the closest allowed size that doesn't yet have an icon.
-  std::set<int> allowed_sizes(extension_misc::kExtensionIconSizes,
-                              extension_misc::kExtensionIconSizes +
-                                  extension_misc::kNumExtensionIconSizes);
-
   web_app_info_.generated_icon_color = SK_ColorTRANSPARENT;
-  // Determine the color that will be used for the icon's background. For this
-  // the dominant color of the first icon found is used.
-  if (downloaded_icons.size()) {
-    color_utils::GridSampler sampler;
-    web_app_info_.generated_icon_color =
-        color_utils::CalculateKMeanColorOfBitmap(downloaded_icons[0]);
-  }
-
-  std::set<int> generate_sizes = SizesToGenerate();
-
-  std::map<int, SkBitmap> generated_icons;
-  // Icons are always generated, replacing the icons that were downloaded. This
-  // is done so that the icons are consistent across machines.
-  // TODO(benwells): Use blob sync once it is available to sync the downloaded
-  // icons, and then only generate when there are required sizes missing.
-  GenerateIcons(generate_sizes, web_app_info_.app_url,
-                web_app_info_.generated_icon_color, &generated_icons);
-
-  ReplaceWebAppIcons(generated_icons, &web_app_info_);
+  UpdateWebAppInfoIcons(downloaded_icons, &web_app_info_);
   favicon_downloader_.reset();
 
   if (!contents_) {
     // The web contents can be null in tests.
     OnBubbleCompleted(true, web_app_info_);
     return;
   }
 
   Browser* browser = chrome::FindBrowserWithWebContents(contents_);
   if (!browser) {
@@ skipping 95 matching lines @@
       callback_.Run(nullptr, web_app_info_);
       break;
     default:
       NOTREACHED();
       break;
   }
 }
 
 void CreateOrUpdateBookmarkApp(ExtensionService* service,
                                WebApplicationInfo* web_app_info) {
-  scoped_refptr<extensions::CrxInstaller> installer(
-      extensions::CrxInstaller::CreateSilent(service));
-  installer->set_error_on_unsupported_requirements(true);
-  if (web_app_info->icons.empty()) {
-    std::map<int, SkBitmap> bitmap_map;
-    GenerateIcons(SizesToGenerate(), web_app_info->app_url,
-                  web_app_info->generated_icon_color, &bitmap_map);
-    ReplaceWebAppIcons(bitmap_map, web_app_info);
-  }
-
-  installer->InstallWebApp(*web_app_info);
+  scoped_refptr<BookmarkAppInstaller> installer(
+      new BookmarkAppInstaller(service, *web_app_info));
+  installer->Run();
 }
 
 void GetWebApplicationInfoFromApp(
     content::BrowserContext* browser_context,
     const extensions::Extension* extension,
     const base::Callback<void(const WebApplicationInfo&)> callback) {
   if (!extension->from_bookmark()) {
     callback.Run(WebApplicationInfo());
     return;
   }
@@ skipping 22 matching lines @@
       extension, info_list, base::Bind(&OnIconsLoaded, web_app_info, callback));
 }
 
 bool IsValidBookmarkAppUrl(const GURL& url) {
   URLPattern origin_only_pattern(Extension::kValidWebExtentSchemes);
   origin_only_pattern.SetMatchAllURLs(true);
   return url.is_valid() && origin_only_pattern.MatchesURL(url);
 }
 
 }  // namespace extensions