diff options
19 files changed, 28 insertions, 198 deletions
diff --git a/chrome/browser/extensions/api/extension_action/browser_action_unittest.cc b/chrome/browser/extensions/api/extension_action/browser_action_unittest.cc deleted file mode 100644 index 8060e9e..0000000 --- a/chrome/browser/extensions/api/extension_action/browser_action_unittest.cc +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/files/file_util.h" -#include "chrome/browser/extensions/extension_service_test_with_install.h" -#include "chrome/common/extensions/api/extension_action/action_info.h" - -namespace extensions { -namespace { - -class BrowserActionUnitTest : public ExtensionServiceTestWithInstall { -}; - -TEST_F(BrowserActionUnitTest, MultiIcons) { - InitializeEmptyExtensionService(); - base::FilePath path = - data_dir().AppendASCII("api_test/browser_action/multi_icons"); - ASSERT_TRUE(base::PathExists(path)); - - const Extension* extension = PackAndInstallCRX(path, INSTALL_NEW); - - EXPECT_EQ(0U, extension->install_warnings().size()); - const ActionInfo* browser_action_info = - ActionInfo::GetBrowserActionInfo(extension); - ASSERT_TRUE(browser_action_info); - - const ExtensionIconSet& icons = browser_action_info->default_icon; - - // Extension can provide arbitrary sizes. - EXPECT_EQ(4u, icons.map().size()); - EXPECT_EQ("icon19.png", icons.Get(19, ExtensionIconSet::MATCH_EXACTLY)); - EXPECT_EQ("icon24.png", icons.Get(24, ExtensionIconSet::MATCH_EXACTLY)); - EXPECT_EQ("icon24.png", icons.Get(31, ExtensionIconSet::MATCH_EXACTLY)); - EXPECT_EQ("icon38.png", icons.Get(38, ExtensionIconSet::MATCH_EXACTLY)); -} - -TEST_F(BrowserActionUnitTest, MissingIconInWebstoreCrx) { - InitializeEmptyExtensionService(); - base::FilePath path = - data_dir().AppendASCII("api_test/browser_action/missing_icon"); - ASSERT_TRUE(base::PathExists(path)); - - const Extension* extension = PackAndInstallCRX(path, INSTALL_NEW); - - ASSERT_EQ(1U, extension->install_warnings().size()); - EXPECT_EQ("Could not load extension icon 'icon19.png'.", - extension->install_warnings().front().message); - const ActionInfo* browser_action_info = - ActionInfo::GetBrowserActionInfo(extension); - ASSERT_TRUE(browser_action_info); - - const ExtensionIconSet& icons = browser_action_info->default_icon; - - EXPECT_EQ(2u, icons.map().size()); - EXPECT_EQ("icon24.png", icons.Get(24, ExtensionIconSet::MATCH_EXACTLY)); - EXPECT_EQ("icon38.png", icons.Get(38, ExtensionIconSet::MATCH_EXACTLY)); -} - -TEST_F(BrowserActionUnitTest, MissingIconInCommandLine) { - InitializeEmptyExtensionService(); - base::FilePath path = - data_dir().AppendASCII("api_test/browser_action/missing_icon"); - ASSERT_TRUE(base::PathExists(path)); - - PackAndInstallCRXWithLocation(path, Manifest::COMMAND_LINE, INSTALL_FAILED); -} - -} // namespace -} // namespace extensions diff --git a/chrome/browser/extensions/extension_action_manager_unittest.cc b/chrome/browser/extensions/extension_action_manager_unittest.cc index 617c41c..f809bbc 100644 --- a/chrome/browser/extensions/extension_action_manager_unittest.cc +++ b/chrome/browser/extensions/extension_action_manager_unittest.cc @@ -84,7 +84,6 @@ scoped_refptr<Extension> ExtensionActionManagerTest::BuildExtension( .Set("icons", std::move(extension_icons)) .Set(action_type, std::move(action)) .Set("name", std::string("Test Extension").append(id)))) - .SetLocation(Manifest::UNPACKED) .SetID(id) .Build(); registry_->AddEnabled(extension); diff --git a/chrome/browser/extensions/extension_service_test_with_install.cc b/chrome/browser/extensions/extension_service_test_with_install.cc index 8f93b2c..f2cedef 100644 --- a/chrome/browser/extensions/extension_service_test_with_install.cc +++ b/chrome/browser/extensions/extension_service_test_with_install.cc @@ -126,20 +126,6 @@ const Extension* ExtensionServiceTestWithInstall::PackAndInstallCRX( Extension::NO_FLAGS); } -const Extension* ExtensionServiceTestWithInstall::PackAndInstallCRXWithLocation( - const base::FilePath& dir_path, - Manifest::Location location, - InstallState install_state) { - // TODO(devlin): reuse another function instead of reimplementing here. - base::FilePath crx_path; - base::ScopedTempDir temp_dir; - EXPECT_TRUE(temp_dir.CreateUniqueTempDir()); - crx_path = temp_dir.path().AppendASCII("temp.crx"); - - PackCRX(dir_path, base::FilePath(), crx_path); - return InstallCRXWithLocation(crx_path, location, install_state); -} - // Attempts to install an extension. Use INSTALL_FAILED if the installation // is expected to fail. // If |install_state| is INSTALL_UPDATED, and |expected_old_name| is diff --git a/chrome/browser/extensions/extension_service_test_with_install.h b/chrome/browser/extensions/extension_service_test_with_install.h index bb9e6dc..0be2d9a 100644 --- a/chrome/browser/extensions/extension_service_test_with_install.h +++ b/chrome/browser/extensions/extension_service_test_with_install.h @@ -58,9 +58,6 @@ class ExtensionServiceTestWithInstall : public ExtensionServiceTestBase, InstallState install_state); const Extension* PackAndInstallCRX(const base::FilePath& dir_path, InstallState install_state); - const Extension* PackAndInstallCRXWithLocation(const base::FilePath& dir_path, - Manifest::Location location, - InstallState install_state); const Extension* InstallCRX(const base::FilePath& path, InstallState install_state, diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 603f5a2..e278b93 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -433,7 +433,6 @@ 'browser/extensions/api/dial/dial_service_unittest.cc', 'browser/extensions/api/easy_unlock_private/easy_unlock_private_api_chromeos_unittest.cc', 'browser/extensions/api/experience_sampling_private/experience_sampling_private_api_unittest.cc', - 'browser/extensions/api/extension_action/browser_action_unittest.cc', 'browser/extensions/api/extension_action/extension_action_prefs_unittest.cc', 'browser/extensions/api/file_handlers/api_file_handler_util_unittest.cc', 'browser/extensions/api/file_handlers/mime_util_unittest.cc', diff --git a/chrome/common/extensions/api/extension_action/action_info.cc b/chrome/common/extensions/api/extension_action/action_info.cc index 8e68d32..9185d13 100644 --- a/chrome/common/extensions/api/extension_action/action_info.cc +++ b/chrome/common/extensions/api/extension_action/action_info.cc @@ -52,7 +52,7 @@ ActionInfo::~ActionInfo() { } // static -scoped_ptr<ActionInfo> ActionInfo::Load(Extension* extension, +scoped_ptr<ActionInfo> ActionInfo::Load(const Extension* extension, const base::DictionaryValue* dict, base::string16* error) { scoped_ptr<ActionInfo> result(new ActionInfo()); @@ -92,7 +92,7 @@ scoped_ptr<ActionInfo> ActionInfo::Load(Extension* extension, std::string default_icon; if (dict->GetDictionary(keys::kPageActionDefaultIcon, &icons_value)) { if (!manifest_handler_helpers::LoadIconsFromDictionary( - extension, icons_value, &result->default_icon, error)) { + icons_value, &result->default_icon, error)) { return scoped_ptr<ActionInfo>(); } } else if (dict->GetString(keys::kPageActionDefaultIcon, &default_icon) && diff --git a/chrome/common/extensions/api/extension_action/action_info.h b/chrome/common/extensions/api/extension_action/action_info.h index 3ad0a25..09e497c 100644 --- a/chrome/common/extensions/api/extension_action/action_info.h +++ b/chrome/common/extensions/api/extension_action/action_info.h @@ -32,7 +32,7 @@ struct ActionInfo { }; // Loads an ActionInfo from the given DictionaryValue. - static scoped_ptr<ActionInfo> Load(Extension* extension, + static scoped_ptr<ActionInfo> Load(const Extension* extension, const base::DictionaryValue* dict, base::string16* error); diff --git a/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc b/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc index 7a0a64c..fdab935 100644 --- a/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc +++ b/chrome/common/extensions/api/extension_action/browser_action_manifest_unittest.cc @@ -86,8 +86,6 @@ TEST_F(BrowserActionManifestTest, .Set("19", "icon19.png") .Set("24", "icon24.png") .Set("38", "icon38.png"))))))) - // Don't check for existence of icons during parsing. - .SetLocation(Manifest::UNPACKED) .Build(); ASSERT_TRUE(extension.get()); diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_icons_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_icons_unittest.cc index 0a7a41f..e73ba82 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_icons_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_icons_unittest.cc @@ -15,22 +15,9 @@ namespace extensions { class IconsManifestTest : public ChromeManifestTest { }; -// Icons that don't exist on disk should be ignored (for most manifest -// locations). -TEST_F(IconsManifestTest, IgnoreNonExistentIcons) { - scoped_refptr<extensions::Extension> extension( - LoadAndExpectSuccess("normalize_icon_paths.json", Manifest::INTERNAL)); - const ExtensionIconSet& icons = IconsInfo::GetIcons(extension.get()); - - EXPECT_EQ("", icons.Get(extension_misc::EXTENSION_ICON_BITTY, - ExtensionIconSet::MATCH_EXACTLY)); - EXPECT_EQ("", icons.Get(extension_misc::EXTENSION_ICON_MEDIUM, - ExtensionIconSet::MATCH_EXACTLY)); -} - TEST_F(IconsManifestTest, NormalizeIconPaths) { scoped_refptr<extensions::Extension> extension( - LoadAndExpectSuccess("normalize_icon_paths.json", Manifest::UNPACKED)); + LoadAndExpectSuccess("normalize_icon_paths.json")); const ExtensionIconSet& icons = IconsInfo::GetIcons(extension.get()); EXPECT_EQ("16.png", icons.Get(extension_misc::EXTENSION_ICON_BITTY, @@ -41,7 +28,7 @@ TEST_F(IconsManifestTest, NormalizeIconPaths) { TEST_F(IconsManifestTest, IconSizes) { scoped_refptr<extensions::Extension> extension( - LoadAndExpectSuccess("init_icon_size.json", Manifest::UNPACKED)); + LoadAndExpectSuccess("init_icon_size.json")); const ExtensionIconSet& icons = IconsInfo::GetIcons(extension.get()); EXPECT_EQ("16.png", icons.Get(extension_misc::EXTENSION_ICON_BITTY, diff --git a/chrome/test/data/extensions/api_test/browser_action/missing_icon/manifest.json b/chrome/test/data/extensions/api_test/browser_action/missing_icon/manifest.json deleted file mode 100644 index 7f6df45..0000000 --- a/chrome/test/data/extensions/api_test/browser_action/missing_icon/manifest.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "name": "A test extension that tests multiple browser action icons", - "version": "1.0", - "manifest_version": 2, - "browser_action": { - "default_icon": { - "19": "icon19.png", - "24": "icon24.png", - "38": "icon38.png" - } - } -} diff --git a/chrome/test/data/extensions/api_test/browser_action/multi_icons/manifest.json b/chrome/test/data/extensions/api_test/browser_action/multi_icons/manifest.json deleted file mode 100644 index 694d8102..0000000 --- a/chrome/test/data/extensions/api_test/browser_action/multi_icons/manifest.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "name": "A test extension that tests multiple browser action icons", - "version": "1.0", - "manifest_version": 2, - "browser_action": { - "default_icon": { - "19": "icon19.png", - "24": "icon24.png", - "31": "icon24.png", - "38": "icon38.png" - } - } -} diff --git a/extensions/common/file_util.cc b/extensions/common/file_util.cc index 3ddf6d0..90a7846 100644 --- a/extensions/common/file_util.cc +++ b/extensions/common/file_util.cc @@ -56,6 +56,16 @@ enum SafeInstallationFlag { }; SafeInstallationFlag g_use_safe_installation = DEFAULT; +// Returns true if the given file path exists and is not zero-length. +bool ValidateFilePath(const base::FilePath& path) { + int64 size = 0; + if (!base::PathExists(path) || !base::GetFileSize(path, &size) || size == 0) { + return false; + } + + return true; +} + // Returns true if the extension installation should flush all files and the // directory. bool UseSafeInstallation() { diff --git a/extensions/common/file_util.h b/extensions/common/file_util.h index 609b27c..3fda56a 100644 --- a/extensions/common/file_util.h +++ b/extensions/common/file_util.h @@ -72,9 +72,6 @@ scoped_ptr<base::DictionaryValue> LoadManifest( const base::FilePath::CharType* manifest_filename, std::string* error); -// Returns true if the given file path exists and is not zero-length. -bool ValidateFilePath(const base::FilePath& path); - // Returns true if the given extension object is valid and consistent. // May also append a series of warning messages to |warnings|, but they // should not prevent the extension from running. diff --git a/extensions/common/file_util_unittest.cc b/extensions/common/file_util_unittest.cc index afbcf3c..0bfb8b9 100644 --- a/extensions/common/file_util_unittest.cc +++ b/extensions/common/file_util_unittest.cc @@ -419,39 +419,19 @@ TEST_F(FileUtilTest, WarnOnPrivateKey) { "extension includes the key file.*ext_root.a_key.pem")); } -// Try to install an extension with a zero-length icon file. -TEST_F(FileUtilTest, CheckZeroLengthAndMissingIconFile) { - base::FilePath install_dir; - ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir)); - - base::FilePath ext_dir = - install_dir.AppendASCII("file_util").AppendASCII("bad_icon"); - - std::string error; - scoped_refptr<Extension> extension(file_util::LoadExtension( - ext_dir, Manifest::INTERNAL, Extension::NO_FLAGS, &error)); - EXPECT_TRUE(extension); - ASSERT_EQ(2U, extension->install_warnings().size()); - - EXPECT_EQ("Could not load extension icon 'missing-icon.png'.", - extension->install_warnings()[0].message); - EXPECT_EQ("Could not load extension icon 'icon.png'.", - extension->install_warnings()[1].message); -} - -// Try to install an unpacked extension with a zero-length icon file. -TEST_F(FileUtilTest, CheckZeroLengthAndMissingIconFileUnpacked) { +TEST_F(FileUtilTest, CheckZeroLengthIconFile) { base::FilePath install_dir; ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir)); + // Try to install an extension with a zero-length icon file. base::FilePath ext_dir = install_dir.AppendASCII("file_util").AppendASCII("bad_icon"); std::string error; scoped_refptr<Extension> extension(file_util::LoadExtension( ext_dir, Manifest::UNPACKED, Extension::NO_FLAGS, &error)); - EXPECT_FALSE(extension); - EXPECT_EQ("Could not load extension icon 'missing-icon.png'.", error); + EXPECT_TRUE(extension.get() == NULL); + EXPECT_STREQ("Could not load extension icon 'icon.png'.", error.c_str()); } TEST_F(FileUtilTest, ExtensionURLToRelativeFilePath) { diff --git a/extensions/common/manifest_handler_helpers.cc b/extensions/common/manifest_handler_helpers.cc index 36af224d..bfc9e33 100644 --- a/extensions/common/manifest_handler_helpers.cc +++ b/extensions/common/manifest_handler_helpers.cc @@ -13,10 +13,7 @@ #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 { @@ -35,8 +32,7 @@ bool NormalizeAndValidatePath(std::string* path) { return true; } -bool LoadIconsFromDictionary(Extension* extension, - const base::DictionaryValue* icons_value, +bool LoadIconsFromDictionary(const base::DictionaryValue* icons_value, ExtensionIconSet* icons, base::string16* error) { DCHECK(icons); @@ -53,23 +49,7 @@ bool LoadIconsFromDictionary(Extension* extension, return false; } - // 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())); - } + icons->Add(size, icon_path); } return true; } diff --git a/extensions/common/manifest_handler_helpers.h b/extensions/common/manifest_handler_helpers.h index e1f901a..b0249c3 100644 --- a/extensions/common/manifest_handler_helpers.h +++ b/extensions/common/manifest_handler_helpers.h @@ -6,11 +6,9 @@ #define EXTENSIONS_COMMON_MANIFEST_HANDLER_HELPERS_H_ #include <string> -#include <vector> #include "base/memory/scoped_ptr.h" #include "base/strings/string16.h" -#include "extensions/common/install_warning.h" class ExtensionIconSet; @@ -19,9 +17,6 @@ class DictionaryValue; } namespace extensions { - -class Extension; - namespace manifest_handler_helpers { // Strips leading slashes from the file path. Returns true iff the final path is @@ -30,10 +25,8 @@ bool NormalizeAndValidatePath(std::string* path); // Loads icon paths defined in dictionary |icons_value| into ExtensionIconSet // |icons|. |icons_value| is a dictionary value {icon size -> icon path}. -// Returns success. If load fails, |error| will be set. Non-failure warnings may -// be added to |extension|. -bool LoadIconsFromDictionary(Extension* extension, - const base::DictionaryValue* icons_value, +// Returns success. If load fails, |error| will be set. +bool LoadIconsFromDictionary(const base::DictionaryValue* icons_value, ExtensionIconSet* icons, base::string16* error); diff --git a/extensions/common/manifest_handlers/icons_handler.cc b/extensions/common/manifest_handlers/icons_handler.cc index 52597e5..20ad025 100644 --- a/extensions/common/manifest_handlers/icons_handler.cc +++ b/extensions/common/manifest_handlers/icons_handler.cc @@ -64,7 +64,7 @@ bool IconsHandler::Parse(Extension* extension, base::string16* error) { } if (!manifest_handler_helpers::LoadIconsFromDictionary( - extension, icons_dict, &icons_info->icons, error)) { + icons_dict, &icons_info->icons, error)) { return false; } diff --git a/extensions/test/data/file_util/bad_icon/manifest.json b/extensions/test/data/file_util/bad_icon/manifest.json index 27b7dac..e3846e4 100644 --- a/extensions/test/data/file_util/bad_icon/manifest.json +++ b/extensions/test/data/file_util/bad_icon/manifest.json @@ -2,9 +2,7 @@ "name": "My extension with a zero-length icon", "version": "1.0", "icons": { - "16": "good-icon.png", - "32": "missing-icon.png", - "64": "icon.png" + "16": "icon.png" }, "manifest_version": 2 } diff --git a/extensions/utility/unpacker.cc b/extensions/utility/unpacker.cc index be0a4b5..c230f82 100644 --- a/extensions/utility/unpacker.cc +++ b/extensions/utility/unpacker.cc @@ -187,8 +187,9 @@ bool Unpacker::Run() { // Decode any images that the browser needs to display. std::set<base::FilePath> image_paths = ExtensionsClient::Get()->GetBrowserImagePaths(extension.get()); - for (const base::FilePath& path : image_paths) { - if (!AddDecodedImage(path)) + for (std::set<base::FilePath>::iterator it = image_paths.begin(); + it != image_paths.end(); ++it) { + if (!AddDecodedImage(*it)) return false; // Error was already reported. } |