Makes mus not bring window to front on activation changes

Makes mus not bring window to front on activation changes

The window manager should be doing it. To do otherwise means mus is
likely conflicting with the wm. Additionally changing the activation
as we were meant clients weren't getting notified and got out of sync
with mus.

BUG=652421
TEST=covered by tests
R=msw@chromium.org

Committed: https://crrev.com/3bddafc6834c9487e6223168120f5de7ecbf1d2b
Cr-Commit-Position: refs/heads/master@{#422652}

[sky, 2016-10-03 23:32:43]

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

[commit-bot: I haz the power, 2016-10-03 23:34:07]

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

[msw, 2016-10-03 23:42:02]

some questions

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/display.cc
File services/ui/ws/display.cc (right):

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/display.cc#n...
services/ui/ws/display.cc:354: // Don't do anything here. We assume the
windowmanager handles restacking. If
nit: 'window manager'?

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/window_manag...
File services/ui/ws/window_manager_client_unittest.cc (left):

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/window_manag...
services/ui/ws/window_manager_client_unittest.cc:881:
EXPECT_GT(ValidIndexOf(parent->children(), child1),
Should we EXPECT_LT (or invert the args to match the check below), or is there
no expected outcome?

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/window_manag...
services/ui/ws/window_manager_client_unittest.cc:897:
EXPECT_GT(ValidIndexOf(parent->children(), child2),
Why is this still valid if the check above isn't?

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/window_manag...
File services/ui/ws/window_manager_client_unittest.cc (right):

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/window_manag...
services/ui/ws/window_manager_client_unittest.cc:883: // Set focus on |child21|.
This should activate |child2|, and raise it over
Is the 'raise it over |child1|.' comment still valid?

[sky, 2016-10-03 23:51:18]

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

[commit-bot: I haz the power, 2016-10-03 23:51:45]

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

[sky, 2016-10-03 23:53:29]

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/display.cc
File services/ui/ws/display.cc (right):

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/display.cc#n...
services/ui/ws/display.cc:354: // Don't do anything here. We assume the
windowmanager handles restacking. If
On 2016/10/03 23:42:01, msw wrote:
> nit: 'window manager'?

Done.

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/window_manag...
File services/ui/ws/window_manager_client_unittest.cc (left):

https://codereview.chromium.org/2391853002/diff/1/services/ui/ws/window_manag...
services/ui/ws/window_manager_client_unittest.cc:881:
EXPECT_GT(ValidIndexOf(parent->children(), child1),
On 2016/10/03 23:42:01, msw wrote:
> Should we EXPECT_LT (or invert the args to match the check below), or is there
> no expected outcome?

I've changed these around to be more explicit. The order shouldn't change.

[msw, 2016-10-03 23:59:47]

lgtm with nits

https://codereview.chromium.org/2391853002/diff/20001/services/ui/ws/window_m...
File services/ui/ws/window_manager_client_unittest.cc (right):

https://codereview.chromium.org/2391853002/diff/20001/services/ui/ws/window_m...
services/ui/ws/window_manager_client_unittest.cc:865: // NOTE: |child2| is after
|child1| as |child2| is a transient child of
nit: do you mean "after |child3|"?

https://codereview.chromium.org/2391853002/diff/20001/services/ui/ws/window_m...
services/ui/ws/window_manager_client_unittest.cc:870: // Set focus on |child11|,
order of windows should not change.
nit: child1

[sky, 2016-10-04 00:04:32]

https://codereview.chromium.org/2391853002/diff/20001/services/ui/ws/window_m...
File services/ui/ws/window_manager_client_unittest.cc (right):

https://codereview.chromium.org/2391853002/diff/20001/services/ui/ws/window_m...
services/ui/ws/window_manager_client_unittest.cc:865: // NOTE: |child2| is after
|child1| as |child2| is a transient child of
On 2016/10/03 23:59:47, msw wrote:
> nit: do you mean "after |child3|"?

GAH! You are right. I reordered this so that it goes 0, 1, 2 here and below.

https://codereview.chromium.org/2391853002/diff/20001/services/ui/ws/window_m...
services/ui/ws/window_manager_client_unittest.cc:870: // Set focus on |child11|,
order of windows should not change.
On 2016/10/03 23:59:47, msw wrote:
> nit: child1

It is child11 though? Look at next line.

[sky, 2016-10-04 00:04:44]

The CQ bit was checked by sky@chromium.org

[sky, 2016-10-04 00:04:44]

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

[commit-bot: I haz the power, 2016-10-04 00:04:56]

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

[msw, 2016-10-04 00:11:05]

lgtm, thanks!

https://codereview.chromium.org/2391853002/diff/20001/services/ui/ws/window_m...
File services/ui/ws/window_manager_client_unittest.cc (right):

https://codereview.chromium.org/2391853002/diff/20001/services/ui/ws/window_m...
services/ui/ws/window_manager_client_unittest.cc:870: // Set focus on |child11|,
order of windows should not change.
On 2016/10/04 00:04:32, sky wrote:
> On 2016/10/03 23:59:47, msw wrote:
> > nit: child1
> 
> It is child11 though? Look at next line.

D'oh, you're right; sorry.

[commit-bot: I haz the power, 2016-10-04 01:12:11]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-04 01:15:18]

Description was changed from

==========
Makes mus not bring window to front on activation changes

The window manager should be doing it. To do otherwise means mus is
likely conflicting with the wm. Additionally changing the activation
as we were meant clients weren't getting notified and got out of sync
with mus.

BUG=652421
TEST=covered by tests
R=msw@chromium.org
==========

to

==========
Makes mus not bring window to front on activation changes

The window manager should be doing it. To do otherwise means mus is
likely conflicting with the wm. Additionally changing the activation
as we were meant clients weren't getting notified and got out of sync
with mus.

BUG=652421
TEST=covered by tests
R=msw@chromium.org

Committed: https://crrev.com/3bddafc6834c9487e6223168120f5de7ecbf1d2b
Cr-Commit-Position: refs/heads/master@{#422652}
==========

[commit-bot: I haz the power, 2016-10-04 01:15:19]

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