diff options
Diffstat (limited to 'chrome')
16 files changed, 124 insertions, 96 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 760a76a..e62c0ec 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -4668,18 +4668,6 @@ Update checks have repeatedly failed for the extension "<ph name="EXTENSION_NAME Could not decode image: '<ph name="IMAGE_NAME">$1<ex>image.png</ex></ph>' </message> - <!-- This version of the message is used temporarily, until we have the - blog post up describing the transition process. --> - <message name="IDS_EXTENSION_MANIFEST_VERSION_OLD" desc="Warning message when a developer tries to load an extension that uses a manifest version that will soon be disallowed."> - Support for manifest version 1 is being phased out. Please <ph name="LINK_START">$1<ex><a></ex></ph>upgrade to version 2<ph name="LINK_END">$2<ex></a></ex></ph>. - </message> - - <!-- We'll switch to this version of the message once we have a blog post - written for the first link. --> - <message name="IDS_EXTENSION_MANIFEST_VERSION_OLD_2_LINKS" desc="Warning message when a developer tries to load an extension that uses a manifest version that will soon be disallowed."> - Support for manifest version 1 is being <ph name="LINK_START_1">$1<ex><a></ex></ph>phased out<ph name="LINK_END_1">$2<ex></a></ex></ph>. Please <ph name="LINK_START_2">$3<ex><a></ex></ph>upgrade to version 2<ph name="LINK_END_2">$4<ex></a></ex></ph>. - </message> - <!-- Extension installed bubble --> <message name="IDS_EXTENSION_BUNDLE_INSTALLED_HEADING_EXTENSIONS" desc="First line in the content area of the extension bundle installed bubble. Instructs which extensions were installed."> The following extensions are now installed: diff --git a/chrome/browser/extensions/api/extension_action/page_action_apitest.cc b/chrome/browser/extensions/api/extension_action/page_action_apitest.cc index b2ffd0c..3fd45a3 100644 --- a/chrome/browser/extensions/api/extension_action/page_action_apitest.cc +++ b/chrome/browser/extensions/api/extension_action/page_action_apitest.cc @@ -145,7 +145,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, PageActionRemovePopup) { // Tests old-style pageActions API that is deprecated but we don't want to // break. IN_PROC_BROWSER_TEST_F(ExtensionApiTest, OldPageActions) { - ASSERT_TRUE(RunExtensionTestIgnoreManifestWarnings("page_action/old_api")) << + ASSERT_TRUE(RunExtensionTestAllowOldManifestVersion("page_action/old_api")) << message_; const Extension* extension = GetSingleLoadedExtension(); ASSERT_TRUE(extension) << message_; diff --git a/chrome/browser/extensions/extension_apitest.cc b/chrome/browser/extensions/extension_apitest.cc index 86a048a..c149d2b 100644 --- a/chrome/browser/extensions/extension_apitest.cc +++ b/chrome/browser/extensions/extension_apitest.cc @@ -124,7 +124,15 @@ bool ExtensionApiTest::RunExtensionTestIncognito(const char* extension_name) { bool ExtensionApiTest::RunExtensionTestIgnoreManifestWarnings( const char* extension_name) { return RunExtensionTestImpl( - extension_name, "", kFlagEnableFileAccess | kFlagIgnoreManifestWarnings); + extension_name, "", kFlagIgnoreManifestWarnings); +} + +bool ExtensionApiTest::RunExtensionTestAllowOldManifestVersion( + const char* extension_name) { + return RunExtensionTestImpl( + extension_name, + "", + kFlagEnableFileAccess | kFlagAllowOldManifestVersions); } bool ExtensionApiTest::RunComponentExtensionTest(const char* extension_name) { @@ -194,6 +202,10 @@ bool ExtensionApiTest::RunExtensionTestImpl(const char* extension_name, browser_test_flags |= ExtensionBrowserTest::kFlagEnableFileAccess; if (flags & kFlagIgnoreManifestWarnings) browser_test_flags |= ExtensionBrowserTest::kFlagIgnoreManifestWarnings; + if (flags & kFlagAllowOldManifestVersions) { + browser_test_flags |= + ExtensionBrowserTest::kFlagAllowOldManifestVersions; + } extension = LoadExtensionWithFlags(extension_path, browser_test_flags); } if (!extension) { diff --git a/chrome/browser/extensions/extension_apitest.h b/chrome/browser/extensions/extension_apitest.h index b5f7e3e..a923455 100644 --- a/chrome/browser/extensions/extension_apitest.h +++ b/chrome/browser/extensions/extension_apitest.h @@ -32,10 +32,11 @@ class Extension; // chrome.test.fail // (4) Verify expected browser state. // TODO(erikkay): There should also be a way to drive events in these tests. - class ExtensionApiTest : public ExtensionBrowserTest { public: // Flags used to configure how the tests are run. + // TODO(aa): Many of these are dupes of ExtensionBrowserTest::Flags. Combine + // somehow? enum Flags { kFlagNone = 0, @@ -54,9 +55,12 @@ class ExtensionApiTest : public ExtensionBrowserTest { // Launch the extension as a platform app. kFlagLaunchPlatformApp = 1 << 4, - // Don't fail when the loaded manifest has warnings (should only be used - // when testing deprecated features). - kFlagIgnoreManifestWarnings = 1 << 5 + // Don't fail when the loaded manifest has warnings. + kFlagIgnoreManifestWarnings = 1 << 5, + + // Allow manifest versions older that Extension::kModernManifestVersion. + // Used to test old manifest features. + kFlagAllowOldManifestVersions = 1 << 6, }; ExtensionApiTest(); @@ -116,6 +120,9 @@ class ExtensionApiTest : public ExtensionBrowserTest { // Same as RunExtensionTest, but ignores any warnings in the manifest. bool RunExtensionTestIgnoreManifestWarnings(const char* extension_name); + // Same as RunExtensionTest, allow old manifest ersions. + bool RunExtensionTestAllowOldManifestVersion(const char* extension_name); + // Same as RunExtensionTest, but loads extension as component. bool RunComponentExtensionTest(const char* extension_name); diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc index 45f9e58..a529281 100644 --- a/chrome/browser/extensions/extension_browsertest.cc +++ b/chrome/browser/extensions/extension_browsertest.cc @@ -82,6 +82,8 @@ const Extension* ExtensionBrowserTest::LoadExtensionWithFlags( scoped_refptr<extensions::UnpackedInstaller> installer( extensions::UnpackedInstaller::Create(service)); installer->set_prompt_for_plugins(false); + installer->set_require_modern_manifest_version( + (flags & kFlagAllowOldManifestVersions) == 0); installer->Load(path); content::RunMessageLoop(); } diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h index 06b67fb..aef8c36 100644 --- a/chrome/browser/extensions/extension_browsertest.h +++ b/chrome/browser/extensions/extension_browsertest.h @@ -37,7 +37,11 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest, // Don't fail when the loaded manifest has warnings (should only be used // when testing deprecated features). - kFlagIgnoreManifestWarnings = 1 << 2 + kFlagIgnoreManifestWarnings = 1 << 2, + + // Allow older manifest versions (typically these can't be loaded - we allow + // them for testing). + kFlagAllowOldManifestVersions = 1 << 3, }; ExtensionBrowserTest(); diff --git a/chrome/browser/extensions/extension_creator.cc b/chrome/browser/extensions/extension_creator.cc index 7c24389..7c06df6 100644 --- a/chrome/browser/extensions/extension_creator.cc +++ b/chrome/browser/extensions/extension_creator.cc @@ -84,7 +84,8 @@ bool ExtensionCreator::InitializeInput( } bool ExtensionCreator::ValidateManifest(const FilePath& extension_dir, - crypto::RSAPrivateKey* key_pair) { + crypto::RSAPrivateKey* key_pair, + int run_flags) { std::vector<uint8> public_key_bytes; if (!key_pair->ExportPublicKey(&public_key_bytes)) { error_message_ = @@ -102,12 +103,17 @@ bool ExtensionCreator::ValidateManifest(const FilePath& extension_dir, // Load the extension once. We don't really need it, but this does a lot of // useful validation of the structure. + int create_flags = + Extension::FOLLOW_SYMLINKS_ANYWHERE | Extension::ERROR_ON_PRIVATE_KEY; + if (run_flags & kRequireModernManifestVersion) + create_flags |= Extension::REQUIRE_MODERN_MANIFEST_VERSION; + scoped_refptr<Extension> extension( extension_file_util::LoadExtension( extension_dir, extension_id, Extension::INTERNAL, - Extension::FOLLOW_SYMLINKS_ANYWHERE | Extension::ERROR_ON_PRIVATE_KEY, + create_flags, &error_message_)); return !!extension.get(); } @@ -297,7 +303,9 @@ bool ExtensionCreator::Run(const FilePath& extension_dir, return false; // Perform some extra validation by loading the extension. - if (!ValidateManifest(extension_dir, key_pair.get())) + // TODO(aa): Can this go before creating the key pair? This would mean not + // passing ID into LoadExtension which seems OK. + if (!ValidateManifest(extension_dir, key_pair.get(), run_flags)) return false; ScopedTempDir temp_dir; diff --git a/chrome/browser/extensions/extension_creator.h b/chrome/browser/extensions/extension_creator.h index a7ec922..86243ba 100644 --- a/chrome/browser/extensions/extension_creator.h +++ b/chrome/browser/extensions/extension_creator.h @@ -30,7 +30,8 @@ class ExtensionCreator { // Settings to specify treatment of special or ignorable error conditions. enum RunFlags { kNoRunFlags = 0x0, - kOverwriteCRX = 0x1 + kOverwriteCRX = 0x1, + kRequireModernManifestVersion = 0x2, }; // Categories of error that may need special handling on the UI end. @@ -63,7 +64,8 @@ class ExtensionCreator { // Validates the manifest by trying to load the extension. bool ValidateManifest(const FilePath& extension_dir, - crypto::RSAPrivateKey* key_pair); + crypto::RSAPrivateKey* key_pair, + int run_flags); // Reads private key from |private_key_path|. crypto::RSAPrivateKey* ReadInputKey(const FilePath& private_key_path); diff --git a/chrome/browser/extensions/pack_extension_job.cc b/chrome/browser/extensions/pack_extension_job.cc index 5dcfd9f..690ccc3 100644 --- a/chrome/browser/extensions/pack_extension_job.cc +++ b/chrome/browser/extensions/pack_extension_job.cc @@ -22,7 +22,7 @@ PackExtensionJob::PackExtensionJob(Client* client, const FilePath& key_file, int run_flags) : client_(client), key_file_(key_file), asynchronous_(true), - run_flags_(run_flags) { + run_flags_(run_flags | ExtensionCreator::kRequireModernManifestVersion) { root_directory_ = root_directory.StripTrailingSeparators(); CHECK(BrowserThread::GetCurrentThreadIdentifier(&client_thread_id_)); } diff --git a/chrome/browser/extensions/pack_extension_job.h b/chrome/browser/extensions/pack_extension_job.h index 4039d19..f6d0dbb 100644 --- a/chrome/browser/extensions/pack_extension_job.h +++ b/chrome/browser/extensions/pack_extension_job.h @@ -67,7 +67,8 @@ class PackExtensionJob : public base::RefCountedThreadSafe<PackExtensionJob> { FilePath crx_file_out_; FilePath key_file_out_; bool asynchronous_; - int run_flags_; // Bitset of ExtensionCreator::RunFlags values + int run_flags_; // Bitset of ExtensionCreator::RunFlags values - we always + // assume kRequireModernManifestVersion, though. DISALLOW_COPY_AND_ASSIGN(PackExtensionJob); }; diff --git a/chrome/browser/extensions/unpacked_installer.cc b/chrome/browser/extensions/unpacked_installer.cc index f314c4b..00b7c46 100644 --- a/chrome/browser/extensions/unpacked_installer.cc +++ b/chrome/browser/extensions/unpacked_installer.cc @@ -89,7 +89,8 @@ scoped_refptr<UnpackedInstaller> UnpackedInstaller::Create( UnpackedInstaller::UnpackedInstaller(ExtensionService* extension_service) : service_weak_(extension_service->AsWeakPtr()), - prompt_for_plugins_(true) { + prompt_for_plugins_(true), + require_modern_manifest_version_(true) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); } @@ -119,21 +120,11 @@ void UnpackedInstaller::LoadFromCommandLine(const FilePath& path_in) { return; } - std::string id = Extension::GenerateIdForPath(extension_path_); - bool allow_file_access = - Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD); - if (service_weak_->extension_prefs()->HasAllowFileAccessSetting(id)) - allow_file_access = service_weak_->extension_prefs()->AllowFileAccess(id); - - int flags = Extension::REQUIRE_MODERN_MANIFEST_VERSION; - if (allow_file_access) - flags |= Extension::ALLOW_FILE_ACCESS; - std::string error; scoped_refptr<const Extension> extension(extension_file_util::LoadExtension( extension_path_, Extension::LOAD, - flags | Extension::FOLLOW_SYMLINKS_ANYWHERE, + GetFlags(), &error)); if (!extension) { @@ -144,6 +135,22 @@ void UnpackedInstaller::LoadFromCommandLine(const FilePath& path_in) { OnLoaded(extension); } +int UnpackedInstaller::GetFlags() { + std::string id = Extension::GenerateIdForPath(extension_path_); + bool allow_file_access = + Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD); + if (service_weak_->extension_prefs()->HasAllowFileAccessSetting(id)) + allow_file_access = service_weak_->extension_prefs()->AllowFileAccess(id); + + int result = Extension::FOLLOW_SYMLINKS_ANYWHERE; + if (allow_file_access) + result |= Extension::ALLOW_FILE_ACCESS; + if (require_modern_manifest_version_) + result |= Extension::REQUIRE_MODERN_MANIFEST_VERSION; + + return result; +} + bool UnpackedInstaller::IsLoadingUnpackedAllowed() const { if (!service_weak_) return true; @@ -171,30 +178,19 @@ void UnpackedInstaller::CheckExtensionFileAccess() { return; } - std::string id = Extension::GenerateIdForPath(extension_path_); - // Unpacked extensions default to allowing file access, but if that has been - // overridden, don't reset the value. - bool allow_file_access = - Extension::ShouldAlwaysAllowFileAccess(Extension::LOAD); - if (service_weak_->extension_prefs()->HasAllowFileAccessSetting(id)) - allow_file_access = service_weak_->extension_prefs()->AllowFileAccess(id); - BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind( - &UnpackedInstaller::LoadWithFileAccess, - this, allow_file_access)); + &UnpackedInstaller::LoadWithFileAccess, this, GetFlags())); } -void UnpackedInstaller::LoadWithFileAccess(bool allow_file_access) { +void UnpackedInstaller::LoadWithFileAccess(int flags) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - int flags = Extension::REQUIRE_MODERN_MANIFEST_VERSION; - if (allow_file_access) - flags |= Extension::ALLOW_FILE_ACCESS; + std::string error; scoped_refptr<const Extension> extension(extension_file_util::LoadExtension( extension_path_, Extension::LOAD, - flags | Extension::FOLLOW_SYMLINKS_ANYWHERE, + flags, &error)); if (!extension) { diff --git a/chrome/browser/extensions/unpacked_installer.h b/chrome/browser/extensions/unpacked_installer.h index 2cc3ec4..0aa1f93 100644 --- a/chrome/browser/extensions/unpacked_installer.h +++ b/chrome/browser/extensions/unpacked_installer.h @@ -41,6 +41,15 @@ class UnpackedInstaller bool prompt_for_plugins() { return prompt_for_plugins_; } void set_prompt_for_plugins(bool val) { prompt_for_plugins_ = val; } + // Allows overriding of whether modern manifest versions are required; + // intended for testing. + bool require_modern_manifest_version() const { + return require_modern_manifest_version_; + } + void set_require_modern_manifest_version(bool val) { + require_modern_manifest_version_ = val; + } + private: friend class base::RefCountedThreadSafe<UnpackedInstaller>; @@ -59,7 +68,7 @@ class UnpackedInstaller // what file access flags to pass to extension_file_util::LoadExtension. void GetAbsolutePath(); void CheckExtensionFileAccess(); - void LoadWithFileAccess(bool allow_file_access); + void LoadWithFileAccess(int flags); // Notify the frontend that there was an error loading an extension. void ReportExtensionLoadError(const std::string& error); @@ -67,6 +76,9 @@ class UnpackedInstaller // Called when an unpacked extension has been loaded and installed. void OnLoaded(const scoped_refptr<const Extension>& extension); + // Helper to get the Extension::CreateFlags for the installing extension. + int GetFlags(); + base::WeakPtr<ExtensionService> service_weak_; // The pathname of the directory to load from, which is an absolute path @@ -77,6 +89,10 @@ class UnpackedInstaller // loading. bool prompt_for_plugins_; + // Whether to require the extension installed to have a modern manifest + // version. + bool require_modern_manifest_version_; + DISALLOW_COPY_AND_ASSIGN(UnpackedInstaller); }; diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index a65b7a1..8c654fa 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -73,7 +73,7 @@ namespace extensions { namespace { -const int kModernManifestVersion = 1; +const int kModernManifestVersion = 2; const int kPEMOutputColumns = 65; const char kOverrideExtentUrlPatternFormat[] = "chrome://%s/*"; @@ -1358,18 +1358,10 @@ bool Extension::LoadManifestVersion(string16* error) { manifest_version_ < kModernManifestVersion && !CommandLine::ForCurrentProcess()->HasSwitch( switches::kAllowLegacyExtensionManifests)) { - *error = ASCIIToUTF16(errors::kInvalidManifestVersion); - return false; - } - - if (location() == LOAD && manifest_version_ == 1) { - install_warnings_.push_back(Extension::InstallWarning( - Extension::InstallWarning::FORMAT_HTML, - l10n_util::GetStringFUTF8( - IDS_EXTENSION_MANIFEST_VERSION_OLD, - ASCIIToUTF16("<a href='http://code.google.com/chrome/extensions/" - "manifestVersion.html' target='_blank'>"), - ASCIIToUTF16("</a>")))); + *error = ExtensionErrorUtils::FormatErrorMessageUTF16( + errors::kInvalidManifestVersionOld, + base::IntToString(kModernManifestVersion)); + return false; } return true; diff --git a/chrome/common/extensions/extension_manifest_constants.cc b/chrome/common/extensions/extension_manifest_constants.cc index ad5e446..660ff75 100644 --- a/chrome/common/extensions/extension_manifest_constants.cc +++ b/chrome/common/extensions/extension_manifest_constants.cc @@ -329,7 +329,11 @@ const char kInvalidLaunchValueContainer[] = const char kInvalidManifest[] = "Manifest file is invalid."; const char kInvalidManifestVersion[] = - "The 'manifest_version' key must be present and set to 2 (without quotes)."; + "Invalid value for 'manifest_version'. Must be an integer greater than " + "zero."; +const char kInvalidManifestVersionOld[] = + "The 'manifest_version' key must be present and set to * (without quotes). " + "See developer.chrome.com/extensions/manifestVersion.html for details."; const char kInvalidMatch[] = "Invalid value for 'content_scripts[*].matches[*]': *"; const char kInvalidMatchCount[] = diff --git a/chrome/common/extensions/extension_manifest_constants.h b/chrome/common/extensions/extension_manifest_constants.h index 2831022..ae6fd01 100644 --- a/chrome/common/extensions/extension_manifest_constants.h +++ b/chrome/common/extensions/extension_manifest_constants.h @@ -246,6 +246,7 @@ namespace extension_manifest_errors { extern const char kInvalidLaunchValueContainer[]; extern const char kInvalidManifest[]; extern const char kInvalidManifestVersion[]; + extern const char kInvalidManifestVersionOld[]; extern const char kInvalidMatch[]; extern const char kInvalidMatchCount[]; extern const char kInvalidMatches[]; diff --git a/chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc b/chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc index cdb0915..1f53bca 100644 --- a/chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc +++ b/chrome/common/extensions/manifest_tests/extension_manifests_manifest_version_unittest.cc @@ -11,7 +11,7 @@ using extensions::Extension; namespace errors = extension_manifest_errors; -TEST_F(ExtensionManifestTest, ManifestVersionWarning) { +TEST_F(ExtensionManifestTest, ManifestVersionError) { scoped_ptr<DictionaryValue> manifest1(new DictionaryValue()); manifest1->SetString("name", "Miles"); manifest1->SetString("version", "0.55"); @@ -24,39 +24,34 @@ TEST_F(ExtensionManifestTest, ManifestVersionWarning) { struct { const char* test_name; + bool require_modern_manifest_version; DictionaryValue* manifest; - bool expect_warning; + bool expect_error; } test_data[] = { - { "default_manifest_version", manifest1.get(), true }, - { "manifest_version_1", manifest2.get(), true }, - { "manifest_version_2", manifest3.get(), false } + { "require_modern_with_default", true, manifest1.get(), true }, + { "require_modern_with_v1", true, manifest2.get(), true }, + { "require_modern_with_v2", true, manifest3.get(), false }, + { "dont_require_modern_with_default", false, manifest1.get(), false }, + { "dont_require_modern_with_v1", false, manifest2.get(), false }, + { "dont_require_modern_with_v2", false, manifest3.get(), false }, }; for (size_t i = 0; i < ARRAYSIZE_UNSAFE(test_data); ++i) { - scoped_refptr<Extension> extension( - LoadAndExpectSuccess(Manifest(test_data[i].manifest, - test_data[i].test_name), - Extension::LOAD)); - if (test_data[i].expect_warning) { - EXPECT_THAT( - extension->install_warnings(), - testing::Contains( - testing::Field( - &Extension::InstallWarning::message, - testing::HasSubstr( - "Support for manifest version 1 is being phased out.")))) - << test_data[i].test_name; + int create_flags = Extension::NO_FLAGS; + if (test_data[i].require_modern_manifest_version) + create_flags |= Extension::REQUIRE_MODERN_MANIFEST_VERSION; + if (test_data[i].expect_error) { + LoadAndExpectError( + Manifest(test_data[i].manifest, + test_data[i].test_name), + errors::kInvalidManifestVersionOld, + Extension::LOAD, + create_flags); } else { - EXPECT_THAT( - extension->install_warnings(), - testing::Not( - testing::Contains( - testing::Field( - &Extension::InstallWarning::message, - testing::HasSubstr( - "Support for manifest version 1 is being phased " - "out."))))) - << test_data[i].test_name; + LoadAndExpectSuccess(Manifest(test_data[i].manifest, + test_data[i].test_name), + Extension::LOAD, + create_flags); } } } |