aura-mus: Implement stacking in DesktopWindowTreeHostMus.

aura-mus: Implement DesktopWindowTreeHostMus::StackAtTop().

Adds API to the window server so that a client can request that one of
their windows gets stacked on top of everything else.

BUG=663617
Review-Url: https://codereview.chromium.org/2633233003
Cr-Commit-Position: refs/heads/master@{#445211}
Committed: https://chromium.googlesource.com/chromium/src/+/a77d1f27b7d151775ba412bd522344e060736d02

[Elliot Glaysher, 2017-01-17 22:06:07]

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

[commit-bot: I haz the power, 2017-01-17 22:06: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, 2017-01-17 23:03:19]

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

[commit-bot: I haz the power, 2017-01-17 23:03:20]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-01-18 21:02:07]

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

[commit-bot: I haz the power, 2017-01-18 21:03:29]

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

[Elliot Glaysher, 2017-01-18 21:14:44]

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

[commit-bot: I haz the power, 2017-01-18 21:15:26]

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

[Elliot Glaysher, 2017-01-18 21:26:19]

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

[commit-bot: I haz the power, 2017-01-18 21:26:49]

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, 2017-01-18 22:31:26]

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

[commit-bot: I haz the power, 2017-01-18 22:31:27]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-01-19 20:43:59]

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

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

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, 2017-01-19 20:55:59]

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

[commit-bot: I haz the power, 2017-01-19 20:56:00]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Elliot Glaysher, 2017-01-19 20:59:34]

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

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

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, 2017-01-19 21:57:38]

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

[commit-bot: I haz the power, 2017-01-19 21:57:38]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-01-19 22:06:07]

Description was changed from

==========
aura-mus: Implement stacking in DesktopWindowTreeHostMus.

BUG=663617
==========

to

==========
aura-mus: Implement DesktopWindowTreeHostMus::StackAtTop().

Adds API to the window server so that a client can request that one of
their windows gets stacked on top of everything else.

BUG=663617
==========

[Elliot Glaysher, 2017-01-19 22:06:14]

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

[Elliot Glaysher, 2017-01-19 22:06:22]

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

[commit-bot: I haz the power, 2017-01-19 22:06:56]

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

[sky, 2017-01-19 22:20:59]

https://codereview.chromium.org/2633233003/diff/120001/services/ui/public/int...
File services/ui/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2633233003/diff/120001/services/ui/public/int...
services/ui/public/interfaces/window_tree.mojom:267: // Stacks the window above
everything.
everything -> all sibling windows.

https://codereview.chromium.org/2633233003/diff/120001/services/ui/ws/window_...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2633233003/diff/120001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:1627: DVLOG(1) << "StackAtTop ignored (already at
top)";
As this isn't a failure (response is true), how about changing to 3. 3 better
matches similar places in this file.

https://codereview.chromium.org/2633233003/diff/120001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:1632: ServerWindow* relative_window =
parent->children().back();
Please call through to the accesspolicy. See line 1579 for an example. The
access policy should only allow this if the client doesn't own the window, but
the windowmanager does, and the implementation should redirect to the window
manager. Ash will then handle the reorder.

[Elliot Glaysher, 2017-01-19 22:52:56]

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

[commit-bot: I haz the power, 2017-01-19 22:53:57]

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

[Elliot Glaysher, 2017-01-20 01:03:44]

https://codereview.chromium.org/2633233003/diff/120001/services/ui/public/int...
File services/ui/public/interfaces/window_tree.mojom (right):

https://codereview.chromium.org/2633233003/diff/120001/services/ui/public/int...
services/ui/public/interfaces/window_tree.mojom:267: // Stacks the window above
everything.
On 2017/01/19 22:20:59, sky wrote:
> everything -> all sibling windows.

Done.

https://codereview.chromium.org/2633233003/diff/120001/services/ui/ws/window_...
File services/ui/ws/window_tree.cc (right):

