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

Unified Diff: chrome/browser/nacl_host/nacl_process_host.cc

Issue 8397001: Open NaCl IRT file only once at startup (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 9 years, 2 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: chrome/browser/nacl_host/nacl_process_host.cc
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index ebf3bbfadd4b433a08ae1189c1eba12612b34f57..0ac0872173197cfd253cfe973215dd8173f3c73b 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -52,17 +52,21 @@ struct NaClProcessHost::NaClInternal {
std::vector<nacl::Handle> sockets_for_sel_ldr;
};
+bool NaClProcessHost::RunningOnWOW64() {
+#if defined(OS_WIN)
+ return (base::win::OSInfo::GetInstance()->wow64_status() ==
+ base::win::OSInfo::WOW64_ENABLED);
+#else
+ return false;
+#endif
+}
+
NaClProcessHost::NaClProcessHost(const std::wstring& url)
: BrowserChildProcessHost(NACL_LOADER_PROCESS),
reply_msg_(NULL),
internal_(new NaClInternal()),
- running_on_wow64_(false),
ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
set_name(WideToUTF16Hack(url));
-#if defined(OS_WIN)
- running_on_wow64_ = (base::win::OSInfo::GetInstance()->wow64_status() ==
- base::win::OSInfo::WOW64_ENABLED);
-#endif
}
NaClProcessHost::~NaClProcessHost() {
@@ -105,6 +109,11 @@ bool NaClProcessHost::Launch(
return false;
}
+ if (irt_platform_file_ == base::kInvalidPlatformFileValue) {
+ LOG(ERROR) << "Cannot launch NaCl process after IRT file open failed";
+ return false;
+ }
+
// Rather than creating a socket pair in the renderer, and passing
// one side through the browser to sel_ldr, socket pairs are created
// in the browser and then passed to the renderer and sel_ldr.
@@ -180,7 +189,7 @@ bool NaClProcessHost::LaunchSelLdr() {
// On Windows we might need to start the broker process to launch a new loader
#if defined(OS_WIN)
- if (running_on_wow64_) {
+ if (RunningOnWOW64()) {
return NaClBrokerService::GetInstance()->LaunchLoader(
this, ASCIIToWide(channel_id()));
} else {
@@ -202,7 +211,7 @@ void NaClProcessHost::OnProcessLaunchedByBroker(base::ProcessHandle handle) {
base::TerminationStatus NaClProcessHost::GetChildTerminationStatus(
int* exit_code) {
- if (running_on_wow64_)
+ if (RunningOnWOW64())
return base::GetTerminationStatus(handle(), exit_code);
return BrowserChildProcessHost::GetChildTerminationStatus(exit_code);
}
@@ -225,25 +234,13 @@ void NaClProcessHost::OnChildDied() {
BrowserChildProcessHost::OnChildDied();
}
-FilePath::StringType NaClProcessHost::GetIrtLibraryFilename() {
- bool on_x86_64 = running_on_wow64_;
-#if defined(__x86_64__)
- on_x86_64 = true;
-#endif
- if (on_x86_64) {
- return FILE_PATH_LITERAL("nacl_irt_x86_64.nexe");
- } else {
- return FILE_PATH_LITERAL("nacl_irt_x86_32.nexe");
- }
-}
+base::PlatformFile NaClProcessHost::irt_platform_file_;
+
+void NaClProcessHost::OpenIrtLibraryFile() {
+ irt_platform_file_ = base::kInvalidPlatformFileValue;
+
+ FilePath irt_filepath;
-void NaClProcessHost::OnProcessLaunched() {
- // TODO(mseaborn): Opening the IRT file every time a NaCl process is
- // launched probably does not work with auto-update on Linux. We
- // might need to open the file on startup. If so, we would need to
- // ensure that NaCl's ELF loader does not use lseek() on the shared
- // IRT file descriptor, otherwise there would be a race condition.
- FilePath irt_path;
// Allow the IRT library to be overridden via an environment
// variable. This allows the NaCl/Chromium integration bot to
// specify a newly-built IRT rather than using a prebuilt one
@@ -251,41 +248,80 @@ void NaClProcessHost::OnProcessLaunched() {
// variable that the standalone NaCl PPAPI plugin accepts.
const char* irt_path_var = getenv("NACL_IRT_LIBRARY");
if (irt_path_var != NULL) {
- FilePath::StringType string(irt_path_var,
- irt_path_var + strlen(irt_path_var));
- irt_path = FilePath(string);
+ FilePath::StringType path_string(
+ irt_path_var, const_cast<const char*>(strchr(irt_path_var, '\0')));
+ irt_filepath = FilePath(path_string);
} else {
FilePath plugin_dir;
if (!PathService::Get(chrome::DIR_INTERNAL_PLUGINS, &plugin_dir)) {
LOG(ERROR) << "Failed to locate the plugins directory";
- delete this;
return;
}
- irt_path = plugin_dir.Append(GetIrtLibraryFilename());
+
+ bool on_x86_64 = RunningOnWOW64();
+#if defined(__x86_64__)
+ on_x86_64 = true;
+#endif
+ FilePath::StringType irt_name;
+ if (on_x86_64) {
+ irt_name = FILE_PATH_LITERAL("nacl_irt_x86_64.nexe");
+ } else {
+ irt_name = FILE_PATH_LITERAL("nacl_irt_x86_32.nexe");
+ }
+
+ irt_filepath = plugin_dir.Append(irt_name);
}
- if (!base::FileUtilProxy::CreateOrOpen(
- BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE),
- irt_path,
- base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ,
- base::Bind(&NaClProcessHost::OpenIrtFileDone,
- weak_factory_.GetWeakPtr()))) {
- delete this;
+ base::PlatformFileError error_code;
+ irt_platform_file_ = base::CreatePlatformFile(irt_filepath,
+ base::PLATFORM_FILE_OPEN |
+ base::PLATFORM_FILE_READ,
+ NULL,
+ &error_code);
+ if (error_code != base::PLATFORM_FILE_OK) {
+ LOG(ERROR) << "Failed to open NaCl IRT file \""
+ << irt_filepath.LossyDisplayName()
+ << "\": " << error_code;
}
}
-void NaClProcessHost::OpenIrtFileDone(base::PlatformFileError error_code,
- base::PassPlatformFile file,
- bool created) {
+void NaClProcessHost::OnProcessLaunched() {
+ SendStart(irt_platform_file_);
+}
+
+static bool SendHandleToSelLdr(
+ base::ProcessHandle processh,
+ nacl::Handle sourceh, bool close_source,
+ std::vector<nacl::FileDescriptor> *handles_for_sel_ldr) {
+#if defined(OS_WIN)
+ HANDLE channel;
+ int flags = DUPLICATE_SAME_ACCESS;
+ if (close_source)
+ flags |= DUPLICATE_CLOSE_SOURCE;
+ if (!DuplicateHandle(GetCurrentProcess(),
+ reinterpret_cast<HANDLE>(sourceh),
+ processh,
+ &channel,
+ 0, // Unused given DUPLICATE_SAME_ACCESS.
+ FALSE,
+ flags)) {
+ LOG(ERROR) << "DuplicateHandle() failed";
+ return false;
+ }
+ handles_for_sel_ldr->push_back(
+ reinterpret_cast<nacl::FileDescriptor>(channel));
+#else
+ nacl::FileDescriptor channel;
+ channel.fd = sourceh;
+ channel.auto_close = close_source;
+ handles_for_sel_ldr->push_back(channel);
+#endif
+ return true;
+}
+
+void NaClProcessHost::SendStart(base::PlatformFile irt_file) {
std::vector<nacl::FileDescriptor> handles_for_renderer;
base::ProcessHandle nacl_process_handle;
- bool have_irt_file = false;
- if (base::PLATFORM_FILE_OK == error_code) {
- internal_->sockets_for_sel_ldr.push_back(file.ReleaseValue());
- have_irt_file = true;
- } else {
- LOG(ERROR) << "Failed to open the NaCl IRT library file";
- }
for (size_t i = 0; i < internal_->sockets_for_renderer.size(); i++) {
#if defined(OS_WIN)
@@ -345,28 +381,18 @@ void NaClProcessHost::OpenIrtFileDone(base::PlatformFileError error_code,
std::vector<nacl::FileDescriptor> handles_for_sel_ldr;
for (size_t i = 0; i < internal_->sockets_for_sel_ldr.size(); i++) {
-#if defined(OS_WIN)
- HANDLE channel;
- if (!DuplicateHandle(GetCurrentProcess(),
- reinterpret_cast<HANDLE>(
- internal_->sockets_for_sel_ldr[i]),
- handle(),
- &channel,
- 0, // Unused given DUPLICATE_SAME_ACCESS.
- FALSE,
- DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
- LOG(ERROR) << "DuplicateHandle() failed";
+ if (!SendHandleToSelLdr(handle(),
+ internal_->sockets_for_sel_ldr[i], true,
+ &handles_for_sel_ldr)) {
delete this;
return;
}
- handles_for_sel_ldr.push_back(
- reinterpret_cast<nacl::FileDescriptor>(channel));
-#else
- nacl::FileDescriptor channel;
- channel.fd = internal_->sockets_for_sel_ldr[i];
- channel.auto_close = true;
- handles_for_sel_ldr.push_back(channel);
-#endif
+ }
+
+ // Send over the IRT file handle. We don't close our own copy!
+ if (!SendHandleToSelLdr(handle(), irt_file, false, &handles_for_sel_ldr)) {
+ delete this;
+ return;
}
#if defined(OS_MACOSX)
@@ -391,7 +417,7 @@ void NaClProcessHost::OpenIrtFileDone(base::PlatformFileError error_code,
handles_for_sel_ldr.push_back(memory_fd);
#endif
- Send(new NaClProcessMsg_Start(handles_for_sel_ldr, have_irt_file));
+ Send(new NaClProcessMsg_Start(handles_for_sel_ldr));
internal_->sockets_for_sel_ldr.clear();
}

Powered by Google App Engine
This is Rietveld 408576698