chromeos: Renames WmWindowAura to WmWindow

chromeos: Renames WmWindowAura to WmWindow

BUG=671246
TEST=none
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2629643002
Cr-Commit-Position: refs/heads/master@{#443408}
Committed: https://chromium.googlesource.com/chromium/src/+/5dbc91d59a8f1fbfd4987f8539ee032fe29481c3

[sky, 2017-01-12 18:13:46]

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

https://codereview.chromium.org/2629643002/diff/40001/ash/common/wm_window.h#...
ash/common/wm_window.h:91: static aura::Window* GetAuraWindow(WmWindow*
wm_window) {
I started trying to remove this, and realized as this function deals with null I
can't just bulk replace with window->aura_window(). So, for the time being I
left it. I'll deal with it later.

[sky, 2017-01-12 18:13:51]

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

[sky, 2017-01-12 18:14:26]

Description was changed from

==========
Renames WmWindowAura to WmWindow

BUG=671246
TEST=none
R=jamescook@chromium.org
==========

to

==========
chromeos: Renames WmWindowAura to WmWindow

BUG=671246
TEST=none
R=jamescook@chromium.org
==========

[sky, 2017-01-12 18:14:26]

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

[sky, 2017-01-12 18:14:37]

+reveman for exo

[commit-bot: I haz the power, 2017-01-12 18:14:42]

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

[James Cook, 2017-01-12 18:18:37]

Are you blocked on this? I can look later this afternoon if that's OK.

[sky, 2017-01-12 18:25:25]

I'm not blocked. Take your time.

  -Scott

On Thu, Jan 12, 2017 at 10:18 AM,  <jamescook@chromium.org> wrote:
> Are you blocked on this? I can look later this afternoon if that's OK.
>
>
> https://codereview.chromium.org/2629643002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[reveman, 2017-01-12 19:03:26]

components/exo lgtm

[commit-bot: I haz the power, 2017-01-12 19:18:46]

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

[commit-bot: I haz the power, 2017-01-12 19:18:48]

Dry run: This issue passed the CQ dry run.

[James Cook, 2017-01-12 20:45:57]

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

https://codereview.chromium.org/2629643002/diff/40001/ash/common/wm_window.h#...
ash/common/wm_window.h:61: // static Get() function to obtain a WmWindow from an
aura::Window.
nit: I would add a note explaining why this class exists (instead of using
aura::Window directly).

You might also comment on lifetime wrt window property and Destroy() method.

https://codereview.chromium.org/2629643002/diff/40001/ash/common/wm_window.h#...
ash/common/wm_window.h:91: static aura::Window* GetAuraWindow(WmWindow*
wm_window) {
On 2017/01/12 18:13:46, sky wrote:
> I started trying to remove this, and realized as this function deals with null
I
> can't just bulk replace with window->aura_window(). So, for the time being I
> left it. I'll deal with it later.

Acknowledged.

https://codereview.chromium.org/2629643002/diff/40001/ash/common/wm_window.h#...
ash/common/wm_window.h:368: protected:
Are there still subclasses of WmWindow? WmWindowMus went away, right? So why do
we need a protected section?

https://codereview.chromium.org/2629643002/diff/40001/ash/common/wm_window.h#...
ash/common/wm_window.h:372: static bool HasInstance(const aura::Window* window);
Does this have callers outside the now-deleted WmWindowMus?

https://codereview.chromium.org/2629643002/diff/40001/ash/mus/accelerators/ac...
File ash/mus/accelerators/accelerator_controller_unittest.cc (right):

https://codereview.chromium.org/2629643002/diff/40001/ash/mus/accelerators/ac...
ash/mus/accelerators/accelerator_controller_unittest.cc:588:
WmWindow::Get(CreateTestWindow(gfx::Rect(5, 5, 20, 20)));
Is the WmWindow::Get() required here?

https://codereview.chromium.org/2629643002/diff/40001/ash/wm/drag_window_cont...
File ash/wm/drag_window_controller.cc (right):

https://codereview.chromium.org/2629643002/diff/40001/ash/wm/drag_window_cont...
ash/wm/drag_window_controller.cc:9: #include "ash/common/wm_window.h"
It doesn't look like WmWindow is used in this file.

https://codereview.chromium.org/2629643002/diff/40001/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/2629643002/diff/40001/ash/wm/window_util.cc#n...
ash/wm/window_util.cc:13: #include "ash/common/wm_window.h"
WmWindow not used?

[sky, 2017-01-12 20:58:19]

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

[sky, 2017-01-12 20:58:27]

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

https://codereview.chromium.org/2629643002/diff/40001/ash/common/wm_window.h#...
ash/common/wm_window.h:61: // static Get() function to obtain a WmWindow from an
aura::Window.
On 2017/01/12 20:45:56, James Cook wrote:
> nit: I would add a note explaining why this class exists (instead of using
> aura::Window directly).
> 
> You might also comment on lifetime wrt window property and Destroy() method.

Done.

https://codereview.chromium.org/2629643002/diff/40001/ash/common/wm_window.h#...
ash/common/wm_window.h:368: protected:
On 2017/01/12 20:45:57, James Cook wrote:
> Are there still subclasses of WmWindow? WmWindowMus went away, right? So why
do
> we need a protected section?

Done.

https://codereview.chromium.org/2629643002/diff/40001/ash/common/wm_window.h#...
ash/common/wm_window.h:372: static bool HasInstance(const aura::Window* window);
On 2017/01/12 20:45:57, James Cook wrote:
> Does this have callers outside the now-deleted WmWindowMus?

Indeed you're right, it's not needed anymore. Removed (and observers() below).

https://codereview.chromium.org/2629643002/diff/40001/ash/mus/accelerators/ac...
File ash/mus/accelerators/accelerator_controller_unittest.cc (right):

https://codereview.chromium.org/2629643002/diff/40001/ash/mus/accelerators/ac...
ash/mus/accelerators/accelerator_controller_unittest.cc:588:
WmWindow::Get(CreateTestWindow(gfx::Rect(5, 5, 20, 20)));
On 2017/01/12 20:45:57, James Cook wrote:
> Is the WmWindow::Get() required here?

Nope. Removed.

https://codereview.chromium.org/2629643002/diff/40001/ash/wm/drag_window_cont...
File ash/wm/drag_window_controller.cc (right):

https://codereview.chromium.org/2629643002/diff/40001/ash/wm/drag_window_cont...
ash/wm/drag_window_controller.cc:9: #include "ash/common/wm_window.h"
On 2017/01/12 20:45:57, James Cook wrote:
> It doesn't look like WmWindow is used in this file.

Done.

https://codereview.chromium.org/2629643002/diff/40001/ash/wm/window_util.cc
File ash/wm/window_util.cc (right):

https://codereview.chromium.org/2629643002/diff/40001/ash/wm/window_util.cc#n...
ash/wm/window_util.cc:13: #include "ash/common/wm_window.h"
On 2017/01/12 20:45:57, James Cook wrote:
> WmWindow not used?

Done.

[sky, 2017-01-12 21:27:25]

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

[commit-bot: I haz the power, 2017-01-12 21:28:06]

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

[James Cook, 2017-01-12 22:17:13]

LGTM

[sky, 2017-01-12 22:19:58]

The CQ bit was unchecked by sky@chromium.org

[sky, 2017-01-12 22:20:05]

The CQ bit was checked by sky@chromium.org

[sky, 2017-01-12 22:20:06]

The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org
Link to the patchset: https://codereview.chromium.org/2629643002/#ps60001
(title: "feedback")

[commit-bot: I haz the power, 2017-01-12 22:20:19]

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, 2017-01-12 23:34:17]

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

[commit-bot: I haz the power, 2017-01-12 23:34:18]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[sky, 2017-01-12 23:37:40]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2017-01-12 23:38:37]

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, 2017-01-13 00:26:18]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484264260609260,
"parent_rev": "8c4dae156ba6495186bebe76aa4be4dc76bd7cd5", "commit_rev":
"5dbc91d59a8f1fbfd4987f8539ee032fe29481c3"}

[commit-bot: I haz the power, 2017-01-13 00:26:48]

Description was changed from

==========
chromeos: Renames WmWindowAura to WmWindow

BUG=671246
TEST=none
R=jamescook@chromium.org
==========

to

==========
chromeos: Renames WmWindowAura to WmWindow

BUG=671246
TEST=none
R=jamescook@chromium.org

Review-Url: https://codereview.chromium.org/2629643002
Cr-Commit-Position: refs/heads/master@{#443408}
Committed:
https://chromium.googlesource.com/chromium/src/+/5dbc91d59a8f1fbfd4987f8539ee...
==========

[commit-bot: I haz the power, 2017-01-13 00:26:50]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/5dbc91d59a8f1fbfd4987f8539ee...