mozart: Initial commit of the launcher.

mozart: Initial commit of the launcher.

The launcher is a simple application which binds a ViewTree to
a NativeViewport, thereby allowing Mozart based applications to be
shown on screen.

Note: This version does not support input yet.

R=abarth@google.com, jamesr@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/c2c28845f9a11b370e7addc97dd7b64960123df0

[jeffbrown, 2015-10-24 02:40:04]

jeffbrown@google.com changed reviewers:
+ abarth@google.com, jamesr@chromium.org

[abarth, 2015-10-24 06:07:21]

LGTM

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
File services/ui/launcher/launcher_app.cc (right):

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_app.cc:49: // Match the Nexus 5 aspect ratio
initially.
<3

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_app.cc:96: if (view_tree_.get())
Is the |.get()| needed here?

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
File services/ui/launcher/launcher_app.h (right):

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_app.h:42: void LaunchClient(std::string app_url);
const std::string&

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_app.h:58: mojo::ui::ViewTokenPtr
client_view_token_;
nit: We don't usually use |client_| prefixes for these sorts of variables.  This
pointer doesn't represent the client of the interface.  It represents the remote
object, which is just a view provider.

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
File services/ui/launcher/launcher_view_tree.cc (right):

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_view_tree.cc:70:
SetRoot(mojo::ui::ViewTokenPtr());
Should we terminate the launcher in this case?

[jamesr, 2015-10-26 20:13:25]

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
File services/ui/launcher/launcher_view_tree.cc (right):

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_view_tree.cc:37: if (root_.get())
StructPtr types have appropriate bool conversions, so you can just say

if (root_)
  ...

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_view_tree.cc:76: if (!root_.get())
if (!root_)

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_view_tree.cc:87: params.Pass(), true
/*provide_size*/,
maybe this should be an enum

[jeffbrown, 2015-10-27 02:05:03]

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
File services/ui/launcher/launcher_app.cc (right):

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_app.cc:96: if (view_tree_.get())
On 2015/10/24 06:07:20, abarth wrote:
> Is the |.get()| needed here?

Apparently not.  Keep getting confused between the smart pointer types with bool
conversions and those without.  eg. unique_ptr<> doesn't

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
File services/ui/launcher/launcher_view_tree.cc (right):

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_view_tree.cc:37: if (root_.get())
On 2015/10/26 20:13:25, jamesr wrote:
> StructPtr types have appropriate bool conversions, so you can just say
> 
> if (root_)
>   ...

Done.

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_view_tree.cc:70:
SetRoot(mojo::ui::ViewTokenPtr());
On 2015/10/24 06:07:21, abarth wrote:
> Should we terminate the launcher in this case?

Probably.  I'll leave a TODO.

Shutdown in Mojo is hard right now.

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_view_tree.cc:76: if (!root_.get())
On 2015/10/26 20:13:25, jamesr wrote:
> if (!root_)

Done.

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_view_tree.cc:87: params.Pass(), true
/*provide_size*/,
On 2015/10/26 20:13:24, jamesr wrote:
> maybe this should be an enum

I'm going to remove it for now.  Layout needs a lot more thought.

[kulakowski, 2015-10-27 16:04:18]

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

[kulakowski, 2015-10-27 16:04:18]

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
File services/ui/launcher/launcher_app.cc (right):

https://codereview.chromium.org/1404423007/diff/1/services/ui/launcher/launch...
services/ui/launcher/launcher_app.cc:96: if (view_tree_.get())
On 2015/10/27 02:05:03, jeffbrown wrote:
> On 2015/10/24 06:07:20, abarth wrote:
> > Is the |.get()| needed here?
> 
> Apparently not.  Keep getting confused between the smart pointer types with
bool
> conversions and those without.  eg. unique_ptr<> doesn't

Yes it does. http://en.cppreference.com/w/cpp/memory/unique_ptr/operator_bool

[jamesr, 2015-10-27 23:01:35]

lgtm

https://codereview.chromium.org/1404423007/diff/40001/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/1404423007/diff/40001/PRESUBMIT.py#newcode646
PRESUBMIT.py:646: r"^services/ui/launcher/launcher_app\.cc$",
you shouldn't need this

https://codereview.chromium.org/1404423007/diff/40001/services/ui/launcher/la...
File services/ui/launcher/launcher_app.cc (right):

https://codereview.chromium.org/1404423007/diff/40001/services/ui/launcher/la...
services/ui/launcher/launcher_app.cc:102: LOG(INFO) << "Launching " << app_url
<< " with Mozart...";
VLOG(). this is just going to annoy everyone as is - folks using the system but
not working on this part don't really care

[jeffbrown, 2015-10-27 23:31:38]

https://codereview.chromium.org/1404423007/diff/40001/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/1404423007/diff/40001/PRESUBMIT.py#newcode646
PRESUBMIT.py:646: r"^services/ui/launcher/launcher_app\.cc$",
On 2015/10/27 23:01:35, jamesr wrote:
> you shouldn't need this

Done.

https://codereview.chromium.org/1404423007/diff/40001/services/ui/launcher/la...
File services/ui/launcher/launcher_app.cc (right):

https://codereview.chromium.org/1404423007/diff/40001/services/ui/launcher/la...
services/ui/launcher/launcher_app.cc:102: LOG(INFO) << "Launching " << app_url
<< " with Mozart...";
On 2015/10/27 23:01:35, jamesr wrote:
> VLOG(). this is just going to annoy everyone as is - folks using the system
but
> not working on this part don't really care

I was thinking of the launcher as a command-line tool so it made sense to
provide some interactive feedback.  I'll remove this.

[jeffbrown, 2015-10-28 01:46:59]

Committed patchset #4 (id:60001) manually as
c2c28845f9a11b370e7addc97dd7b64960123df0 (presubmit successful).