Always allow ServiceRegistry to be used with UtilityProcessHosts.

Always allow ServiceRegistry to be used with UtilityProcessHosts.

BUG=597124
Committed: https://crrev.com/780142d2bca3cebc026771ef19457faba38c54b3
Cr-Commit-Position: refs/heads/master@{#384472}

[Anand Mistry (off Chromium), 2016-03-31 22:18:25]

amistry@chromium.org changed reviewers:
+ avi@chromium.org

[Avi (use Gerrit), 2016-03-31 22:22:33]

lgtm

[Anand Mistry (off Chromium), 2016-03-31 22:57:55]

amistry@chromium.org changed reviewers:
+ reillyg@chromium.org, thestig@chromium.org

[Anand Mistry (off Chromium), 2016-03-31 22:57:56]

thestig@chromium.org: For //chrome

reillyg@chromium.org: For //extensions

[Lei Zhang, 2016-03-31 23:05:46]

lgtm

https://codereview.chromium.org/1844233003/diff/20001/chrome/test/base/run_al...
File chrome/test/base/run_all_unittests.cc (right):

https://codereview.chromium.org/1844233003/diff/20001/chrome/test/base/run_al...
chrome/test/base/run_all_unittests.cc:19:
scoped_ptr<mojo::edk::test::ScopedIPCSupport> ipc_support;
Just:

scoped_ptr<mojo::edk::test::ScopedIPCSupport> ipc_support(new foo);

[Anand Mistry (off Chromium), 2016-03-31 23:10:38]

https://codereview.chromium.org/1844233003/diff/20001/chrome/test/base/run_al...
File chrome/test/base/run_all_unittests.cc (right):

https://codereview.chromium.org/1844233003/diff/20001/chrome/test/base/run_al...
chrome/test/base/run_all_unittests.cc:19:
scoped_ptr<mojo::edk::test::ScopedIPCSupport> ipc_support;
On 2016/03/31 23:05:46, Lei Zhang wrote:
> Just:
> 
> scoped_ptr<mojo::edk::test::ScopedIPCSupport> ipc_support(new foo);

Well, there's no reason for it to be a scoped_ptr, so I've just removed that
part.

[Reilly Grant (use Gerrit), 2016-03-31 23:18:31]

https://codereview.chromium.org/1844233003/diff/40001/extensions/test/extensi...
File extensions/test/extensions_unittests_main.cc (right):

https://codereview.chromium.org/1844233003/diff/40001/extensions/test/extensi...
extensions/test/extensions_unittests_main.cc:117: base::TestIOThread
test_io_thread(base::TestIOThread::kAutoStart);
I'm a little nervous about adding this as there may be tests that want to create
a TestIOThread for their own purposes and the general assumption is that while
browser tests run with a mostly full environment unit tests do not.

There don't appear to be any tests that do this at the moment. How many tests in
this executable need Mojo IPC support?

[Reilly Grant (use Gerrit), 2016-03-31 23:19:51]

On 2016/03/31 at 23:18:31, Reilly Grant wrote:
>
https://codereview.chromium.org/1844233003/diff/40001/extensions/test/extensi...
> File extensions/test/extensions_unittests_main.cc (right):
> 
>
https://codereview.chromium.org/1844233003/diff/40001/extensions/test/extensi...
> extensions/test/extensions_unittests_main.cc:117: base::TestIOThread
test_io_thread(base::TestIOThread::kAutoStart);
> I'm a little nervous about adding this as there may be tests that want to
create a TestIOThread for their own purposes and the general assumption is that
while browser tests run with a mostly full environment unit tests do not.
> 
> There don't appear to be any tests that do this at the moment. How many tests
in this executable need Mojo IPC support?

If creating a TestIOThread here doesn't prevent a test from creating its own
then this lgtm to me since thestig@ was okay with adding the same to
chrome/test/base/run_all_unittests.cc.

[Anand Mistry (off Chromium), 2016-03-31 23:30:41]

On 2016/03/31 23:19:51, Reilly Grant wrote:
> On 2016/03/31 at 23:18:31, Reilly Grant wrote:
> >
>
https://codereview.chromium.org/1844233003/diff/40001/extensions/test/extensi...
> > File extensions/test/extensions_unittests_main.cc (right):
> > 
> >
>
https://codereview.chromium.org/1844233003/diff/40001/extensions/test/extensi...
> > extensions/test/extensions_unittests_main.cc:117: base::TestIOThread
> test_io_thread(base::TestIOThread::kAutoStart);
> > I'm a little nervous about adding this as there may be tests that want to
> create a TestIOThread for their own purposes and the general assumption is
that
> while browser tests run with a mostly full environment unit tests do not.
> > 
> > There don't appear to be any tests that do this at the moment. How many
tests
> in this executable need Mojo IPC support?

None, except that the point of this change is to make Mojo available to all
utility processes which is one step towards my goal of converting all utility
process IPCs. So, for extensions_unittests, that's the SanboxUnpacker tests
which use an in-process utility thread, and hence start to "use" Mojo as a
result of this change.

> 
> If creating a TestIOThread here doesn't prevent a test from creating its own
> then this lgtm to me since thestig@ was okay with adding the same to
> chrome/test/base/run_all_unittests.cc.

In this case, it's _an_ IO thread, not _the_ browser IO thread. So you can have
nay number of of these threads. This particular one is only used by Mojo itself
and can't be used for anything else.

[Anand Mistry (off Chromium), 2016-03-31 23:30:52]

The CQ bit was checked by amistry@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-31 23:31:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1844233003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1844233003/40001

[Reilly Grant (use Gerrit), 2016-03-31 23:33:52]

On 2016/03/31 at 23:30:41, amistry wrote:
> On 2016/03/31 23:19:51, Reilly Grant wrote:
> > On 2016/03/31 at 23:18:31, Reilly Grant wrote:
> > >
> >
https://codereview.chromium.org/1844233003/diff/40001/extensions/test/extensi...
> > > File extensions/test/extensions_unittests_main.cc (right):
> > > 
> > >
> >
https://codereview.chromium.org/1844233003/diff/40001/extensions/test/extensi...
> > > extensions/test/extensions_unittests_main.cc:117: base::TestIOThread
> > test_io_thread(base::TestIOThread::kAutoStart);
> > > I'm a little nervous about adding this as there may be tests that want to
> > create a TestIOThread for their own purposes and the general assumption is
that
> > while browser tests run with a mostly full environment unit tests do not.
> > > 
> > > There don't appear to be any tests that do this at the moment. How many
tests
> > in this executable need Mojo IPC support?
> 
> None, except that the point of this change is to make Mojo available to all
utility processes which is one step towards my goal of converting all utility
process IPCs. So, for extensions_unittests, that's the SanboxUnpacker tests
which use an in-process utility thread, and hence start to "use" Mojo as a
result of this change.
> 
> > 
> > If creating a TestIOThread here doesn't prevent a test from creating its own
> > then this lgtm to me since thestig@ was okay with adding the same to
> > chrome/test/base/run_all_unittests.cc.
> 
> In this case, it's _an_ IO thread, not _the_ browser IO thread. So you can
have nay number of of these threads. This particular one is only used by Mojo
itself and can't be used for anything else.

Got it!

[commit-bot: I haz the power, 2016-04-01 01:09:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-01 01:09:01]

Dry run: This issue passed the CQ dry run.

[Anand Mistry (off Chromium), 2016-04-01 02:08:19]

The CQ bit was checked by amistry@chromium.org

[Anand Mistry (off Chromium), 2016-04-01 02:08:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1844233003/#ps40001
(title: "Remove scoped_ptr usage.")

[commit-bot: I haz the power, 2016-04-01 02:08:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1844233003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1844233003/40001

[commit-bot: I haz the power, 2016-04-01 02:18:47]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-01 02:20:04]

Description was changed from

==========
Always allow ServiceRegistry to be used with UtilityProcessHosts.

BUG=597124
==========

to

==========
Always allow ServiceRegistry to be used with UtilityProcessHosts.

BUG=597124

Committed: https://crrev.com/780142d2bca3cebc026771ef19457faba38c54b3
Cr-Commit-Position: refs/heads/master@{#384472}
==========

[commit-bot: I haz the power, 2016-04-01 02:20:06]

Patchset 3 (id:??) landed as
https://crrev.com/780142d2bca3cebc026771ef19457faba38c54b3
Cr-Commit-Position: refs/heads/master@{#384472}