Utility process: initialize GTK when the sandbox is disabled.

content/utility/utility_main.cc

Index: content/utility/utility_main.cc
diff --git a/content/utility/utility_main.cc b/content/utility/utility_main.cc
index c93c1eca5366ae0c3c53f75af3de4a88593262fc..0f64c866892f2675992aa819462a0f03558954d5 100644
--- a/content/utility/utility_main.cc
+++ b/content/utility/utility_main.cc
@@ -17,6 +17,12 @@
 #include "sandbox/win/src/sandbox.h"
 #endif
 
+#if defined(TOOLKIT_GTK)
+#include <gtk/gtk.h>
+
+#include "ui/gfx/gtk_util.h"
+#endif
+
 namespace content {
 
 // Mainline routine for running as the utility process.
@@ -25,9 +31,41 @@ int UtilityMain(const MainFunctionParams& parameters) {
   base::MessageLoop main_message_loop;
   base::PlatformThread::SetName("CrUtilityMain");
 
+#if defined(OS_POSIX)
+  // The utility process is used to load plugins (see OnLoadPlugins() in
+  // utility_thread_impl.cc). Some plugins expect the browser to have loaded
+  // GLib/GTK.
+  // Due to bugs in GLib we need to initialize GLib/GTK before we start threads,
+  // see crbug.com/309093.
+
+  bool do_init_gtk = false;
+
+#if defined(OS_LINUX)
+  // On Linux, we only initialize GLib/GTK if we're not sandboxed.

[jln (very slow on Chromium), 2013/10/22 01:23:55]

This is super weird-looking. The utility process really should know whether it's
started from the Zygote or not.

Could you add this information to |parameters| instead?

[Jorge Lucangeli Obes, 2013/10/22 16:14:54]

Well, we do know our cmdline flags, and we use --no-sandbox to sidestep the
Zygote.

I feel more comfortable looking at the actual state of the process rather than
whatever we got passed in |parameters|.

[jln (very slow on Chromium), 2013/10/22 16:42:40]

It's error prone though. You could have a DCHECK that checks that the states
matches what it should be, don't you think it's cleaner?

[Jorge Lucangeli Obes, 2013/10/22 17:56:14]

Done.

+  LinuxSandbox::GetInstance()->PreinitializeSandbox();
+  const int sandbox_status = LinuxSandbox::GetInstance()->GetStatus();
+  do_init_gtk = !(sandbox_status & kSandboxLinuxSUID);
+#endif
+
+#if defined(TOOLKIT_GTK)
+  if (do_init_gtk) {
+    // g_thread_init() is deprecated since glib 2.31.0, please see release note:
+    // http://mail.gnome.org/archives/gnome-announce-list/2011-October/msg00041.html
+#if !(GLIB_CHECK_VERSION(2, 31, 0))
+    if (!g_thread_get_initialized()) {
+      g_thread_init(NULL);
+    }
+#endif
+    gfx::GtkInitFromCommandLine(*CommandLine::ForCurrentProcess());
+  }
+#endif
+#endif
+
 #if defined(OS_LINUX)
-  // Initialize the sandbox before any thread is created.
-  LinuxSandbox::InitializeSandbox();
+  // If we didn't initialize GLib/GTK, initialize the sandbox.
+  if (!do_init_gtk) {
+    LinuxSandbox::InitializeSandbox();

[jln (very slow on Chromium), 2013/10/22 01:23:55]

This is ugly. You should create a "const bool is_sandboxing_desired" and use
that.

Hopefully it can just come from |parameters| as suggested above. If not you
could have one localized hack above to set that bool, which you would then use
twice.

However, with the current "fake" BPF policy, there is no point in not
initializing the sandbox earlier, but I agree it looks weird. While you're at
it, add a comment that the BPF sandbox is a TODO().

[Jorge Lucangeli Obes, 2013/10/22 16:14:54]

GTK starts threads, so we actually do need to know whether GTK was initialized.
We can deduce it from whether we're sandboxed, but we need to take it into
account.

[jln (very slow on Chromium), 2013/10/22 16:42:40]

I realize, but again, there should be some "logic" that decides what we're
doing. If we start the (empty) sandbox because gtk was not started, we don't
really know that we're starting the sandbox when it matters.

In any case, for now, until a real BPF policy exists, you can
InitializeSandbox() before GTK.

[Jorge Lucangeli Obes, 2013/10/22 16:58:43]

I'm worried that we'll change the BPF policy and things are gonna break, but
I'll put a big fat TODO there and in sandbox_seccomp_bpf_linux.cc.

+  }
 #endif
 
   ChildProcess utility_process;