diff options
author | thestig@chromium.org <thestig@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-06 04:41:54 +0000 |
---|---|---|
committer | thestig@chromium.org <thestig@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-06 04:41:54 +0000 |
commit | 158cdc931a3537cb881000829c8bff37b6574c51 (patch) | |
tree | 9f24483841f3c1c5b5e12579a8045c19cbe92902 | |
parent | be28cdf164a12c5afc401c59cd52be51d085e76a (diff) | |
download | chromium_src-158cdc931a3537cb881000829c8bff37b6574c51.zip chromium_src-158cdc931a3537cb881000829c8bff37b6574c51.tar.gz chromium_src-158cdc931a3537cb881000829c8bff37b6574c51.tar.bz2 |
Revert 204389 "Set up NaClChromeMainArgs number_of_cores member ..."
> Set up NaClChromeMainArgs number_of_cores member so apps can size threadpools
> appropriately
>
> The outer sandbox on Linux and OSX was preventing
> sysconf(_SC_NPROCESSORS_ONLN) from succeeding, so that NaCl applications that
> need the number of processors/cores to, e.g., figure out the appropriate size
> for threadpools were getting a bogus value (-1 when using newlib and 1 when
> using glibc).
>
> This is a reland of r195198 (https://chromiumcodereview.appspot.com/14238013)
> which was reverted because of a test failure on an XP bot. Having built (on
> Win7) and moved the filesystem to an XP laptop to verify that no test failure
> occurs, and bradn having helped to verify on a bot (VM configured similarly to
> chrome's build waterfall bots), we suspect that the original test failure was
> a bot issue (test timed out!).
>
> TEST= browser_tests --gtest_filter=*ysconf*
> BUG= 176522
>
> Review URL: https://chromiumcodereview.appspot.com/16047003
TBR=bsy@google.com
Review URL: https://codereview.chromium.org/15813021
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204416 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | chrome/nacl/nacl_helper_linux.cc | 12 | ||||
-rw-r--r-- | chrome/nacl/nacl_listener.cc | 8 | ||||
-rw-r--r-- | chrome/nacl/nacl_listener.h | 13 | ||||
-rw-r--r-- | chrome/nacl/nacl_main.cc | 9 | ||||
-rw-r--r-- | chrome/test/data/nacl/nacl_test_data.gyp | 17 | ||||
-rw-r--r-- | chrome/test/data/nacl/sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.cc | 106 | ||||
-rw-r--r-- | chrome/test/data/nacl/sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.html | 59 | ||||
-rw-r--r-- | chrome/test/nacl/nacl_browsertest.cc | 38 |
8 files changed, 4 insertions, 258 deletions
diff --git a/chrome/nacl/nacl_helper_linux.cc b/chrome/nacl/nacl_helper_linux.cc index 5c56285..f8f32c4 100644 --- a/chrome/nacl/nacl_helper_linux.cc +++ b/chrome/nacl/nacl_helper_linux.cc @@ -40,8 +40,7 @@ namespace { // if (!child) { // Note: this code doesn't attempt to support the SECCOMP sandbox. void BecomeNaClLoader(const std::vector<int>& child_fds, - size_t prereserved_sandbox_size, - int number_of_cores) { + size_t prereserved_sandbox_size) { VLOG(1) << "NaCl loader: setting up IPC descriptor"; // don't need zygote FD any more if (HANDLE_EINTR(close(kNaClZygoteDescriptor)) != 0) @@ -52,7 +51,6 @@ void BecomeNaClLoader(const std::vector<int>& child_fds, base::MessageLoopForIO main_message_loop; NaClListener listener; listener.set_prereserved_sandbox_size(prereserved_sandbox_size); - listener.set_number_of_cores(number_of_cores); listener.Listen(); _exit(0); } @@ -60,8 +58,7 @@ void BecomeNaClLoader(const std::vector<int>& child_fds, // Some of this code was lifted from // content/browser/zygote_main_linux.cc:ForkWithRealPid() void HandleForkRequest(const std::vector<int>& child_fds, - size_t prereserved_sandbox_size, - int number_of_cores) { + size_t prereserved_sandbox_size) { VLOG(1) << "nacl_helper: forking"; pid_t childpid = fork(); if (childpid < 0) { @@ -99,7 +96,7 @@ void HandleForkRequest(const std::vector<int>& child_fds, if (HANDLE_EINTR(close(child_fds[kNaClParentFDIndex])) != 0) LOG(ERROR) << "close(child_fds[kNaClParentFDIndex]) failed"; if (validack) { - BecomeNaClLoader(child_fds, prereserved_sandbox_size, number_of_cores); + BecomeNaClLoader(child_fds, prereserved_sandbox_size); } else { LOG(ERROR) << "Failed to synch with zygote"; } @@ -237,7 +234,6 @@ int main(int argc, char* argv[]) { #endif std::vector<int> empty; // for SendMsg() calls size_t prereserved_sandbox_size = CheckReservedAtZero(); - int number_of_cores = sysconf(_SC_NPROCESSORS_ONLN); CheckRDebug(argv[0]); @@ -274,7 +270,7 @@ int main(int argc, char* argv[]) { } else if (msglen == sizeof(kNaClForkRequest) - 1 && memcmp(buf, kNaClForkRequest, msglen) == 0) { if (kNaClParentFDIndex + 1 == fds.size()) { - HandleForkRequest(fds, prereserved_sandbox_size, number_of_cores); + HandleForkRequest(fds, prereserved_sandbox_size); continue; // fork succeeded. Note: child does not return } else { LOG(ERROR) << "nacl_helper: unexpected number of fds, got " diff --git a/chrome/nacl/nacl_listener.cc b/chrome/nacl/nacl_listener.cc index ac03a31..e771496 100644 --- a/chrome/nacl/nacl_listener.cc +++ b/chrome/nacl/nacl_listener.cc @@ -7,10 +7,6 @@ #include <errno.h> #include <stdlib.h> -#if defined(OS_POSIX) -#include <unistd.h> -#endif - #include "base/command_line.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" @@ -178,9 +174,6 @@ NaClListener::NaClListener() : shutdown_event_(true, false), #if defined(OS_LINUX) prereserved_sandbox_size_(0), #endif -#if defined(OS_POSIX) - number_of_cores_(-1), // unknown/error -#endif main_loop_(NULL) { io_thread_.StartWithOptions( base::Thread::Options(base::MessageLoop::TYPE_IO, 0)); @@ -267,7 +260,6 @@ void NaClListener::OnStart(const nacl::NaClStartParams& params) { LOG(ERROR) << "Failed to dup() the urandom FD"; return; } - args->number_of_cores = number_of_cores_; args->create_memory_object_func = CreateMemoryObject; # if defined(OS_MACOSX) CHECK(handles.size() >= 1); diff --git a/chrome/nacl/nacl_listener.h b/chrome/nacl/nacl_listener.h index a6cec25..2936eee 100644 --- a/chrome/nacl/nacl_listener.h +++ b/chrome/nacl/nacl_listener.h @@ -34,11 +34,6 @@ class NaClListener : public IPC::Listener { prereserved_sandbox_size_ = prereserved_sandbox_size; } #endif -#if defined(OS_POSIX) - void set_number_of_cores(int number_of_cores) { - number_of_cores_ = number_of_cores; - } -#endif private: void OnStart(const nacl::NaClStartParams& params); @@ -56,14 +51,6 @@ class NaClListener : public IPC::Listener { #if defined(OS_LINUX) size_t prereserved_sandbox_size_; #endif -#if defined(OS_POSIX) - // The outer sandbox on Linux and OSX prevents - // sysconf(_SC_NPROCESSORS) from working; in Windows, there are no - // problems with invoking GetSystemInfo. Therefore, only in - // OS_POSIX do we need to supply the number of cores into the - // NaClChromeMainArgs object. - int number_of_cores_; -#endif // Used to identify what thread we're on. base::MessageLoop* main_loop_; diff --git a/chrome/nacl/nacl_main.cc b/chrome/nacl/nacl_main.cc index 112a9c4..fa4835e 100644 --- a/chrome/nacl/nacl_main.cc +++ b/chrome/nacl/nacl_main.cc @@ -33,12 +33,6 @@ int NaClMain(const content::MainFunctionParams& parameters) { bool no_sandbox = parsed_command_line.HasSwitch(switches::kNoSandbox); platform.InitSandboxTests(no_sandbox); -#if defined(OS_POSIX) - // The number of cores must be obtained before the invocation of - // platform.EnableSandbox(), so cannot simply be inlined below. - int number_of_cores = sysconf(_SC_NPROCESSORS_ONLN); -#endif - if (!no_sandbox) { platform.EnableSandbox(); } @@ -46,9 +40,6 @@ int NaClMain(const content::MainFunctionParams& parameters) { if (sandbox_test_result) { NaClListener listener; -#if defined(OS_POSIX) - listener.set_number_of_cores(number_of_cores); -#endif listener.Listen(); } else { // This indirectly prevents the test-harness-success-cookie from being set, diff --git a/chrome/test/data/nacl/nacl_test_data.gyp b/chrome/test/data/nacl/nacl_test_data.gyp index 93d70fd..c9512e3 100644 --- a/chrome/test/data/nacl/nacl_test_data.gyp +++ b/chrome/test/data/nacl/nacl_test_data.gyp @@ -59,23 +59,6 @@ }, }, { - 'target_name': 'sysconf_nprocessors_onln_test', - 'type': 'none', - 'variables': { - 'nexe_target': 'sysconf_nprocessors_onln_test', - 'build_newlib': 1, - 'build_glibc': 1, - 'build_pnacl_newlib': 1, - 'nexe_destination_dir': 'nacl_test_data', - 'sources': [ - 'sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.cc', - ], - 'test_files': [ - 'sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.html', - ], - }, - }, - { 'target_name': 'ppapi_test_lib', 'type': 'none', 'variables': { diff --git a/chrome/test/data/nacl/sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.cc b/chrome/test/data/nacl/sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.cc deleted file mode 100644 index 6df8959..0000000 --- a/chrome/test/data/nacl/sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.cc +++ /dev/null @@ -1,106 +0,0 @@ -/* - * Copyright 2013 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. - */ - -/* - * Post-message based test for simple rpc based access to sysconf result. - */ - -#include <assert.h> -#include <inttypes.h> -#include <stdio.h> -#include <stdlib.h> -#include <string.h> -#include <sys/fcntl.h> -#include <unistd.h> - -#include <sys/nacl_syscalls.h> - -#include "ppapi/cpp/instance.h" -#include "ppapi/cpp/module.h" -#include "ppapi/cpp/var.h" - -void NumProcessors(const pp::Var& message_data, std::string* out) { - int num_cores; - char string_rep[16]; - - num_cores = sysconf(_SC_NPROCESSORS_ONLN); - snprintf(string_rep, sizeof string_rep, "%d", num_cores); - *out = string_rep; -} - -struct PostMessageHandlerDesc { - char const *request; - void (*handler)(const pp::Var& message_data, std::string* out); -}; - -// This object represents one time the page says <embed>. -class MyInstance : public pp::Instance { - public: - explicit MyInstance(PP_Instance instance) : pp::Instance(instance) {} - virtual ~MyInstance() {} - virtual void HandleMessage(const pp::Var& message_data); -}; - -// HandleMessage gets invoked when postMessage is called on the DOM -// element associated with this plugin instance. In this case, if we -// are given a string, we'll post a message back to JavaScript with a -// reply string -- essentially treating this as a string-based RPC. -void MyInstance::HandleMessage(const pp::Var& message_data) { - static struct PostMessageHandlerDesc kMsgHandlers[] = { - { "nprocessors", NumProcessors }, - { reinterpret_cast<char const *>(NULL), - reinterpret_cast<void (*)(const pp::Var&, std::string*)>(NULL) } - }; - - if (message_data.is_string()) { - std::string op_name(message_data.AsString()); - size_t len; - - fprintf(stderr, "Searching for handler for request \"%s\".\n", - op_name.c_str()); - - std::string sb; - - for (size_t ix = 0; kMsgHandlers[ix].request != NULL; ++ix) { - if (op_name == kMsgHandlers[ix].request) { - fprintf(stderr, "found at index %u\n", ix); - kMsgHandlers[ix].handler(message_data, &sb); - break; - } - } - - len = strlen(sb.c_str()); - fprintf(stderr, "posting reply len %d\n", len); - fprintf(stderr, "posting reply \"%s\".\n", sb.c_str()); - fflush(stderr); - - PostMessage(pp::Var(sb)); - fprintf(stderr, "returning\n"); - fflush(stderr); - } -} - -// This object is the global object representing this plugin library as long -// as it is loaded. -class MyModule : public pp::Module { - public: - MyModule() : pp::Module() {} - virtual ~MyModule() {} - - // Override CreateInstance to create your customized Instance object. - virtual pp::Instance* CreateInstance(PP_Instance instance) { - return new MyInstance(instance); - } -}; - -namespace pp { - -// Factory function for your specialization of the Module object. -Module* CreateModule() { - return new MyModule(); -} - -} // namespace pp diff --git a/chrome/test/data/nacl/sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.html b/chrome/test/data/nacl/sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.html deleted file mode 100644 index 0e58289..0000000 --- a/chrome/test/data/nacl/sysconf_nprocessors_onln/sysconf_nprocessors_onln_test.html +++ /dev/null @@ -1,59 +0,0 @@ -<!-- - Copyright 2013 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. ---> -<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" - "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> -<html> - <head> - <meta http-equiv="Pragma" content="no-cache" /> - <meta http-equiv="Expires" content="-1" /> - <script type="text/javascript" src="nacltest.js"></script> - <title>Native Client _SC_NPROCESSORS_ONLN Test</title> - </head> - - <body id="body"> - <h1>Native Client _SC_NPROCESSORS_ONLN Test</h1> - <script type="text/javascript"> - //<![CDATA[ -var tester = new Tester($('body')); -var gotExpected = false; -var args = getTestArguments({'cpu_count' : - 'THIS TEST CANNOT RUN STANDALONE -- run using browser_test instead'}); - -function setupTests(tester, plugin) { - tester.addAsyncTest('TestSysconfNprocessors', function(status) { - plugin.addEventListener('message', function handler(message_event) { - this.removeEventListener('message', handler, false); - status.assertEqual(message_event.data, args.cpu_count); - status.pass(); - }, false); - plugin.postMessage('nprocessors'); - }); -} - -function runTests() { - setupTests(tester, $('naclModule')); - tester.waitFor($('naclModule')); - tester.run(); -} - //]]> - </script> - <div> - <embed id="naclModule" - name="naclModule" - width=400 height=400 - src="sysconf_nprocessors_onln_test.nmf" - basic_tests="2" - stress_tests="0" - style="background-color:gray" - type="application/x-nacl" /> - </div> - <script type="text/javascript"> - //<![CDATA[ -runTests(); - //]]> - </script> - </body> -</html> diff --git a/chrome/test/nacl/nacl_browsertest.cc b/chrome/test/nacl/nacl_browsertest.cc index 95bc213..8f6e066 100644 --- a/chrome/test/nacl/nacl_browsertest.cc +++ b/chrome/test/nacl/nacl_browsertest.cc @@ -2,13 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <stdio.h> -#if defined(OS_POSIX) -#include <unistd.h> -#elif defined(OS_WIN) -#include <windows.h> -#endif - #include "chrome/test/nacl/nacl_browsertest_util.h" namespace { @@ -25,7 +18,6 @@ namespace { #define MAYBE_CrossOriginFail DISABLED_CrossOriginFail #define MAYBE_SameOriginCookie DISABLED_SameOriginCookie #define MAYBE_CORSNoCookie DISABLED_CORSNoCookie -#define MAYBE_SysconfNprocessorsOnln DISABLED_SysconfNprocessorsOnln #else #define MAYBE_SimpleLoad SimpleLoad #define MAYBE_ExitStatus0 ExitStatus0 @@ -37,7 +29,6 @@ namespace { #define MAYBE_CrossOriginFail CrossOriginFail #define MAYBE_SameOriginCookie SameOriginCookie #define MAYBE_CORSNoCookie CORSNoCookie -#define MAYBE_SysconfNprocessorsOnln SysconfNprocessorsOnln #endif NACL_BROWSER_TEST_F(NaClBrowserTest, MAYBE_SimpleLoad, { @@ -67,35 +58,6 @@ NACL_BROWSER_TEST_F(NaClBrowserTest, MAYBE_ProgressEvents, { RunNaClIntegrationTest(FILE_PATH_LITERAL("ppapi_progress_events.html")); }) -// Some versions of Visual Studio does not like preprocessor -// conditionals inside the argument of a macro, so we put the -// conditionals on a helper function. We are already in an anonymous -// namespace, so the name of the helper is not visible in external -// scope. -#if defined(OS_POSIX) -base::FilePath::StringType NumberOfCoresAsFilePathString() { - char string_rep[23]; - snprintf(string_rep, sizeof string_rep, "%ld", sysconf(_SC_NPROCESSORS_ONLN)); - return string_rep; -} -#elif defined(OS_WIN) -base::FilePath::StringType NumberOfCoresAsFilePathString() { - wchar_t string_rep[23]; - SYSTEM_INFO system_info; - GetSystemInfo(&system_info); - _snwprintf_s(string_rep, sizeof string_rep, _TRUNCATE, L"%u", - system_info.dwNumberOfProcessors); - return string_rep; -} -#endif - -NACL_BROWSER_TEST_F(NaClBrowserTest, MAYBE_SysconfNprocessorsOnln, { - base::FilePath::StringType path = - FILE_PATH_LITERAL("sysconf_nprocessors_onln_test.html?cpu_count="); - path = path + NumberOfCoresAsFilePathString(); - RunNaClIntegrationTest(path); -}) - IN_PROC_BROWSER_TEST_F(NaClBrowserTestStatic, MAYBE_CrossOriginCORS) { RunLoadTest(FILE_PATH_LITERAL("cross_origin/cors.html")); } |