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

Unified Diff: extensions/common/manifest_handler_helpers.cc

Issue 1547413002: Reland of Change extension icon load errors to warnings (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: 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 bfc9e3378c4ca9f1f46eb83c30b734e0215dbf9e..36af224d69e9d21f417728620dbf8a2a5989c34e 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 @@
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 @@
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