Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(609)

Unified Diff: extensions/common/manifest_handler_helpers.cc

Issue 1537473003: Change extension icon load errors to warnings (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: ultimate weirdness Created 5 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: extensions/common/manifest_handler_helpers.cc
diff --git a/extensions/common/manifest_handler_helpers.cc b/extensions/common/manifest_handler_helpers.cc
index 4dc2f6b0837f204a39a30ca28eef5d691f556714..f04d0f83fc341536ce6fe90a0b26e8b1dd3e1133 100644
--- a/extensions/common/manifest_handler_helpers.cc
+++ b/extensions/common/manifest_handler_helpers.cc
@@ -11,8 +11,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 {
@@ -31,7 +33,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);
@@ -48,7 +51,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;
}

Powered by Google App Engine
This is Rietveld 408576698