mus: Fix ash::GetRootWindowController for child windows/widgets

mus: Fix ash::GetRootWindowController for child windows/widgets

Under mus there is one RootWindow per top-level widget, similar to desktop
aura. This means we can't just use the aura::Window's root to find the
RootWindowController -- sometimes it will be null. Instead we need to find
the display's root window's RootWindowController.

This has been a cause of crashes under mus. I looked into refactoring the
callers of GetRootWindowController(), but there are ~100 places in the code
that call into it, either directly or indirectly, and it seemed better to
fix it here.

BUG=607755
TEST=RootWindowControolerTest.GetRootWindowController, existing ash_unittests

Committed: https://crrev.com/5d74ac0d16386f72e1426107d78aba51c221c230
Cr-Commit-Position: refs/heads/master@{#393337}

[James Cook, 2016-05-11 21:16:58]

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

[James Cook, 2016-05-11 21:16:59]

oshima, please take a look (sky is on vacation).

We don't have ash_unittests running yet under mus, but I've verified the
behavior manually with mus ash, and once we get the tests running
RootWindowControllerTest.GetRootWindowController will exercise the behavior I
added.

[oshima, 2016-05-12 00:40:56]

https://codereview.chromium.org/1975433002/diff/1/ash/root_window_controller.cc
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/1975433002/diff/1/ash/root_window_controller....
ash/root_window_controller.cc:1073: return
GetRootWindowSettings(root_window)->controller;
This behaves differently if the most of the root window is located outside of
the root window. I *think* this will not happen in *ash UI, although I'm not
100% sure.

I wonder if we should at least use the transient parent's root window?

[James Cook, 2016-05-12 16:43:12]

https://codereview.chromium.org/1975433002/diff/1/ash/root_window_controller.cc
File ash/root_window_controller.cc (right):

https://codereview.chromium.org/1975433002/diff/1/ash/root_window_controller....
ash/root_window_controller.cc:1073: return
GetRootWindowSettings(root_window)->controller;
On 2016/05/12 00:40:56, oshima wrote:
> This behaves differently if the most of the root window is located outside of
> the root window. I *think* this will not happen in *ash UI, although I'm not
> 100% sure.
> 
> I wonder if we should at least use the transient parent's root window?

I'm sorry, I don't completely understand.

I understand in mus ash that if a widget's window is mostly outside the root
window for display A it might choose display B's root window. However, if
|root_window| really is the root of the display then it should return the
correct RootWindowController, right? Or can the display's root window be mostly
outside the display?  Should I use the origin of the window?

I don't understand what you're suggesting about transient parents. Since this is
a "root window" on mus it should be the top-level aura window for a widget. I'm
not sure how the transient parent helps.

The behavior here in classic ash is identical to the old code.

[James Cook, 2016-05-12 18:26:26]

oshima, PTAL

(Talked offline. The correct behavior depends on which display "owns" the window
in
mus, which isn't clear yet for windows that partially overlap multiple displays.
Likewise, a "RootWindowController" is a weird concept in mus ash, since there
are
many more roots. For mus we probably need something different to do actions like
look up the shelf for a display. For now we're just going to return the
RootWindowController for the primary display, until mus multi-display support is
further along.)

[oshima, 2016-05-12 18:40:42]

lgtm

[James Cook, 2016-05-12 19:13:26]

The CQ bit was checked by jamescook@chromium.org

[commit-bot: I haz the power, 2016-05-12 19:13:47]

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

[commit-bot: I haz the power, 2016-05-12 19:57:22]

Committed patchset #2 (id:20001)

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

Description was changed from

==========
mus: Fix ash::GetRootWindowController for child windows/widgets

Under mus there is one RootWindow per top-level widget, similar to desktop
aura. This means we can't just use the aura::Window's root to find the
RootWindowController -- sometimes it will be null. Instead we need to find
the display's root window's RootWindowController.

This has been a cause of crashes under mus. I looked into refactoring the
callers of GetRootWindowController(), but there are ~100 places in the code
that call into it, either directly or indirectly, and it seemed better to
fix it here.

BUG=607755
TEST=RootWindowControolerTest.GetRootWindowController, existing ash_unittests
==========

to

==========
mus: Fix ash::GetRootWindowController for child windows/widgets

Under mus there is one RootWindow per top-level widget, similar to desktop
aura. This means we can't just use the aura::Window's root to find the
RootWindowController -- sometimes it will be null. Instead we need to find
the display's root window's RootWindowController.

This has been a cause of crashes under mus. I looked into refactoring the
callers of GetRootWindowController(), but there are ~100 places in the code
that call into it, either directly or indirectly, and it seemed better to
fix it here.

BUG=607755
TEST=RootWindowControolerTest.GetRootWindowController, existing ash_unittests

Committed: https://crrev.com/5d74ac0d16386f72e1426107d78aba51c221c230
Cr-Commit-Position: refs/heads/master@{#393337}
==========

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

Patchset 2 (id:??) landed as
https://crrev.com/5d74ac0d16386f72e1426107d78aba51c221c230
Cr-Commit-Position: refs/heads/master@{#393337}