Creates mus implementation of WmShelf

Creates mus implementation of WmShelf

At this point it's just a stub.

BUG=603369
TEST=none
R=msw@chromium.org

Committed: https://crrev.com/88028961efbe33410aa37e21160bf2bdbcec0a8b
Cr-Commit-Position: refs/heads/master@{#394497}

[sky, 2016-05-17 23:51:26]

In case you haven't seen any of the ash/wm/common code it's the windowmanagement
code shared between mash and ash. ash/wm/common/wm_shelf is the shelf related
code needed by ash/wm/common. WmShelfMus is the implementation for mash, and at
this point it's mostly just a stub.

[msw, 2016-05-18 00:37:31]

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus.cc
File mash/wm/bridge/wm_shelf_mus.cc (right):

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus...
mash/wm/bridge/wm_shelf_mus.cc:13: #define NOTIMPLEMENTED() DVLOG(1) <<
"notimplemented"
Did you actually mean to land this? Why isn't NOTIMPLEMENTED() okay as-is?

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus.h
File mash/wm/bridge/wm_shelf_mus.h (right):

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus...
mash/wm/bridge/wm_shelf_mus.h:27: // ash::wm::WmShelf:
Hmm, I'm trying to understand why this is needed in addition to
ash/shelf/shelf.h... Shouldn't that Shelf class itself support either window
type for use by the ash or mash wm? (isn't the mash wm part of ash_sysui? if
not, wouldn't this need to be a mojo interface like ShelfController?) Sorry, I'm
not up to speed here.

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus...
mash/wm/bridge/wm_shelf_mus.h:39: base::ObserverList<ash::wm::WmShelfObserver>
observers_;
Make these private (here or above).

[sky, 2016-05-18 16:18:32]

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus.cc
File mash/wm/bridge/wm_shelf_mus.cc (right):

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus...
mash/wm/bridge/wm_shelf_mus.cc:13: #define NOTIMPLEMENTED() DVLOG(1) <<
"notimplemented"
On 2016/05/18 00:37:30, msw wrote:
> Did you actually mean to land this? Why isn't NOTIMPLEMENTED() okay as-is?

Yes. They are too spammy at this point to be useful.

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus.h
File mash/wm/bridge/wm_shelf_mus.h (right):

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus...
mash/wm/bridge/wm_shelf_mus.h:27: // ash::wm::WmShelf:
On 2016/05/18 00:37:30, msw wrote:
> Hmm, I'm trying to understand why this is needed in addition to
> ash/shelf/shelf.h... Shouldn't that Shelf class itself support either window
> type for use by the ash or mash wm? (isn't the mash wm part of ash_sysui? if
> not, wouldn't this need to be a mojo interface like ShelfController?) Sorry,
I'm
> not up to speed here.

The mash wm (aka desktop_wm) is not part of ash_sysui.

As part of window management mash wm needs to know and communicate with the
shelf. For example, the mash wm may need to hide the shelf in certain
situations, and it needs to know shelf alignment for positioning other windows.
ash::wm::WmShelf is the shelf api needed by both the window management code in
ash and mash. This class, WmShelfMus, is that implementation for mash.

ash contains at least two sets of functionality:
. window management.
. creating various shell widgets.

The mash wm wants the window management from ash, but not the rest of it.
Additionally the window management code in mash should be in terms of
mus::Window, not aura::Window. I refactored the window management code in ash so
that it is not in terms of aura::Window and is self contained. By self contained
I mean the code does not depend upon the rest of ash or aura::Window. This code
lives in ash/wm/common. The window management code defines a handful of pure
virtual classes. The implementation for ash in terms of aura lives in
ash/wm/aura. The implementation of these classes for mash lives in
mash/wm/bridge. If you think of the dependencies of ash and mash_wm,
ash/wm/common is a piece that both depend upon with very minimal dependencies it
self (
https://chromium.googlesource.com/chromium/src/+/master/ash/wm/common/DEPS ).

So, WmShelfMus is the implementation of the shelf api needed by the common
window management code for mash. As to should this be a mojom. The porting
layer, WmShelf, shouldn't be a mojom, but as you say, this class will need to
communicate via a mojom.

When mash is shipping we can nuke the porting classes (such as
WmShelf/WmShelfMus/WmShelfAura) and have the common window management code talk
directly to mus/mash types.

Make sense?

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus...
mash/wm/bridge/wm_shelf_mus.h:39: base::ObserverList<ash::wm::WmShelfObserver>
observers_;
On 2016/05/18 00:37:30, msw wrote:
> Make these private (here or above).

Done.

[msw, 2016-05-18 17:25:35]

lgtm

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus.h
File mash/wm/bridge/wm_shelf_mus.h (right):

https://codereview.chromium.org/1991553002/diff/1/mash/wm/bridge/wm_shelf_mus...
mash/wm/bridge/wm_shelf_mus.h:27: // ash::wm::WmShelf:
On 2016/05/18 16:18:32, sky wrote:
> On 2016/05/18 00:37:30, msw wrote:
> > Hmm, I'm trying to understand why this is needed in addition to
> > ash/shelf/shelf.h... Shouldn't that Shelf class itself support either window
> > type for use by the ash or mash wm? (isn't the mash wm part of ash_sysui? if
> > not, wouldn't this need to be a mojo interface like ShelfController?) Sorry,
> I'm
> > not up to speed here.
> 
> The mash wm (aka desktop_wm) is not part of ash_sysui.
> 
> As part of window management mash wm needs to know and communicate with the
> shelf. For example, the mash wm may need to hide the shelf in certain
> situations, and it needs to know shelf alignment for positioning other
windows.
> ash::wm::WmShelf is the shelf api needed by both the window management code in
> ash and mash. This class, WmShelfMus, is that implementation for mash.
> 
> ash contains at least two sets of functionality:
> . window management.
> . creating various shell widgets.
> 
> The mash wm wants the window management from ash, but not the rest of it.
> Additionally the window management code in mash should be in terms of
> mus::Window, not aura::Window. I refactored the window management code in ash
so
> that it is not in terms of aura::Window and is self contained. By self
contained
> I mean the code does not depend upon the rest of ash or aura::Window. This
code
> lives in ash/wm/common. The window management code defines a handful of pure
> virtual classes. The implementation for ash in terms of aura lives in
> ash/wm/aura. The implementation of these classes for mash lives in
> mash/wm/bridge. If you think of the dependencies of ash and mash_wm,
> ash/wm/common is a piece that both depend upon with very minimal dependencies
it
> self (
> https://chromium.googlesource.com/chromium/src/+/master/ash/wm/common/DEPS ).
> 
> So, WmShelfMus is the implementation of the shelf api needed by the common
> window management code for mash. As to should this be a mojom. The porting
> layer, WmShelf, shouldn't be a mojom, but as you say, this class will need to
> communicate via a mojom.
> 
> When mash is shipping we can nuke the porting classes (such as
> WmShelf/WmShelfMus/WmShelfAura) and have the common window management code
talk
> directly to mus/mash types.
> 
> Make sense?

That does make sense, thanks for the thoughtful explanation.

[sky, 2016-05-18 18:29:13]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-05-18 18:30:06]

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

[commit-bot: I haz the power, 2016-05-18 19:17:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-18 19:18:36]

Description was changed from

==========
Creates mus implementation of WmShelf

At this point it's just a stub.

BUG=603369
TEST=none
R=msw@chromium.org
==========

to

==========
Creates mus implementation of WmShelf

At this point it's just a stub.

BUG=603369
TEST=none
R=msw@chromium.org

Committed: https://crrev.com/88028961efbe33410aa37e21160bf2bdbcec0a8b
Cr-Commit-Position: refs/heads/master@{#394497}
==========

[commit-bot: I haz the power, 2016-05-18 19:18:37]

Patchset 3 (id:??) landed as
https://crrev.com/88028961efbe33410aa37e21160bf2bdbcec0a8b
Cr-Commit-Position: refs/heads/master@{#394497}