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

Unified Diff: extensions/browser/sandboxed_unpacker.cc

Issue 2697463002: Convert utility process extension Unpacker IPC to mojo (Closed)
Patch Set: Take #4, declare the IPC enum traits in the message file, try build fix. Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: extensions/browser/sandboxed_unpacker.cc
diff --git a/extensions/browser/sandboxed_unpacker.cc b/extensions/browser/sandboxed_unpacker.cc
index 043dcd629576231049b486cd8baf1ccf59e29a88..d813fe8da779135a0f0269079a03ef0f2876316c 100644
--- a/extensions/browser/sandboxed_unpacker.cc
+++ b/extensions/browser/sandboxed_unpacker.cc
@@ -14,23 +14,19 @@
#include "base/command_line.h"
#include "base/files/file_util.h"
#include "base/json/json_string_value_serializer.h"
-#include "base/message_loop/message_loop.h"
#include "base/metrics/histogram_macros.h"
-#include "base/numerics/safe_conversions.h"
#include "base/path_service.h"
#include "base/sequenced_task_runner.h"
-#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/sequenced_worker_pool.h"
#include "build/build_config.h"
#include "components/crx_file/crx_file.h"
#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/utility_process_host.h"
-#include "content/public/common/common_param_traits.h"
#include "extensions/common/constants.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_l10n_util.h"
-#include "extensions/common/extension_utility_messages.h"
+#include "extensions/common/extension_unpacker.mojom.h"
+#include "extensions/common/extension_utility_types.h"
#include "extensions/common/extensions_client.h"
#include "extensions/common/file_util.h"
#include "extensions/common/manifest_constants.h"
@@ -43,7 +39,6 @@
using base::ASCIIToUTF16;
using content::BrowserThread;
-using content::UtilityProcessHost;
using crx_file::CrxFile;
// The following macro makes histograms that record the length of paths
@@ -224,11 +219,9 @@ SandboxedUnpacker::SandboxedUnpacker(
SandboxedUnpackerClient* client)
: client_(client),
extensions_dir_(extensions_dir),
- got_response_(false),
location_(location),
creation_flags_(creation_flags),
- unpacker_io_task_runner_(unpacker_io_task_runner),
- utility_wrapper_(new UtilityHostWrapper) {
+ unpacker_io_task_runner_(unpacker_io_task_runner) {
// Tracking for crbug.com/692069. The location must be valid. If it's invalid,
// the utility process kills itself for a bad IPC.
CHECK_GT(location, Manifest::INVALID_LOCATION);
@@ -318,8 +311,8 @@ void SandboxedUnpacker::StartWithCrx(const CRXFileInfo& crx_info) {
link_free_crx_path);
BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
- base::Bind(&SandboxedUnpacker::StartUnzipOnIOThread,
- this, link_free_crx_path));
+ base::Bind(&SandboxedUnpacker::UnzipOnIOThread, this,
+ link_free_crx_path));
}
void SandboxedUnpacker::StartWithDirectory(const std::string& extension_id,
@@ -342,9 +335,9 @@ void SandboxedUnpacker::StartWithDirectory(const std::string& extension_id,
return;
}
- BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
- base::Bind(&SandboxedUnpacker::StartUnpackOnIOThread,
- this, extension_root_));
+ BrowserThread::PostTask(
+ BrowserThread::IO, FROM_HERE,
+ base::Bind(&SandboxedUnpacker::UnpackOnIOThread, this, extension_root_));
}
SandboxedUnpacker::~SandboxedUnpacker() {
@@ -353,28 +346,28 @@ SandboxedUnpacker::~SandboxedUnpacker() {
temp_dir_.Take();
}
-bool SandboxedUnpacker::OnMessageReceived(const IPC::Message& message) {
- bool handled = true;
- IPC_BEGIN_MESSAGE_MAP(SandboxedUnpacker, message)
- IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnzipToDir_Succeeded,
- OnUnzipToDirSucceeded)
- IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnzipToDir_Failed,
- OnUnzipToDirFailed)
- IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnpackExtension_Succeeded,
- OnUnpackExtensionSucceeded)
- IPC_MESSAGE_HANDLER(ExtensionUtilityHostMsg_UnpackExtension_Failed,
- OnUnpackExtensionFailed)
- IPC_MESSAGE_UNHANDLED(handled = false)
- IPC_END_MESSAGE_MAP()
- return handled;
-}
+void SandboxedUnpacker::StartUtilityProcessIfNeeded() {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-void SandboxedUnpacker::OnProcessCrashed(int exit_code) {
- // Don't report crashes if they happen after we got a response.
- if (got_response_)
+ if (utility_process_mojo_client_)
return;
- // Utility process crashed while trying to install.
+ utility_process_mojo_client_ = base::MakeUnique<
+ 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();
+
ReportFailure(
UTILITY_PROCESS_CRASHED_WHILE_TRYING_TO_INSTALL,
l10n_util::GetStringFUTF16(
@@ -384,60 +377,73 @@ void SandboxedUnpacker::OnProcessCrashed(int exit_code) {
l10n_util::GetStringUTF16(IDS_EXTENSION_INSTALL_PROCESS_CRASHED));
}
-void SandboxedUnpacker::StartUnzipOnIOThread(const base::FilePath& crx_path) {
- if (!utility_wrapper_->StartIfNeeded(temp_dir_.GetPath(), this,
- unpacker_io_task_runner_)) {
- ReportFailure(
- COULD_NOT_START_UTILITY_PROCESS,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- FailureReasonToString16(COULD_NOT_START_UTILITY_PROCESS)));
- return;
- }
+void SandboxedUnpacker::UnzipOnIOThread(const base::FilePath& crx_path) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+ StartUtilityProcessIfNeeded();
+
DCHECK(crx_path.DirName() == temp_dir_.GetPath());
base::FilePath unzipped_dir =
crx_path.DirName().AppendASCII(kTempExtensionName);
- utility_wrapper_->host()->Send(
- new ExtensionUtilityMsg_UnzipToDir(crx_path, unzipped_dir));
+
+ utility_process_mojo_client_->service()->Unzip(
+ crx_path, unzipped_dir,
+ base::Bind(&SandboxedUnpacker::UnzipDone, this, unzipped_dir));
}
-void SandboxedUnpacker::StartUnpackOnIOThread(
- const base::FilePath& directory_path) {
- if (!utility_wrapper_->StartIfNeeded(temp_dir_.GetPath(), this,
- unpacker_io_task_runner_)) {
- ReportFailure(
- COULD_NOT_START_UTILITY_PROCESS,
- l10n_util::GetStringFUTF16(
- IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
- FailureReasonToString16(COULD_NOT_START_UTILITY_PROCESS)));
+void SandboxedUnpacker::UnzipDone(const base::FilePath& directory,
+ bool success) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+ if (!success) {
+ utility_process_mojo_client_.reset();
+ ReportFailure(UNZIP_FAILED,
+ l10n_util::GetStringUTF16(IDS_EXTENSION_PACKAGE_UNZIP_ERROR));
return;
}
- DCHECK(directory_path.DirName() == temp_dir_.GetPath());
- utility_wrapper_->host()->Send(new ExtensionUtilityMsg_UnpackExtension(
- directory_path, extension_id_, location_, creation_flags_));
-}
-void SandboxedUnpacker::OnUnzipToDirSucceeded(const base::FilePath& directory) {
BrowserThread::PostTask(
BrowserThread::IO, FROM_HERE,
- base::Bind(&SandboxedUnpacker::StartUnpackOnIOThread, this, directory));
+ base::Bind(&SandboxedUnpacker::UnpackOnIOThread, this, directory));
}
-void SandboxedUnpacker::OnUnzipToDirFailed(const std::string& error) {
- got_response_ = true;
- utility_wrapper_ = nullptr;
- ReportFailure(UNZIP_FAILED,
- l10n_util::GetStringUTF16(IDS_EXTENSION_PACKAGE_UNZIP_ERROR));
+void SandboxedUnpacker::UnpackOnIOThread(const base::FilePath& directory) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+ StartUtilityProcessIfNeeded();
+
+ DCHECK(directory.DirName() == temp_dir_.GetPath());
+
+ utility_process_mojo_client_->service()->Unpack(
+ directory, extension_id_, location_, creation_flags_,
+ base::Bind(&SandboxedUnpacker::UnpackDone, this));
}
-void SandboxedUnpacker::OnUnpackExtensionSucceeded(
- const base::DictionaryValue& manifest) {
+void SandboxedUnpacker::UnpackDone(
+ const base::string16& error,
+ std::unique_ptr<base::DictionaryValue> manifest) {
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+ utility_process_mojo_client_.reset();
+
+ if (!error.empty()) {
+ unpacker_io_task_runner_->PostTask(
+ FROM_HERE,
+ base::Bind(&SandboxedUnpacker::UnpackExtensionFailed, this, error));
+ return;
+ }
+
+ unpacker_io_task_runner_->PostTask(
+ FROM_HERE, base::Bind(&SandboxedUnpacker::UnpackExtensionSucceeded, this,
+ base::Passed(&manifest)));
+}
+
+void SandboxedUnpacker::UnpackExtensionSucceeded(
+ std::unique_ptr<base::DictionaryValue> manifest) {
CHECK(unpacker_io_task_runner_->RunsTasksOnCurrentThread());
- got_response_ = true;
- utility_wrapper_ = nullptr;
std::unique_ptr<base::DictionaryValue> final_manifest(
- RewriteManifestFile(manifest));
+ RewriteManifestFile(*manifest));
if (!final_manifest)
return;
@@ -477,13 +483,12 @@ void SandboxedUnpacker::OnUnpackExtensionSucceeded(
if (!RewriteCatalogFiles())
return;
- ReportSuccess(manifest, install_icon);
+ ReportSuccess(std::move(manifest), install_icon);
}
-void SandboxedUnpacker::OnUnpackExtensionFailed(const base::string16& error) {
+void SandboxedUnpacker::UnpackExtensionFailed(const base::string16& error) {
CHECK(unpacker_io_task_runner_->RunsTasksOnCurrentThread());
- got_response_ = true;
- utility_wrapper_ = nullptr;
+
ReportFailure(
UNPACKER_CLIENT_FAILED,
l10n_util::GetStringFUTF16(IDS_EXTENSION_PACKAGE_ERROR_MESSAGE, error));
@@ -572,13 +577,12 @@ base::string16 SandboxedUnpacker::FailureReasonToString16(
return ASCIIToUTF16("UNZIP_FAILED");
case DIRECTORY_MOVE_FAILED:
return ASCIIToUTF16("DIRECTORY_MOVE_FAILED");
- case COULD_NOT_START_UTILITY_PROCESS:
- return ASCIIToUTF16("COULD_NOT_START_UTILITY_PROCESS");
case NUM_FAILURE_REASONS:
NOTREACHED();
return base::string16();
}
+
NOTREACHED();
return base::string16();
}
@@ -648,7 +652,6 @@ bool SandboxedUnpacker::ValidateSignature(const base::FilePath& crx_path,
void SandboxedUnpacker::ReportFailure(FailureReason reason,
const base::string16& error) {
- utility_wrapper_ = nullptr;
UMA_HISTOGRAM_ENUMERATION("Extensions.SandboxUnpackFailureReason", reason,
NUM_FAILURE_REASONS);
if (!crx_unpack_start_time_.is_null())
@@ -665,9 +668,8 @@ void SandboxedUnpacker::ReportFailure(FailureReason reason,
}
void SandboxedUnpacker::ReportSuccess(
- const base::DictionaryValue& original_manifest,
+ const std::unique_ptr<base::DictionaryValue>& original_manifest,
const SkBitmap& install_icon) {
- utility_wrapper_ = nullptr;
UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackSuccess", 1);
if (!crx_unpack_start_time_.is_null())
@@ -677,8 +679,11 @@ void SandboxedUnpacker::ReportSuccess(
DCHECK(!temp_dir_.GetPath().empty());
// Client takes ownership of temporary directory and extension.
+ // TODO(https://crbug.com/699528): we should consider transferring the
+ // ownership of original_manifest to our clients as well.
client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_,
- &original_manifest, extension_.get(), install_icon);
+ original_manifest.get(), extension_.get(),
+ install_icon);
extension_ = NULL;
}
@@ -898,42 +903,4 @@ void SandboxedUnpacker::Cleanup() {
}
}
-SandboxedUnpacker::UtilityHostWrapper::UtilityHostWrapper() {}
-
-bool SandboxedUnpacker::UtilityHostWrapper::StartIfNeeded(
- const base::FilePath& exposed_dir,
- const scoped_refptr<UtilityProcessHostClient>& client,
- const scoped_refptr<base::SequencedTaskRunner>& client_task_runner) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
- if (!utility_host_) {
- utility_host_ =
- UtilityProcessHost::Create(client, client_task_runner)->AsWeakPtr();
- utility_host_->SetName(
- l10n_util::GetStringUTF16(IDS_UTILITY_PROCESS_EXTENSION_UNPACKER_NAME));
-
- // Grant the subprocess access to our temp dir so it can write out files.
- DCHECK(!exposed_dir.empty());
- utility_host_->SetExposedDir(exposed_dir);
- if (!utility_host_->StartBatchMode()) {
- utility_host_.reset();
- return false;
- }
- }
- return true;
-}
-
-content::UtilityProcessHost* SandboxedUnpacker::UtilityHostWrapper::host()
- const {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
- return utility_host_.get();
-}
-
-SandboxedUnpacker::UtilityHostWrapper::~UtilityHostWrapper() {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
- if (utility_host_) {
- utility_host_->EndBatchMode();
- utility_host_.reset();
- }
-}
-
} // namespace extensions

Powered by Google App Engine
This is Rietveld 408576698