mash: Create and show a shelf in mash.

mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Call WmShell::ShutDown before destruction in WindowManager.
Teardown PointerWatcherEventRouter after window destruction.

BUG=557406, 612631, 615155, 621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org,sky@chromium.org

Committed: https://crrev.com/ccb5d69efde4c915618f8334fa4a5231a09d3bb7
Cr-Commit-Position: refs/heads/master@{#412630}

[msw, 2016-08-15 22:39:43]

Description was changed from

==========
mash: WIP DO NOT COMMIT Create and show a shelf in mash.

BUG=557406,615155
==========

to

==========
mash: WIP DO NOT COMMIT Create and show a shelf in mash.

BUG=557406,612631,615155,
==========

[msw, 2016-08-15 23:05:25]

Description was changed from

==========
mash: WIP DO NOT COMMIT Create and show a shelf in mash.

BUG=557406,612631,615155,
==========

to

==========
mash: WIP DO NOT COMMIT Create and show a shelf in mash.

BUG=557406,612631,615155,621112
==========

[msw, 2016-08-16 00:31:46]

Description was changed from

==========
mash: WIP DO NOT COMMIT Create and show a shelf in mash.

BUG=557406,612631,615155,621112
==========

to

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
(these are ash/common ready, just waiting on oshima's CL)

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.

BUG=557406,612631,615155,621112
==========

[msw, 2016-08-16 19:48:13]

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

[commit-bot: I haz the power, 2016-08-16 19:48:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2016-08-16 20:22:08]

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

[commit-bot: I haz the power, 2016-08-16 20:22:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2016-08-16 20:42:54]

Description was changed from

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
(these are ash/common ready, just waiting on oshima's CL)

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.

BUG=557406,612631,615155,621112
==========

to

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
Move Shelf ownership to WmShelf (eases Shelf deprecation).

WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org
==========

[msw, 2016-08-16 20:42:54]

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

[msw, 2016-08-16 20:44:07]

Hey James, please take a look; thanks!

[msw, 2016-08-16 20:44:13]

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

[commit-bot: I haz the power, 2016-08-16 20:44:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 21:15:47]

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

[commit-bot: I haz the power, 2016-08-16 21:15:49]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[James Cook, 2016-08-16 23:21:35]

https://codereview.chromium.org/2247503002/diff/120001/ash/ash.gyp
File ash/ash.gyp (left):

https://codereview.chromium.org/2247503002/diff/120001/ash/ash.gyp#oldcode144
ash/ash.gyp:144: 'common/shelf/shelf_icon_observer.h',
File not deleted in CL?

https://codereview.chromium.org/2247503002/diff/120001/ash/aura/wm_shelf_aura.cc
File ash/aura/wm_shelf_aura.cc (right):

https://codereview.chromium.org/2247503002/diff/120001/ash/aura/wm_shelf_aura...
ash/aura/wm_shelf_aura.cc:54: void
WmShelfAura::SetShelfLayoutManager(ShelfLayoutManager* manager) {
I suspect this only needs to be called once to set it and once to clear it. If
so, I would prefer separate Set and Clear methods with DCHECKs.

https://codereview.chromium.org/2247503002/diff/120001/ash/aura/wm_shelf_aura...
ash/aura/wm_shelf_aura.cc:64: bezel_event_handler_.reset(new
ShelfBezelEventHandler(this));
Do you need to reconstruct the auto_hide_event_filter_ if it existed before?

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/shelf...
File ash/common/shelf/shelf_view.h (left):

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/shelf...
ash/common/shelf/shelf_view.h:405: base::ObserverList<ShelfIconObserver>
observers_;
Yeah, I like getting rid of this. Good call.

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/wm_sh...
File ash/common/shelf/wm_shelf.h (right):

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/wm_sh...
ash/common/shelf/wm_shelf.h:42: }
Ugh, the SetShelf / SetShelfLayoutManager lifetime management is hard to
understand. Could you comment, either here or below, on the expected lifecycle
of ShelfLayoutManager, and who owns it?

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/wm_sh...
ash/common/shelf/wm_shelf.h:116: void SetKeyboardBoundsForTesting(const
gfx::Rect& bounds);
nit: Either keep the comment or rename to SetVirtualKeyboardBoundsForTesting

https://codereview.chromium.org/2247503002/diff/120001/ash/mus/bridge/wm_shel...
File ash/mus/bridge/wm_shelf_mus.cc (right):

https://codereview.chromium.org/2247503002/diff/120001/ash/mus/bridge/wm_shel...
ash/mus/bridge/wm_shelf_mus.cc:31: SetShelf(base::WrapUnique(shelf));
I know I said something different in person, but now that I see the code I would
prefer that Shelf have separate copies owned by ash::RootWindowController and
WmShelfMus. This will emphasize that classes outside of WmShelf control its
lifecycle.

https://codereview.chromium.org/2247503002/diff/120001/ash/shelf/shelf_view_u...
File ash/shelf/shelf_view_unittest.cc (right):

https://codereview.chromium.org/2247503002/diff/120001/ash/shelf/shelf_view_u...
ash/shelf/shelf_view_unittest.cc:90: int icon_positions_changed() const { return
icon_positions_changed_; }
return type bool

https://codereview.chromium.org/2247503002/diff/120001/ash/shelf/shelf_widget.cc
File ash/shelf/shelf_widget.cc (left):

https://codereview.chromium.org/2247503002/diff/120001/ash/shelf/shelf_widget...
ash/shelf/shelf_widget.cc:7: #include "ash/aura/wm_shelf_aura.h"
Awesome.

[msw, 2016-08-16 23:25:39]

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

[commit-bot: I haz the power, 2016-08-16 23:26:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 00:10:26]

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

[commit-bot: I haz the power, 2016-08-17 00:10:28]

Dry run: This issue passed the CQ dry run.

[msw, 2016-08-17 00:34:25]

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

[commit-bot: I haz the power, 2016-08-17 00:34:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2016-08-17 01:08:33]

Description was changed from

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
Move Shelf ownership to WmShelf (eases Shelf deprecation).

WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org
==========

to

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Workaround mash_unittests shutdown crashes with shelf:
-Check for WmShell instance in ShelfWindowWatcher dtor.
-Check for window_tree_client_ in PointerWatcherEventRouter.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org
==========

[msw, 2016-08-17 01:09:02]

Hey James, please take another look; thanks!

https://codereview.chromium.org/2247503002/diff/120001/ash/ash.gyp
File ash/ash.gyp (left):

https://codereview.chromium.org/2247503002/diff/120001/ash/ash.gyp#oldcode144
ash/ash.gyp:144: 'common/shelf/shelf_icon_observer.h',
On 2016/08/16 23:21:34, James Cook (slow reviews) wrote:
> File not deleted in CL?

Done.

https://codereview.chromium.org/2247503002/diff/120001/ash/aura/wm_shelf_aura.cc
File ash/aura/wm_shelf_aura.cc (right):

https://codereview.chromium.org/2247503002/diff/120001/ash/aura/wm_shelf_aura...
ash/aura/wm_shelf_aura.cc:54: void
WmShelfAura::SetShelfLayoutManager(ShelfLayoutManager* manager) {
On 2016/08/16 23:21:35, James Cook (slow reviews) wrote:
> I suspect this only needs to be called once to set it and once to clear it. If
> so, I would prefer separate Set and Clear methods with DCHECKs.

Done. (I wrote ClearShelfLayoutManager, but then decided it could simply be
inlined in WillDeleteShelfLayoutManager)

https://codereview.chromium.org/2247503002/diff/120001/ash/aura/wm_shelf_aura...
ash/aura/wm_shelf_aura.cc:64: bezel_event_handler_.reset(new
ShelfBezelEventHandler(this));
On 2016/08/16 23:21:34, James Cook (slow reviews) wrote:
> Do you need to reconstruct the auto_hide_event_filter_ if it existed before?

I don't think so; that's created on WillChangeVisibilityState.

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/shelf...
File ash/common/shelf/shelf_view.h (left):

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/shelf...
ash/common/shelf/shelf_view.h:405: base::ObserverList<ShelfIconObserver>
observers_;
On 2016/08/16 23:21:35, James Cook (slow reviews) wrote:
> Yeah, I like getting rid of this. Good call.

Acknowledged.

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/wm_sh...
File ash/common/shelf/wm_shelf.h (right):

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/wm_sh...
ash/common/shelf/wm_shelf.h:42: }
On 2016/08/16 23:21:35, James Cook (slow reviews) wrote:
> Ugh, the SetShelf / SetShelfLayoutManager lifetime management is hard to
> understand. Could you comment, either here or below, on the expected lifecycle
> of ShelfLayoutManager, and who owns it?

Done; I expanded on the local members' declaration comments.

https://codereview.chromium.org/2247503002/diff/120001/ash/common/shelf/wm_sh...
ash/common/shelf/wm_shelf.h:116: void SetKeyboardBoundsForTesting(const
gfx::Rect& bounds);
On 2016/08/16 23:21:35, James Cook (slow reviews) wrote:
> nit: Either keep the comment or rename to SetVirtualKeyboardBoundsForTesting

Done (renamed for self-explanatory name).

https://codereview.chromium.org/2247503002/diff/120001/ash/mus/bridge/wm_shel...
File ash/mus/bridge/wm_shelf_mus.cc (right):

https://codereview.chromium.org/2247503002/diff/120001/ash/mus/bridge/wm_shel...
ash/mus/bridge/wm_shelf_mus.cc:31: SetShelf(base::WrapUnique(shelf));
On 2016/08/16 23:21:35, James Cook (slow reviews) wrote:
> I know I said something different in person, but now that I see the code I
would
> prefer that Shelf have separate copies owned by ash::RootWindowController and
> WmShelfMus. This will emphasize that classes outside of WmShelf control its
> lifecycle.

Done.

https://codereview.chromium.org/2247503002/diff/120001/ash/shelf/shelf_view_u...
File ash/shelf/shelf_view_unittest.cc (right):

https://codereview.chromium.org/2247503002/diff/120001/ash/shelf/shelf_view_u...
ash/shelf/shelf_view_unittest.cc:90: int icon_positions_changed() const { return
icon_positions_changed_; }
On 2016/08/16 23:21:35, James Cook (slow reviews) wrote:
> return type bool

Done.

https://codereview.chromium.org/2247503002/diff/120001/ash/shelf/shelf_widget.cc
File ash/shelf/shelf_widget.cc (left):

https://codereview.chromium.org/2247503002/diff/120001/ash/shelf/shelf_widget...
ash/shelf/shelf_widget.cc:7: #include "ash/aura/wm_shelf_aura.h"
On 2016/08/16 23:21:35, James Cook (slow reviews) wrote:
> Awesome.

Acknowledged.

[commit-bot: I haz the power, 2016-08-17 01:24:25]

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

[commit-bot: I haz the power, 2016-08-17 01:24:26]

Dry run: This issue passed the CQ dry run.

[James Cook, 2016-08-17 03:29:39]

Code LG, just a couple questions.

https://codereview.chromium.org/2247503002/diff/100002/ash/common/shelf/shelf...
File ash/common/shelf/shelf_window_watcher.cc (right):

https://codereview.chromium.org/2247503002/diff/100002/ash/common/shelf/shelf...
ash/common/shelf/shelf_window_watcher.cc:105: if (WmShell::HasInstance())
Why is this needed? ShelfWindowWatcher is owned by WmShell and reset in
WmShell::Shutdown(), so I would expect there to always be a WmShell instance
around.

https://codereview.chromium.org/2247503002/diff/100002/ash/mus/bridge/wm_shel...
File ash/mus/bridge/wm_shelf_mus.cc (right):

https://codereview.chromium.org/2247503002/diff/100002/ash/mus/bridge/wm_shel...
ash/mus/bridge/wm_shelf_mus.cc:14: #include "base/memory/ptr_util.h"
not needed?

https://codereview.chromium.org/2247503002/diff/100002/ash/root_window_contro...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/2247503002/diff/100002/ash/root_window_contro...
ash/root_window_controller.cc:617: // CloseChildWindows may be called twice
during the shutdown of ash unittests.
Ugh. Thanks for documenting it.

https://codereview.chromium.org/2247503002/diff/100002/ui/views/mus/pointer_w...
File ui/views/mus/pointer_watcher_event_router.cc (right):

https://codereview.chromium.org/2247503002/diff/100002/ui/views/mus/pointer_w...
ui/views/mus/pointer_watcher_event_router.cc:50: if (window_tree_client_)
Where are we tearing down the window tree client before code removes its
PointerWatchers?

[msw, 2016-08-17 17:41:13]

Patchset #10 (id:170001) has been deleted

[msw, 2016-08-17 17:45:16]

Description was changed from

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Workaround mash_unittests shutdown crashes with shelf:
-Check for WmShell instance in ShelfWindowWatcher dtor.
-Check for window_tree_client_ in PointerWatcherEventRouter.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org
==========

to

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Call WmShell::ShutDown before destruction in WindowManager.
Teardown PointerWatcherEventRouter after window destruction.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org
==========

[msw, 2016-08-17 17:45:44]

Hey James, please take another look; thanks!

https://codereview.chromium.org/2247503002/diff/100002/ash/common/shelf/shelf...
File ash/common/shelf/shelf_window_watcher.cc (right):

https://codereview.chromium.org/2247503002/diff/100002/ash/common/shelf/shelf...
ash/common/shelf/shelf_window_watcher.cc:105: if (WmShell::HasInstance())
On 2016/08/17 03:29:39, James Cook (slow reviews) wrote:
> Why is this needed? ShelfWindowWatcher is owned by WmShell and reset in
> WmShell::Shutdown(), so I would expect there to always be a WmShell instance
> around.

Fixed instead by calling WmShell::Shutdown in WindowManager; thanks!
(seems like we should have been doing that all along)

https://codereview.chromium.org/2247503002/diff/100002/ash/mus/bridge/wm_shel...
File ash/mus/bridge/wm_shelf_mus.cc (right):

https://codereview.chromium.org/2247503002/diff/100002/ash/mus/bridge/wm_shel...
ash/mus/bridge/wm_shelf_mus.cc:14: #include "base/memory/ptr_util.h"
On 2016/08/17 03:29:39, James Cook (slow reviews) wrote:
> not needed?

Done.

https://codereview.chromium.org/2247503002/diff/100002/ash/root_window_contro...
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/2247503002/diff/100002/ash/root_window_contro...
ash/root_window_controller.cc:617: // CloseChildWindows may be called twice
during the shutdown of ash unittests.
On 2016/08/17 03:29:39, James Cook (slow reviews) wrote:
> Ugh. Thanks for documenting it.

Acknowledged. We can try to fix this separately.

https://codereview.chromium.org/2247503002/diff/100002/ui/views/mus/pointer_w...
File ui/views/mus/pointer_watcher_event_router.cc (right):

https://codereview.chromium.org/2247503002/diff/100002/ui/views/mus/pointer_w...
ui/views/mus/pointer_watcher_event_router.cc:50: if (window_tree_client_)
On 2016/08/17 03:29:39, James Cook (slow reviews) wrote:
> Where are we tearing down the window tree client before code removes its
> PointerWatchers?

The WindowTreeClient dtor (1) calls OnWillDestroyClient, which cleared
|window_tree_client_| (below at line 95), *then* (2) destroys windows like the
Shelf, where ShelfView's ShelfTooltipManager is a PointerWatcher, then (3) calls
OnDidDestroyClient. We should support Windows owning PointerWatcher objects, so
I changed PointerWatcherEventRouter to clear |window_tree_client_| in (3)
OnDidDestroyClient, after (2) destroying windows.

[msw, 2016-08-17 17:45:56]

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

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

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[James Cook, 2016-08-17 18:31:51]

LGTM. Nice patch -- it'll be good to see the shelf again.

https://codereview.chromium.org/2247503002/diff/190001/ui/views/mus/pointer_w...
File ui/views/mus/pointer_watcher_event_router.cc (right):

https://codereview.chromium.org/2247503002/diff/190001/ui/views/mus/pointer_w...
ui/views/mus/pointer_watcher_event_router.cc:47:
DCHECK(pointer_watchers_.HasObserver(watcher));
FYI - We'll have to see how this goes. I vaguely remember seeing some code that
could attempt to remove the same PointerWatcher twice. That said, removing twice
seems like a dumb thing to do, so I support the DCHECK.

[msw, 2016-08-17 18:54:19]

Description was changed from

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Call WmShell::ShutDown before destruction in WindowManager.
Teardown PointerWatcherEventRouter after window destruction.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org
==========

to

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Call WmShell::ShutDown before destruction in WindowManager.
Teardown PointerWatcherEventRouter after window destruction.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org,sky@chromium.org
==========

[msw, 2016-08-17 18:54:19]

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

[msw, 2016-08-17 18:55:17]

Scott, please take a look for ui/views/* OWNERS review.
Also, FYI on ash/mus/window_manager.cc, feel free to look at more.

https://codereview.chromium.org/2247503002/diff/190001/ui/views/mus/pointer_w...
File ui/views/mus/pointer_watcher_event_router.cc (right):

https://codereview.chromium.org/2247503002/diff/190001/ui/views/mus/pointer_w...
ui/views/mus/pointer_watcher_event_router.cc:47:
DCHECK(pointer_watchers_.HasObserver(watcher));
On 2016/08/17 18:31:50, James Cook (slow reviews) wrote:
> FYI - We'll have to see how this goes. I vaguely remember seeing some code
that
> could attempt to remove the same PointerWatcher twice. That said, removing
twice
> seems like a dumb thing to do, so I support the DCHECK.

Acknowledged.

[commit-bot: I haz the power, 2016-08-17 19:19:04]

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

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

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[sky, 2016-08-17 19:50:36]

LGTM

https://codereview.chromium.org/2247503002/diff/190001/ui/views/mus/pointer_w...
File ui/views/mus/pointer_watcher_event_router.cc (right):

https://codereview.chromium.org/2247503002/diff/190001/ui/views/mus/pointer_w...
ui/views/mus/pointer_watcher_event_router.cc:93:
DCHECK(!pointer_watchers_.might_have_observers());
I suspect we'll have to remove this, but we can try it. My concern is that
PointerWatcherEventRouter::OnDidDestroyClient may be called before other places
do their cleanup.

[msw, 2016-08-17 19:59:54]

The CQ bit was checked by msw@chromium.org

[commit-bot: I haz the power, 2016-08-17 20:00:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-17 20:38:13]

Description was changed from

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Call WmShell::ShutDown before destruction in WindowManager.
Teardown PointerWatcherEventRouter after window destruction.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org,sky@chromium.org
==========

to

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Call WmShell::ShutDown before destruction in WindowManager.
Teardown PointerWatcherEventRouter after window destruction.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org,sky@chromium.org
==========

[commit-bot: I haz the power, 2016-08-17 20:38:14]

Committed patchset #10 (id:190001)

[commit-bot: I haz the power, 2016-08-17 20:40:12]

Description was changed from

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Call WmShell::ShutDown before destruction in WindowManager.
Teardown PointerWatcherEventRouter after window destruction.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org,sky@chromium.org
==========

to

==========
mash: Create and show a shelf in mash.

Allow mash to use the shelf, widget, and layout manager.
Delay moving files to ease oshima's concurrent Shelf CL.

Consolidate WmShelf[Aura|Mus] functionality on WmShelf.
WmShelfMus ctor inits Shelf, ShelfView, ShelfWidget, etc.
Use a ShelfDelegateStub for now; add TODO for real impl.
WmShelfAura still uses root event handlers; shelf dimmer.

Nix ShelfIconObserver interface; use WmShelfObserver.
Nix redundant mash RootWindowController::CreateStatusArea.
Change ShelfWidget's WmShelfAura* member to WmShelf*.

Call WmShell::ShutDown before destruction in WindowManager.
Teardown PointerWatcherEventRouter after window destruction.

BUG=557406,612631,615155,621112
TEST=Automated; no cros changes; chrome --mash has a shelf.
R=jamescook@chromium.org,sky@chromium.org

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

[commit-bot: I haz the power, 2016-08-17 20:40:14]

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