ash/mash: Introduce a ShelfDelegate implementation for mash.

ash/mash: Introduce a ShelfDelegate implementation for mash.

Introduce ShelfDelegateMus to provide the ShelfDelegate implementation, which
talks to the mash-wm to receive updates about the user-windows, and updates the
ShelfModel in ash as needed.

BUG=585141
Committed: https://crrev.com/618dd9e47a812b47b72ff28e4dc54a2e7a8b5839
Cr-Commit-Position: refs/heads/master@{#374701}

[sadrul, 2016-02-09 17:50:48]

sadrul@chromium.org changed reviewers:
+ msw@chromium.org, sky@chromium.org

[msw, 2016-02-09 18:31:26]

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/shelf_del...
File ash/mus/sysui/shelf_delegate_mus.cc (right):

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/shelf_del...
ash/mus/sysui/shelf_delegate_mus.cc:50: ShelfMenuModel*
CreateApplicationMenu(int event_flags) override {
nit: follow base class ordering after CreateContextMenu: CanPin,
CreateApplicationMenu, IsDraggabe

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/shelf_del...
ash/mus/sysui/shelf_delegate_mus.cc:165: NOTIMPLEMENTED();
Were my old title/focus change impls not easily convertible?

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/sysui_app...
File ash/mus/sysui/sysui_application.cc (right):

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/sysui_app...
ash/mus/sysui/sysui_application.cc:103: ui::ContextFactory* factory =
aura::Env::GetInstance()->context_factory();
This seems like a tangential workaround; care to add a comment?

https://codereview.chromium.org/1684633002/diff/20001/ash/screen_util.cc
File ash/screen_util.cc (right):

https://codereview.chromium.org/1684633002/diff/20001/ash/screen_util.cc#newc...
ash/screen_util.cc:71: // sized/positioned yet. Use the bounds of the display in
this case.
Could this always use the display bounds? When are they different?

[sky, 2016-02-09 20:44:08]

https://codereview.chromium.org/1684633002/diff/20001/ash/screen_util.cc
File ash/screen_util.cc (right):

https://codereview.chromium.org/1684633002/diff/20001/ash/screen_util.cc#newc...
ash/screen_util.cc:72: gfx::Display display =
We shouldn't be calling this code right? I think it best to add a NOTIMPLEMENTED
or warning that we need to fix this.

[sadrul, 2016-02-10 07:59:44]

Patchset #3 (id:40001) has been deleted

[sadrul, 2016-02-10 08:04:41]

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/shelf_del...
File ash/mus/sysui/shelf_delegate_mus.cc (right):

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/shelf_del...
ash/mus/sysui/shelf_delegate_mus.cc:50: ShelfMenuModel*
CreateApplicationMenu(int event_flags) override {
On 2016/02/09 18:31:26, msw wrote:
> nit: follow base class ordering after CreateContextMenu: CanPin,
> CreateApplicationMenu, IsDraggabe

Done.

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/shelf_del...
ash/mus/sysui/shelf_delegate_mus.cc:165: NOTIMPLEMENTED();
On 2016/02/09 18:31:26, msw wrote:
> Were my old title/focus change impls not easily convertible?

ShelfItem in mash is slightly different from the ShelfItem in ash, and needs
slightly different way of updating the ShelfModel in ash. I have gone ahead and
implemented these.

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/sysui_app...
File ash/mus/sysui/sysui_application.cc (right):

https://codereview.chromium.org/1684633002/diff/20001/ash/mus/sysui/sysui_app...
ash/mus/sysui/sysui_application.cc:103: ui::ContextFactory* factory =
aura::Env::GetInstance()->context_factory();
On 2016/02/09 18:31:26, msw wrote:
> This seems like a tangential workaround; care to add a comment?

Split into a separate CL: https://codereview.chromium.org/1679023007

https://codereview.chromium.org/1684633002/diff/20001/ash/screen_util.cc
File ash/screen_util.cc (right):

https://codereview.chromium.org/1684633002/diff/20001/ash/screen_util.cc#newc...
ash/screen_util.cc:71: // sized/positioned yet. Use the bounds of the display in
this case.
On 2016/02/09 18:31:26, msw wrote:
> Could this always use the display bounds? When are they different?

Looks like it could always use the display bounds (at least, the trybots aren't
complaining: https://codereview.chromium.org/1688453002/). However, it doesn't
work right for mustash if we always use the display size. I haven't looked
closely enough to figure out why. But ideally, as sky@ notes below, we wouldn't
be running this code for mustash at all.

https://codereview.chromium.org/1684633002/diff/20001/ash/screen_util.cc#newc...
ash/screen_util.cc:72: gfx::Display display =
On 2016/02/09 20:44:08, sky wrote:
> We shouldn't be calling this code right? I think it best to add a
NOTIMPLEMENTED
> or warning that we need to fix this.

Added a NOTIMPLEMENTED() here. We do still need this temporary workaround to get
the shelf to show up on screen at the right position though, so I kept that
part.

[sky, 2016-02-10 17:28:23]

LGTM

[msw, 2016-02-10 17:33:29]

lgtm

[sadrul, 2016-02-10 19:22:42]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-02-10 19:22:42]

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

[commit-bot: I haz the power, 2016-02-10 19:24:39]

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

[commit-bot: I haz the power, 2016-02-10 19:38:31]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-02-16 22:30:28]

Description was changed from

==========
ash/mash: Introduce a ShelfDelegate implementation for mash.

Introduce ShelfDelegateMus to provide the ShelfDelegate implementation, which
talks to the mash-wm to receive updates about the user-windows, and updates the
ShelfModel in ash as needed.

BUG=585141
==========

to

==========
ash/mash: Introduce a ShelfDelegate implementation for mash.

Introduce ShelfDelegateMus to provide the ShelfDelegate implementation, which
talks to the mash-wm to receive updates about the user-windows, and updates the
ShelfModel in ash as needed.

BUG=585141

Committed: https://crrev.com/618dd9e47a812b47b72ff28e4dc54a2e7a8b5839
Cr-Commit-Position: refs/heads/master@{#374701}
==========

[commit-bot: I haz the power, 2016-02-16 22:30:29]

Patchset 4 (id:??) landed as
https://crrev.com/618dd9e47a812b47b72ff28e4dc54a2e7a8b5839
Cr-Commit-Position: refs/heads/master@{#374701}