[Linux] Call g_thread_init() in UtilityThreadImpl::OnLoadPlugins().

[Linux] Call g_thread_init() in UtilityThreadImpl::OnLoadPlugins().

This allows plugins that call into glib for NP_GetMIMEDescription to load
without crashing.

BUG=71661, 105252
TEST=Have libTotem plugins installed, go to about:plugins and they are present.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113021

[piman, 2011-12-02 18:13:39]

http://codereview.chromium.org/8774048/diff/1/content/utility/utility_thread_...
File content/utility/utility_thread_impl.cc (right):

http://codereview.chromium.org/8774048/diff/1/content/utility/utility_thread_...
content/utility/utility_thread_impl.cc:137: g_thread_init(NULL);
It seems that we should do that once and for all in the constructor, rather than
every time we get here.
Also, any reason not to do the full GTK initialization like in the plugin
process?

[Robert Sesek, 2011-12-02 18:37:44]

http://codereview.chromium.org/8774048/diff/1/content/utility/utility_thread_...
File content/utility/utility_thread_impl.cc (right):

http://codereview.chromium.org/8774048/diff/1/content/utility/utility_thread_...
content/utility/utility_thread_impl.cc:137: g_thread_init(NULL);
On 2011/12/02 18:13:39, piman wrote:
> It seems that we should do that once and for all in the constructor, rather
than
> every time we get here.

Why? The only IPC message that requires this is LoadPlugins, and the process is
destroyed at the end of this method.

> Also, any reason not to do the full GTK initialization like in the plugin
> process?

Because nothing is actually being put onscreen at this stage, I don't think GTK
needs to be fully initialized. I can do it if you think it's necessary, though.

[piman, 2011-12-02 18:45:21]

http://codereview.chromium.org/8774048/diff/1/content/utility/utility_thread_...
File content/utility/utility_thread_impl.cc (right):

http://codereview.chromium.org/8774048/diff/1/content/utility/utility_thread_...
content/utility/utility_thread_impl.cc:137: g_thread_init(NULL);
On 2011/12/02 18:37:44, rsesek wrote:
> On 2011/12/02 18:13:39, piman wrote:
> > It seems that we should do that once and for all in the constructor, rather
> than
> > every time we get here.
> 
> Why? The only IPC message that requires this is LoadPlugins, and the process
is
> destroyed at the end of this method.

Today. If you want to leave things here, then please add an assert that it's
only called exactly once.

> 
> > Also, any reason not to do the full GTK initialization like in the plugin
> > process?
> 
> Because nothing is actually being put onscreen at this stage, I don't think
GTK
> needs to be fully initialized. I can do it if you think it's necessary,
though.

I can imagine plugins wanting to query X extensions or whatever, and relying on
gtk to be fully initialized. Plugins are scary, very often broken in subtle
ways. Best is to provide them with a friendly environment.

[Robert Sesek, 2011-12-02 19:19:42]

Thanks for the review.

http://codereview.chromium.org/8774048/diff/1/content/utility/utility_thread_...
File content/utility/utility_thread_impl.cc (right):

http://codereview.chromium.org/8774048/diff/1/content/utility/utility_thread_...
content/utility/utility_thread_impl.cc:137: g_thread_init(NULL);
On 2011/12/02 18:45:21, piman wrote:
> On 2011/12/02 18:37:44, rsesek wrote:
> > On 2011/12/02 18:13:39, piman wrote:
> > > It seems that we should do that once and for all in the constructor,
rather
> > than
> > > every time we get here.
> > 
> > Why? The only IPC message that requires this is LoadPlugins, and the process
> is
> > destroyed at the end of this method.
> 
> Today. If you want to leave things here, then please add an assert that it's
> only called exactly once.

Moving to ctor > adding an assert. Done.

> > > Also, any reason not to do the full GTK initialization like in the plugin
> > > process?
> > 
> > Because nothing is actually being put onscreen at this stage, I don't think
> GTK
> > needs to be fully initialized. I can do it if you think it's necessary,
> though.
> 
> I can imagine plugins wanting to query X extensions or whatever, and relying
on
> gtk to be fully initialized. Plugins are scary, very often broken in subtle
> ways. Best is to provide them with a friendly environment.

OK. Done.

[piman, 2011-12-02 22:10:49]

lgtm

[commit-bot: I haz the power, 2011-12-02 22:13:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsesek@chromium.org/8774048/3002

[commit-bot: I haz the power, 2011-12-02 22:13:39]

Presubmit check for 8774048-3002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for: content/utility/utility_thread_impl.cc

Presubmit checks took 1.1s to calculate.

[Robert Sesek, 2011-12-02 22:16:27]

+jam for OWNERS approval

[jam, 2011-12-04 23:00:14]

lgtm

[commit-bot: I haz the power, 2011-12-05 15:48:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsesek@chromium.org/8774048/3002

[commit-bot: I haz the power, 2011-12-05 21:05:16]

Change committed as 113021