Reland of Change extension icon load errors to warnings (patchset #2 id:300001 of https://codereview.chromium.org/1554583002/ )

Reason for revert:
this revert managed to break the tree

Original issue's description:
> Revert of Change extension icon load errors to warnings (patchset #8 id:140001 of https://codereview.chromium.org/1537473003/ )
>
> Reason for revert:
> file read from wrong thread
>
> Original issue's description:
> > Change extension icon load errors to warnings
> >
> > During the Extension parsing step, check if the icon file exists and if not, remove that entry from the dictionary.
> >
> > Keep the same check during the validation phase and don't apply the workaround to unpacked extensions. This will more strongly discourage new extensions from making this mistake.
> >
> > BUG=570249
> >
> > Committed: https://crrev.com/6e8e7d1c49657e82d0e8f2518ad463794346321b
> > Cr-Commit-Position: refs/heads/master@{#366253}
>
> TBR=rdevlin.cronin@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=570249
>
> Committed: https://crrev.com/36dab10ffd04f3e38285bda1e4af07a7de081a31
> Cr-Commit-Position: refs/heads/master@{#367080}

TBR=rdevlin.cronin@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=570249

Review URL: https://codereview.chromium.org/1547413002

Cr-Commit-Position: refs/heads/master@{#367081}

1 files changed, 22 insertions, 2 deletions

diff --git a/extensions/common/manifest_handler_helpers.cc b/extensions/common/manifest_handler_helpers.cc
index bfc9e33..36af224d 100644
--- a/extensions/common/manifest_handler_helpers.cc
+++ b/extensions/common/manifest_handler_helpers.cc
@@ -13,7 +13,10 @@
 #include "extensions/common/error_utils.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_icon_set.h"
+#include "extensions/common/file_util.h"
 #include "extensions/common/manifest_constants.h"
+#include "grit/extensions_strings.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace extensions {
 
@@ -32,7 +35,8 @@ bool NormalizeAndValidatePath(std::string* path) {
   return true;
 }
 
-bool LoadIconsFromDictionary(const base::DictionaryValue* icons_value,
+bool LoadIconsFromDictionary(Extension* extension,
+                             const base::DictionaryValue* icons_value,
                              ExtensionIconSet* icons,
                              base::string16* error) {
   DCHECK(icons);
@@ -49,7 +53,23 @@ bool LoadIconsFromDictionary(const base::DictionaryValue* icons_value,
       return false;
     }
 
-    icons->Add(size, icon_path);
+    // For backwards compatibility, only warn (don't error out) if an icon is
+    // missing. Component extensions can skip this check as their icons are not
+    // located on disk. Unpacked extensions skip this check and fail later
+    // during validation if the file isn't present. See crbug.com/570249
+    // TODO(estade|devlin): remove this workaround and let install fail in the
+    // validate step a few releases after M49. See http://crbug.com/571193
+    if (Manifest::IsComponentLocation(extension->location()) ||
+        Manifest::IsUnpackedLocation(extension->location()) ||
+        file_util::ValidateFilePath(
+            extension->GetResource(icon_path).GetFilePath())) {
+      icons->Add(size, icon_path);
+    } else {
+      extension->AddInstallWarning(InstallWarning(
+          l10n_util::GetStringFUTF8(IDS_EXTENSION_LOAD_ICON_FAILED,
+                                    base::UTF8ToUTF16(icon_path)),
+          std::string()));
+    }
   }
   return true;
 }