Fails InstallableManager if a selected badge icon cannot be fetched.

chrome/browser/installable/installable_manager.cc

Index: chrome/browser/installable/installable_manager.cc
diff --git a/chrome/browser/installable/installable_manager.cc b/chrome/browser/installable/installable_manager.cc
index b0bf4496513b77bdd42977cfe2e53435a498d2dc..0b41618adda76ed1b55b9445467d64ef8afdced1 100644
--- a/chrome/browser/installable/installable_manager.cc
+++ b/chrome/browser/installable/installable_manager.cc
@@ -177,7 +177,12 @@ InstallableStatusCode InstallableManager::GetErrorCode(
       return icon.error;
   }
 
-  // Do not report badge icon's error because badge icon is optional.
+  if (params.fetch_valid_badge_icon) {
+    IconProperty& icon = icons_[ParamsForBadgeIcon(params)];
+    if (icon.error != NO_ERROR_DETECTED)
+      return icon.error;

[dominickn, 2017/04/05 02:57:24]

Can you edit the following error messages in installable_logging so they're more
generic?

kCannotDownloadIconMessage -> "could not download a required icon from the
manifest"

kNoIconAvailableMessage -> "downloaded manifest icon was empty or corrupted"

[F, 2017/04/06 15:35:35]

Done.

+  }
+
   return NO_ERROR_DETECTED;
 }
 
@@ -305,10 +310,10 @@ void InstallableManager::WorkOnTask() {
     CheckInstallable();
   } else if (params.fetch_valid_primary_icon &&
              !IsIconFetched(ParamsForPrimaryIcon(params))) {
-    CheckAndFetchBestIcon(ParamsForPrimaryIcon(params));
+    CheckAndFetchBestIcon(ParamsForPrimaryIcon(params), true);
   } else if (params.fetch_valid_badge_icon &&
              !IsIconFetched(ParamsForBadgeIcon(params))) {
-    CheckAndFetchBestIcon(ParamsForBadgeIcon(params));
+    CheckAndFetchBestIcon(ParamsForBadgeIcon(params), false);
   } else {
     NOTREACHED();
   }
@@ -435,7 +440,8 @@ void InstallableManager::OnDidCheckHasServiceWorker(
   WorkOnTask();
 }
 
-void InstallableManager::CheckAndFetchBestIcon(const IconParams& params) {
+void InstallableManager::CheckAndFetchBestIcon(const IconParams& params,
+                                               bool is_required) {
   DCHECK(!manifest().IsEmpty());
 
   int ideal_icon_size_in_px = std::get<0>(params);
@@ -450,7 +456,8 @@ void InstallableManager::CheckAndFetchBestIcon(const IconParams& params) {
       icon_purpose);
 
   if (icon_url.is_empty()) {
-    icon.error = NO_ACCEPTABLE_ICON;
+    if (is_required)

[dominickn, 2017/04/05 02:57:24]

I misinterpreted why you did it this way the first time I read this.

Instead of having is_required here, can you amend the conditional in
GetErrorCode() to be:

// If the error is NO_ACCEPTABLE_ICON, there is no icon suitable as a badge in
the
// manifest. Ignore this case since we only want to fail the check if there was
a
// suitable badge icon specified and we couldn't fetch it.
if (icon.error != NO_ERROR_DETECTED && icon.error != NO_ACCEPTABLE_ICON)
  return icon.error;

That way, we don't have the misleading NO_ERROR_DETECTED - we just ignore the
error when it's caused by a suitable badge icon not being present.

[F, 2017/04/06 15:35:35]

Done. This is a much better solution!

+      icon.error = NO_ACCEPTABLE_ICON;
   } else {
     bool can_download_icon = ManifestIconDownloader::Download(
         GetWebContents(), icon_url, ideal_icon_size_in_px,