linux: move gtk_init to the right thread

linux: move gtk_init to the right thread

In the plugin process, gtk_init needs to be called from the UI thread (PluginThread) not the main thread (PluginMain) which is an IO thread.

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

[jam, 2009-07-09 23:06:52]

lgtm

http://codereview.chromium.org/149424/diff/1/3
File chrome/plugin/plugin_thread.cc (right):

http://codereview.chromium.org/149424/diff/1/3#newcode61
Line 61: // TODO(evanm): hoist this up nearer to where we have argc/argv.
this TODO can now be removed?

[Evan Martin, 2009-07-09 23:11:29]

LGTM

http://codereview.chromium.org/149424/diff/1/3
File chrome/plugin/plugin_thread.cc (right):

http://codereview.chromium.org/149424/diff/1/3#newcode61
Line 61: // TODO(evanm): hoist this up nearer to where we have argc/argv.
On 2009/07/09 23:06:52, John Abd-El-Malek wrote:
> this TODO can now be removed?

My TODO was thinking we should run this code from main().  It seems weird to
construct this argc specially when we have argc one or two stack frames up...

[jam, 2009-07-09 23:16:47]

http://codereview.chromium.org/149424/diff/1/3
File chrome/plugin/plugin_thread.cc (right):

http://codereview.chromium.org/149424/diff/1/3#newcode61
Line 61: // TODO(evanm): hoist this up nearer to where we have argc/argv.
On 2009/07/09 23:11:29, Evan Martin wrote:
> On 2009/07/09 23:06:52, John Abd-El-Malek wrote:
> > this TODO can now be removed?
> 
> My TODO was thinking we should run this code from main().  It seems weird to
> construct this argc specially when we have argc one or two stack frames up...

Not anymore, this is in a different thread which is why I was saying it doesn't
make sense anymore.

[Antoine Labour, 2009-07-09 23:20:58]

On Thu, Jul 9, 2009 at 4:16 PM, <jam@chromium.org> wrote:

>
> http://codereview.chromium.org/149424/diff/1/3
> File chrome/plugin/plugin_thread.cc (right):
>
> http://codereview.chromium.org/149424/diff/1/3#newcode61
> Line 61: // TODO(evanm): hoist this up nearer to where we have
> argc/argv.
> On 2009/07/09 23:11:29, Evan Martin wrote:
>
>> On 2009/07/09 23:06:52, John Abd-El-Malek wrote:
>> > this TODO can now be removed?
>>
>
>  My TODO was thinking we should run this code from main().  It seems
>>
> weird to
>
>> construct this argc specially when we have argc one or two stack
>>
> frames up...
>
> Not anymore, this is in a different thread which is why I was saying it
> doesn't make sense anymore.


We /could/ add the original argv as a char ** to CommandLine. It's a minor
gain though, if gtk_init may modify the strings, we still need to make
copies (for thread safety).



>
>
> http://codereview.chromium.org/149424
>



-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

[Antoine Labour, 2009-07-10 00:06:55]

On 2009/07/09 23:20:58, Antoine Labour wrote:
> On Thu, Jul 9, 2009 at 4:16 PM, <jam@chromium.org> wrote:
> 
> >
> > http://codereview.chromium.org/149424/diff/1/3
> > File chrome/plugin/plugin_thread.cc (right):
> >
> > http://codereview.chromium.org/149424/diff/1/3#newcode61
> > Line 61: // TODO(evanm): hoist this up nearer to where we have
> > argc/argv.
> > On 2009/07/09 23:11:29, Evan Martin wrote:
> >
> >> On 2009/07/09 23:06:52, John Abd-El-Malek wrote:
> >> > this TODO can now be removed?
> >>
> >
> >  My TODO was thinking we should run this code from main().  It seems
> >>
> > weird to
> >
> >> construct this argc specially when we have argc one or two stack
> >>
> > frames up...
> >
> > Not anymore, this is in a different thread which is why I was saying it
> > doesn't make sense anymore.
> 
> 
> We /could/ add the original argv as a char ** to CommandLine. It's a minor
> gain though, if gtk_init may modify the strings, we still need to make
> copies (for thread safety).
> 
> 
> 
> >
> >
> > http://codereview.chromium.org/149424
> >

I will check in with the TODO removed, let me know if you disagree.

[Evan Martin, 2009-07-10 01:17:16]

On 2009/07/10 00:06:55, Antoine Labour wrote:
> On 2009/07/09 23:20:58, Antoine Labour wrote:
> > On Thu, Jul 9, 2009 at 4:16 PM, <jam@chromium.org> wrote:
> > 
> > >
> > > http://codereview.chromium.org/149424/diff/1/3
> > > File chrome/plugin/plugin_thread.cc (right):
> > >
> > > http://codereview.chromium.org/149424/diff/1/3#newcode61
> > > Line 61: // TODO(evanm): hoist this up nearer to where we have
> > > argc/argv.
> > > On 2009/07/09 23:11:29, Evan Martin wrote:
> > >
> > >> On 2009/07/09 23:06:52, John Abd-El-Malek wrote:
> > >> > this TODO can now be removed?
> > >>
> > >
> > >  My TODO was thinking we should run this code from main().  It seems
> > >>
> > > weird to
> > >
> > >> construct this argc specially when we have argc one or two stack
> > >>
> > > frames up...
> > >
> > > Not anymore, this is in a different thread which is why I was saying it
> > > doesn't make sense anymore.
> > 
> > 
> > We /could/ add the original argv as a char ** to CommandLine. It's a minor
> > gain though, if gtk_init may modify the strings, we still need to make
> > copies (for thread safety).
> > 
> > 
> > 
> > >
> > >
> > > http://codereview.chromium.org/149424
> > >
> 
> I will check in with the TODO removed, let me know if you disagree.

LGTM