Changing ZipFileInstaller to use the Unzip service.
As part of the effort to deprecate UtilityProcessHost, changing
extensions::UnzipInstaller to use the new Unzip service instead of
UtilityProcessMojoClient.
This remove the last use of a utility process for extensions, and as a
result the extensions/utility directory can be removed.
Tbr: tsepez@chromium.org,finnur@chromium.org
Bug: 799220
Change-Id: Ibe4c7f0c16909c99ed572822718ac56a6bcb57fa
Reviewed-on: https://chromium-review.googlesource.com/937902
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542303}diff --git a/.gn b/.gn
index 50a8a2f..375c60c 100644
--- a/.gn
+++ b/.gn
@@ -113,7 +113,6 @@
"//extensions/renderer:unit_tests",
"//extensions/shell:browser_tests",
"//extensions/shell:unit_tests",
- "//extensions/utility:unit_tests",
"//gin/*",
"//google_apis/*",
"//google_update/*",
diff --git a/chrome/browser/extensions/BUILD.gn b/chrome/browser/extensions/BUILD.gn
index 2fbe7a4..fdcee0a 100644
--- a/chrome/browser/extensions/BUILD.gn
+++ b/chrome/browser/extensions/BUILD.gn
@@ -853,6 +853,7 @@
"//components/sync_sessions",
"//components/translate/core/browser",
"//components/undo",
+ "//components/unzip_service/public/cpp",
"//components/update_client",
"//components/url_matcher",
"//components/user_prefs",
diff --git a/chrome/browser/extensions/DEPS b/chrome/browser/extensions/DEPS
index 5e0ad1d..73dab839 100644
--- a/chrome/browser/extensions/DEPS
+++ b/chrome/browser/extensions/DEPS
@@ -37,6 +37,9 @@
# TODO(mash): Remove. http://crbug.com/678705
"+ash/shell.h",
],
+ "zipfile_installer_unittest.cc": [
+ "+services/data_decoder",
+ ],
"test_extension_system.cc": [
"+services/data_decoder",
],
diff --git a/chrome/browser/extensions/zipfile_installer_unittest.cc b/chrome/browser/extensions/zipfile_installer_unittest.cc
index b347b3f7..43994cc 100644
--- a/chrome/browser/extensions/zipfile_installer_unittest.cc
+++ b/chrome/browser/extensions/zipfile_installer_unittest.cc
@@ -2,8 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <vector>
+
#include "base/bind.h"
#include "base/command_line.h"
+#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/memory/ref_counted.h"
#include "base/message_loop/message_loop.h"
@@ -19,12 +22,16 @@
#include "chrome/browser/extensions/test_extension_system.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/test/base/testing_profile.h"
+#include "components/unzip_service/unzip_service.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "content/public/test/test_utils.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/browser/extension_registry_observer.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
+#include "services/data_decoder/data_decoder_service.h"
+#include "services/service_manager/public/cpp/connector.h"
+#include "services/service_manager/public/cpp/test/test_connector_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_CHROMEOS)
@@ -81,12 +88,27 @@
base::Closure quit_closure;
};
+struct UnzipFileFilterTestCase {
+ const base::FilePath::CharType* input;
+ const bool should_unzip;
+};
+
} // namespace
class ZipFileInstallerTest : public testing::Test {
public:
ZipFileInstallerTest()
- : browser_threads_(content::TestBrowserThreadBundle::IO_MAINLOOP) {}
+ : browser_threads_(content::TestBrowserThreadBundle::IO_MAINLOOP) {
+ service_manager::TestConnectorFactory::NameToServiceMap services;
+ services.insert(std::make_pair("data_decoder",
+ data_decoder::DataDecoderService::Create()));
+ services.insert(
+ std::make_pair("unzip_service", unzip::UnzipService::CreateService()));
+ test_connector_factory_ =
+ service_manager::TestConnectorFactory::CreateForServices(
+ std::move(services));
+ connector_ = test_connector_factory_->CreateConnector();
+ }
void SetUp() override {
extensions::LoadErrorReporter::Init(/*enable_noisy_errors=*/false);
@@ -121,8 +143,8 @@
.AppendASCII("zipfile_installer")
.AppendASCII(zip_name);
ASSERT_TRUE(base::PathExists(original_path)) << original_path.value();
-
zipfile_installer_ = ZipFileInstaller::Create(
+ connector_.get(),
MakeRegisterInExtensionServiceCallback(extension_service_));
base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -131,6 +153,17 @@
observer_.WaitForInstall(expect_error);
}
+ void RunZipFileFilterTest(
+ const std::vector<UnzipFileFilterTestCase>& cases,
+ base::RepeatingCallback<bool(const base::FilePath&)>& filter) {
+ for (size_t i = 0; i < cases.size(); ++i) {
+ base::FilePath input(cases[i].input);
+ bool observed = filter.Run(input);
+ EXPECT_EQ(cases[i].should_unzip, observed)
+ << "i: " << i << ", input: " << input.value();
+ }
+ }
+
protected:
scoped_refptr<ZipFileInstaller> zipfile_installer_;
@@ -148,6 +181,11 @@
// ChromeOS needs a user manager to instantiate an extension service.
chromeos::ScopedTestUserManager test_user_manager_;
#endif
+
+ private:
+ std::unique_ptr<service_manager::TestConnectorFactory>
+ test_connector_factory_;
+ std::unique_ptr<service_manager::Connector> connector_;
};
TEST_F(ZipFileInstallerTest, GoodZip) {
@@ -165,4 +203,49 @@
EXPECT_EQ(observer_.last_extension_installed, kIdForPublicKey);
}
+TEST_F(ZipFileInstallerTest, NonTheme_FileExtractionFilter) {
+ const std::vector<UnzipFileFilterTestCase> cases = {
+ {FILE_PATH_LITERAL("foo"), true},
+ {FILE_PATH_LITERAL("foo.nexe"), true},
+ {FILE_PATH_LITERAL("foo.dll"), true},
+ {FILE_PATH_LITERAL("foo.jpg.exe"), false},
+ {FILE_PATH_LITERAL("foo.exe"), false},
+ {FILE_PATH_LITERAL("foo.EXE"), false},
+ {FILE_PATH_LITERAL("file_without_extension"), true},
+ };
+ base::RepeatingCallback<bool(const base::FilePath&)> filter =
+ base::BindRepeating(&ZipFileInstaller::ShouldExtractFile, false);
+ RunZipFileFilterTest(cases, filter);
+}
+
+TEST_F(ZipFileInstallerTest, Theme_FileExtractionFilter) {
+ const std::vector<UnzipFileFilterTestCase> cases = {
+ {FILE_PATH_LITERAL("image.jpg"), true},
+ {FILE_PATH_LITERAL("IMAGE.JPEG"), true},
+ {FILE_PATH_LITERAL("test/image.bmp"), true},
+ {FILE_PATH_LITERAL("test/IMAGE.gif"), true},
+ {FILE_PATH_LITERAL("test/image.WEBP"), true},
+ {FILE_PATH_LITERAL("test/dir/file.image.png"), true},
+ {FILE_PATH_LITERAL("manifest.json"), true},
+ {FILE_PATH_LITERAL("other.html"), false},
+ {FILE_PATH_LITERAL("file_without_extension"), true},
+ };
+ base::RepeatingCallback<bool(const base::FilePath&)> filter =
+ base::BindRepeating(&ZipFileInstaller::ShouldExtractFile, true);
+ RunZipFileFilterTest(cases, filter);
+}
+
+TEST_F(ZipFileInstallerTest, ManifestExtractionFilter) {
+ const std::vector<UnzipFileFilterTestCase> cases = {
+ {FILE_PATH_LITERAL("manifest.json"), true},
+ {FILE_PATH_LITERAL("MANIFEST.JSON"), true},
+ {FILE_PATH_LITERAL("test/manifest.json"), false},
+ {FILE_PATH_LITERAL("manifest.json/test"), false},
+ {FILE_PATH_LITERAL("other.file"), false},
+ };
+ base::RepeatingCallback<bool(const base::FilePath&)> filter =
+ base::BindRepeating(&ZipFileInstaller::IsManifestFile);
+ RunZipFileFilterTest(cases, filter);
+}
+
} // namespace extensions
diff --git a/chrome/browser/ui/webui/extensions/install_extension_handler.cc b/chrome/browser/ui/webui/extensions/install_extension_handler.cc
index 953306e..cafe0181 100644
--- a/chrome/browser/ui/webui/extensions/install_extension_handler.cc
+++ b/chrome/browser/ui/webui/extensions/install_extension_handler.cc
@@ -20,6 +20,7 @@
#include "content/public/browser/web_ui.h"
#include "content/public/browser/web_ui_data_source.h"
#include "content/public/common/drop_data.h"
+#include "content/public/common/service_manager_connection.h"
#include "extensions/browser/extension_system.h"
#include "extensions/common/feature_switch.h"
#include "net/base/filename_util.h"
@@ -100,6 +101,7 @@
if (file_display_name_.MatchesExtension(FILE_PATH_LITERAL(".zip"))) {
ZipFileInstaller::Create(
+ content::ServiceManagerConnection::GetForProcess()->GetConnector(),
MakeRegisterInExtensionServiceCallback(
ExtensionSystem::Get(profile)->extension_service()))
->LoadFromZipFile(file_to_install_);
diff --git a/chrome/utility/BUILD.gn b/chrome/utility/BUILD.gn
index 2481d41..4dba9dbd 100644
--- a/chrome/utility/BUILD.gn
+++ b/chrome/utility/BUILD.gn
@@ -116,7 +116,6 @@
"//chrome/common/extensions/api",
"//chrome/services/media_gallery_util:lib",
"//chrome/services/removable_storage_writer:lib",
- "//extensions/utility",
]
public_deps += [ "//chrome/common/extensions/api" ]
diff --git a/chrome/utility/DEPS b/chrome/utility/DEPS
index 37bfc09..4f9afedc1 100644
--- a/chrome/utility/DEPS
+++ b/chrome/utility/DEPS
@@ -29,7 +29,6 @@
"+content/public/utility",
"+extensions/common",
"+extensions/buildflags",
- "+extensions/utility",
"+media",
"+services/network/public",
"+services/network/url_request_context_builder_mojo.h",
diff --git a/chrome/utility/chrome_content_utility_client.cc b/chrome/utility/chrome_content_utility_client.cc
index aabd610..c3a84e6 100644
--- a/chrome/utility/chrome_content_utility_client.cc
+++ b/chrome/utility/chrome_content_utility_client.cc
@@ -51,7 +51,6 @@
#include "chrome/services/removable_storage_writer/public/mojom/constants.mojom.h"
#include "chrome/services/removable_storage_writer/removable_storage_writer_service.h"
#include "chrome/utility/extensions/extensions_handler.h"
-#include "extensions/utility/utility_handler.h"
#if defined(OS_WIN)
#include "chrome/services/wifi_util_win/public/mojom/constants.mojom.h"
#include "chrome/services/wifi_util_win/wifi_util_win_service.h"
@@ -118,10 +117,6 @@
ChromeContentUtilityClient::~ChromeContentUtilityClient() = default;
void ChromeContentUtilityClient::UtilityThreadStarted() {
-#if BUILDFLAG(ENABLE_EXTENSIONS)
- extensions::utility_handler::UtilityThreadStarted();
-#endif
-
#if defined(OS_WIN)
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
utility_process_running_elevated_ = command_line->HasSwitch(
@@ -137,10 +132,6 @@
return;
auto registry = std::make_unique<service_manager::BinderRegistry>();
-#if BUILDFLAG(ENABLE_EXTENSIONS)
- extensions::utility_handler::ExposeInterfacesToBrowser(
- registry.get(), utility_process_running_elevated_);
-#endif
// If our process runs with elevated privileges, only add elevated Mojo
// interfaces to the interface registry.
if (!utility_process_running_elevated_) {
diff --git a/chrome/utility/extensions/DEPS b/chrome/utility/extensions/DEPS
index a62f289..4959379 100644
--- a/chrome/utility/extensions/DEPS
+++ b/chrome/utility/extensions/DEPS
@@ -1,5 +1,5 @@
include_rules = [
"+extensions/buildflags",
"+extensions/common",
- "+extensions/utility",
+ "+extensions/features",
]
diff --git a/extensions/BUILD.gn b/extensions/BUILD.gn
index a071e57..bf9ba62 100644
--- a/extensions/BUILD.gn
+++ b/extensions/BUILD.gn
@@ -223,7 +223,6 @@
"//extensions/common:unit_tests",
"//extensions/renderer:unit_tests",
"//extensions/shell:unit_tests",
- "//extensions/utility:unit_tests",
"//services/data_decoder:lib",
"//services/service_manager/public/cpp/test:test_support",
"//ui/gl:test_support",
diff --git a/extensions/browser/BUILD.gn b/extensions/browser/BUILD.gn
index 28da43b..8dc8564 100644
--- a/extensions/browser/BUILD.gn
+++ b/extensions/browser/BUILD.gn
@@ -615,6 +615,8 @@
"//components/pref_registry:pref_registry",
"//components/prefs:test_support",
"//components/sync_preferences:test_support",
+ "//components/unzip_service:lib",
+ "//components/unzip_service/public/cpp:test_support",
"//components/update_client",
"//components/url_matcher",
"//components/user_prefs",
@@ -629,8 +631,10 @@
"//extensions/strings",
"//ipc:test_support",
"//net:test_support",
+ "//services/data_decoder:lib",
"//services/data_decoder/public/cpp:test_support",
"//services/device/public/mojom",
+ "//services/service_manager/public/cpp/test:test_support",
"//storage/browser:test_support",
"//third_party/leveldatabase",
"//third_party/zlib/google:zip",
diff --git a/extensions/browser/DEPS b/extensions/browser/DEPS
index 3d5d48d..5d4816a 100644
--- a/extensions/browser/DEPS
+++ b/extensions/browser/DEPS
@@ -8,6 +8,7 @@
"+components/sync",
"+components/sync_preferences",
"+components/update_client",
+ "+components/unzip_service/public",
"+components/variations",
"+components/version_info",
"+components/web_cache",
@@ -56,4 +57,8 @@
"+chrome/test/base/testing_profile.h",
"+chrome/test/base/ui_test_utils.h",
],
+ "sandboxed_unpacker_unittest.cc": [
+ "+components/unzip_service",
+ "+services/data_decoder",
+ ],
}
diff --git a/extensions/browser/sandboxed_unpacker.cc b/extensions/browser/sandboxed_unpacker.cc
index afec71d..54ee0eb 100644
--- a/extensions/browser/sandboxed_unpacker.cc
+++ b/extensions/browser/sandboxed_unpacker.cc
@@ -25,15 +25,16 @@
#include "base/strings/utf_string_conversions.h"
#include "build/build_config.h"
#include "components/crx_file/crx_verifier.h"
+#include "components/unzip_service/public/cpp/unzip.h"
#include "content/public/browser/browser_thread.h"
#include "extensions/browser/api/declarative_net_request/utils.h"
#include "extensions/browser/extension_file_task_runner.h"
+#include "extensions/browser/zipfile_installer.h"
#include "extensions/common/api/declarative_net_request/dnr_manifest_data.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_l10n_util.h"
#include "extensions/common/extension_resource_path_normalizer.h"
-#include "extensions/common/extension_unpacker.mojom.h"
#include "extensions/common/extension_utility_types.h"
#include "extensions/common/extensions_client.h"
#include "extensions/common/features/feature_channel.h"
@@ -233,6 +234,9 @@
CHECK_GT(location, Manifest::INVALID_LOCATION);
CHECK_LT(location, Manifest::NUM_LOCATIONS);
+ // The connector should not be bound to any thread yet.
+ DCHECK(!connector_->IsBound());
+
// Use a random instance ID to guarantee the connection is to a new data
// decoder service (running in its own process).
data_decoder_identity_ = service_manager::Identity(
@@ -323,9 +327,20 @@
PATH_LENGTH_HISTOGRAM("Extensions.SandboxUnpackLinkFreeCrxPathLength",
link_free_crx_path);
- BrowserThread::PostTask(
- BrowserThread::IO, FROM_HERE,
- base::BindOnce(&SandboxedUnpacker::Unzip, this, link_free_crx_path));
+ // Make sure to create the directory where the extension will be unzipped, as
+ // the unzipper service requires it.
+ base::FilePath unzipped_dir =
+ link_free_crx_path.DirName().AppendASCII(kTempExtensionName);
+ base::File::Error error;
+ if (!base::CreateDirectoryAndGetError(unzipped_dir, &error)) {
+ LOG(ERROR) << "Failed to created directory " << unzipped_dir.value()
+ << " with error " << error;
+ ReportFailure(UNZIP_FAILED,
+ l10n_util::GetStringUTF16(IDS_EXTENSION_PACKAGE_UNZIP_ERROR));
+ return;
+ }
+
+ Unzip(link_free_crx_path, unzipped_dir);
}
void SandboxedUnpacker::StartWithDirectory(const std::string& extension_id,
@@ -348,8 +363,8 @@
return;
}
- BrowserThread::PostTask(
- BrowserThread::IO, FROM_HERE,
+ unpacker_io_task_runner_->PostTask(
+ FROM_HERE,
base::BindOnce(&SandboxedUnpacker::Unpack, this, extension_root_));
}
@@ -373,63 +388,23 @@
unpacker_io_task_runner_->DeleteSoon(FROM_HERE, std::move(connector_));
}
-void SandboxedUnpacker::StartUtilityProcessIfNeeded() {
- DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-
- if (utility_process_mojo_client_)
- return;
-
- utility_process_mojo_client_ = std::make_unique<
- content::UtilityProcessMojoClient<mojom::ExtensionUnpacker>>(
- l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_EXTENSION_UNPACKER_NAME));
- utility_process_mojo_client_->set_error_callback(
- base::Bind(&SandboxedUnpacker::UtilityProcessCrashed, this));
-
- utility_process_mojo_client_->set_exposed_directory(temp_dir_.GetPath());
-
- utility_process_mojo_client_->Start();
-}
-
-void SandboxedUnpacker::UtilityProcessCrashed() {
- DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-
- utility_process_mojo_client_.reset();
-
- unpacker_io_task_runner_->PostTask(
- FROM_HERE,
- base::BindOnce(
- &SandboxedUnpacker::ReportFailure, this,
- UTILITY_PROCESS_CRASHED_WHILE_TRYING_TO_INSTALL,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- ASCIIToUTF16("UTILITY_PROCESS_CRASHED_WHILE_TRYING_TO_INSTALL")) +
- ASCIIToUTF16(". ") +
- l10n_util::GetStringUTF16(
- IDS_EXTENSION_INSTALL_PROCESS_CRASHED)));
-}
-
-void SandboxedUnpacker::Unzip(const base::FilePath& crx_path) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
- StartUtilityProcessIfNeeded();
+void SandboxedUnpacker::Unzip(const base::FilePath& crx_path,
+ const base::FilePath& unzipped_dir) {
+ DCHECK(unpacker_io_task_runner_->RunsTasksInCurrentSequence());
DCHECK(crx_path.DirName() == temp_dir_.GetPath());
- base::FilePath unzipped_dir =
- crx_path.DirName().AppendASCII(kTempExtensionName);
- utility_process_mojo_client_->service()->Unzip(
- crx_path, unzipped_dir,
- base::BindOnce(&SandboxedUnpacker::UnzipDone, this, unzipped_dir));
+ ZipFileInstaller::Create(connector_.get(),
+ base::BindOnce(&SandboxedUnpacker::UnzipDone, this))
+ ->LoadFromZipFileInDir(crx_path, unzipped_dir);
}
-void SandboxedUnpacker::UnzipDone(const base::FilePath& directory,
- bool success) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
+void SandboxedUnpacker::UnzipDone(const base::FilePath& zip_file,
+ const base::FilePath& unzip_dir,
+ const std::string& error) {
+ DCHECK(unpacker_io_task_runner_->RunsTasksInCurrentSequence());
- utility_process_mojo_client_.reset();
-
- if (!success) {
- utility_process_mojo_client_.reset();
+ if (!error.empty()) {
unpacker_io_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(
@@ -438,22 +413,18 @@
return;
}
- BrowserThread::PostTask(
- BrowserThread::IO, FROM_HERE,
- base::BindOnce(&SandboxedUnpacker::Unpack, this, directory));
+ Unpack(unzip_dir);
}
void SandboxedUnpacker::Unpack(const base::FilePath& directory) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ DCHECK(unpacker_io_task_runner_->RunsTasksInCurrentSequence());
DCHECK(directory.DirName() == temp_dir_.GetPath());
base::FilePath manifest_path = extension_root_.Append(kManifestFilename);
- unpacker_io_task_runner_->PostTask(
- FROM_HERE,
- base::BindOnce(
- &SandboxedUnpacker::ParseJsonFile, this, manifest_path,
- base::BindOnce(&SandboxedUnpacker::ReadManifestDone, this)));
+
+ ParseJsonFile(manifest_path,
+ base::BindOnce(&SandboxedUnpacker::ReadManifestDone, this));
}
void SandboxedUnpacker::ReadManifestDone(
@@ -607,6 +578,12 @@
l10n_util::GetStringFUTF16(IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
ASCIIToUTF16("ERROR_SAVING_THEME_IMAGE"));
break;
+ case ImageSanitizer::Status::kServiceError:
+ failure_reason = UTILITY_PROCESS_CRASHED_WHILE_TRYING_TO_INSTALL;
+ error = l10n_util::GetStringFUTF16(
+ IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
+ ASCIIToUTF16("ERROR_UTILITY_PROCESS_CRASH"));
+ break;
default:
NOTREACHED();
break;
@@ -786,7 +763,6 @@
void SandboxedUnpacker::UnpackExtensionFailed(const base::string16& error) {
DCHECK(unpacker_io_task_runner_->RunsTasksInCurrentSequence());
-
ReportFailure(
UNPACKER_CLIENT_FAILED,
l10n_util::GetStringFUTF16(IDS_EXTENSION_PACKAGE_ERROR_MESSAGE, error));
diff --git a/extensions/browser/sandboxed_unpacker.h b/extensions/browser/sandboxed_unpacker.h
index 9ec05ce..24ea37f 100644
--- a/extensions/browser/sandboxed_unpacker.h
+++ b/extensions/browser/sandboxed_unpacker.h
@@ -16,7 +16,6 @@
#include "base/optional.h"
#include "base/strings/string_piece.h"
#include "base/time/time.h"
-#include "content/public/browser/utility_process_mojo_client.h"
#include "extensions/browser/crx_file_info.h"
#include "extensions/browser/image_sanitizer.h"
#include "extensions/browser/install/crx_install_error.h"
@@ -40,10 +39,6 @@
namespace extensions {
class Extension;
-namespace mojom {
-class ExtensionUnpacker;
-}
-
class SandboxedUnpackerClient
: public base::RefCountedDeleteOnSequence<SandboxedUnpackerClient> {
public:
@@ -85,9 +80,8 @@
// SandboxedUnpacker does work to optionally unpack and then validate/sanitize
// an extension, either starting from a crx file, or else an already unzipped
-// directory (eg., from a differential update). This is done in a sandboxed
-// subprocess to protect the browser process from parsing complex data formats
-// like JPEG or JSON from untrusted sources.
+// directory (eg., from a differential update). The parsing of complex data
+// formats like JPEG or JSON is performed in specific, sandboxed services.
//
// Unpacking an extension using this class makes changes to its source, such as
// transcoding all images to PNG, parsing all message catalogs, and rewriting
@@ -96,8 +90,7 @@
//
// Lifetime management:
//
-// This class is ref-counted by each call it makes to itself on another thread,
-// and by UtilityProcessMojoClient.
+// This class is ref-counted by each call it makes to itself on another thread.
//
// Additionally, we hold a reference to our own client so that the client lives
// long enough to receive the result of unpacking.
@@ -229,15 +222,12 @@
bool ValidateSignature(const base::FilePath& crx_path,
const std::string& expected_hash);
- // Ensures the utility process is created.
- void StartUtilityProcessIfNeeded();
-
- // Utility process crashed or failed while trying to install.
- void UtilityProcessCrashed();
-
// Unzips the extension into directory.
- void Unzip(const base::FilePath& crx_path);
- void UnzipDone(const base::FilePath& directory, bool success);
+ void Unzip(const base::FilePath& crx_path,
+ const base::FilePath& unzipped_dir);
+ void UnzipDone(const base::FilePath& zip_file,
+ const base::FilePath& unzip_dir,
+ const std::string& error);
// Unpacks the extension in directory and returns the manifest.
void Unpack(const base::FilePath& directory);
@@ -342,10 +332,6 @@
// Sequenced task runner where file I/O operations will be performed.
scoped_refptr<base::SequencedTaskRunner> unpacker_io_task_runner_;
- // Utility client used for sending tasks to the utility process.
- std::unique_ptr<content::UtilityProcessMojoClient<mojom::ExtensionUnpacker>>
- utility_process_mojo_client_;
-
// The normalized path of the install icon path, retrieved from the manifest.
base::FilePath install_icon_path_;
diff --git a/extensions/browser/sandboxed_unpacker_unittest.cc b/extensions/browser/sandboxed_unpacker_unittest.cc
index a61be79..a378d30 100644
--- a/extensions/browser/sandboxed_unpacker_unittest.cc
+++ b/extensions/browser/sandboxed_unpacker_unittest.cc
@@ -17,6 +17,8 @@
#include "base/threading/thread_task_runner_handle.h"
#include "base/values.h"
#include "components/crx_file/id_util.h"
+#include "components/unzip_service/public/cpp/test_unzip_service.h"
+#include "components/unzip_service/unzip_service.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "content/public/test/test_utils.h"
@@ -28,8 +30,10 @@
#include "extensions/common/switches.h"
#include "extensions/strings/grit/extensions_strings.h"
#include "extensions/test/test_extensions_client.h"
+#include "services/data_decoder/data_decoder_service.h"
#include "services/data_decoder/public/cpp/test_data_decoder_service.h"
#include "services/service_manager/public/cpp/connector.h"
+#include "services/service_manager/public/cpp/test/test_connector_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "third_party/zlib/google/zip.h"
@@ -128,10 +132,30 @@
// It will delete itself.
client_ = new MockSandboxedUnpackerClient;
- sandboxed_unpacker_ = new SandboxedUnpacker(
- test_data_decoder_service_.connector()->Clone(), Manifest::INTERNAL,
- Extension::NO_FLAGS, extensions_dir_.GetPath(),
- base::ThreadTaskRunnerHandle::Get(), client_);
+ InitSanboxedUnpacker(/*data_decode_service=*/nullptr,
+ /*unzip_service=*/nullptr);
+ }
+
+ void InitSanboxedUnpacker(
+ std::unique_ptr<service_manager::Service> data_decode_service,
+ std::unique_ptr<service_manager::Service> unzip_service) {
+ service_manager::TestConnectorFactory::NameToServiceMap services;
+ if (!data_decode_service)
+ data_decode_service = data_decoder::DataDecoderService::Create();
+ if (!unzip_service)
+ unzip_service = unzip::UnzipService::CreateService();
+ services.insert(
+ std::make_pair("data_decoder", std::move(data_decode_service)));
+ services.insert(std::make_pair("unzip_service", std::move(unzip_service)));
+ test_connector_factory_ =
+ service_manager::TestConnectorFactory::CreateForServices(
+ std::move(services));
+ connector_ = test_connector_factory_->CreateConnector();
+
+ sandboxed_unpacker_ =
+ new SandboxedUnpacker(connector_->Clone(), Manifest::INTERNAL,
+ Extension::NO_FLAGS, extensions_dir_.GetPath(),
+ base::ThreadTaskRunnerHandle::Get(), client_);
}
void TearDown() override {
@@ -178,20 +202,6 @@
client_->WaitForUnpack();
}
- void SimulateUtilityProcessCrash() {
- sandboxed_unpacker_->CreateTempDirectory();
-
- content::BrowserThread::PostTask(
- content::BrowserThread::IO, FROM_HERE,
- base::Bind(&SandboxedUnpacker::StartUtilityProcessIfNeeded,
- sandboxed_unpacker_));
-
- content::BrowserThread::PostTask(
- content::BrowserThread::IO, FROM_HERE,
- base::Bind(&SandboxedUnpacker::UtilityProcessCrashed,
- sandboxed_unpacker_));
- }
-
bool InstallSucceeded() const { return !client_->temp_dir().empty(); }
base::FilePath GetInstallPath() const {
@@ -222,12 +232,14 @@
}
protected:
- data_decoder::TestDataDecoderService test_data_decoder_service_;
base::ScopedTempDir extensions_dir_;
MockSandboxedUnpackerClient* client_;
scoped_refptr<SandboxedUnpacker> sandboxed_unpacker_;
std::unique_ptr<content::InProcessUtilityThreadHelper>
in_process_utility_thread_helper_;
+ std::unique_ptr<service_manager::TestConnectorFactory>
+ test_connector_factory_;
+ std::unique_ptr<service_manager::Connector> connector_;
};
TEST_F(SandboxedUnpackerTest, EmptyDefaultLocale) {
@@ -353,6 +365,34 @@
EXPECT_EQ(base::string16(), GetInstallError());
}
+// The following tests simulate the utility services failling.
+TEST_F(SandboxedUnpackerTest, UnzipperServiceFails) {
+ InitSanboxedUnpacker(
+ /*data_decoder_service=*/nullptr,
+ std::make_unique<unzip::CrashyUnzipService>());
+ SetupUnpacker("good_package.crx", "");
+ EXPECT_FALSE(InstallSucceeded());
+ EXPECT_FALSE(GetInstallError().empty());
+}
+
+TEST_F(SandboxedUnpackerTest, JsonParserFails) {
+ InitSanboxedUnpacker(std::make_unique<data_decoder::CrashyDataDecoderService>(
+ /*crash_json=*/true, /*crash_image=*/false),
+ /*unzip_service=*/nullptr);
+ SetupUnpacker("good_package.crx", "");
+ EXPECT_FALSE(InstallSucceeded());
+ EXPECT_FALSE(GetInstallError().empty());
+}
+
+TEST_F(SandboxedUnpackerTest, ImageDecoderFails) {
+ InitSanboxedUnpacker(std::make_unique<data_decoder::CrashyDataDecoderService>(
+ /*crash_json=*/false, /*crash_image=*/true),
+ /*unzip_service=*/nullptr);
+ SetupUnpacker("good_package.crx", "");
+ EXPECT_FALSE(InstallSucceeded());
+ EXPECT_FALSE(GetInstallError().empty());
+}
+
// SandboxedUnpacker is ref counted and is reference by callbacks and
// InterfacePtrs. This tests that it gets deleted as expected (so that no extra
// refs are left).
@@ -364,25 +404,4 @@
TestSandboxedUnpackerDeleted("bad_image.crx", /*expect_success=*/false);
}
-class SandboxedUnpackerTestWithRealIOThread : public SandboxedUnpackerTest {
- public:
- SandboxedUnpackerTestWithRealIOThread()
- : SandboxedUnpackerTest(
- content::TestBrowserThreadBundle::REAL_IO_THREAD) {}
-
- void TearDown() override {
- // The utility process task could still be running. Ensure it is fully
- // finished before ending the test.
- content::RunAllPendingInMessageLoop(content::BrowserThread::IO);
- SandboxedUnpackerTest::TearDown();
- }
-};
-
-TEST_F(SandboxedUnpackerTestWithRealIOThread, UtilityProcessCrash) {
- SimulateUtilityProcessCrash();
- client_->WaitForUnpack();
- // Check that there is an error message.
- EXPECT_NE(base::string16(), GetInstallError());
-}
-
} // namespace extensions
diff --git a/extensions/browser/zipfile_installer.cc b/extensions/browser/zipfile_installer.cc
index b24bbab..b18e9588 100644
--- a/extensions/browser/zipfile_installer.cc
+++ b/extensions/browser/zipfile_installer.cc
@@ -6,19 +6,33 @@
#include "base/files/file_util.h"
#include "base/path_service.h"
+#include "base/task_runner_util.h"
#include "base/task_scheduler/post_task.h"
+#include "components/unzip_service/public/cpp/unzip.h"
+#include "components/unzip_service/public/interfaces/unzipper.mojom.h"
#include "extensions/browser/extension_file_task_runner.h"
-#include "extensions/common/extension_unpacker.mojom.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/manifest.h"
#include "extensions/strings/grit/extensions_strings.h"
+#include "services/data_decoder/public/cpp/safe_json_parser.h"
+#include "services/service_manager/public/cpp/connector.h"
#include "ui/base/l10n/l10n_util.h"
+namespace extensions {
+
namespace {
-const char kExtensionHandlerTempDirError[] =
+constexpr char kExtensionHandlerTempDirError[] =
"Could not create temporary directory for zipped extension.";
-const char kExtensionHandlerFileUnzipError[] =
+constexpr char kExtensionHandlerFileUnzipError[] =
"Could not unzip extension for install.";
+constexpr const base::FilePath::CharType* kAllowedThemeFiletypes[] = {
+ FILE_PATH_LITERAL(".bmp"), FILE_PATH_LITERAL(".gif"),
+ FILE_PATH_LITERAL(".jpeg"), FILE_PATH_LITERAL(".jpg"),
+ FILE_PATH_LITERAL(".json"), FILE_PATH_LITERAL(".png"),
+ FILE_PATH_LITERAL(".webp")};
+
base::Optional<base::FilePath> PrepareAndGetUnzipDir(
const base::FilePath& zip_file) {
base::AssertBlockingAllowed();
@@ -36,31 +50,59 @@
return unzip_dir;
}
-} // namespace
+base::Optional<std::string> ReadFileContent(const base::FilePath& path) {
+ base::AssertBlockingAllowed();
-namespace extensions {
+ std::string content;
+ return base::ReadFileToString(path, &content) ? content
+ : base::Optional<std::string>();
+}
+
+} // namespace
// static
scoped_refptr<ZipFileInstaller> ZipFileInstaller::Create(
+ service_manager::Connector* connector,
DoneCallback done_callback) {
+ DCHECK(connector);
DCHECK(done_callback);
- return base::WrapRefCounted(new ZipFileInstaller(std::move(done_callback)));
+ return base::WrapRefCounted(
+ new ZipFileInstaller(connector, std::move(done_callback)));
}
void ZipFileInstaller::LoadFromZipFile(const base::FilePath& zip_file) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ LoadFromZipFileImpl(zip_file, base::FilePath());
+}
+
+void ZipFileInstaller::LoadFromZipFileInDir(const base::FilePath& zip_file,
+ const base::FilePath& unzip_dir) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ DCHECK(!unzip_dir.empty());
+ LoadFromZipFileImpl(zip_file, unzip_dir);
+}
+
+void ZipFileInstaller::LoadFromZipFileImpl(const base::FilePath& zip_file,
+ const base::FilePath& unzip_dir) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(!zip_file.empty());
zip_file_ = zip_file;
+ if (!unzip_dir.empty()) {
+ Unzip(unzip_dir);
+ return;
+ }
+
base::PostTaskAndReplyWithResult(
GetExtensionFileTaskRunner().get(), FROM_HERE,
base::BindOnce(&PrepareAndGetUnzipDir, zip_file),
base::BindOnce(&ZipFileInstaller::Unzip, this));
}
-ZipFileInstaller::ZipFileInstaller(DoneCallback done_callback)
- : done_callback_(std::move(done_callback)) {}
+ZipFileInstaller::ZipFileInstaller(service_manager::Connector* connector,
+ DoneCallback done_callback)
+ : done_callback_(std::move(done_callback)), connector_(connector) {}
ZipFileInstaller::~ZipFileInstaller() = default;
@@ -71,29 +113,76 @@
ReportFailure(std::string(kExtensionHandlerTempDirError));
return;
}
- DCHECK(!utility_process_mojo_client_);
- utility_process_mojo_client_ = std::make_unique<
- content::UtilityProcessMojoClient<mojom::ExtensionUnpacker>>(
- l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_ZIP_FILE_INSTALLER_NAME));
- utility_process_mojo_client_->set_error_callback(
- base::Bind(&ZipFileInstaller::UnzipDone, this, *unzip_dir, false));
+ unzip::UnzipWithFilter(
+ connector_->Clone(), zip_file_, *unzip_dir,
+ base::BindRepeating(&ZipFileInstaller::IsManifestFile),
+ base::BindOnce(&ZipFileInstaller::ManifestUnzipped, this, *unzip_dir));
+}
- utility_process_mojo_client_->set_exposed_directory(*unzip_dir);
+void ZipFileInstaller::ManifestUnzipped(const base::FilePath& unzip_dir,
+ bool success) {
+ if (!success) {
+ ReportFailure(kExtensionHandlerFileUnzipError);
+ return;
+ }
- utility_process_mojo_client_->Start();
+ base::PostTaskAndReplyWithResult(
+ GetExtensionFileTaskRunner().get(), FROM_HERE,
+ base::BindOnce(&ReadFileContent, unzip_dir.Append(kManifestFilename)),
+ base::BindOnce(&ZipFileInstaller::ManifestRead, this, unzip_dir));
+}
- utility_process_mojo_client_->service()->Unzip(
- zip_file_, *unzip_dir,
- base::BindOnce(&ZipFileInstaller::UnzipDone, this, *unzip_dir));
+void ZipFileInstaller::ManifestRead(
+ const base::FilePath& unzip_dir,
+ base::Optional<std::string> manifest_content) {
+ if (!manifest_content) {
+ ReportFailure(std::string(kExtensionHandlerFileUnzipError));
+ return;
+ }
+
+ data_decoder::SafeJsonParser::Parse(
+ connector_, *manifest_content,
+ base::Bind(&ZipFileInstaller::ManifestParsed, this, unzip_dir),
+ base::Bind(&ZipFileInstaller::ManifestParsingFailed, this));
+}
+
+void ZipFileInstaller::ManifestParsingFailed(const std::string& error) {
+ ReportFailure(std::string(kExtensionHandlerFileUnzipError));
+}
+
+void ZipFileInstaller::ManifestParsed(
+ const base::FilePath& unzip_dir,
+ std::unique_ptr<base::Value> manifest_value) {
+ std::unique_ptr<base::DictionaryValue> manifest_dictionary =
+ base::DictionaryValue::From(std::move(manifest_value));
+ if (!manifest_dictionary) {
+ ReportFailure(std::string(kExtensionHandlerFileUnzipError));
+ return;
+ }
+
+ Manifest manifest(Manifest::INTERNAL, std::move(manifest_dictionary));
+
+ unzip::UnzipFilterCallback filter = base::BindRepeating(
+ [](bool is_theme, const base::FilePath& file_path) -> bool {
+ // Note that we ignore the manifest as it has already been extracted and
+ // would cause the unzipping to fail.
+ return ZipFileInstaller::ShouldExtractFile(is_theme, file_path) &&
+ !ZipFileInstaller::IsManifestFile(file_path);
+ },
+ manifest.is_theme());
+
+ // TODO(crbug.com/645263): This silently ignores blocked file types.
+ // Add install warnings.
+ unzip::UnzipWithFilter(
+ connector_->Clone(), zip_file_, unzip_dir, filter,
+ base::BindOnce(&ZipFileInstaller::UnzipDone, this, unzip_dir));
}
void ZipFileInstaller::UnzipDone(const base::FilePath& unzip_dir,
bool success) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- utility_process_mojo_client_.reset();
-
if (!success) {
ReportFailure(kExtensionHandlerFileUnzipError);
return;
@@ -108,4 +197,26 @@
std::move(done_callback_).Run(zip_file_, base::FilePath(), error);
}
+// static
+bool ZipFileInstaller::ShouldExtractFile(bool is_theme,
+ const base::FilePath& file_path) {
+ if (is_theme) {
+ const base::FilePath::StringType extension =
+ base::ToLowerASCII(file_path.FinalExtension());
+ // Allow filenames with no extension.
+ if (extension.empty())
+ return true;
+ return base::ContainsValue(kAllowedThemeFiletypes, extension);
+ }
+ return !base::FilePath::CompareEqualIgnoreCase(file_path.FinalExtension(),
+ FILE_PATH_LITERAL(".exe"));
+}
+
+// static
+bool ZipFileInstaller::IsManifestFile(const base::FilePath& file_path) {
+ CHECK(!file_path.IsAbsolute());
+ return base::FilePath::CompareEqualIgnoreCase(file_path.value(),
+ kManifestFilename);
+}
+
} // namespace extensions
diff --git a/extensions/browser/zipfile_installer.h b/extensions/browser/zipfile_installer.h
index 944e99a..9d44b5f 100644
--- a/extensions/browser/zipfile_installer.h
+++ b/extensions/browser/zipfile_installer.h
@@ -8,20 +8,24 @@
#include <memory>
#include <string>
+#include "base/callback.h"
#include "base/files/file_path.h"
+#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
#include "base/sequence_checker.h"
-#include "content/public/browser/utility_process_mojo_client.h"
+#include "base/values.h"
+
+namespace service_manager {
+class Connector;
+}
namespace extensions {
-namespace mojom {
-class ExtensionUnpacker;
-}
-
-// ZipFileInstaller unzips an extension in a utility process.
+// ZipFileInstaller unzips an extension safely using the Unzipper and
+// SafeJSONParser services.
// This class is not thread-safe: it is bound to the sequence it is created on.
class ZipFileInstaller : public base::RefCountedThreadSafe<ZipFileInstaller> {
public:
@@ -34,18 +38,38 @@
const std::string& error)>;
// Creates a ZipFileInstaller that invokes |done_callback| when done.
- static scoped_refptr<ZipFileInstaller> Create(DoneCallback done_callback);
+ static scoped_refptr<ZipFileInstaller> Create(
+ service_manager::Connector* connector,
+ DoneCallback done_callback);
+ // Creates a temporary directory and unzips the extension in it.
void LoadFromZipFile(const base::FilePath& zip_file);
+ // Unzips the extension in |unzip_dir|.
+ void LoadFromZipFileInDir(const base::FilePath& zip_file,
+ const base::FilePath& unzip_dir);
+
private:
friend class base::RefCountedThreadSafe<ZipFileInstaller>;
+ FRIEND_TEST_ALL_PREFIXES(ZipFileInstallerTest, NonTheme_FileExtractionFilter);
+ FRIEND_TEST_ALL_PREFIXES(ZipFileInstallerTest, Theme_FileExtractionFilter);
+ FRIEND_TEST_ALL_PREFIXES(ZipFileInstallerTest, ManifestExtractionFilter);
- explicit ZipFileInstaller(DoneCallback done_callback);
+ ZipFileInstaller(service_manager::Connector* connector,
+ DoneCallback done_callback);
~ZipFileInstaller();
+ void LoadFromZipFileImpl(const base::FilePath& zip_file,
+ const base::FilePath& unzip_dir);
+
// Unzip an extension into |unzip_dir| and load it with an UnpackedInstaller.
void Unzip(base::Optional<base::FilePath> unzip_dir);
+ void ManifestUnzipped(const base::FilePath& unzip_dir, bool success);
+ void ManifestRead(const base::FilePath& unzip_dir,
+ base::Optional<std::string> manifest_content);
+ void ManifestParsingFailed(const std::string& error);
+ void ManifestParsed(const base::FilePath& unzip_dir,
+ std::unique_ptr<base::Value> manifest);
void UnzipDone(const base::FilePath& unzip_dir, bool success);
// On failure, report the |error| reason.
@@ -54,12 +78,18 @@
// Callback invoked when unzipping has finished.
DoneCallback done_callback_;
+ // Whether a file should be extracted as part of installing an
+ // extension/theme. Protects against unused or potentially hamrful files.
+ static bool ShouldExtractFile(bool is_theme, const base::FilePath& file_path);
+
+ // Returns true if |file_path| points to an extension manifest.
+ static bool IsManifestFile(const base::FilePath& file_path);
+
// File containing the extension to unzip.
base::FilePath zip_file_;
- // Utility process used to perform the unzip.
- std::unique_ptr<content::UtilityProcessMojoClient<mojom::ExtensionUnpacker>>
- utility_process_mojo_client_;
+ // Connector to the ServiceManager. Bound to the UI thread.
+ service_manager::Connector* connector_;
SEQUENCE_CHECKER(sequence_checker_);
diff --git a/extensions/common/BUILD.gn b/extensions/common/BUILD.gn
index 0450e04..eb30402e 100644
--- a/extensions/common/BUILD.gn
+++ b/extensions/common/BUILD.gn
@@ -23,7 +23,6 @@
if (enable_extensions) {
mojom("mojo") {
sources = [
- "extension_unpacker.mojom",
"mojo/app_window.mojom",
"mojo/guest_view.mojom",
"mojo/keep_alive.mojom",
@@ -35,7 +34,6 @@
public_deps = [
"//content/public/common:interfaces",
- "//mojo/common:common_custom_types",
"//ui/gfx/geometry/mojo",
"//url/mojom:url_mojom_gurl",
]
diff --git a/extensions/common/extension_unpacker.mojom b/extensions/common/extension_unpacker.mojom
deleted file mode 100644
index f29ce63..0000000
--- a/extensions/common/extension_unpacker.mojom
+++ /dev/null
@@ -1,16 +0,0 @@
-// Copyright 2017 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.
-
-// Safe chrome extension unpacker service provided by the utility process
-// and exposed by mojo policy to the chrome browser process.
-
-module extensions.mojom;
-
-import "mojo/common/file_path.mojom";
-
-interface ExtensionUnpacker {
- // Unzip |file| into the directory |path|.
- Unzip(mojo.common.mojom.FilePath file,
- mojo.common.mojom.FilePath path) => (bool success);
-};
diff --git a/extensions/shell/BUILD.gn b/extensions/shell/BUILD.gn
index c02e77fb..5b493a18 100644
--- a/extensions/shell/BUILD.gn
+++ b/extensions/shell/BUILD.gn
@@ -65,7 +65,6 @@
"//extensions/shell/common/api",
"//extensions/shell/common/api:api_registration",
"//extensions/shell/common/api:extensions_features",
- "//extensions/utility",
"//skia",
"//third_party/WebKit/public:blink",
"//third_party/cld_3/src/src:cld_3",
@@ -183,8 +182,6 @@
"renderer/shell_content_renderer_client.h",
"renderer/shell_extensions_renderer_client.cc",
"renderer/shell_extensions_renderer_client.h",
- "utility/shell_content_utility_client.cc",
- "utility/shell_content_utility_client.h",
]
if (use_aura) {
diff --git a/extensions/shell/app/DEPS b/extensions/shell/app/DEPS
index 5b7cd97..a92aad2 100644
--- a/extensions/shell/app/DEPS
+++ b/extensions/shell/app/DEPS
@@ -4,5 +4,6 @@
"+components/nacl",
"+content/public/app",
"+content/public/browser",
+ "+content/public/utility",
"+sandbox",
]
diff --git a/extensions/shell/app/shell_main_delegate.cc b/extensions/shell/app/shell_main_delegate.cc
index 823dbfc5..42c0f22 100644
--- a/extensions/shell/app/shell_main_delegate.cc
+++ b/extensions/shell/app/shell_main_delegate.cc
@@ -12,13 +12,13 @@
#include "components/nacl/common/buildflags.h"
#include "content/public/browser/browser_main_runner.h"
#include "content/public/common/content_switches.h"
+#include "content/public/utility/content_utility_client.h"
#include "content/shell/common/shell_switches.h"
#include "extensions/common/extension_paths.h"
#include "extensions/shell/browser/default_shell_browser_main_delegate.h"
#include "extensions/shell/browser/shell_content_browser_client.h"
#include "extensions/shell/common/shell_content_client.h"
#include "extensions/shell/renderer/shell_content_renderer_client.h"
-#include "extensions/shell/utility/shell_content_utility_client.h"
#include "ui/base/resource/resource_bundle.h"
#if defined(OS_CHROMEOS)
@@ -168,11 +168,6 @@
return renderer_client_.get();
}
-content::ContentUtilityClient* ShellMainDelegate::CreateContentUtilityClient() {
- utility_client_.reset(CreateShellContentUtilityClient());
- return utility_client_.get();
-}
-
void ShellMainDelegate::ProcessExiting(const std::string& process_type) {
logging::CloseLogFile();
}
@@ -202,7 +197,7 @@
content::ContentUtilityClient*
ShellMainDelegate::CreateShellContentUtilityClient() {
- return new ShellContentUtilityClient();
+ return new content::ContentUtilityClient();
}
void ShellMainDelegate::InitializeResourceBundle() {
diff --git a/extensions/shell/app/shell_main_delegate.h b/extensions/shell/app/shell_main_delegate.h
index 1f005c6..c43f620 100644
--- a/extensions/shell/app/shell_main_delegate.h
+++ b/extensions/shell/app/shell_main_delegate.h
@@ -31,7 +31,6 @@
void PreSandboxStartup() override;
content::ContentBrowserClient* CreateContentBrowserClient() override;
content::ContentRendererClient* CreateContentRendererClient() override;
- content::ContentUtilityClient* CreateContentUtilityClient() override;
void ProcessExiting(const std::string& process_type) override;
#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_ANDROID)
void ZygoteStarting(std::vector<std::unique_ptr<content::ZygoteForkDelegate>>*
diff --git a/extensions/shell/utility/DEPS b/extensions/shell/utility/DEPS
deleted file mode 100644
index 8ad521e..0000000
--- a/extensions/shell/utility/DEPS
+++ /dev/null
@@ -1,3 +0,0 @@
-include_rules = [
- "+content/public/utility",
-]
diff --git a/extensions/shell/utility/shell_content_utility_client.cc b/extensions/shell/utility/shell_content_utility_client.cc
deleted file mode 100644
index 75d72622..0000000
--- a/extensions/shell/utility/shell_content_utility_client.cc
+++ /dev/null
@@ -1,17 +0,0 @@
-// Copyright 2014 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 "extensions/shell/utility/shell_content_utility_client.h"
-
-namespace extensions {
-
-ShellContentUtilityClient::ShellContentUtilityClient() = default;
-
-ShellContentUtilityClient::~ShellContentUtilityClient() = default;
-
-void ShellContentUtilityClient::UtilityThreadStarted() {
- utility_handler::UtilityThreadStarted();
-}
-
-} // namespace extensions
diff --git a/extensions/shell/utility/shell_content_utility_client.h b/extensions/shell/utility/shell_content_utility_client.h
deleted file mode 100644
index 4baaf17..0000000
--- a/extensions/shell/utility/shell_content_utility_client.h
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright 2014 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.
-
-#ifndef EXTENSIONS_SHELL_UTILITY_SHELL_CONTENT_UTILITY_CLIENT_H_
-#define EXTENSIONS_SHELL_UTILITY_SHELL_CONTENT_UTILITY_CLIENT_H_
-
-#include "content/public/utility/content_utility_client.h"
-#include "extensions/utility/utility_handler.h"
-
-namespace extensions {
-
-class ShellContentUtilityClient : public content::ContentUtilityClient {
- public:
- ShellContentUtilityClient();
- ~ShellContentUtilityClient() override;
-
- // content::ContentUtilityClient:
- void UtilityThreadStarted() override;
-};
-
-} // namespace extensions
-
-#endif // EXTENSIONS_SHELL_UTILITY_SHELL_CONTENT_UTILITY_CLIENT_H_
diff --git a/extensions/strings/extensions_strings.grd b/extensions/strings/extensions_strings.grd
index f3ea305..0fe77a51d 100644
--- a/extensions/strings/extensions_strings.grd
+++ b/extensions/strings/extensions_strings.grd
@@ -385,17 +385,6 @@
Shaped windows are not supported.
</message>
</if>
-
- <!-- Utility process names. Please keep alphabetized. -->
- <message name="IDS_UTILITY_PROCESS_EXTENSION_UNPACKER_NAME" desc="The name of the utility process used for unpacking extensions.">
- Extension Unpacker
- </message>
- <message name="IDS_UTILITY_PROCESS_MANIFEST_PARSER_NAME" desc="The name of the utility process used for parsing extension manifests.">
- Extension Manifest Parser
- </message>
- <message name="IDS_UTILITY_PROCESS_ZIP_FILE_INSTALLER_NAME" desc="The name of the utility process used for unpacking zip files.">
- Zip File Installer
- </message>
</messages>
</release>
</grit>
diff --git a/extensions/test/test_content_utility_client.cc b/extensions/test/test_content_utility_client.cc
index 10c2c16..69895ce 100644
--- a/extensions/test/test_content_utility_client.cc
+++ b/extensions/test/test_content_utility_client.cc
@@ -17,10 +17,7 @@
TestContentUtilityClient::~TestContentUtilityClient() = default;
void TestContentUtilityClient::UtilityThreadStarted() {
- utility_handler::UtilityThreadStarted();
-
auto registry = std::make_unique<service_manager::BinderRegistry>();
- utility_handler::ExposeInterfacesToBrowser(registry.get(), false);
content::ChildThread::Get()
->GetServiceManagerConnection()
->AddConnectionFilter(std::make_unique<content::SimpleConnectionFilter>(
diff --git a/extensions/test/test_content_utility_client.h b/extensions/test/test_content_utility_client.h
index 18531eb..b2edbd6b 100644
--- a/extensions/test/test_content_utility_client.h
+++ b/extensions/test/test_content_utility_client.h
@@ -6,7 +6,6 @@
#define EXTENSIONS_TEST_TEST_CONTENT_UTILITY_CLIENT_H_
#include "content/public/utility/content_utility_client.h"
-#include "extensions/utility/utility_handler.h"
namespace extensions {
diff --git a/extensions/utility/BUILD.gn b/extensions/utility/BUILD.gn
deleted file mode 100644
index de71ac1..0000000
--- a/extensions/utility/BUILD.gn
+++ /dev/null
@@ -1,42 +0,0 @@
-# Copyright 2014 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.
-
-import("//build/config/features.gni")
-import("//extensions/buildflags/buildflags.gni")
-
-assert(enable_extensions)
-
-source_set("utility") {
- sources = [
- "utility_handler.cc",
- "utility_handler.h",
- ]
-
- # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
- configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
-
- deps = [
- "//content/public/common",
- "//content/public/utility",
- "//extensions/common",
- "//skia",
- ]
-}
-
-source_set("unit_tests") {
- testonly = true
- sources = [
- "utility_handler_unittest.cc",
- ]
- deps = [
- ":utility",
- "//base",
- "//extensions:test_support",
- "//extensions/common",
- "//extensions/strings",
- "//testing/gtest",
- "//third_party/zlib/google:zip",
- "//ui/base",
- ]
-}
diff --git a/extensions/utility/DEPS b/extensions/utility/DEPS
deleted file mode 100644
index 48d2f8d..0000000
--- a/extensions/utility/DEPS
+++ /dev/null
@@ -1,6 +0,0 @@
-include_rules = [
- "+content/public/utility",
- "+content/public/child",
- "+net",
- "+third_party/zlib/google",
-]
diff --git a/extensions/utility/utility_handler.cc b/extensions/utility/utility_handler.cc
deleted file mode 100644
index 40d7116e..0000000
--- a/extensions/utility/utility_handler.cc
+++ /dev/null
@@ -1,182 +0,0 @@
-// Copyright 2014 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 "extensions/utility/utility_handler.h"
-
-#include <memory>
-#include <string>
-#include <utility>
-
-#include "base/command_line.h"
-#include "base/files/file_path.h"
-#include "base/files/file_util.h"
-#include "base/json/json_file_value_serializer.h"
-#include "base/location.h"
-#include "base/task_scheduler/post_task.h"
-#include "content/public/utility/utility_thread.h"
-#include "extensions/common/constants.h"
-#include "extensions/common/extension_l10n_util.h"
-#include "extensions/common/extension_unpacker.mojom.h"
-#include "extensions/common/extensions_client.h"
-#include "extensions/common/features/feature_channel.h"
-#include "extensions/common/features/feature_session_type.h"
-#include "extensions/common/manifest.h"
-#include "extensions/common/manifest_constants.h"
-#include "extensions/strings/grit/extensions_strings.h"
-#include "mojo/public/cpp/bindings/strong_binding.h"
-#include "third_party/zlib/google/zip.h"
-#include "ui/base/l10n/l10n_util.h"
-#include "ui/base/ui_base_switches.h"
-
-namespace extensions {
-
-namespace {
-
-constexpr const base::FilePath::CharType* kAllowedThemeFiletypes[] = {
- FILE_PATH_LITERAL(".bmp"), FILE_PATH_LITERAL(".gif"),
- FILE_PATH_LITERAL(".jpeg"), FILE_PATH_LITERAL(".jpg"),
- FILE_PATH_LITERAL(".json"), FILE_PATH_LITERAL(".png"),
- FILE_PATH_LITERAL(".webp")};
-
-std::unique_ptr<base::DictionaryValue> ReadManifest(
- const base::FilePath& extension_dir,
- std::string* error) {
- DCHECK(error);
- base::FilePath manifest_path = extension_dir.Append(kManifestFilename);
- if (!base::PathExists(manifest_path)) {
- *error = manifest_errors::kInvalidManifest;
- return nullptr;
- }
-
- JSONFileValueDeserializer deserializer(manifest_path);
- std::unique_ptr<base::Value> root = deserializer.Deserialize(NULL, error);
- if (!root) {
- return nullptr;
- }
-
- if (!root->is_dict()) {
- *error = manifest_errors::kInvalidManifest;
- return nullptr;
- }
-
- return base::DictionaryValue::From(std::move(root));
-}
-
-class ExtensionUnpackerImpl : public extensions::mojom::ExtensionUnpacker {
- public:
- ExtensionUnpackerImpl() = default;
- ~ExtensionUnpackerImpl() override = default;
-
- static void Create(extensions::mojom::ExtensionUnpackerRequest request) {
- mojo::MakeStrongBinding(std::make_unique<ExtensionUnpackerImpl>(),
- std::move(request));
- }
-
- private:
- // extensions::mojom::ExtensionUnpacker:
- void Unzip(const base::FilePath& file,
- const base::FilePath& path,
- UnzipCallback callback) override {
- // Move unzip operation to background thread to avoid blocking the main
- // utility thread for extended amont of time. For example, this prevents
- // extension unzipping block receipt of the connection complete
- // notification for the utility process channel to the browser process,
- // which could cause the utility process to terminate itself due to browser
- // process being considered unreachable.
- base::PostTaskWithTraitsAndReplyWithResult(
- FROM_HERE,
- {base::TaskPriority::USER_BLOCKING, base::MayBlock(),
- base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
- base::BindOnce(&ExtensionUnpackerImpl::UnzipOnBackgroundTaskRunner,
- file, path),
- std::move(callback));
- }
-
- static bool UnzipFileManifestIntoPath(
- const base::FilePath& file,
- const base::FilePath& path,
- std::unique_ptr<base::DictionaryValue>* manifest) {
- if (zip::UnzipWithFilterCallback(
- file, path, base::BindRepeating(&utility_handler::IsManifestFile),
- false)) {
- std::string error;
- *manifest = ReadManifest(path, &error);
- return error.empty() && manifest->get();
- }
-
- return false;
- }
-
- static bool UnzipFileIntoPath(
- const base::FilePath& file,
- const base::FilePath& path,
- std::unique_ptr<base::DictionaryValue> manifest) {
- Manifest internal(Manifest::INTERNAL, std::move(manifest));
- // TODO(crbug.com/645263): This silently ignores blocked file types.
- // Add install warnings.
- return zip::UnzipWithFilterCallback(
- file, path,
- base::BindRepeating(&utility_handler::ShouldExtractFile,
- internal.is_theme()),
- true /* log_skipped_files */);
- }
-
- // Unzips the extension from |file| to |path|.
- // Returns whether the unzip operation succeeded.
- static bool UnzipOnBackgroundTaskRunner(const base::FilePath& file,
- const base::FilePath& path) {
- std::unique_ptr<base::DictionaryValue> manifest;
- if (!UnzipFileManifestIntoPath(file, path, &manifest))
- return false;
-
- return UnzipFileIntoPath(file, path, std::move(manifest));
- }
-
- DISALLOW_COPY_AND_ASSIGN(ExtensionUnpackerImpl);
-};
-
-} // namespace
-
-namespace utility_handler {
-
-void UtilityThreadStarted() {
- base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
- std::string lang = command_line->GetSwitchValueASCII(switches::kLang);
- if (!lang.empty())
- extension_l10n_util::SetProcessLocale(lang);
-}
-
-void ExposeInterfacesToBrowser(service_manager::BinderRegistry* registry,
- bool running_elevated) {
- // If our process runs with elevated privileges, only add elevated Mojo
- // interfaces to the interface registry.
- if (running_elevated)
- return;
-
- registry->AddInterface(base::Bind(&ExtensionUnpackerImpl::Create),
- base::ThreadTaskRunnerHandle::Get());
-}
-
-bool ShouldExtractFile(bool is_theme, const base::FilePath& file_path) {
- if (is_theme) {
- const base::FilePath::StringType extension =
- base::ToLowerASCII(file_path.FinalExtension());
- // Allow filenames with no extension.
- if (extension.empty())
- return true;
- return base::ContainsValue(kAllowedThemeFiletypes, extension);
- }
- return !base::FilePath::CompareEqualIgnoreCase(file_path.FinalExtension(),
- FILE_PATH_LITERAL(".exe"));
-}
-
-bool IsManifestFile(const base::FilePath& file_path) {
- CHECK(!file_path.IsAbsolute());
- return base::FilePath::CompareEqualIgnoreCase(file_path.value(),
- kManifestFilename);
-}
-
-} // namespace utility_handler
-
-} // namespace extensions
diff --git a/extensions/utility/utility_handler.h b/extensions/utility/utility_handler.h
deleted file mode 100644
index 7a361d9..0000000
--- a/extensions/utility/utility_handler.h
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright 2014 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.
-
-#ifndef EXTENSIONS_UTILITY_UTILITY_HANDLER_H_
-#define EXTENSIONS_UTILITY_UTILITY_HANDLER_H_
-
-#include "services/service_manager/public/cpp/binder_registry.h"
-
-namespace base {
-class FilePath;
-}
-
-namespace extensions {
-
-namespace utility_handler {
-
-void UtilityThreadStarted();
-
-void ExposeInterfacesToBrowser(service_manager::BinderRegistry* registry,
- bool running_elevated);
-
-bool ShouldExtractFile(bool is_theme, const base::FilePath& file_path);
-
-bool IsManifestFile(const base::FilePath& file_path);
-
-} // namespace utility_handler
-
-} // namespace extensions
-
-#endif // EXTENSIONS_UTILITY_UTILITY_HANDLER_H_
diff --git a/extensions/utility/utility_handler_unittest.cc b/extensions/utility/utility_handler_unittest.cc
deleted file mode 100644
index db2c0293..0000000
--- a/extensions/utility/utility_handler_unittest.cc
+++ /dev/null
@@ -1,77 +0,0 @@
-// Copyright 2018 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 <vector>
-
-#include "base/bind.h"
-#include "base/files/file_path.h"
-#include "extensions/utility/utility_handler.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace extensions {
-
-using UtilityHandlerTest = testing::Test;
-
-struct UnzipFileFilterTestCase {
- const base::FilePath::CharType* input;
- const bool should_unzip;
-};
-
-void RunZipFileFilterTest(
- const std::vector<UnzipFileFilterTestCase>& cases,
- base::RepeatingCallback<bool(const base::FilePath&)>& filter) {
- for (size_t i = 0; i < cases.size(); ++i) {
- base::FilePath input(cases[i].input);
- bool observed = filter.Run(input);
- EXPECT_EQ(cases[i].should_unzip, observed)
- << "i: " << i << ", input: " << input.value();
- }
-}
-
-TEST_F(UtilityHandlerTest, NonTheme_FileExtractionFilter) {
- const std::vector<UnzipFileFilterTestCase> cases = {
- {FILE_PATH_LITERAL("foo"), true},
- {FILE_PATH_LITERAL("foo.nexe"), true},
- {FILE_PATH_LITERAL("foo.dll"), true},
- {FILE_PATH_LITERAL("foo.jpg.exe"), false},
- {FILE_PATH_LITERAL("foo.exe"), false},
- {FILE_PATH_LITERAL("foo.EXE"), false},
- {FILE_PATH_LITERAL("file_without_extension"), true},
- };
- base::RepeatingCallback<bool(const base::FilePath&)> filter =
- base::BindRepeating(&utility_handler::ShouldExtractFile, false);
- RunZipFileFilterTest(cases, filter);
-}
-
-TEST_F(UtilityHandlerTest, Theme_FileExtractionFilter) {
- const std::vector<UnzipFileFilterTestCase> cases = {
- {FILE_PATH_LITERAL("image.jpg"), true},
- {FILE_PATH_LITERAL("IMAGE.JPEG"), true},
- {FILE_PATH_LITERAL("test/image.bmp"), true},
- {FILE_PATH_LITERAL("test/IMAGE.gif"), true},
- {FILE_PATH_LITERAL("test/image.WEBP"), true},
- {FILE_PATH_LITERAL("test/dir/file.image.png"), true},
- {FILE_PATH_LITERAL("manifest.json"), true},
- {FILE_PATH_LITERAL("other.html"), false},
- {FILE_PATH_LITERAL("file_without_extension"), true},
- };
- base::RepeatingCallback<bool(const base::FilePath&)> filter =
- base::BindRepeating(&utility_handler::ShouldExtractFile, true);
- RunZipFileFilterTest(cases, filter);
-}
-
-TEST_F(UtilityHandlerTest, ManifestExtractionFilter) {
- const std::vector<UnzipFileFilterTestCase> cases = {
- {FILE_PATH_LITERAL("manifest.json"), true},
- {FILE_PATH_LITERAL("MANIFEST.JSON"), true},
- {FILE_PATH_LITERAL("test/manifest.json"), false},
- {FILE_PATH_LITERAL("manifest.json/test"), false},
- {FILE_PATH_LITERAL("other.file"), false},
- };
- base::RepeatingCallback<bool(const base::FilePath&)> filter =
- base::BindRepeating(&utility_handler::IsManifestFile);
- RunZipFileFilterTest(cases, filter);
-}
-
-} // namespace extensions
