Use base::MessageLoopProxy instead of MessageLoop in ProxyConfigServiceLinux.

Use base::MessageLoopProxy instead of MessageLoop in ProxyConfigServiceLinux.

This will allow using that class on the main NPAPI plugin thread in 
chromoting.

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

[Ryan Sleevi, 2012-04-24 00:44:30]

-me, +willchan

For some reason, I believe Will might be in process of changing this code to the
TaskRunner interface. I'm not sure if he's started that yet, but he'll probably
be a better reviewer as I believe he was working on it recently.

Will: If not, this should be punted to... Eric? I'm not that familiar with this
code, so I'm probably not the best reviewer.

[willchan no longer on Chromium, 2012-04-24 00:46:29]

Redirecting to Eric. Although yeah, I think we should just use
base::SequencedTaskRunner.

On Mon, Apr 23, 2012 at 5:44 PM, <rsleevi@chromium.org> wrote:

> -me, +willchan
>
> For some reason, I believe Will might be in process of changing this code
> to the
> TaskRunner interface. I'm not sure if he's started that yet, but he'll
> probably
> be a better reviewer as I believe he was working on it recently.
>
> Will: If not, this should be punted to... Eric? I'm not that familiar with
> this
> code, so I'm probably not the best reviewer.
>
>
http://codereview.chromium.**org/10140010/<http://codereview.chromium.org/101...
>

[Sergey Ulanov, 2012-04-24 01:13:48]

I agree  that moving to SequencedTaskRunner would make sense. Problem is that it
would require much bigger refactoring of this code. E.g. it still uses inotify
and MessageLoop::WatchFileDescriptor() and we would need to fix it to use
base::files::FilePathWatcher.

So I would like to land this CL as is - it will allow use to fix the linked bug
for remoting, we can refactor this code further later.

On 2012/04/24 00:46:29, willchan wrote:
> Redirecting to Eric. Although yeah, I think we should just use
> base::SequencedTaskRunner.
> 
> On Mon, Apr 23, 2012 at 5:44 PM, <mailto:rsleevi@chromium.org> wrote:
> 
> > -me, +willchan
> >
> > For some reason, I believe Will might be in process of changing this code
> > to the
> > TaskRunner interface. I'm not sure if he's started that yet, but he'll
> > probably
> > be a better reviewer as I believe he was working on it recently.
> >
> > Will: If not, this should be punted to... Eric? I'm not that familiar with
> > this
> > code, so I'm probably not the best reviewer.
> >
> >
>
http://codereview.chromium.**org/10140010/%3Chttp://codereview.chromium.org/1...>
> >

[willchan no longer on Chromium, 2012-04-24 01:24:20]

Sounds reasonable.
On Apr 23, 2012 6:13 PM, <sergeyu@chromium.org> wrote:

> I agree  that moving to SequencedTaskRunner would make sense. Problem is
> that it
> would require much bigger refactoring of this code. E.g. it still uses
> inotify
> and MessageLoop::**WatchFileDescriptor() and we would need to fix it to
> use
> base::files::FilePathWatcher.
>
> So I would like to land this CL as is - it will allow use to fix the
> linked bug
> for remoting, we can refactor this code further later.
>
> On 2012/04/24 00:46:29, willchan wrote:
>
>> Redirecting to Eric. Although yeah, I think we should just use
>> base::SequencedTaskRunner.
>>
>
>  On Mon, Apr 23, 2012 at 5:44 PM, <mailto:rsleevi@chromium.org> wrote:
>>
>
>  > -me, +willchan
>> >
>> > For some reason, I believe Will might be in process of changing this
>> code
>> > to the
>> > TaskRunner interface. I'm not sure if he's started that yet, but he'll
>> > probably
>> > be a better reviewer as I believe he was working on it recently.
>> >
>> > Will: If not, this should be punted to... Eric? I'm not that familiar
>> with
>> > this
>> > code, so I'm probably not the best reviewer.
>> >
>> >
>>
>
> http://codereview.chromium.****org/10140010/%3Chttp://coderev**
> iew.chromium.org/10140010/ <http://codereview.chromium.org/10140010/>>
>
>> >
>>
>
>
>
>
http://codereview.chromium.**org/10140010/<http://codereview.chromium.org/101...
>

[Mike Mammarella, 2012-04-24 01:32:45]

The changes in this CL look good (though I'm not an owner, despite writing a lot
of this code), but as in the previous email thread, the GConf library is not
thread-safe and must only be accessed from the UI thread. If you have a
MessageLoopProxy for the UI thread and it's just a type problem that you don't
have the required MessageLoop, then that's fine - but if you are trying to use a
thread other than the UI thread, you're just asking for problems.

[Sergey Ulanov, 2012-04-24 02:10:46]

Yes the main plugin thread is a UI thread, it's just the plugin doesn't
have access to the corresponding MessageLoop for that thread.

On Mon, Apr 23, 2012 at 6:32 PM, <mdm@chromium.org> wrote:

> The changes in this CL look good (though I'm not an owner, despite writing
> a lot
> of this code), but as in the previous email thread, the GConf library is
> not
> thread-safe and must only be accessed from the UI thread. If you have a
> MessageLoopProxy for the UI thread and it's just a type problem that you
> don't
> have the required MessageLoop, then that's fine - but if you are trying to
> use a
> thread other than the UI thread, you're just asking for problems.
>
>
http://codereview.chromium.**org/10140010/<http://codereview.chromium.org/101...
>

[Mike Mammarella, 2012-04-24 02:33:21]

OK, LGTM then. But you'll still need an owner.

(You mean *the* UI thread, not *a* UI thread, right?)

[Sergey Ulanov, 2012-04-24 03:00:45]

On Mon, Apr 23, 2012 at 7:33 PM, <mdm@chromium.org> wrote:

> OK, LGTM then. But you'll still need an owner.
>
> (You mean *the* UI thread, not *a* UI thread, right?)
>

Right. It's the main thread in a plugin process. There may be multiple
plugin processes, each with it's own UI thread :-).



>
>
http://codereview.chromium.**org/10140010/<http://codereview.chromium.org/101...
>

[willchan no longer on Chromium, 2012-04-24 18:53:44]

lgtm

[commit-bot: I haz the power, 2012-04-24 18:57:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10140010/1

[commit-bot: I haz the power, 2012-04-24 21:13:29]

Change committed as 133767