summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorgregoryd@google.com <gregoryd@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-17 02:06:33 +0000
committergregoryd@google.com <gregoryd@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-17 02:06:33 +0000
commit2a4d754e56ca9ed588201257b1e521d96b6aaa17 (patch)
tree656d7ba03658f2e13464047a6159deabaa5d2ac5
parentcc9858559216b315ef95415396f612109c6ef5f3 (diff)
downloadchromium_src-2a4d754e56ca9ed588201257b1e521d96b6aaa17.zip
chromium_src-2a4d754e56ca9ed588201257b1e521d96b6aaa17.tar.gz
chromium_src-2a4d754e56ca9ed588201257b1e521d96b6aaa17.tar.bz2
Don't keep a pointer to NaClBrokerHost in NaClBrokerService - use ChildProcessHost::Iterator instead. This change fixes the crash that occured if the browser was closed while the NaCl module was still running (found by nacl_ui_tests).
Also make nacl_ui_tests depend on nacl win64 binary (required to run the tests on Win64) BUG=none TEST=nacl_ui_tests Review URL: http://codereview.chromium.org/875005 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@41804 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/nacl_host/nacl_broker_host.cc15
-rw-r--r--chrome/browser/nacl_host/nacl_broker_host.h5
-rw-r--r--chrome/browser/nacl_host/nacl_broker_service.cc58
-rw-r--r--chrome/browser/nacl_host/nacl_broker_service.h11
-rw-r--r--chrome/browser/nacl_host/nacl_process_host.cc4
-rw-r--r--chrome/chrome_tests.gypi4
-rw-r--r--chrome/common/nacl_messages_internal.h3
-rw-r--r--chrome/nacl/broker_thread.cc1
8 files changed, 30 insertions, 71 deletions
diff --git a/chrome/browser/nacl_host/nacl_broker_host.cc b/chrome/browser/nacl_host/nacl_broker_host.cc
index 9c33430..1ff5461 100644
--- a/chrome/browser/nacl_host/nacl_broker_host.cc
+++ b/chrome/browser/nacl_host/nacl_broker_host.cc
@@ -56,15 +56,10 @@ bool NaClBrokerHost::Init() {
void NaClBrokerHost::OnMessageReceived(const IPC::Message& msg) {
IPC_BEGIN_MESSAGE_MAP(NaClBrokerHost, msg)
- IPC_MESSAGE_HANDLER(NaClProcessMsg_BrokerReady, OnBrokerReady)
IPC_MESSAGE_HANDLER(NaClProcessMsg_LoaderLaunched, OnLoaderLaunched)
IPC_END_MESSAGE_MAP()
}
-void NaClBrokerHost::OnBrokerReady() {
- NaClBrokerService::GetInstance()->OnBrokerStarted();
-}
-
bool NaClBrokerHost::LaunchLoader(
const std::wstring& loader_channel_id) {
return Send(new NaClProcessMsg_LaunchLoaderThroughBroker(loader_channel_id));
@@ -79,13 +74,3 @@ void NaClBrokerHost::StopBroker() {
stopping_ = true;
Send(new NaClProcessMsg_StopBroker());
}
-
-void NaClBrokerHost::OnChildDied() {
- if (!stopping_) {
- // If the broker stops unexpectedly (and not when asked by the broker
- // service), we need to notify the broker service. In any other case
- // the broker service may have a new broker host pointer by this time.
- NaClBrokerService::GetInstance()->OnBrokerDied();
- }
- ChildProcessHost::OnChildDied();
-}
diff --git a/chrome/browser/nacl_host/nacl_broker_host.h b/chrome/browser/nacl_host/nacl_broker_host.h
index 47c3e95..ced46e9 100644
--- a/chrome/browser/nacl_host/nacl_broker_host.h
+++ b/chrome/browser/nacl_host/nacl_broker_host.h
@@ -26,9 +26,6 @@ class NaClBrokerHost : public ChildProcessHost {
// Stop the broker process.
void StopBroker();
- protected:
- virtual void OnChildDied();
-
private:
// ResourceDispatcherHost::Receiver implementation:
virtual URLRequestContext* GetRequestContext(
@@ -37,8 +34,6 @@ class NaClBrokerHost : public ChildProcessHost {
virtual bool CanShutdown() { return true; }
- // Handler for NaClProcessMsg_BrokerReady message (sent by the broker process)
- void OnBrokerReady();
// Handler for NaClProcessMsg_LoaderLaunched message
void OnLoaderLaunched(const std::wstring& loader_channel_id,
base::ProcessHandle handle);
diff --git a/chrome/browser/nacl_host/nacl_broker_service.cc b/chrome/browser/nacl_host/nacl_broker_service.cc
index 69c919b..b4c54d7 100644
--- a/chrome/browser/nacl_host/nacl_broker_service.cc
+++ b/chrome/browser/nacl_host/nacl_broker_service.cc
@@ -13,9 +13,7 @@ NaClBrokerService* NaClBrokerService::GetInstance() {
}
NaClBrokerService::NaClBrokerService()
- : broker_started_(false),
- broker_host_(NULL),
- loaders_running_(0),
+ : loaders_running_(0),
resource_dispatcher_host_(NULL),
initialized_(false) {
}
@@ -23,40 +21,32 @@ NaClBrokerService::NaClBrokerService()
void NaClBrokerService::Init(ResourceDispatcherHost* resource_dispatcher_host) {
if (!initialized_)
resource_dispatcher_host_ = resource_dispatcher_host;
-
- if (broker_host_ == NULL)
- StartBroker();
-
initialized_ = true;
}
bool NaClBrokerService::StartBroker() {
- broker_host_.reset(new NaClBrokerHost(resource_dispatcher_host_));
- if (!broker_host_->Init()) {
- // Initialization failed, we will not retry in the future
- broker_host_.reset(NULL);
+ NaClBrokerHost* broker_host = new NaClBrokerHost(resource_dispatcher_host_);
+ if (!broker_host->Init()) {
+ delete broker_host;
+ return false;
}
- return (broker_host_ != NULL);
+ return true;
}
bool NaClBrokerService::LaunchLoader(NaClProcessHost* nacl_process_host,
const std::wstring& loader_channel_id) {
// Add task to the list
pending_launches_[loader_channel_id] = nacl_process_host;
- if (broker_started_) {
- // If the broker is not ready yet
- // we will call LaunchLoader in OnBrokerStarted
- broker_host_->LaunchLoader(loader_channel_id);
- }
- return true;
-}
+ NaClBrokerHost* broker_host = GetBrokerHost();
-void NaClBrokerService::OnBrokerStarted() {
- PendingLaunchesMap::iterator it;
- for (it = pending_launches_.begin(); it != pending_launches_.end(); it++)
- broker_host_->LaunchLoader(it->first);
+ if (!broker_host) {
+ if (!StartBroker())
+ return false;
+ broker_host = GetBrokerHost();
+ }
+ broker_host->LaunchLoader(loader_channel_id);
- broker_started_ = true;
+ return true;
}
void NaClBrokerService::OnLoaderLaunched(const std::wstring& channel_id,
@@ -75,16 +65,18 @@ void NaClBrokerService::OnLoaderLaunched(const std::wstring& channel_id,
void NaClBrokerService::OnLoaderDied() {
--loaders_running_;
// Stop the broker only if there are no loaders running or being launched.
- if (loaders_running_ + pending_launches_.size() == 0 &&
- broker_host_ != NULL) {
- broker_host_->StopBroker();
- // Reset the pointer to the broker host.
- OnBrokerDied();
+ NaClBrokerHost* broker_host = GetBrokerHost();
+ if (loaders_running_ + pending_launches_.size() == 0 && broker_host != NULL) {
+ broker_host->StopBroker();
}
}
-void NaClBrokerService::OnBrokerDied() {
- // NaClBrokerHost object will be destructed by ChildProcessHost
- broker_started_ = false;
- broker_host_.release();
+NaClBrokerHost* NaClBrokerService::GetBrokerHost() {
+ for (ChildProcessHost::Iterator iter(ChildProcessInfo::NACL_BROKER_PROCESS);
+ !iter.Done();
+ ++iter) {
+ NaClBrokerHost* broker_host = static_cast<NaClBrokerHost*>(*iter);
+ return broker_host;
+ }
+ return NULL;
}
diff --git a/chrome/browser/nacl_host/nacl_broker_service.h b/chrome/browser/nacl_host/nacl_broker_service.h
index 8518cd4..3eb40f6 100644
--- a/chrome/browser/nacl_host/nacl_broker_service.h
+++ b/chrome/browser/nacl_host/nacl_broker_service.h
@@ -28,10 +28,6 @@ class NaClBrokerService {
bool LaunchLoader(NaClProcessHost* client,
const std::wstring& loader_channel_id);
- // Called by NaClBrokerHost to notify the service
- // that the broker was launched.
- void OnBrokerStarted();
-
// Called by NaClBrokerHost to notify the service that a loader was launched.
void OnLoaderLaunched(const std::wstring& channel_id,
base::ProcessHandle handle);
@@ -39,9 +35,6 @@ class NaClBrokerService {
// Called by NaClProcessHost when a loader process is terminated
void OnLoaderDied();
- // Called by NaClBrokerHost when the broker process is terminated.
- void OnBrokerDied();
-
private:
typedef std::map<std::wstring, NaClProcessHost*>
PendingLaunchesMap;
@@ -51,8 +44,8 @@ class NaClBrokerService {
NaClBrokerService();
~NaClBrokerService() {}
- bool broker_started_;
- scoped_ptr<NaClBrokerHost> broker_host_;
+ NaClBrokerHost* GetBrokerHost();
+
int loaders_running_;
bool initialized_;
ResourceDispatcherHost* resource_dispatcher_host_;
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index 58b689f..1e3bcff 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -96,8 +96,8 @@ bool NaClProcessHost::LaunchSelLdr() {
#if defined(OS_WIN)
if (running_on_wow64_) {
NaClBrokerService::GetInstance()->Init(resource_dispatcher_host_);
- NaClBrokerService::GetInstance()->LaunchLoader(this,
- ASCIIToWide(channel_id()));
+ return NaClBrokerService::GetInstance()->LaunchLoader(this,
+ ASCIIToWide(channel_id()));
} else // NO_LINT
#endif
ChildProcessHost::Launch(
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index c809459..df5793d 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -452,11 +452,9 @@
'test/nacl/nacl_test.cc',
],
'conditions': [
- # TODO(gregoryd): This test will run on Windows only at first.
- # Refer to ui_tests target above if any problems appear when trying
- # to enable it on other platforms.
['OS=="win"', {
'dependencies': [
+ 'chrome_nacl_win64',
'crash_service', # run time dependency
'security_tests', # run time dependency
'test_support_common',
diff --git a/chrome/common/nacl_messages_internal.h b/chrome/common/nacl_messages_internal.h
index 9eec408..abde832 100644
--- a/chrome/common/nacl_messages_internal.h
+++ b/chrome/common/nacl_messages_internal.h
@@ -23,9 +23,6 @@ IPC_BEGIN_MESSAGES(NaClProcess)
std::wstring, /* channel ID for the loader */
base::ProcessHandle /* loader process handle */)
- // Notify the browser process that the broker is ready (sent by the broker)
- IPC_MESSAGE_CONTROL0(NaClProcessMsg_BrokerReady)
-
// Notify the broker that all loader processes have been terminated and it
// should shutdown.
IPC_MESSAGE_CONTROL0(NaClProcessMsg_StopBroker)
diff --git a/chrome/nacl/broker_thread.cc b/chrome/nacl/broker_thread.cc
index f94d49d..a199c08 100644
--- a/chrome/nacl/broker_thread.cc
+++ b/chrome/nacl/broker_thread.cc
@@ -74,6 +74,5 @@ void NaClBrokerThread::OnChannelConnected(int32 peer_pid) {
bool res = base::OpenProcessHandle(peer_pid, &browser_handle_);
DCHECK(res);
ChildProcess::current()->AddRefProcess();
- Send(new NaClProcessMsg_BrokerReady());
}