Fix event targeting for sub-windows of system modal windows in mus+ash.

Fix event targeting for sub-windows of system modal windows in mus+ash.

If a non-client frame is set to a system modal, its embedded sub-window
should be able to receive events.

For example, creating a JS alert() creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
Review-Url: https://codereview.chromium.org/2737003002
Cr-Commit-Position: refs/heads/master@{#456058}
Committed: https://chromium.googlesource.com/chromium/src/+/5f8a6330d36c11874bb8aaf136f3ea4e508ae8ee

[Hadi, 2017-03-07 19:32:51]

Description was changed from

==========
Fix event targetting in presence of modal windows.

BUG=
==========

to

==========
Fix event targeting for sub-windows of modal windows.

If a non-client frame is set to modal, its embedded sub-window should be
able to receive events.

For example, creating a js modal creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
==========

[Hadi, 2017-03-07 19:34:58]

Description was changed from

==========
Fix event targeting for sub-windows of modal windows.

If a non-client frame is set to modal, its embedded sub-window should be
able to receive events.

For example, creating a js modal creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
==========

to

==========
Fix event targeting for sub-windows of modal windows in mus+ash.

If a non-client frame is set to modal, its embedded sub-window should be
able to receive events.

For example, creating a js modal creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
==========

[Hadi, 2017-03-07 19:35:33]

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

[commit-bot: I haz the power, 2017-03-07 19:36:42]

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

[Hadi, 2017-03-07 19:41:33]

Description was changed from

==========
Fix event targeting for sub-windows of modal windows in mus+ash.

If a non-client frame is set to modal, its embedded sub-window should be
able to receive events.

For example, creating a js modal creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
==========

to

==========
Fix event targeting for sub-windows of system modal windows in mus+ash.

If a non-client frame is set to a system modal, its embedded sub-window
should be able to receive events.

For example, creating a js alert() creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
==========

[Hadi, 2017-03-07 19:41:43]

Description was changed from

==========
Fix event targeting for sub-windows of system modal windows in mus+ash.

If a non-client frame is set to a system modal, its embedded sub-window
should be able to receive events.

For example, creating a js alert() creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
==========

to

==========
Fix event targeting for sub-windows of system modal windows in mus+ash.

If a non-client frame is set to a system modal, its embedded sub-window
should be able to receive events.

For example, creating a JS alert() creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
==========

[commit-bot: I haz the power, 2017-03-07 20:35:04]

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

[commit-bot: I haz the power, 2017-03-07 20:35:05]

Dry run: This issue passed the CQ dry run.

[Hadi, 2017-03-09 20:37:03]

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

[commit-bot: I haz the power, 2017-03-09 20:37:21]

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

[Hadi, 2017-03-09 20:37:23]

moshayedi@chromium.org changed reviewers:
+ mohsen@chromium.org, sky@chromium.org

[Hadi, 2017-03-09 20:37:24]

PTAL.

[commit-bot: I haz the power, 2017-03-09 21:21:34]

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

[commit-bot: I haz the power, 2017-03-09 21:21:35]

Dry run: This issue passed the CQ dry run.

[sky, 2017-03-09 23:23:26]

LGTM

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/event_di...
File services/ui/ws/event_dispatcher_unittest.cc (right):

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/event_di...
services/ui/ws/event_dispatcher_unittest.cc:1568: auto w1 =
CreateChildWindow(WindowId(1, 3));
excessive use of auto makes for hard to read code. I would prefer you be
explicit and use std::unique_ptr<ServerWindow> as the rest of the code does in
this class.

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/event_di...
services/ui/ws/event_dispatcher_unittest.cc:1601: ASSERT_TRUE(details);
You should redirect the index to this so if there is a failure you know where.

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/modal_wi...
File services/ui/ws/modal_window_controller.cc (right):

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/modal_wi...
services/ui/ws/modal_window_controller.cc:86: // TODO(moshayedi):
crbug.com/697127. Handle windows which are modal to
Update comment? In fact is the bug you're fixing the same as 697127?

[Hadi, 2017-03-10 14:02:38]

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/event_di...
File services/ui/ws/event_dispatcher_unittest.cc (right):

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/event_di...
services/ui/ws/event_dispatcher_unittest.cc:1568: auto w1 =
CreateChildWindow(WindowId(1, 3));
On 2017/03/09 23:23:26, sky wrote:
> excessive use of auto makes for hard to read code. I would prefer you be
> explicit and use std::unique_ptr<ServerWindow> as the rest of the code does in
> this class.

Done.

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/event_di...
services/ui/ws/event_dispatcher_unittest.cc:1601: ASSERT_TRUE(details);
On 2017/03/09 23:23:26, sky wrote:
> You should redirect the index to this so if there is a failure you know where.

Done.

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/modal_wi...
File services/ui/ws/modal_window_controller.cc (right):

https://codereview.chromium.org/2737003002/diff/20001/services/ui/ws/modal_wi...
services/ui/ws/modal_window_controller.cc:86: // TODO(moshayedi):
crbug.com/697127. Handle windows which are modal to
On 2017/03/09 23:23:26, sky wrote:
> Update comment? In fact is the bug you're fixing the same as 697127?

These bugs are not the same. 697127 refers to modal windows such as print
dialog, cast dialog, etc. which are modal to children of their parent but not
the parent itself.

[Hadi, 2017-03-10 14:02:42]

The CQ bit was checked by moshayedi@chromium.org

[Hadi, 2017-03-10 14:02:43]

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

[commit-bot: I haz the power, 2017-03-10 14:02:48]

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-03-10 14:51:06]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489154562576100,
"parent_rev": "ea4cd6e84d00c433d05fda8a30156e704a654f3c", "commit_rev":
"5f8a6330d36c11874bb8aaf136f3ea4e508ae8ee"}

[commit-bot: I haz the power, 2017-03-10 14:51:45]

Description was changed from

==========
Fix event targeting for sub-windows of system modal windows in mus+ash.

If a non-client frame is set to a system modal, its embedded sub-window
should be able to receive events.

For example, creating a JS alert() creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213
==========

to

==========
Fix event targeting for sub-windows of system modal windows in mus+ash.

If a non-client frame is set to a system modal, its embedded sub-window
should be able to receive events.

For example, creating a JS alert() creates a non-client frame and sets it
to MODAL_TYPE_SYSTEM. Previously users couldn't click on the OK inside
the client window since all events were routed to the non-client frame.

BUG=699213

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

[commit-bot: I haz the power, 2017-03-10 14:51:46]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/5f8a6330d36c11874bb8aaf136f3...