Enable multiprocess by default on desktop mojo_runner.

Enable multiprocess by default on desktop mojo_runner.

This makes apptests also run in multiprocess and use the new EDK as well.

BUG=478251, 546644, 551083
Committed: https://crrev.com/14d2827bffed6b8c4928001eb42d9f4dda23dd81
Cr-Commit-Position: refs/heads/master@{#358664}

Committed: https://crrev.com/2e0f873f6dd6c21b5d17ea625f889e19c97218ff
Cr-Commit-Position: refs/heads/master@{#359060}

[jam, 2015-11-07 04:59:07]

Description was changed from

==========
Enable multiprocess by default on desktop mojo_runner.

This makes apptests also run in multiprocess and use the new EDK as well.

BUG=478251, 546644, 551083
==========

to

==========
Enable multiprocess by default on desktop mojo_runner.

This makes apptests also run in multiprocess and use the new EDK as well.

BUG=478251, 546644, 551083
==========

[jam, 2015-11-07 04:59:08]

jam@chromium.org changed reviewers:
+ sky@chromium.org

[jam, 2015-11-07 05:00:31]

https://codereview.chromium.org/1434563004/diff/20001/mojo/message_pump/handl...
File mojo/message_pump/handle_watcher.cc (right):

https://codereview.chromium.org/1434563004/diff/20001/mojo/message_pump/handl...
mojo/message_pump/handle_watcher.cc:226:
base::LeakySingletonTraits<WatcherThreadManager>>::
we need to leak this because otherwise when the process dies, AtExitManager
waits for destruction which waits till the handle watcher thread is joined. but
that can't happen since the pump uses mojo message pipes to wake up the pump.
since mojo edk has been shutdown already, this never completes.

[sky, 2015-11-08 16:26:36]

LGTM

https://codereview.chromium.org/1434563004/diff/20001/mojo/message_pump/handl...
File mojo/message_pump/handle_watcher.cc (right):

https://codereview.chromium.org/1434563004/diff/20001/mojo/message_pump/handl...
mojo/message_pump/handle_watcher.cc:226:
base::LeakySingletonTraits<WatcherThreadManager>>::
On 2015/11/07 05:00:31, jam wrote:
> we need to leak this because otherwise when the process dies, AtExitManager
> waits for destruction which waits till the handle watcher thread is joined.
but
> that can't happen since the pump uses mojo message pipes to wake up the pump.
> since mojo edk has been shutdown already, this never completes.

Please add this comment to the code here.

[jam, 2015-11-09 18:20:46]

The CQ bit was checked by jam@chromium.org

[jam, 2015-11-09 18:20:46]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1434563004/#ps120001
(title: "merge")

[jam, 2015-11-09 18:20:58]

https://codereview.chromium.org/1434563004/diff/20001/mojo/message_pump/handl...
File mojo/message_pump/handle_watcher.cc (right):

https://codereview.chromium.org/1434563004/diff/20001/mojo/message_pump/handl...
mojo/message_pump/handle_watcher.cc:226:
base::LeakySingletonTraits<WatcherThreadManager>>::
On 2015/11/08 16:26:36, sky wrote:
> On 2015/11/07 05:00:31, jam wrote:
> > we need to leak this because otherwise when the process dies, AtExitManager
> > waits for destruction which waits till the handle watcher thread is joined.
> but
> > that can't happen since the pump uses mojo message pipes to wake up the
pump.
> > since mojo edk has been shutdown already, this never completes.
> 
> Please add this comment to the code here.

Done.

[jam, 2015-11-09 18:20:58]

https://codereview.chromium.org/1434563004/diff/20001/mojo/message_pump/handl...
File mojo/message_pump/handle_watcher.cc (right):

https://codereview.chromium.org/1434563004/diff/20001/mojo/message_pump/handl...
mojo/message_pump/handle_watcher.cc:226:
base::LeakySingletonTraits<WatcherThreadManager>>::
On 2015/11/08 16:26:36, sky wrote:
> On 2015/11/07 05:00:31, jam wrote:
> > we need to leak this because otherwise when the process dies, AtExitManager
> > waits for destruction which waits till the handle watcher thread is joined.
> but
> > that can't happen since the pump uses mojo message pipes to wake up the
pump.
> > since mojo edk has been shutdown already, this never completes.
> 
> Please add this comment to the code here.

Done.

[commit-bot: I haz the power, 2015-11-09 18:21:24]

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

[commit-bot: I haz the power, 2015-11-09 21:44:42]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-11-09 21:45:13]

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

[msw, 2015-11-09 22:06:03]

msw@chromium.org changed reviewers:
+ msw@chromium.org

[msw, 2015-11-09 22:06:04]

This causes MediaAppTest.Lifetime mojo:media_apptests to fail on win:
http://build.chromium.org/p/chromium.mojo/builders/Chromium%20Mojo%20Windows/...
Taking a look and +cc xhwang

[jam, 2015-11-09 23:26:27]

On 2015/11/09 22:06:04, msw wrote:
> This causes MediaAppTest.Lifetime mojo:media_apptests to fail on win:
>
http://build.chromium.org/p/chromium.mojo/builders/Chromium%20Mojo%20Windows/...
> Taking a look and +cc xhwang

yep looking into it, also Linux. not sure why this isn't showing up on the CQ?

[jam, 2015-11-09 23:27:46]

A revert of this CL (patchset #7 id:120001) has been created in
https://codereview.chromium.org/1423723007/ by jam@chromium.org.

The reason for reverting is: This is failing on CQ and waterfall now. It might
have conflicted with another patch that landed in the meantime..

[jam, 2015-11-09 23:27:58]

Description was changed from

==========
Enable multiprocess by default on desktop mojo_runner.

This makes apptests also run in multiprocess and use the new EDK as well.

BUG=478251, 546644, 551083

Committed: https://crrev.com/14d2827bffed6b8c4928001eb42d9f4dda23dd81
Cr-Commit-Position: refs/heads/master@{#358664}
==========

to

==========
Enable multiprocess by default on desktop mojo_runner.

This makes apptests also run in multiprocess and use the new EDK as well.

BUG=478251, 546644, 551083

Committed: https://crrev.com/14d2827bffed6b8c4928001eb42d9f4dda23dd81
Cr-Commit-Position: refs/heads/master@{#358664}
==========

[msw, 2015-11-09 23:30:33]

On 2015/11/09 23:27:46, jam wrote:
> A revert of this CL (patchset #7 id:120001) has been created in
> https://codereview.chromium.org/1423723007/ by mailto:jam@chromium.org.
> 
> The reason for reverting is: This is failing on CQ and waterfall now. It might
> have conflicted with another patch that landed in the meantime..

looks like https://codereview.chromium.org/1437523002 actually broke linux, but
maybe only in combination with this patchset?

[msw, 2015-11-10 00:01:48]

"Chromium Mojo Linux" cycled green after the revert:
http://build.chromium.org/p/chromium.mojo/builders/Chromium%20Mojo%20Linux/bu...
Hopefully the same will happen on a *future* build of "Linux Tests"... 34040?
http://build.chromium.org/p/chromium.linux/builders/Linux%20Tests/builds/34040

The actual mojo:media_apptests defect is very weird...
On my local build, the test passes locally when run directly:
mojo_runner.exe --use-temporary-user-data-dir
--gtest_filter=MediaAppTest.Lifetime mojo:media_apptests
With the same exact build, invoking apptest_runner fails like on the bot, with
"ERROR: Test exited with no output."
mojo/tools/apptest_runner.py out/Release/ --apptest-filter=mojo:media_apptests
I wonder if the output coming from the apptest module isn't actually printed
until after the shell unloads the module? (which is potentially after
mojo_runner.exe exits since this CL leaks the modules via WatcherThreadManager?)

[jam, 2015-11-11 03:49:53]

The CQ bit was checked by jam@chromium.org

[jam, 2015-11-11 03:49:54]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1434563004/#ps180001
(title: "merge")

[commit-bot: I haz the power, 2015-11-11 03:50:34]

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

[commit-bot: I haz the power, 2015-11-11 05:21:19]

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

[commit-bot: I haz the power, 2015-11-11 05:21:20]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jam, 2015-11-11 05:29:32]

The CQ bit was checked by jam@chromium.org

[commit-bot: I haz the power, 2015-11-11 05:29:43]

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

[commit-bot: I haz the power, 2015-11-11 06:30:58]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-11-11 06:32:21]

Patchset 10 (id:??) landed as
https://crrev.com/2e0f873f6dd6c21b5d17ea625f889e19c97218ff
Cr-Commit-Position: refs/heads/master@{#359060}