Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(13)

Issue 2745143004: Inform window manager about modal windows in mus+ash. (Closed)

Created:
8 months, 1 week ago by Hadi
Modified:
8 months ago
Reviewers:
Tom Sepez, sadrul, sky
CC:
chromium-reviews, rjkroege, sadrul, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, kalyank, darin (slow to review)
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Inform window manager about modal windows in mus+ash. We need to inform window manager about modal windows. This is necessary because for example SystemModalContainerLayoutManager needs to know about modal types so it can create the fullscreen grey background for system modals. After this change we have some of the WM effects for modal windows (e.g. animating system modal windows when clicked outside of it), but for the rest we need more wiring (e.g. no grey background yet) which will be done in subsequent CLs. BUG=645996 Review-Url: https://codereview.chromium.org/2745143004 Cr-Commit-Position: refs/heads/master@{#458065} Committed: https://chromium.googlesource.com/chromium/src/+/edab7eb543505b23c3618767d40e2aa4f4ddeb19

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Total comments: 6

Patch Set 4 : Addressed feedback. #

Patch Set 5 : Added a test. #

Total comments: 1

Patch Set 6 : Addressed feedback. #

Patch Set 7 : rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -2 lines) Patch
M services/ui/public/interfaces/window_manager.mojom View 1 2 3 4 5 6 2 chunks +2 lines, -0 lines 0 comments Download
M services/ui/ws/test_utils.h View 1 2 3 4 5 6 3 chunks +3 lines, -0 lines 0 comments Download
M services/ui/ws/test_utils.cc View 1 2 3 4 5 6 1 chunk +4 lines, -0 lines 0 comments Download
M services/ui/ws/window_tree.cc View 1 2 3 4 5 6 2 chunks +24 lines, -2 lines 0 comments Download
M services/ui/ws/window_tree_client_unittest.cc View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M services/ui/ws/window_tree_unittest.cc View 1 2 3 4 5 6 1 chunk +33 lines, -0 lines 0 comments Download
M ui/aura/mus/window_tree_client.h View 1 2 3 4 5 6 2 chunks +2 lines, -0 lines 0 comments Download
M ui/aura/mus/window_tree_client.cc View 1 2 3 4 5 6 1 chunk +6 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 47 (34 generated)
Hadi
PTAL. How do we write tests for changes like this? I couldn't find tests for ...
8 months, 1 week ago (2017-03-14 19:38:46 UTC) #8
sky
On 2017/03/14 19:38:46, Hadi wrote: > PTAL. > > How do we write tests for ...
8 months, 1 week ago (2017-03-14 21:15:26 UTC) #14
sky
https://codereview.chromium.org/2745143004/diff/40001/services/ui/ws/window_tree.cc File services/ui/ws/window_tree.cc (right): https://codereview.chromium.org/2745143004/diff/40001/services/ui/ws/window_tree.cc#newcode1298 services/ui/ws/window_tree.cc:1298: WindowTree* wm_tree = GetWindowManagerDisplayRoot(window) As ash always allows the ...
8 months, 1 week ago (2017-03-14 21:16:23 UTC) #15
Hadi
Addressed feedback and added a test to window_tree_unittest.cc to check that it forwards the change ...
8 months ago (2017-03-17 16:16:32 UTC) #22
Hadi
tsepez@ can you please review window_manager.mojom? Thanks.
8 months ago (2017-03-17 16:18:43 UTC) #24
Tom Sepez
lgtm
8 months ago (2017-03-17 17:03:12 UTC) #27
sky
LGTM https://codereview.chromium.org/2745143004/diff/80001/services/ui/ws/window_tree_unittest.cc File services/ui/ws/window_tree_unittest.cc (right): https://codereview.chromium.org/2745143004/diff/80001/services/ui/ws/window_tree_unittest.cc#newcode1400 services/ui/ws/window_tree_unittest.cc:1400: TestWindowTreeBinding* child_binding; = nullptr
8 months ago (2017-03-17 19:07:06 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2745143004/100001
8 months ago (2017-03-17 19:10:52 UTC) #35
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/386109)
8 months ago (2017-03-17 20:31:42 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2745143004/100001
8 months ago (2017-03-18 05:14:21 UTC) #39
commit-bot: I haz the power
Failed to apply patch for ui/aura/mus/window_tree_client.h: While running git apply --index -p1; error: patch failed: ...
8 months ago (2017-03-18 07:30:04 UTC) #41
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2745143004/120001
8 months ago (2017-03-20 13:17:25 UTC) #44
commit-bot: I haz the power
8 months ago (2017-03-20 15:16:25 UTC) #47
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/edab7eb543505b23c3618767d40e...

Powered by Google App Engine
This is Rietveld efc10ee0f