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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 1 week ago by Hadi
Modified:
2 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
Commit queue not available (can’t edit this change).

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 ...
2 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 ...
2 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 ...
2 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 ...
2 months, 1 week ago (2017-03-17 16:16:32 UTC) #22
Hadi
tsepez@ can you please review window_manager.mojom? Thanks.
2 months, 1 week ago (2017-03-17 16:18:43 UTC) #24
Tom Sepez
lgtm
2 months, 1 week 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
2 months, 1 week 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
2 months, 1 week 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)
2 months, 1 week 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
2 months, 1 week 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: ...
2 months, 1 week 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
2 months ago (2017-03-20 13:17:25 UTC) #44
commit-bot: I haz the power
2 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06