Wires up WmWindowMus::SetBoundsInScreen()

Wires up WmWindowMus::SetBoundsInScreen()

As part of this I realized SetLockedToRoot and
SetDescendantsStayInSameRootWindow meant the same thing, so I nuked
the later.

In order to test this I need multiple display support, which isn't
quite working yet (in particular shutdown is currently
problematic).

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

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

[sky, 2016-09-15 03:54:30]

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

[commit-bot: I haz the power, 2016-09-15 03:54:51]

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-09-15 04:39:54]

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

[commit-bot: I haz the power, 2016-09-15 04:39:55]

Dry run: This issue passed the CQ dry run.

[James Cook, 2016-09-15 16:31:41]

Just nits.

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

https://codereview.chromium.org/2344783002/diff/20001/ash/aura/wm_window_aura...
ash/aura/wm_window_aura.cc:559: return
window_->GetProperty(kStayInSameRootWindowKey);
It's a little weird that many of the functions are now called LockedToRoot, but
the window property is still StayInSameRootWindow and some of the utility
functions reference StayInSameRootWindow. Can you change things to be consistent
everywhere?

https://codereview.chromium.org/2344783002/diff/20001/ash/common/wm/window_po...
File ash/common/wm/window_positioning_utils.cc (right):

https://codereview.chromium.org/2344783002/diff/20001/ash/common/wm/window_po...
ash/common/wm/window_positioning_utils.cc:56: gfx::Rect parent_bounds_in_screen
= transient_child->GetBoundsInScreen();
Another naming nit/question: Does this have anything to do with a parent? It
looks like it just moves the child between containers and resets it to have the
same |bounds_in_screen|. Is this just a bad name (I see it in the old code), or
am I missing something?

https://codereview.chromium.org/2344783002/diff/20001/ash/common/wm/window_po...
ash/common/wm/window_positioning_utils.cc:127: const gfx::Rect& bounds,
How about |bounds_in_screen|? Coordinate systems have been driving me bonkers
recently.

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

https://codereview.chromium.org/2344783002/diff/20001/ash/common/wm_window.h#...
ash/common/wm_window.h:207: //   display if allowed (see SetLockedToRoot()).
Thanks for collapsing these two.

https://codereview.chromium.org/2344783002/diff/20001/ash/mus/bridge/wm_windo...
File ash/mus/bridge/wm_window_mus.cc (right):

https://codereview.chromium.org/2344783002/diff/20001/ash/mus/bridge/wm_windo...
ash/mus/bridge/wm_window_mus.cc:554: DCHECK(GetParent());  // Aura code assumed
a client, so this does too.
"a client"?  A child?

https://codereview.chromium.org/2344783002/diff/20001/ash/mus/bridge/wm_windo...
ash/mus/bridge/wm_window_mus.cc:555: if (static_cast<const
WmWindowMus*>(GetParent())
Any particular reason this casts to const? (My aging eyes mistook it for a
const_cast at first.)

[sky, 2016-09-15 17:35:03]

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

https://codereview.chromium.org/2344783002/diff/20001/ash/aura/wm_window_aura...
ash/aura/wm_window_aura.cc:559: return
window_->GetProperty(kStayInSameRootWindowKey);
On 2016/09/15 16:31:40, James Cook wrote:
> It's a little weird that many of the functions are now called LockedToRoot,
but
> the window property is still StayInSameRootWindow and some of the utility
> functions reference StayInSameRootWindow. Can you change things to be
consistent
> everywhere?

I went with LockedToRoot everywhere.

https://codereview.chromium.org/2344783002/diff/20001/ash/common/wm/window_po...
File ash/common/wm/window_positioning_utils.cc (right):

https://codereview.chromium.org/2344783002/diff/20001/ash/common/wm/window_po...
ash/common/wm/window_positioning_utils.cc:56: gfx::Rect parent_bounds_in_screen
= transient_child->GetBoundsInScreen();
On 2016/09/15 16:31:40, James Cook wrote:
> Another naming nit/question: Does this have anything to do with a parent? It
> looks like it just moves the child between containers and resets it to have
the
> same |bounds_in_screen|. Is this just a bad name (I see it in the old code),
or
> am I missing something?

You're right. Updated.

https://codereview.chromium.org/2344783002/diff/20001/ash/common/wm/window_po...
ash/common/wm/window_positioning_utils.cc:127: const gfx::Rect& bounds,
On 2016/09/15 16:31:40, James Cook wrote:
> How about |bounds_in_screen|? Coordinate systems have been driving me bonkers
> recently.

Done.

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

https://codereview.chromium.org/2344783002/diff/20001/ash/common/wm_window.h#...
ash/common/wm_window.h:207: //   display if allowed (see SetLockedToRoot()).
On 2016/09/15 16:31:40, James Cook wrote:
> Thanks for collapsing these two.

It was my mistake for introducing the two in the first place!

https://codereview.chromium.org/2344783002/diff/20001/ash/mus/bridge/wm_windo...
File ash/mus/bridge/wm_window_mus.cc (right):

https://codereview.chromium.org/2344783002/diff/20001/ash/mus/bridge/wm_windo...
ash/mus/bridge/wm_window_mus.cc:554: DCHECK(GetParent());  // Aura code assumed
a client, so this does too.
On 2016/09/15 16:31:40, James Cook wrote:
> "a client"?  A child?

parent actually. Updated.

https://codereview.chromium.org/2344783002/diff/20001/ash/mus/bridge/wm_windo...
ash/mus/bridge/wm_window_mus.cc:555: if (static_cast<const
WmWindowMus*>(GetParent())
On 2016/09/15 16:31:40, James Cook wrote:
> Any particular reason this casts to const? (My aging eyes mistook it for a
> const_cast at first.)

Actually, yes. WmWindow has the inline implementation of the non-const version
(as suggested in effective c++):
  WmWindow* GetParent() {
    return const_cast<WmWindow*>(
        const_cast<const WmWindow*>(this)->GetParent());
  }

and the const version if pure virtual:
  virtual const WmWindow* GetParent() const = 0;

A ramification of this is that in a subclass (WmWindowMus or WmWindowAura) when
you call GetParent() it's resolved to the const version as that is what the
class declares. So, if I don't do the static_cast to const here I get:

../../ash/mus/bridge/wm_window_mus.cc:555:7: error: static_cast from 'const
ash::WmWindow *' to 'ash::mus::WmWindowMus *' casts away qualifiers
  if (static_cast<WmWindowMus*>(GetParent())

[sky, 2016-09-15 17:35:15]

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

[commit-bot: I haz the power, 2016-09-15 17:35:39]

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-09-15 18:20:29]

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

[commit-bot: I haz the power, 2016-09-15 18:20:30]

Dry run: This issue passed the CQ dry run.

[James Cook, 2016-09-15 21:06:50]

LGTM

[sky, 2016-09-15 21:58:33]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2016-09-15 21:58:52]

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-09-15 22:14:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-15 22:17:45]

Description was changed from

==========
Wires up WmWindowMus::SetBoundsInScreen()

As part of this I realized SetLockedToRoot and
SetDescendantsStayInSameRootWindow meant the same thing, so I nuked
the later.

In order to test this I need multiple display support, which isn't
quite working yet (in particular shutdown is currently
problematic).

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

to

==========
Wires up WmWindowMus::SetBoundsInScreen()

As part of this I realized SetLockedToRoot and
SetDescendantsStayInSameRootWindow meant the same thing, so I nuked
the later.

In order to test this I need multiple display support, which isn't
quite working yet (in particular shutdown is currently
problematic).

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

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

[commit-bot: I haz the power, 2016-09-15 22:17:45]

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