Add mus_demo mojo app.

Add mus_demo mojo app.

Simple mojo app that connects to mus, creates a new window and draws
into the window. Draws a spinning square.

Provides a simple way to test mus graphics output without the complexity
of starting mash.

BUG=599162
Committed: https://crrev.com/bd601e5a5d235c1c933190286c04db14b458971e
Cr-Commit-Position: refs/heads/master@{#384259}

[kylechar, 2016-03-29 15:28:55]

kylechar@chromium.org changed reviewers:
+ rjkroege@chromium.org

[kylechar, 2016-03-29 16:48:13]

I've removed WindowTreeClient as it turned out to be unnecessary for the demo.

[rjkroege, 2016-03-29 18:53:23]

There might already be a bug for building a "mus shell". Or if there isn't, file
one.

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
File components/mus/demo/mus_demo.cc (right):

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.cc:30: void DrawSquare(const gfx::Rect& bounds,
double angle, SkCanvas* canvas) {
This is software rasterization -> MUS. A future iteration of this code might
benefit from doing GL drawing as well. You should take this comment as "add a
TODO(kylechar): ...."

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.cc:142: // Copy pixels data into vector that will
be passed to BitmapUploader.
I know you mostly copied this code from what I wrote to get Chrome to draw on
mus but I didn't like it when I wrote it and I still don't like it very much.
:-) Please add a TODO(rjkroege): to make a 1/0-copy bitmap uploader for the
contents of a SkBitmap.

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
File components/mus/demo/mus_demo.h (right):

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:29: namespace mus {
I think it would be better to be in a different name space.

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:33: // simple way to demonstrate the graphic
stack works as intended.
to demonstrate that the

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:63: // Allocate a bitmap the same size as the
window t draw into.
t -> to

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:69: mojo::Connector* connector_ = nullptr;
These are owned elsewhere? if so, adding // NOT OWNED is conventional

If they are not owned elsewhere, how are they managed?

[kylechar, 2016-03-29 20:43:40]

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
File components/mus/demo/mus_demo.cc (right):

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.cc:30: void DrawSquare(const gfx::Rect& bounds,
double angle, SkCanvas* canvas) {
On 2016/03/29 at 18:53:22, rjkroege wrote:
> This is software rasterization -> MUS. A future iteration of this code might
benefit from doing GL drawing as well. You should take this comment as "add a
TODO(kylechar): ...."

Done.

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.cc:142: // Copy pixels data into vector that will
be passed to BitmapUploader.
On 2016/03/29 at 18:53:22, rjkroege wrote:
> I know you mostly copied this code from what I wrote to get Chrome to draw on
mus but I didn't like it when I wrote it and I still don't like it very much.
:-) Please add a TODO(rjkroege): to make a 1/0-copy bitmap uploader for the
contents of a SkBitmap.

Done.

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
File components/mus/demo/mus_demo.h (right):

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:29: namespace mus {
On 2016/03/29 at 18:53:23, rjkroege wrote:
> I think it would be better to be in a different name space.

Done.

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:33: // simple way to demonstrate the graphic
stack works as intended.
On 2016/03/29 at 18:53:22, rjkroege wrote:
> to demonstrate that the

Done.

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:63: // Allocate a bitmap the same size as the
window t draw into.
On 2016/03/29 at 18:53:23, rjkroege wrote:
> t -> to

Done.

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:69: mojo::Connector* connector_ = nullptr;
On 2016/03/29 at 18:53:23, rjkroege wrote:
> These are owned elsewhere? if so, adding // NOT OWNED is conventional
> 
> If they are not owned elsewhere, how are they managed?

I believe they're owned elsewhere (based on other code that uses mus::Window and
Mojo::Connector).

Devils advocate: It wouldn't be a raw pointer if it was owned? I realize older
part of the chrome codebase might not use smart pointers yet but for newer code
it can probably be assumed? If you feel strongly I'll add a comment.

[rjkroege, 2016-03-29 20:56:54]

lgtm

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
File components/mus/demo/mus_demo.h (right):

https://codereview.chromium.org/1832943002/diff/40001/components/mus/demo/mus...
components/mus/demo/mus_demo.h:69: mojo::Connector* connector_ = nullptr;
On 2016/03/29 20:43:39, kylechar wrote:
> On 2016/03/29 at 18:53:23, rjkroege wrote:
> > These are owned elsewhere? if so, adding // NOT OWNED is conventional
> > 
> > If they are not owned elsewhere, how are they managed?
> 
> I believe they're owned elsewhere (based on other code that uses mus::Window
and
> Mojo::Connector).
> 
> Devils advocate: It wouldn't be a raw pointer if it was owned? I realize older
> part of the chrome codebase might not use smart pointers yet but for newer
code
> it can probably be assumed? If you feel strongly I'll add a comment.

My understanding is that the mojo owns infrastructure owns it.

[kylechar, 2016-03-29 21:03:21]

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

[kylechar, 2016-03-29 21:03:22]

Hey msw@, I'm adding you for LGTM on mash/BUILD.gn.

Some BUILD.gn needs to reference //components/mus/demo for GN to know it exists.
Adding it the mash:all target seemed like the correct place...

[msw, 2016-03-29 21:24:31]

https://codereview.chromium.org/1832943002/diff/60001/mash/BUILD.gn
File mash/BUILD.gn (right):

https://codereview.chromium.org/1832943002/diff/60001/mash/BUILD.gn#newcode15
mash/BUILD.gn:15: "//components/mus/demo",
Hmm, this is actually a sibling of mash, not really part of it; how about moving
this to the deps for the top-level gn_only target?

[kylechar, 2016-03-30 13:20:07]

kylechar@chromium.org changed reviewers:
+ dpranke@chromium.org
- msw@chromium.org

[kylechar, 2016-03-30 13:20:07]

If that's more idiomatic then sure, thanks msw@! Moved to gn_all target in
BUILD.gn in the same conditional as the rest of the mash/mojo stuff. Added
dpranke@ for BUILD.gn.

[kylechar, 2016-03-30 16:53:14]

Description was changed from

==========
Add mus_demo mojo app.

Simple mojo app that connects to mus, creates a new window and draws
into the window. Draws a spinning square.

Provides a simple way to test mus graphics output without the complexity
of starting mash.

BUG=
==========

to

==========
Add mus_demo mojo app.

Simple mojo app that connects to mus, creates a new window and draws
into the window. Draws a spinning square.

Provides a simple way to test mus graphics output without the complexity
of starting mash.

BUG=599162
==========

[kylechar, 2016-03-30 16:54:05]

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

[commit-bot: I haz the power, 2016-03-30 16:54:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1832943002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1832943002/80001

[commit-bot: I haz the power, 2016-03-30 17:46:11]

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

[commit-bot: I haz the power, 2016-03-30 17:46:11]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2016-03-31 01:52:15]

lgtm

[kylechar, 2016-03-31 13:06:33]

The CQ bit was checked by kylechar@chromium.org

[kylechar, 2016-03-31 13:06:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org
Link to the patchset: https://codereview.chromium.org/1832943002/#ps80001
(title: "Move from mash/BUILD.gn to BUILD.gn")

[commit-bot: I haz the power, 2016-03-31 13:06:46]

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

[commit-bot: I haz the power, 2016-03-31 13:11:47]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-31 13:12:50]

Description was changed from

==========
Add mus_demo mojo app.

Simple mojo app that connects to mus, creates a new window and draws
into the window. Draws a spinning square.

Provides a simple way to test mus graphics output without the complexity
of starting mash.

BUG=599162
==========

to

==========
Add mus_demo mojo app.

Simple mojo app that connects to mus, creates a new window and draws
into the window. Draws a spinning square.

Provides a simple way to test mus graphics output without the complexity
of starting mash.

BUG=599162

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

[commit-bot: I haz the power, 2016-03-31 13:12:51]

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