mash: Convert FocusCycler to wm common types and move to //ash/common

mash: Convert FocusCycler to wm common types and move to //ash/common

FocusCycler is a dependency of the status area widget code I'm trying to move.

* Introduce WmWindow::GetWidget().
* Move FocusCycler ownership from ash::Shell to WmShell.
* Eliminate extraneous Shell::RotateFocus() method in favor of calling
FocusCycler directly.

I'm intentionally leaving the unit test in place, as it has dependencies that
haven't migrated yet.

BUG=619636
TEST=ash_unittests

Committed: https://crrev.com/26f32092680910d571ea398888263cb4f01f1e10
Cr-Commit-Position: refs/heads/master@{#400354}

[James Cook, 2016-06-16 23:03:44]

Description was changed from

==========
mash: Convert FocusCycler to wm common types and move to //ash/common

FocusCycler is a dependency of the status area widget code I'm trying to move.

* Introduce WmWindow::GetWidget().
* Move FocusCycler ownership from ash::Shell to WmShell.

I'm intentionally leaving the unit test in place, as it has dependencies that
haven't migrated yet.

BUG=619636
TEST=ash_unittests
==========

to

==========
mash: Convert FocusCycler to wm common types and move to //ash/common

FocusCycler is a dependency of the status area widget code I'm trying to move.

* Introduce WmWindow::GetWidget().
* Move FocusCycler ownership from ash::Shell to WmShell.
* Eliminate extraneous Shell::RotateFocus() method in favor of calling
FocusCycler directly.

I'm intentionally leaving the unit test in place, as it has dependencies that
haven't migrated yet.

BUG=619636
TEST=ash_unittests
==========

[James Cook, 2016-06-16 23:08:50]

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

[James Cook, 2016-06-16 23:08:51]

msw, please take a look.

[msw, 2016-06-16 23:57:44]

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

[msw, 2016-06-16 23:57:45]

lgtm with nits; +sky for any WmWindow::GetWidget() thoughts.

https://codereview.chromium.org/2075923002/diff/20001/ash/aura/wm_window_aura.cc
File ash/aura/wm_window_aura.cc (right):

https://codereview.chromium.org/2075923002/diff/20001/ash/aura/wm_window_aura...
ash/aura/wm_window_aura.cc:521: void WmWindowAura::CloseWidget() {
nit: Remove this and inline GetWidget()->Close() for the one caller.

https://codereview.chromium.org/2075923002/diff/20001/ash/common/wm_window.h
File ash/common/wm_window.h (right):

https://codereview.chromium.org/2075923002/diff/20001/ash/common/wm_window.h#...
ash/common/wm_window.h:213: virtual views::Widget* GetWidget() = 0;
Hmm, Scott may have some advice here; but I guess it seems okay to me.
(I wonder if it would be helpful to return the mus wm frame widget...)

https://codereview.chromium.org/2075923002/diff/20001/ash/focus_cycler_unitte...
File ash/focus_cycler_unittest.cc (right):

https://codereview.chromium.org/2075923002/diff/20001/ash/focus_cycler_unitte...
ash/focus_cycler_unittest.cc:62: // TODO(jamescook): Migrate this test to
//ash/common after the status area
nit: cite a bug?

[sky, 2016-06-17 00:08:30]

LGTM

https://codereview.chromium.org/2075923002/diff/20001/ash/common/wm_window.h
File ash/common/wm_window.h (right):

https://codereview.chromium.org/2075923002/diff/20001/ash/common/wm_window.h#...
ash/common/wm_window.h:213: virtual views::Widget* GetWidget() = 0;
On 2016/06/16 23:57:44, msw wrote:
> Hmm, Scott may have some advice here; but I guess it seems okay to me.
> (I wonder if it would be helpful to return the mus wm frame widget...)

I'm ok with this, but I recommend a clearer name. WmWindowMus calls this type of
widget 'internal'. GetInternalWidget? If we need this to return either type,
then GetWidget() is fine.

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

Thanks for the quick review!

https://codereview.chromium.org/2075923002/diff/20001/ash/aura/wm_window_aura.cc
File ash/aura/wm_window_aura.cc (right):

https://codereview.chromium.org/2075923002/diff/20001/ash/aura/wm_window_aura...
ash/aura/wm_window_aura.cc:521: void WmWindowAura::CloseWidget() {
On 2016/06/16 23:57:44, msw wrote:
> nit: Remove this and inline GetWidget()->Close() for the one caller.

I'm going to keep this, since there is a difference in CloseWidget() behavior
for WmWindowMus and I'm changing GetWidget() to GetInternalWidget().

https://codereview.chromium.org/2075923002/diff/20001/ash/common/wm_window.h
File ash/common/wm_window.h (right):

https://codereview.chromium.org/2075923002/diff/20001/ash/common/wm_window.h#...
ash/common/wm_window.h:213: virtual views::Widget* GetWidget() = 0;
On 2016/06/17 00:08:29, sky wrote:
> On 2016/06/16 23:57:44, msw wrote:
> > Hmm, Scott may have some advice here; but I guess it seems okay to me.
> > (I wonder if it would be helpful to return the mus wm frame widget...)
> 
> I'm ok with this, but I recommend a clearer name. WmWindowMus calls this type
of
> widget 'internal'. GetInternalWidget? If we need this to return either type,
> then GetWidget() is fine.

I only need the internal widget, so I went with GetInternalWidget().

https://codereview.chromium.org/2075923002/diff/20001/ash/focus_cycler_unitte...
File ash/focus_cycler_unittest.cc (right):

https://codereview.chromium.org/2075923002/diff/20001/ash/focus_cycler_unitte...
ash/focus_cycler_unittest.cc:62: // TODO(jamescook): Migrate this test to
//ash/common after the status area
On 2016/06/16 23:57:44, msw wrote:
> nit: cite a bug?

Done.

[James Cook, 2016-06-17 03:46:28]

The CQ bit was checked by jamescook@chromium.org

[James Cook, 2016-06-17 03:46:29]

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/2075923002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-06-17 03:46:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2075923002/60001

[commit-bot: I haz the power, 2016-06-17 04:27:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-17 04:28:30]

Description was changed from

==========
mash: Convert FocusCycler to wm common types and move to //ash/common

FocusCycler is a dependency of the status area widget code I'm trying to move.

* Introduce WmWindow::GetWidget().
* Move FocusCycler ownership from ash::Shell to WmShell.
* Eliminate extraneous Shell::RotateFocus() method in favor of calling
FocusCycler directly.

I'm intentionally leaving the unit test in place, as it has dependencies that
haven't migrated yet.

BUG=619636
TEST=ash_unittests
==========

to

==========
mash: Convert FocusCycler to wm common types and move to //ash/common

FocusCycler is a dependency of the status area widget code I'm trying to move.

* Introduce WmWindow::GetWidget().
* Move FocusCycler ownership from ash::Shell to WmShell.
* Eliminate extraneous Shell::RotateFocus() method in favor of calling
FocusCycler directly.

I'm intentionally leaving the unit test in place, as it has dependencies that
haven't migrated yet.

BUG=619636
TEST=ash_unittests

Committed: https://crrev.com/26f32092680910d571ea398888263cb4f01f1e10
Cr-Commit-Position: refs/heads/master@{#400354}
==========

[commit-bot: I haz the power, 2016-06-17 04:28:32]

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