linux: call g_thread_init() at relevant startup points

According to glib docs, we need to do this if it's at all possible
for us to hit glib on multiple threads.  This may be happening
when we grab plugin metadata from the file thread.

Rather than explicitly depending on gthread all over the place,
just put it in with the GTK dep (since anywhere we're using GTK
we ought to init gthread).

(Note that this is *not* initializing the GDK locking system.)

BUG=18957

Review URL: http://codereview.chromium.org/174264

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24207 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 35 insertions, 50 deletions

diff --git a/base/process_util_unittest.cc b/base/process_util_unittest.cc
index 7b30be6..b75bdb8 100644
--- a/base/process_util_unittest.cc
+++ b/base/process_util_unittest.cc
@@ -27,6 +27,11 @@
 namespace base {
 
 class ProcessUtilTest : public MultiProcessTest {
+#if defined(OS_POSIX)
+ public:
+  // Spawn a child process that counts how many file descriptors are open.
+  int CountOpenFDsInChild();
+#endif
 };
 
 MULTIPROCESS_TEST_MAIN(SimpleChildProcess) {
@@ -194,20 +199,11 @@ MULTIPROCESS_TEST_MAIN(ProcessUtilsLeakFDChildProcess) {
   for (int i = STDERR_FILENO + 1; i < max_files; i++) {
     if (i != kChildPipe) {
       if (HANDLE_EINTR(close(i)) != -1) {
-        LOG(WARNING) << "Leaked FD " << i;
         num_open_files += 1;
       }
     }
   }
 
-  // InitLogging always opens a file at startup.
-  int expected_num_open_fds = 1;
-#if defined(OS_LINUX)
-  // On Linux, '/etc/localtime' is opened before the test's main() enters.
-  expected_num_open_fds += 1;
-#endif  // defined(OS_LINUX)
-  num_open_files -= expected_num_open_fds;
-
   int written = HANDLE_EINTR(write(write_pipe, &num_open_files,
                                    sizeof(num_open_files)));
   DCHECK_EQ(static_cast<size_t>(written), sizeof(num_open_files));
@@ -216,40 +212,45 @@ MULTIPROCESS_TEST_MAIN(ProcessUtilsLeakFDChildProcess) {
   return 0;
 }
 
-TEST_F(ProcessUtilTest, FDRemapping) {
-  // Open some files to check they don't get leaked to the child process.
+int ProcessUtilTest::CountOpenFDsInChild() {
   int fds[2];
   if (pipe(fds) < 0)
     NOTREACHED();
-  int pipe_read_fd = fds[0];
-  int pipe_write_fd = fds[1];
-
-  // open some dummy fds to make sure they don't propogate over to the
-  // child process.
-  int dev_null = open("/dev/null", O_RDONLY);
-  int sockets[2];
-  socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);
 
   file_handle_mapping_vector fd_mapping_vec;
-  fd_mapping_vec.push_back(std::pair<int,int>(pipe_write_fd, kChildPipe));
+  fd_mapping_vec.push_back(std::pair<int,int>(fds[1], kChildPipe));
   ProcessHandle handle = this->SpawnChild(L"ProcessUtilsLeakFDChildProcess",
                                           fd_mapping_vec,
                                           false);
-  ASSERT_NE(static_cast<ProcessHandle>(NULL), handle);
-  HANDLE_EINTR(close(pipe_write_fd));
+  CHECK(handle);
+  HANDLE_EINTR(close(fds[1]));
 
   // Read number of open files in client process from pipe;
   int num_open_files = -1;
   ssize_t bytes_read =
-      HANDLE_EINTR(read(pipe_read_fd, &num_open_files, sizeof(num_open_files)));
-  ASSERT_EQ(bytes_read, static_cast<ssize_t>(sizeof(num_open_files)));
-
-  // Make sure 0 fds are leaked to the client.
-  ASSERT_EQ(0, num_open_files);
+      HANDLE_EINTR(read(fds[0], &num_open_files, sizeof(num_open_files)));
+  CHECK(bytes_read == static_cast<ssize_t>(sizeof(num_open_files)));
 
-  EXPECT_TRUE(WaitForSingleProcess(handle, 1000));
+  CHECK(WaitForSingleProcess(handle, 1000));
   base::CloseProcessHandle(handle);
   HANDLE_EINTR(close(fds[0]));
+
+  return num_open_files;
+}
+
+TEST_F(ProcessUtilTest, FDRemapping) {
+  int fds_before = CountOpenFDsInChild();
+
+  // open some dummy fds to make sure they don't propogate over to the
+  // child process.
+  int dev_null = open("/dev/null", O_RDONLY);
+  int sockets[2];
+  socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);
+
+  int fds_after = CountOpenFDsInChild();
+
+  ASSERT_EQ(fds_after, fds_before);
+
   HANDLE_EINTR(close(sockets[0]));
   HANDLE_EINTR(close(sockets[1]));
   HANDLE_EINTR(close(dev_null));
diff --git a/base/test_suite.h b/base/test_suite.h
index 352c0b8..236c3ed 100644
--- a/base/test_suite.h
+++ b/base/test_suite.h
@@ -41,6 +41,7 @@ class TestSuite {
     CommandLine::Init(argc, argv);
     testing::InitGoogleTest(&argc, argv);
 #if defined(OS_LINUX)
+    g_thread_init(NULL);
     gtk_init_check(&argc, &argv);
 #endif
     // Don't add additional code to this constructor.  Instead add it to
diff --git a/build/linux/system.gyp b/build/linux/system.gyp
index 68b52be..e10c905 100644
--- a/build/linux/system.gyp
+++ b/build/linux/system.gyp
@@ -9,15 +9,15 @@
       'type': 'settings',
       'direct_dependent_settings': {
         'cflags': [
-          '<!@(pkg-config --cflags gtk+-2.0)',
+          '<!@(pkg-config --cflags gtk+-2.0 gthread-2.0)',
         ],
       },
       'link_settings': {
         'ldflags': [
-          '<!@(pkg-config --libs-only-L --libs-only-other gtk+-2.0)',
+          '<!@(pkg-config --libs-only-L --libs-only-other gtk+-2.0 gthread-2.0)',
         ],
         'libraries': [
-          '<!@(pkg-config --libs-only-l gtk+-2.0)',
+          '<!@(pkg-config --libs-only-l gtk+-2.0 gthread-2.0)',
         ],
       },
     },
@@ -107,23 +107,6 @@
       },
     },
     {
-      'target_name': 'gthread',
-      'type': 'settings',
-      'direct_dependent_settings': {
-        'cflags': [
-          '<!@(pkg-config --cflags gthread-2.0)',
-        ],
-      },
-      'link_settings': {
-        'ldflags': [
-          '<!@(pkg-config --libs-only-L --libs-only-other gthread-2.0)',
-        ],
-        'libraries': [
-          '<!@(pkg-config --libs-only-l gthread-2.0)',
-        ],
-      },
-    },
-    {
       'target_name': 'x11',
       'type': 'settings',
       'direct_dependent_settings': {
diff --git a/chrome/app/chrome_dll_main.cc b/chrome/app/chrome_dll_main.cc
index 1783117..792d0ad 100644
--- a/chrome/app/chrome_dll_main.cc
+++ b/chrome/app/chrome_dll_main.cc
@@ -554,6 +554,7 @@ int ChromeMain(int argc, const char** argv) {
 #endif
   } else if (process_type.empty()) {
 #if defined(OS_LINUX)
+    g_thread_init(NULL);
     // Glib type system initialization. Needed at least for gconf,
     // used in net/proxy/proxy_config_service_linux.cc. Most likely
     // this is superfluous as gtk_init() ought to do this. It's
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 2ed58c8..c368821 100644
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -3046,8 +3046,6 @@
             '<@(chromium_dependencies)',
             # Needed for chrome_dll_main.cc #include of gtk/gtk.h
             '../build/linux/system.gyp:gtk',
-            # Needed for chrome_dll_main.cc use of g_thread_init
-            '../build/linux/system.gyp:gthread',
           ],
           'sources': [
             'app/chrome_dll_main.cc',
diff --git a/chrome/plugin/plugin_thread.cc b/chrome/plugin/plugin_thread.cc
index ae90e1a..307cbaa 100644
--- a/chrome/plugin/plugin_thread.cc
+++ b/chrome/plugin/plugin_thread.cc
@@ -35,6 +35,7 @@ PluginThread::PluginThread()
   {
     // XEmbed plugins assume they are hosted in a Gtk application, so we need
     // to initialize Gtk in the plugin process.
+    g_thread_init(NULL);
     const std::vector<std::string>& args =
         CommandLine::ForCurrentProcess()->argv();
     int argc = args.size();