New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 710195 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

mus client unit tests initialize OzonePlatform

Project Member Reported by e...@chromium.org, Apr 10 2017

Issue description

Right now, we trigger aura::Env to be in mus client mode by looking for the --primordial-pipe-token command line flag (//ui/aura/env.cc:RunningInsideMus()). If that flag isn't there, Env will initialize the current ozone platform.

We don't initialize ozone inside mus-chrome or mash. We instead go and initialize certain ozone subsystems that are needed (which will DCHECK if  ozone is loaded). (My first attempt at solving this bug was to merely install a new OzonePlatformMus which encapsulated what chrome currently was doing, but this ran into some problems.)

In any test that goes through the AuraTestHelper path (basically everything from //ui/aura and //ui/views), we go through paths in TestScreen which accesses the non-mus screen paths, and creates a platform window tree host which then goes to (on linux) OzonePlatformX11 for its window. As long as test binaries aren't being passed --primordial-pipe-token, they're initializing Ozone, which creates non-mus WindowTreeHosts.

But if you do pass --primordial-pipe-token, things just break because now Ozone isn't being initialized and aura is *still* trying to initialize the native stuff.

(TestScreen can apparently take a WindowTreeClient* and use that, but the code appears to never be used, and actually using causes created WindowTreeHosts to be placed in two different std::unique_ptr<>.)
 

Comment 1 by e...@chromium.org, Apr 10 2017

Components: MUS
Labels: -Pri-3 Proj-Mustash-Testing Pri-2

Comment 2 by e...@chromium.org, Apr 11 2017

Even if you attempt to prevent loading ozone via the UI thread (OzonePlatform::InitializeForUI), you're going to load it on the GPU thread. (OzonePlatform::InitializeForGPU).

It might not be feasible to not have an OzonePlatform instance.

Comment 3 by e...@chromium.org, Apr 11 2017

OK, maybe not.

In the chrome process of mash, we don't perform any ozone initialization except in the window server process.

In an in person discussion with sky, I suggested moving aura::Env from a global object created by the TestSuite to one initialized in each test through the AuraTestHelper. I suggested that so that each test could bring up (and take down) ozone if that specific test used the local ozone platform instead of the mus service. I don't think that's feasible now because of how the gpu threads are brought up and shut down.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 12 2017

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

commit ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3
Author: erg <erg@chromium.org>
Date: Wed Apr 12 20:54:53 2017

[views-mus] Prevent creating a native OzonePlatform in mus tests.

In the views_mus_unittests, we were always creating a native
OzonePlatform instance. Sometimes this was used instead of mus for the
test body. In most cases, it was accessed during test setup. This
modifies the test code to not build an implicit OzonePlatform.

In normal mus builds of chrome, we do not run with an OzonePlatform and
instead manually initialize some of the objects that OzonePlatform
owns. This ensures that AuraTestHelper doesn't create a local
implementation using the same detection method that Env uses to flip
into aura-mus mode.

BUG= 705037 ,  710195 ,  710939 

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

[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ash/system/status_area_widget_unittest.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/aura/env.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/aura/mus/os_exchange_data_provider_mus_unittest.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/aura/test/aura_test_helper.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/base/dragdrop/os_exchange_data_provider_factory.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/base/dragdrop/os_exchange_data_provider_factory.h
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/keyboard/keyboard_controller.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/views/BUILD.gn
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/views/mus/views_mus_test_suite.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/views/mus/views_mus_test_suite.h
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/views/test/widget_test_unittest.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/views/views_test_suite.cc
[modify] https://crrev.com/ac2e0aeea11a2641dfca4d1b192bd1641b5cd5d3/ui/views/views_test_suite.h

Comment 5 by e...@chromium.org, Apr 12 2017

Status: Fixed (was: Assigned)
Components: -MUS Internals>Services>WindowService

Sign in to add a comment