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

Issue 2715533005: Make mus_demo_unittests work with multiple root windows (Closed)

Created:
2 years, 9 months ago by fwang
Modified:
2 years, 7 months ago
Reviewers:
sadrul, sky
CC:
chromium-reviews, rjkroege, tonikitoo, kylechar
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make mus_demo_unittests work with multiple root windows WindowServerTestImpl::EnsureClientHasDrawnWindow verifies whether one root window of the WindowTree is drawn. This currently works well for mus_demo_unittests because the WindowServer only has a single root window. However, MusDemo will open multiple root windows in external mode and hence mus_demo_unittests should really check whether all these windows are drawn. BUG=666958

Patch Set 1 #

Patch Set 2 : Rename test function "EnsureClientHasDrawnRootWindows" and add a parameter to count the expected nu… #

Patch Set 3 : Pass the count to the callback. #

Total comments: 1

Patch Set 4 : Rebase on top of https://codereview.chromium.org/2712203002/#ps80001 #

Patch Set 5 : Add a unit test for multiple windows. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+72 lines, -33 lines) Patch
M ash/mus/app_launch_unittest.cc View 1 2 3 2 chunks +8 lines, -6 lines 0 comments Download
M services/ui/demo/mus_demo_unittests.cc View 1 2 3 4 3 chunks +34 lines, -16 lines 0 comments Download
M services/ui/public/interfaces/window_server_test.mojom View 1 2 1 chunk +1 line, -1 line 0 comments Download
M services/ui/ws/window_server_test_impl.h View 1 2 2 chunks +6 lines, -3 lines 0 comments Download
M services/ui/ws/window_server_test_impl.cc View 1 2 3 4 1 chunk +23 lines, -7 lines 1 comment Download

Depends on Patchset:

Messages

Total messages: 20 (8 generated)
fwang
2 years, 8 months ago (2017-02-23 12:35:52 UTC) #8
sky
The window server knows how many windows a client has, can't it figure out the ...
2 years, 8 months ago (2017-02-23 20:04:49 UTC) #10
fwang
On 2017/02/23 20:04:49, sky wrote: > The window server knows how many windows a client ...
2 years, 8 months ago (2017-02-23 20:19:12 UTC) #11
fwang
On 2017/02/23 20:19:12, fwang wrote: > On 2017/02/23 20:04:49, sky wrote: > > The window ...
2 years, 8 months ago (2017-02-23 20:24:24 UTC) #12
sky
The callback could return the count. That way the test can assert on the count. ...
2 years, 8 months ago (2017-02-23 21:33:15 UTC) #13
fwang
On 2017/02/23 21:33:15, sky wrote: > The callback could return the count. That way the ...
2 years, 8 months ago (2017-02-24 08:29:21 UTC) #14
fwang
https://codereview.chromium.org/2715533005/diff/40001/services/ui/ws/window_server_test_impl.cc File services/ui/ws/window_server_test_impl.cc (right): https://codereview.chromium.org/2715533005/diff/40001/services/ui/ws/window_server_test_impl.cc#newcode53 services/ui/ws/window_server_test_impl.cc:53: "for that client name."; This is maybe not a ...
2 years, 8 months ago (2017-02-24 08:51:15 UTC) #15
sky
How about waiting until you actual need this value? On Fri, Feb 24, 2017 at ...
2 years, 8 months ago (2017-02-24 18:27:38 UTC) #16
fwang
On 2017/02/24 18:27:38, sky wrote: > How about waiting until you actual need this value? ...
2 years, 8 months ago (2017-02-24 18:47:57 UTC) #17
fwang
On 2017/02/24 18:27:38, sky wrote: > How about waiting until you actual need this value? ...
2 years, 8 months ago (2017-03-01 17:24:34 UTC) #18
fwang
On 2017/02/24 18:27:38, sky wrote: > How about waiting until you actual need this value? ...
2 years, 7 months ago (2017-04-03 10:25:22 UTC) #19
fwang
2 years, 7 months ago (2017-04-05 17:32:38 UTC) #20
Message was sent while issue was closed.
Closing that since it does not seem it will be upstreamed for now.

Powered by Google App Engine
This is Rietveld 408576698