Update WebApkInstaller to support badge icon in installation.

chrome/browser/android/webapk/webapk_installer.cc

Index: chrome/browser/android/webapk/webapk_installer.cc
diff --git a/chrome/browser/android/webapk/webapk_installer.cc b/chrome/browser/android/webapk/webapk_installer.cc
index 04575686c1edd21dadc41a9a02ee8df08f6646ba..3352995ea377e1ad72beec9aacfcb189a9fe2881 100644
--- a/chrome/browser/android/webapk/webapk_installer.cc
+++ b/chrome/browser/android/webapk/webapk_installer.cc
@@ -111,7 +111,8 @@ std::string getCurrentAbi() {
 // Must be called on a worker thread because it encodes an SkBitmap.
 std::unique_ptr<webapk::WebApk> BuildWebApkProtoInBackground(
     const ShortcutInfo& shortcut_info,
-    const SkBitmap& shortcut_icon,
+    const SkBitmap& primary_icon,
+    const SkBitmap& badge_icon,
     const std::map<std::string, std::string>& icon_url_to_murmur2_hash,
     bool is_manifest_stale) {
   DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
@@ -139,20 +140,41 @@ std::unique_ptr<webapk::WebApk> BuildWebApkProtoInBackground(
   std::string* scope = web_app_manifest->add_scopes();
   scope->assign(GetScope(shortcut_info).spec());
 
-  webapk::Image* best_image = web_app_manifest->add_icons();
+  webapk::Image* best_primary_icon_image = web_app_manifest->add_icons();
   std::string best_primary_icon_url =
       shortcut_info.best_primary_icon_url.spec();
-  best_image->set_src(best_primary_icon_url);
+  best_primary_icon_image->set_src(best_primary_icon_url);
   auto it = icon_url_to_murmur2_hash.find(best_primary_icon_url);
   if (it != icon_url_to_murmur2_hash.end())
-    best_image->set_hash(it->second);
+    best_primary_icon_image->set_hash(it->second);
   std::vector<unsigned char> png_bytes;
-  gfx::PNGCodec::EncodeBGRASkBitmap(shortcut_icon, false, &png_bytes);
-  best_image->set_image_data(&png_bytes.front(), png_bytes.size());
+  gfx::PNGCodec::EncodeBGRASkBitmap(primary_icon, false, &png_bytes);
+  best_primary_icon_image->set_image_data(&png_bytes.front(), png_bytes.size());
+  best_primary_icon_image->add_usages(webapk::Image::PRIMARY_ICON);
+
+  if (!badge_icon.drawsNothing()) {
+    std::string best_badge_icon_url = shortcut_info.best_badge_icon_url.spec();
+    if (best_badge_icon_url == best_primary_icon_url) {
+      best_primary_icon_image->add_usages(webapk::Image::BADGE_ICON);
+    } else {
+      webapk::Image* best_badge_icon_image = web_app_manifest->add_icons();
+      best_badge_icon_image->set_src(best_badge_icon_url);
+      auto it = icon_url_to_murmur2_hash.find(best_badge_icon_url);
+      if (it != icon_url_to_murmur2_hash.end())
+        best_badge_icon_image->set_hash(it->second);
+      std::vector<unsigned char> png_bytes;
+      gfx::PNGCodec::EncodeBGRASkBitmap(badge_icon, false, &png_bytes);
+      best_badge_icon_image->set_image_data(&png_bytes.front(),
+                                            png_bytes.size());
+      best_badge_icon_image->add_usages(webapk::Image::BADGE_ICON);
+    }
+  }
 
   for (const auto& entry : icon_url_to_murmur2_hash) {
-    if (entry.first == shortcut_info.best_primary_icon_url.spec())
+    if (entry.first == shortcut_info.best_primary_icon_url.spec() ||
+        entry.first == shortcut_info.best_badge_icon_url.spec()) {
       continue;
+    }
     webapk::Image* image = web_app_manifest->add_icons();
     image->set_src(entry.first);
     image->set_hash(entry.second);

[pkotwicz, 2017/03/30 21:19:37]

Can we simplify this logic to:

if (entry.first == shortcut_info.best_primary_icon_url.spec()) {
  SetImageData(image, primary_icon);
  image->add_usages(webapk::Image::PRIMARY_ICON);
} else {
  SetImageData(image, badge_icon);
  image->add_usages(webapk::Image::BADGE_ICON);
}

where SetImageData() is a new function in the anonymous namespace

[F, 2017/04/03 21:57:29]

Resolved by offline. Doing everything inside the loop would break "update when
web manifest is no longer available"

@@ -218,11 +240,12 @@ WebApkInstaller::~WebApkInstaller() {
 // static
 void WebApkInstaller::InstallAsync(content::BrowserContext* context,
                                    const ShortcutInfo& shortcut_info,
-                                   const SkBitmap& shortcut_icon,
+                                   const SkBitmap& primary_icon,
+                                   const SkBitmap& badge_icon,
                                    const FinishCallback& finish_callback) {
   // The installer will delete itself when it is done.
   WebApkInstaller* installer =
-      new WebApkInstaller(context, shortcut_info, shortcut_icon);
+      new WebApkInstaller(context, shortcut_info, primary_icon, badge_icon);
   installer->InstallAsync(finish_callback);
 }
 
@@ -230,15 +253,16 @@ void WebApkInstaller::InstallAsync(content::BrowserContext* context,
 void WebApkInstaller::UpdateAsync(
     content::BrowserContext* context,
     const ShortcutInfo& shortcut_info,
-    const SkBitmap& shortcut_icon,
+    const SkBitmap& primary_icon,
     const std::string& webapk_package,
     int webapk_version,
     const std::map<std::string, std::string>& icon_url_to_murmur2_hash,
     bool is_manifest_stale,
     const FinishCallback& finish_callback) {
   // The installer will delete itself when it is done.
-  WebApkInstaller* installer =
-      new WebApkInstaller(context, shortcut_info, shortcut_icon);
+  SkBitmap empty_badge_icon;
+  WebApkInstaller* installer = new WebApkInstaller(
+      context, shortcut_info, primary_icon, empty_badge_icon);

[pkotwicz, 2017/03/30 21:49:50]

I assume that passing a badge icon for updates is part of a follow up CL

[pkotwicz, 2017/03/31 18:03:27]

After some thought, we actually need to fetch the badge icon in the update path.

The WebAPK server deletes its data after a while. We need to be able to update
OneOffWebApks with badge icons (We don't want a WebAPK with a badge icon to
regress to a WebAPK without a badge icon when it gets updated)

It is not necessary to trigger updates when the badge icon changes. However, we
need to pass the badge icon to UpdateAsync() when we request an update.
WebApkUpdateDataFetcher is probably the easiest place to fetch the badge icon.

[F, 2017/04/03 21:57:29]

I'll do it in follow up CLs. Added TODO in header file.

   installer->UpdateAsync(webapk_package, webapk_version,
                          icon_url_to_murmur2_hash, is_manifest_stale,
                          finish_callback);
@@ -282,8 +306,8 @@ void WebApkInstaller::BuildWebApkProtoInBackgroundForTesting(
     bool is_manifest_stale) {
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
-      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, shortcut_icon_,
-                 icon_url_to_murmur2_hash, is_manifest_stale),
+      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, primary_icon_,
+                 badge_icon_, icon_url_to_murmur2_hash, is_manifest_stale),
       base::Bind(&OnWebApkProtoBuilt, callback));
 }
 
@@ -347,11 +371,13 @@ void WebApkInstaller::OnResult(WebApkInstallResult result) {
 
 WebApkInstaller::WebApkInstaller(content::BrowserContext* browser_context,
                                  const ShortcutInfo& shortcut_info,
-                                 const SkBitmap& shortcut_icon)
+                                 const SkBitmap& primary_icon,
+                                 const SkBitmap& badge_icon)
     : request_context_getter_(
           Profile::FromBrowserContext(browser_context)->GetRequestContext()),
       shortcut_info_(shortcut_info),
-      shortcut_icon_(shortcut_icon),
+      primary_icon_(primary_icon),
+      badge_icon_(badge_icon),
       server_url_(GetServerUrl()),
       webapk_download_url_timeout_ms_(kWebApkDownloadUrlTimeoutMs),
       download_timeout_ms_(kDownloadTimeoutMs),
@@ -382,7 +408,7 @@ void WebApkInstaller::InstallAsync(const FinishCallback& finish_callback) {
   // should be fast because the icon should be in the HTTP cache.
   WebApkIconHasher::DownloadAndComputeMurmur2Hash(
       request_context_getter_, shortcut_info_.best_primary_icon_url,
-      base::Bind(&WebApkInstaller::OnGotIconMurmur2Hash,
+      base::Bind(&WebApkInstaller::OnGotPrimaryIconMurmur2Hash,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
@@ -399,8 +425,8 @@ void WebApkInstaller::UpdateAsync(
 
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
-      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, shortcut_icon_,
-                 icon_url_to_murmur2_hash, is_manifest_stale),
+      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, primary_icon_,
+                 badge_icon_, icon_url_to_murmur2_hash, is_manifest_stale),
       base::Bind(&WebApkInstaller::SendUpdateWebApkRequest,
                  weak_ptr_factory_.GetWeakPtr()));
 }
@@ -452,26 +478,57 @@ void WebApkInstaller::OnURLFetchComplete(const net::URLFetcher* source) {
   OnGotWebApkDownloadUrl(signed_download_url, response->package_name());
 }
 
-void WebApkInstaller::OnGotIconMurmur2Hash(
-    const std::string& icon_murmur2_hash) {
+void WebApkInstaller::OnGotPrimaryIconMurmur2Hash(
+    const std::string& primary_icon_hash) {
   // An empty hash indicates that |icon_hasher_| encountered an error.
-  if (icon_murmur2_hash.empty()) {
+  if (primary_icon_hash.empty()) {
     OnResult(WebApkInstallResult::FAILURE);
     return;
   }
 
+  primary_icon_hash_ = primary_icon_hash;
+
+  if (!shortcut_info_.best_badge_icon_url.is_empty() &&
+      shortcut_info_.best_badge_icon_url.spec() !=
+          shortcut_info_.best_primary_icon_url.spec()) {
+    WebApkIconHasher::DownloadAndComputeMurmur2Hash(
+        request_context_getter_, shortcut_info_.best_badge_icon_url,
+        base::Bind(&WebApkInstaller::OnGotBadgeIconMurmur2Hash,
+                   weak_ptr_factory_.GetWeakPtr()));
+  } else {
+    MapIconUrlToMurmur2Hash();

[pkotwicz, 2017/03/30 21:19:37]

Can we keep the flow linear and call OnGotBadgeIconMurmur2Hash() with an empty
hash instead?

This would enable you to collapse OnGotBadgeIconMurmur2Hash() and
MapIconUrlToMurmur2Hash() into a single function

[F, 2017/04/03 21:57:29]

Done.

+  }
+}
+
+void WebApkInstaller::OnGotBadgeIconMurmur2Hash(
+    const std::string& badge_icon_hash) {
+  // An empty hash indicates that |icon_hasher_| encountered an error.
+  if (badge_icon_hash.empty()) {
+    OnResult(WebApkInstallResult::FAILURE);
+    return;
+  }
+
+  badge_icon_hash_ = badge_icon_hash;
+
+  MapIconUrlToMurmur2Hash();
+}
+
+void WebApkInstaller::MapIconUrlToMurmur2Hash() {
   std::map<std::string, std::string> icon_url_to_murmur2_hash;
   for (const std::string& icon_url : shortcut_info_.icon_urls) {
-    if (icon_url != shortcut_info_.best_primary_icon_url.spec())
-      icon_url_to_murmur2_hash[icon_url] = "";
+    if (icon_url == shortcut_info_.best_primary_icon_url.spec())
+      icon_url_to_murmur2_hash[icon_url] = primary_icon_hash_;
+    else if (icon_url == shortcut_info_.best_badge_icon_url.spec())
+      icon_url_to_murmur2_hash[icon_url] = badge_icon_hash_;
     else
-      icon_url_to_murmur2_hash[icon_url] = icon_murmur2_hash;
+      icon_url_to_murmur2_hash[icon_url] = "";
   }
 
   base::PostTaskAndReplyWithResult(
       GetBackgroundTaskRunner().get(), FROM_HERE,
-      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, shortcut_icon_,
-                 icon_url_to_murmur2_hash, false /* is_manifest_stale */),
+      base::Bind(&BuildWebApkProtoInBackground, shortcut_info_, primary_icon_,
+                 badge_icon_, icon_url_to_murmur2_hash,
+                 false /* is_manifest_stale */),
       base::Bind(&WebApkInstaller::SendCreateWebApkRequest,
                  weak_ptr_factory_.GetWeakPtr()));
 }