New issue
Advanced search Search tips

Issue 663907 link

Starred by 1 user
Status: Archived
Owner:
kylec...@chromium.org
Closed: Nov 2016
Cc:
mustash-bugs@google.com
rjkroege@chromium.org
sadrul@chromium.org
riajiang@chromium.org
sky@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 548429



Sign in to add a comment

DisplayTest in mustash use different display code

Project Member Reported by kylec...@chromium.org, Nov 9 2016 
All of the tests in services/ui/ws/display_unittest.cc are using a test only code for creating/modifying/removing displays. The test code misses important methods that we need to work correctly. Essentially, we are testing test only code.

The tests will be updated to use the same display code as ./chrome --mash. Also the tests should be expanded to ensure that HDPI displays work as expected.
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 11 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/526be5ac7017cdb08fcaba441125b2a3aaec3d92

commit 526be5ac7017cdb08fcaba441125b2a3aaec3d92
Author: kylechar <kylechar@chromium.org>
Date: Fri Nov 11 23:11:25 2016

Remove CreateDefaultDisplay().

Initializing displays after first WM connection seems unnecessary. This
is especially true on Chrome OS devices where there is a significant
delay to query display hardware.

Immediately start display initialization after ui::Service starts. The
timing of this step isn't deterministic anyways. Tests that rely on
displays being created already make a method call to set the number of
displays. Change this call to actually create ws::Display objects
instead.

BUG= 663907 

Review-Url: https://codereview.chromium.org/2484223006
Cr-Commit-Position: refs/heads/master@{#431677}

[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/service.cc
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/service.h
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/cursor_unittest.cc
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/display_unittest.cc
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/test_utils.cc
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/test_utils.h
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/user_display_manager_unittest.cc
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/window_manager_state_unittest.cc
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/window_manager_window_tree_factory_set.cc
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/window_manager_window_tree_factory_set.h
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/window_server.cc
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/window_server.h
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/window_server_delegate.h
[modify] https://crrev.com/526be5ac7017cdb08fcaba441125b2a3aaec3d92/services/ui/ws/window_tree_unittest.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 12 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8624207f2e1fe2961f12bfeae6df773fa1e3f12e

commit 8624207f2e1fe2961f12bfeae6df773fa1e3f12e
Author: kylechar <kylechar@chromium.org>
Date: Sat Nov 12 00:23:50 2016

Delete WindowManagerWindowTreeFactorySetTestApi.

This class can be trivially replaced with a simple function. It no
longer has any need to be a friend class to other classes.

BUG= 663907 

Review-Url: https://codereview.chromium.org/2481013008
Cr-Commit-Position: refs/heads/master@{#431698}

[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/cursor_unittest.cc
[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/display_unittest.cc
[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/test_utils.cc
[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/test_utils.h
[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/user_display_manager_unittest.cc
[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/window_manager_state_unittest.cc
[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/window_manager_window_tree_factory.h
[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/window_manager_window_tree_factory_set.h
[modify] https://crrev.com/8624207f2e1fe2961f12bfeae6df773fa1e3f12e/services/ui/ws/window_tree_unittest.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1df004a488492aa7938dac963ddc5f6fc28f5ce7

commit 1df004a488492aa7938dac963ddc5f6fc28f5ce7
Author: kylechar <kylechar@chromium.org>
Date: Wed Nov 16 20:43:32 2016

Fix ws::Display initialization order.

The initialization order for ws::Display was a bit mixed up. It was not
waiting for the accelerated widget to be available, assuming that it was
available immediately after calling PlatformDisplay::Init() instead.

Reorder ws::Display and ws::PlatformDisplay Init() methods. Make sure
that most work is done in the Init() methods instead of constructors, as
PlatformWindowDelegate::OnAcceleratedWidgetAvailable() can happen
synchronously when creating the PlatformWindow, and we want constructors
to have finished before we start using the objects.

BUG= 663907 

Review-Url: https://codereview.chromium.org/2497303002
Cr-Commit-Position: refs/heads/master@{#432616}

[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/display.cc
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/display.h
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/display_manager.cc
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/platform_display.cc
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/platform_display.h
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/platform_display_delegate.h
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/platform_display_factory.h
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/test_utils.cc
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/test_utils.h
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/user_display_manager_unittest.cc
[modify] https://crrev.com/1df004a488492aa7938dac963ddc5f6fc28f5ce7/services/ui/ws/window_tree_host_factory.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 18 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9298c5ee41f3c4c0078574d72955876ecf8c3b4b

commit 9298c5ee41f3c4c0078574d72955876ecf8c3b4b
Author: kylechar <kylechar@chromium.org>
Date: Fri Nov 18 19:59:20 2016

Cleanup display creation in mus tests.

Create displays via PlatformScreen. DisplayTests will now run the same
code as mustash.

Expand TestPlatformScreen to allow this. It forwards OnDisplayAdded(),
OnDisplayModified() and OnDisplayRemoved() calls. The test is
responsible for making the appropriate calls.

Add new DisplayTests to confirm that creating a display before a WM has
been added works correctly. Also add tests to check that the display
root and WM root ServerWindows are the correct size for a display.

BUG= 663907 

Review-Url: https://codereview.chromium.org/2512593003
Cr-Commit-Position: refs/heads/master@{#433272}

[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/cursor_unittest.cc
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/display_unittest.cc
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/platform_display.cc
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/platform_display.h
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/platform_display_default.h
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/platform_display_factory.h
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/test_utils.cc
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/test_utils.h
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/user_display_manager_unittest.cc
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/window_manager_state_unittest.cc
[modify] https://crrev.com/9298c5ee41f3c4c0078574d72955876ecf8c3b4b/services/ui/ws/window_tree_unittest.cc

Comment 6 by kylec...@chromium.org, Nov 23 2016

Status: Fixed (was: Started)

Comment 7 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 8 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 9 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 10 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 11 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)
►

Sign in to add a comment