mojo: shell::Context should outlive the shell MessageLoop

mojo: shell::Context should outlive the shell MessageLoop

Make sure tests and mojo_mains all work the same way by handing control
of MessageLoops in test to ShellTest{Base, Helper}.

This ensures Applications that live as part of the shell (ViewManager)
can rely on MessageLoop destruction happening first (before their own
destruction) for cleaning up, similar to how this typically works for real
apps (mojo_main_{chromium, standalone}.

BUG=394477
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=286370

[tim (not reviewing), 2014-07-28 16:44:04]

Some context for this:
https://docs.google.com/a/chromium.org/document/d/1tKZw3cnnGEuoOD1gYLmPQtqYHN...

[jamesr, 2014-07-28 20:11:27]

lgtm

https://codereview.chromium.org/420143003/diff/70001/mojo/shell/child_process...
File mojo/shell/child_process_host_unittest.cc (right):

https://codereview.chromium.org/420143003/diff/70001/mojo/shell/child_process...
mojo/shell/child_process_host_unittest.cc:40: Context context;
any reason not to move this context c'tor up before the MessageLoop allocation
like the other ones?

https://codereview.chromium.org/420143003/diff/70001/mojo/shell/context.cc
File mojo/shell/context.cc (right):

https://codereview.chromium.org/420143003/diff/70001/mojo/shell/context.cc#ne...
mojo/shell/context.cc:86: Context::Context() {}
could you DCHECK that base::MessageLoop::current() is NULL here?

[tim (not reviewing), 2014-07-28 22:16:37]

https://codereview.chromium.org/420143003/diff/70001/mojo/shell/child_process...
File mojo/shell/child_process_host_unittest.cc (right):

https://codereview.chromium.org/420143003/diff/70001/mojo/shell/child_process...
mojo/shell/child_process_host_unittest.cc:40: Context context;
On 2014/07/28 20:11:27, jamesr wrote:
> any reason not to move this context c'tor up before the MessageLoop allocation
> like the other ones?

Oops, done.

https://codereview.chromium.org/420143003/diff/70001/mojo/shell/context.cc
File mojo/shell/context.cc (right):

https://codereview.chromium.org/420143003/diff/70001/mojo/shell/context.cc#ne...
mojo/shell/context.cc:86: Context::Context() {}
On 2014/07/28 20:11:27, jamesr wrote:
> could you DCHECK that base::MessageLoop::current() is NULL here?

Good idea. Done.

[darin (slow to review), 2014-07-28 23:02:13]

https://codereview.chromium.org/420143003/diff/90001/mojo/shell/context.cc
File mojo/shell/context.cc (right):

https://codereview.chromium.org/420143003/diff/90001/mojo/shell/context.cc#ne...
mojo/shell/context.cc:87: DCHECK(!base::MessageLoop::current());
perhaps the dtor should have a similar assertion?

https://codereview.chromium.org/420143003/diff/90001/mojo/shell/desktop/mojo_...
File mojo/shell/desktop/mojo_main.cc (right):

https://codereview.chromium.org/420143003/diff/90001/mojo/shell/desktop/mojo_...
mojo/shell/desktop/mojo_main.cc:30: mojo::shell::Context shell_context;
nit: add a comment here?

[tim (not reviewing), 2014-07-29 00:54:57]

https://codereview.chromium.org/420143003/diff/90001/mojo/shell/context.cc
File mojo/shell/context.cc (right):

https://codereview.chromium.org/420143003/diff/90001/mojo/shell/context.cc#ne...
mojo/shell/context.cc:87: DCHECK(!base::MessageLoop::current());
On 2014/07/28 23:02:12, darin wrote:
> perhaps the dtor should have a similar assertion?

Done.

https://codereview.chromium.org/420143003/diff/90001/mojo/shell/desktop/mojo_...
File mojo/shell/desktop/mojo_main.cc (right):

https://codereview.chromium.org/420143003/diff/90001/mojo/shell/desktop/mojo_...
mojo/shell/desktop/mojo_main.cc:30: mojo::shell::Context shell_context;
On 2014/07/28 23:02:13, darin wrote:
> nit: add a comment here?

Done.

[tim (not reviewing), 2014-07-29 05:29:43]

The CQ bit was checked by tim@chromium.org

[commit-bot: I haz the power, 2014-07-29 05:31:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tim@chromium.org/420143003/110001

[commit-bot: I haz the power, 2014-07-29 06:14:13]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  linux_chromium_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jamesr, 2014-07-29 06:17:34]

https://codereview.chromium.org/420143003/diff/110001/mojo/shell/android/mojo...
File mojo/shell/android/mojo_main.cc (right):

https://codereview.chromium.org/420143003/diff/110001/mojo/shell/android/mojo...
mojo/shell/android/mojo_main.cc:40:
shell_context->set_ui_loop(g_java_message_loop.Get().get());
maybe you want g_context.Get()-> here?

[commit-bot: I haz the power, 2014-07-29 06:20:28]

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

[commit-bot: I haz the power, 2014-07-29 06:20:29]

Try jobs failed on following builders:
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)

[tim (not reviewing), 2014-07-29 20:48:43]

https://codereview.chromium.org/420143003/diff/110001/mojo/shell/android/mojo...
File mojo/shell/android/mojo_main.cc (right):

https://codereview.chromium.org/420143003/diff/110001/mojo/shell/android/mojo...
mojo/shell/android/mojo_main.cc:40:
shell_context->set_ui_loop(g_java_message_loop.Get().get());
On 2014/07/29 06:17:34, jamesr wrote:
> maybe you want g_context.Get()-> here?

Yeah; I had verified mojo_shell was working on android with the destruction
reordering but then moved some of the other code around and that broke it (both
this compile error + the set_ui_loop call).  Fixed and verified shell still runs
on android.

[tim (not reviewing), 2014-07-29 21:12:30]

The CQ bit was checked by tim@chromium.org

[commit-bot: I haz the power, 2014-07-29 21:13:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tim@chromium.org/420143003/130001

[commit-bot: I haz the power, 2014-07-30 01:20:54]

Change committed as 286370