Switch FirefoxImporterTest to ChannelMojo.

Switch FirefoxImporterTest to ChannelMojo.

BUG=604282
Committed: https://crrev.com/f2b2c3aea1ae7d78b462329418831517444aab80
Cr-Commit-Position: refs/heads/master@{#404776}

[Anand Mistry (off Chromium), 2016-07-07 06:45:51]

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

[commit-bot: I haz the power, 2016-07-07 06:46:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Anand Mistry (off Chromium), 2016-07-07 06:46:12]

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

[commit-bot: I haz the power, 2016-07-07 07:19:28]

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

[commit-bot: I haz the power, 2016-07-07 07:19:28]

Dry run: This issue passed the CQ dry run.

[gab, 2016-07-07 15:13:11]

gab@chromium.org changed reviewers:
+ isherman@chromium.org

[gab, 2016-07-07 15:13:12]

Ilya is a better for reviewer for the importers (and myself for the
out-of-process import logic to which the importers are black boxes..).

[Ilya Sherman, 2016-07-07 20:14:24]

LGTM -- thanks for working on this!

Food for thought: The CL currently adds quite a bit of Mojo-specific logic to a
test where it reads as boilerplate setup.  If there are other tests that need
similar logic, would it make sense to factor out some of this setup code into
code that could be shared across tests?  This would have both the advantage of
code reuse, and more straightforward code in the actual test files.

https://codereview.chromium.org/2130593003/diff/100001/chrome/utility/importe...
File chrome/utility/importer/DEPS (right):

https://codereview.chromium.org/2130593003/diff/100001/chrome/utility/importe...
chrome/utility/importer/DEPS:10: "firefox_importer_unittest_utils_mac\.cc": [
Hmm, is it really necessary to be so specific?

https://codereview.chromium.org/2130593003/diff/100001/chrome/utility/importe...
File chrome/utility/importer/firefox_importer_unittest_utils_mac.cc (right):

https://codereview.chromium.org/2130593003/diff/100001/chrome/utility/importe...
chrome/utility/importer/firefox_importer_unittest_utils_mac.cc:146: // Setup IPC
channel using ChannelMojo.
nit: s/Setup/Set up

[Anand Mistry (off Chromium), 2016-07-11 06:59:59]

Re refactoring common code:
It turns out, this is the only test which needs this setup code. There's one
other, but it needs to do something special so this setup won't work for it.

Now, we do have multi-process Mojo tests and there is a helper. But it looks
like this test is special enough (the extra environ variable) in a way that
isn't currently supported or needed, that modifying the code to use that might
be more trouble than it's worth. I should also point out that the majority of
code which does need to launch a child uses ChildProcessLauncher, which has all
this setup code.

https://codereview.chromium.org/2130593003/diff/100001/chrome/utility/importe...
File chrome/utility/importer/DEPS (right):

https://codereview.chromium.org/2130593003/diff/100001/chrome/utility/importe...
chrome/utility/importer/DEPS:10: "firefox_importer_unittest_utils_mac\.cc": [
On 2016/07/07 20:14:24, Ilya Sherman wrote:
> Hmm, is it really necessary to be so specific?

This file is the only thing that needs it. I figured it's best to be specific
since nothing else should require it.

https://codereview.chromium.org/2130593003/diff/100001/chrome/utility/importe...
File chrome/utility/importer/firefox_importer_unittest_utils_mac.cc (right):

https://codereview.chromium.org/2130593003/diff/100001/chrome/utility/importe...
chrome/utility/importer/firefox_importer_unittest_utils_mac.cc:146: // Setup IPC
channel using ChannelMojo.
On 2016/07/07 20:14:24, Ilya Sherman wrote:
> nit: s/Setup/Set up

Done.

[Ilya Sherman, 2016-07-11 19:29:52]

Okay, thanks for the explanation.  Still LGTM.

[Anand Mistry (off Chromium), 2016-07-12 03:39:18]

The CQ bit was checked by amistry@chromium.org

[commit-bot: I haz the power, 2016-07-12 03:40:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 03:46:33]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-07-12 03:46:57]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-12 03:48:44]

Description was changed from

==========
Switch FirefoxImporterTest to ChannelMojo.

BUG=604282
==========

to

==========
Switch FirefoxImporterTest to ChannelMojo.

BUG=604282

Committed: https://crrev.com/f2b2c3aea1ae7d78b462329418831517444aab80
Cr-Commit-Position: refs/heads/master@{#404776}
==========

[commit-bot: I haz the power, 2016-07-12 03:48:45]

Patchset 7 (id:??) landed as
https://crrev.com/f2b2c3aea1ae7d78b462329418831517444aab80
Cr-Commit-Position: refs/heads/master@{#404776}