Creates BackgroundShell and uses it in test

Creates BackgroundShell and uses it in test

BackgroundShell creates the necessary mojo state in a background
thread and then vends an ApplicationImpl to the main thread so that
the main thread can use mojo.

I also converted views_apptests to a unittest that makes use of
BackgroundShell.

BUG=577242, 577274
TEST=none
R=ben@chromium.org

Committed: https://crrev.com/468d726832348bc0c6415fb8604ccc4e43c84664
Cr-Commit-Position: refs/heads/master@{#374156}

[sky, 2016-02-04 22:35:18]

Notice that I had to move out icu initialization from context. This is because I
wanted as little changes as possible to calling code, which generally
initializes icu.

I put this code in mojo/shell/background, but maybe a better place is
mojo/shell/public/cpp/background or just mojo/shell/public/cpp (no separate
directory, but separate gn targets). WDYT?

[Ben Goodger (Google), 2016-02-05 00:59:25]

ben@chromium.org changed reviewers:
+ rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-02-05 01:02:01]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-02-05 01:02:01]

lgtm

[Ken Rockot(use gerrit already), 2016-02-05 01:02:07]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-02-05 01:02:48]

Oops - wrong button.

I think we can use this in Chrome, minus the blocking in CreateApplication. So
returning an InterfaceRequest there would make sense.

[sky, 2016-02-05 01:06:23]

I can certainly make it return an InterfaceRequest, but I'm curious
why you don't think chrome would end up blocking on initialize? An
ApplicationImpl that hasn't been initialized isn't really useful. For
example, it has no shell, so you can't attempt to connect to other
apps. I'll also mention that MojoShellConnectionImpl does block
waiting for initialize.

  -Scott

On Thu, Feb 4, 2016 at 5:02 PM,  <rockot@chromium.org> wrote:
> Oops - wrong button.
>
> I think we can use this in Chrome, minus the blocking in CreateApplication.
> So
> returning an InterfaceRequest there would make sense.
>
> https://codereview.chromium.org/1667273002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2016-02-05 04:34:03]

It just seems like it would be an unnecessary source of startup latency.
Maybe I'm overestimating the impact. But it wouldn't be a major challenge
to provide a Shell proxy synchronously even if its concrete ApplicationImpl
were established asynchronously. i.e. we could still make the interface
convenient without blocking browser startup on whatever internal
initialization Mojo needs to do

On Thu, Feb 4, 2016 at 5:06 PM, Scott Violet <sky@chromium.org> wrote:

> I can certainly make it return an InterfaceRequest, but I'm curious
> why you don't think chrome would end up blocking on initialize? An
> ApplicationImpl that hasn't been initialized isn't really useful. For
> example, it has no shell, so you can't attempt to connect to other
> apps. I'll also mention that MojoShellConnectionImpl does block
> waiting for initialize.
>
>   -Scott
>
> On Thu, Feb 4, 2016 at 5:02 PM,  <rockot@chromium.org> wrote:
> > Oops - wrong button.
> >
> > I think we can use this in Chrome, minus the blocking in
> CreateApplication.
> > So
> > returning an InterfaceRequest there would make sense.
> >
> > https://codereview.chromium.org/1667273002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2016-02-08 16:32:21]

I changed the function to return an InterfaceRequest and moved out construction
of the ApplicationImpl to the one place using this code.
Take another look?

[Ken Rockot(use gerrit already), 2016-02-08 16:44:31]

On 2016/02/08 at 16:32:21, sky wrote:
> I changed the function to return an InterfaceRequest and moved out
construction of the ApplicationImpl to the one place using this code.
> Take another look?

Cool, lgtm. I dabbled with embedding this in chrome and I think returning an
InterfaceRequest is the right thing to do. Sometimes we'll get the request from
BackgroundShell, other times we'll get it elsewhere; we'd otherwise like to
share a code path for initializing the delegate + impl, and this makes that
possible. Thanks!

[Ben Goodger (Google), 2016-02-08 19:27:48]

lgtm

[sky, 2016-02-08 19:28:05]

The CQ bit was checked by sky@chromium.org

[sky, 2016-02-08 19:28:05]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/1667273002/#ps100001
(title: "fix android")

[commit-bot: I haz the power, 2016-02-08 19:35:38]

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

[commit-bot: I haz the power, 2016-02-08 19:48:13]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-02-08 19:50:52]

Description was changed from

==========
Creates BackgroundShell and uses it in test

BackgroundShell creates the necessary mojo state in a background
thread and then vends an ApplicationImpl to the main thread so that
the main thread can use mojo.

I also converted views_apptests to a unittest that makes use of
BackgroundShell.

BUG=577242, 577274
TEST=none
R=ben@chromium.org
==========

to

==========
Creates BackgroundShell and uses it in test

BackgroundShell creates the necessary mojo state in a background
thread and then vends an ApplicationImpl to the main thread so that
the main thread can use mojo.

I also converted views_apptests to a unittest that makes use of
BackgroundShell.

BUG=577242, 577274
TEST=none
R=ben@chromium.org

Committed: https://crrev.com/468d726832348bc0c6415fb8604ccc4e43c84664
Cr-Commit-Position: refs/heads/master@{#374156}
==========

[commit-bot: I haz the power, 2016-02-08 19:50:53]

Patchset 6 (id:??) landed as
https://crrev.com/468d726832348bc0c6415fb8604ccc4e43c84664
Cr-Commit-Position: refs/heads/master@{#374156}