https://codereview.chromium.org/2633233003/diff/120001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:1627: DVLOG(1) << "StackAtTop ignored (already at
top)";
On 2017/01/19 22:20:59, sky wrote:
> As this isn't a failure (response is true), how about changing to 3. 3 better
> matches similar places in this file.

Done.

https://codereview.chromium.org/2633233003/diff/120001/services/ui/ws/window_...
services/ui/ws/window_tree.cc:1632: ServerWindow* relative_window =
parent->children().back();
On 2017/01/19 22:20:59, sky wrote:
> Please call through to the accesspolicy. See line 1579 for an example. The
> access policy should only allow this if the client doesn't own the window, but
> the windowmanager does, and the implementation should redirect to the window
> manager. Ash will then handle the reorder.

I've added an access policy check, though per discussion, it isn't forwarding to
the window manager.

[commit-bot: I haz the power, 2017-01-20 01:06:25]

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

[commit-bot: I haz the power, 2017-01-20 01:06:26]

Dry run: This issue passed the CQ dry run.

[sky, 2017-01-20 16:36:11]

https://codereview.chromium.org/2633233003/diff/140001/services/ui/ws/default...
File services/ui/ws/default_access_policy.cc (right):

https://codereview.chromium.org/2633233003/diff/140001/services/ui/ws/default...
services/ui/ws/default_access_policy.cc:160: return
delegate_->HasRootForAccessPolicy(window);
Can you also add a function to the delegate that checks if the window is owned
by the windowmanager, and check that too?

https://codereview.chromium.org/2633233003/diff/140001/ui/views/mus/desktop_w...
File ui/views/mus/desktop_window_tree_host_mus_unittest.cc (right):

https://codereview.chromium.org/2633233003/diff/140001/ui/views/mus/desktop_w...
ui/views/mus/desktop_window_tree_host_mus_unittest.cc:129: if (type == type_ &&
success == success_) {
One potential problem with this pattern is that if there are already changes of
type in the queue this stops when the first one completes.

https://codereview.chromium.org/2633233003/diff/140001/ui/views/mus/mus_clien...
File ui/views/mus/mus_client_test_observer.h (right):

https://codereview.chromium.org/2633233003/diff/140001/ui/views/mus/mus_clien...
ui/views/mus/mus_client_test_observer.h:5: #ifndef
UI_VIEWS_MUS_MUS_CLIENT_TEST_OBSERVER_H_
Can you put this in ui/aura/mus; WindowTreeClientTestObserver and attach it
directly to the WindowTreeClient? That way we aren't adding a test only function
to WindowTreeClientDelegate.

[Elliot Glaysher, 2017-01-20 21:02:49]

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

[commit-bot: I haz the power, 2017-01-20 21:03:27]

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, 2017-01-20 22:02:52]

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

[commit-bot: I haz the power, 2017-01-20 22:02:53]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2017-01-20 22:30:49]

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

[Elliot Glaysher, 2017-01-20 22:31:03]

https://codereview.chromium.org/2633233003/diff/140001/services/ui/ws/default...
File services/ui/ws/default_access_policy.cc (right):

https://codereview.chromium.org/2633233003/diff/140001/services/ui/ws/default...
services/ui/ws/default_access_policy.cc:160: return
delegate_->HasRootForAccessPolicy(window);
On 2017/01/20 16:36:10, sky wrote:
> Can you also add a function to the delegate that checks if the window is owned
> by the windowmanager, and check that too?

Done.

https://codereview.chromium.org/2633233003/diff/140001/ui/views/mus/desktop_w...
File ui/views/mus/desktop_window_tree_host_mus_unittest.cc (right):

https://codereview.chromium.org/2633233003/diff/140001/ui/views/mus/desktop_w...
ui/views/mus/desktop_window_tree_host_mus_unittest.cc:129: if (type == type_ &&
success == success_) {
On 2017/01/20 16:36:10, sky wrote:
> One potential problem with this pattern is that if there are already changes
of
> type in the queue this stops when the first one completes.

Moved this to ui/aura/test/mus/, and made it listen for the first change_id of
|type| after construction.

https://codereview.chromium.org/2633233003/diff/140001/ui/views/mus/mus_clien...
File ui/views/mus/mus_client_test_observer.h (right):

https://codereview.chromium.org/2633233003/diff/140001/ui/views/mus/mus_clien...
ui/views/mus/mus_client_test_observer.h:5: #ifndef
UI_VIEWS_MUS_MUS_CLIENT_TEST_OBSERVER_H_
On 2017/01/20 16:36:10, sky wrote:
> Can you put this in ui/aura/mus; WindowTreeClientTestObserver and attach it
> directly to the WindowTreeClient? That way we aren't adding a test only
function
> to WindowTreeClientDelegate.

Done.

[commit-bot: I haz the power, 2017-01-20 22:31:10]

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

[sky, 2017-01-20 22:36:16]

LGTM

[Elliot Glaysher, 2017-01-20 23:24:40]

The CQ bit was unchecked by erg@chromium.org

[Elliot Glaysher, 2017-01-20 23:24:41]

The CQ bit was checked by erg@chromium.org

[commit-bot: I haz the power, 2017-01-20 23:24:57]

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-20 23:42:33]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1484954681526810,
"parent_rev": "02e248436f3736e6b85ab4c47452ce13611cdd11", "commit_rev":
"a77d1f27b7d151775ba412bd522344e060736d02"}

[commit-bot: I haz the power, 2017-01-20 23:43:03]

Description was changed from

==========
aura-mus: Implement DesktopWindowTreeHostMus::StackAtTop().

Adds API to the window server so that a client can request that one of
their windows gets stacked on top of everything else.

BUG=663617
==========

to

==========
aura-mus: Implement DesktopWindowTreeHostMus::StackAtTop().

Adds API to the window server so that a client can request that one of
their windows gets stacked on top of everything else.

BUG=663617

Review-Url: https://codereview.chromium.org/2633233003
Cr-Commit-Position: refs/heads/master@{#445211}
Committed:
https://chromium.googlesource.com/chromium/src/+/a77d1f27b7d151775ba412bd5223...
==========

[commit-bot: I haz the power, 2017-01-20 23:43:04]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/a77d1f27b7d151775ba412bd5223...

[Wez, 2017-02-08 01:04:29]

Description was changed from

==========
aura-mus: Implement DesktopWindowTreeHostMus::StackAtTop().

Adds API to the window server so that a client can request that one of
their windows gets stacked on top of everything else.

BUG=663617

Review-Url: https://codereview.chromium.org/2633233003
Cr-Commit-Position: refs/heads/master@{#445211}
Committed:
https://chromium.googlesource.com/chromium/src/+/a77d1f27b7d151775ba412bd5223...
==========

to

==========
aura-mus: Implement DesktopWindowTreeHostMus::StackAtTop().

Adds API to the window server so that a client can request that one of
their windows gets stacked on top of everything else.

BUG=663617

Review-Url: https://codereview.chromium.org/2633233003
Cr-Commit-Position: refs/heads/master@{#445211}
Committed:
https://chromium.googlesource.com/chromium/src/+/a77d1f27b7d151775ba412bd5223...
==========

[Wez, 2017-02-08 01:04:29]

wez@chromium.org changed reviewers:
+ wfh@chromium.org

[Wez, 2017-02-08 02:09:10]

Hallo wfh@chromium.org!
Due to a depot_tools patch which mistakenly removed the OWNERS check for
non-source files (see crbug.com/684270), the following files landed in this CL
and need a retrospective review from you:
	services/ui/public/interfaces/window_tree.mojom
Thanks,
Wez

[Will Harris, 2017-02-13 18:33:59]

retroactive lgtm