New issue
Advanced search Search tips

Issue 700142 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Ozone X11 should call XInitThreads() for single process mode

Project Member Reported by kylec...@chromium.org, Mar 9 2017

Issue description

Ozone X11 needs to call XInitThreads() as the first call to X11 when initializing in single process mode. This happens now for mus+ash but doesn't happen in tests.

There is an InitParams object passed into OzonePlatform that specifies if it's single process mode.

Not all tests fill in InitParams, so this will need to be fixed. Anywhere that XInitThreads() is called from now needs to be looked at to ensure OzonePlatform is initialized correctly.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 9 2017

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

commit a0f4b128a7a670ef3540a33966ebb65a27918520
Author: kylechar <kylechar@chromium.org>
Date: Thu Mar 09 23:53:56 2017

Call XInitThreads() for single-process mode.

If Ozone X11 is launched for tests in single process mode then it needs
to call XInitThreads() as the first call to Xlib. This happens for
mus+ash but not during tests. Check |single_process| in InitParams to
see if we are initializing Ozone in single process mode.

Note that not all tests correctly fill out InitParams. This will have to
be fixed as a follow up.

BUG= 700142 
TBR=sadrul@chromium.org

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

[add] https://crrev.com/a0f4b128a7a670ef3540a33966ebb65a27918520/ui/ozone/platform/x11/DEPS
[modify] https://crrev.com/a0f4b128a7a670ef3540a33966ebb65a27918520/ui/ozone/platform/x11/ozone_platform_x11.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 15 2017

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

commit 002b2ee07c580e92cefa2fbc7c067af40c367c44
Author: kylechar <kylechar@chromium.org>
Date: Wed Mar 15 23:50:12 2017

Move Ozone GPU initialization.

Right now OzonePlatform::InitializeForGPU() gets called as part of
gl::init::GetAllowedGLImplementations(). We need to properly set
InitParams when initializing the Ozone GPU so the platform knows if it
should expect to be running in a single process.

Instead, have the GpuInit or tests call InitializeForGPU(). This way
GpuInit can set InitParams::single_process to false and tests can set
InitParams::single_process to true.

Since this CL updates all callers of OzonePlatform::InitializeForGPU()
to use the new version, we can also delete the deprecated version of the
function.

BUG= 700142 , 620934 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

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

[modify] https://crrev.com/002b2ee07c580e92cefa2fbc7c067af40c367c44/content/gpu/in_process_gpu_thread.cc
[modify] https://crrev.com/002b2ee07c580e92cefa2fbc7c067af40c367c44/gpu/ipc/service/gpu_init.cc
[modify] https://crrev.com/002b2ee07c580e92cefa2fbc7c067af40c367c44/ui/gl/init/gl_factory_ozone.cc
[modify] https://crrev.com/002b2ee07c580e92cefa2fbc7c067af40c367c44/ui/gl/test/gl_image_test_support.cc
[modify] https://crrev.com/002b2ee07c580e92cefa2fbc7c067af40c367c44/ui/gl/test/gl_surface_test_support.cc
[modify] https://crrev.com/002b2ee07c580e92cefa2fbc7c067af40c367c44/ui/ozone/public/ozone_platform.cc
[modify] https://crrev.com/002b2ee07c580e92cefa2fbc7c067af40c367c44/ui/ozone/public/ozone_platform.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 16 2017

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

commit 4e7ba20bb8235560cf1d81f8811c52bdb655e795
Author: kylechar <kylechar@chromium.org>
Date: Fri Jun 16 13:11:47 2017

Ozone X11 initialization checks for --single-process.

If Chrome is running with --single-process then make sure that
XInitThreads() is the first X call.

Bug:  700142 , 698780 
Change-Id: I6ebde0f8dfcd0c1eb0183acef219e3f6a7b9a08e
Reviewed-on: https://chromium-review.googlesource.com/537475
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480026}
[modify] https://crrev.com/4e7ba20bb8235560cf1d81f8811c52bdb655e795/ui/ozone/platform/x11/ozone_platform_x11.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 13 2017

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

commit e2c8d813d5838b17e59f44567e6c39201e9dfceb
Author: kylechar <kylechar@chromium.org>
Date: Thu Jul 13 17:17:07 2017

Use OzonePlatform::InitParams everywhere.

Remove the deprecated function that uses default InitParams. Add the
params in the remaining callsites and set |single_process| correctly for
tests. The only exception is in aura::Env::Init() where we don't know
if it is a test.

Bug:  620934 , 700142 
Change-Id: I5e0d61578a423dd42eca38d21ccd1568b9ce5a8b
Reviewed-on: https://chromium-review.googlesource.com/568361
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486421}
[modify] https://crrev.com/e2c8d813d5838b17e59f44567e6c39201e9dfceb/media/gpu/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/e2c8d813d5838b17e59f44567e6c39201e9dfceb/ui/aura/env.cc
[modify] https://crrev.com/e2c8d813d5838b17e59f44567e6c39201e9dfceb/ui/compositor/test/test_suite.cc
[modify] https://crrev.com/e2c8d813d5838b17e59f44567e6c39201e9dfceb/ui/ozone/demo/ozone_demo.cc
[modify] https://crrev.com/e2c8d813d5838b17e59f44567e6c39201e9dfceb/ui/ozone/public/ozone_platform.cc
[modify] https://crrev.com/e2c8d813d5838b17e59f44567e6c39201e9dfceb/ui/ozone/public/ozone_platform.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 18 2017

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

commit 33b93bbe4854966fa0c40745fba90fda24543521
Author: Hung-yu Wu <hywu@chromium.org>
Date: Tue Jul 18 04:07:13 2017

Revert "Use OzonePlatform::InitParams everywhere."

This reverts commit e2c8d813d5838b17e59f44567e6c39201e9dfceb.

Reason for revert: It breaks video_VideoDecodeAccelerator test on all
ChromeOS platforms.
https://bugs.chromium.org/p/chromium/issues/detail?id=744263

Original change's description:
> Use OzonePlatform::InitParams everywhere.
> 
> Remove the deprecated function that uses default InitParams. Add the
> params in the remaining callsites and set |single_process| correctly for
> tests. The only exception is in aura::Env::Init() where we don't know
> if it is a test.
> 
> Bug:  620934 , 700142 
> Change-Id: I5e0d61578a423dd42eca38d21ccd1568b9ce5a8b
> Reviewed-on: https://chromium-review.googlesource.com/568361
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Commit-Queue: kylechar <kylechar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#486421}

TBR=rjkroege@chromium.org,sky@chromium.org,kylechar@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  620934 ,  700142 
Change-Id: I30b11889c9b42ff85e2748d6f0839d3a8de8a7ad
Reviewed-on: https://chromium-review.googlesource.com/575787
Reviewed-by: Wu-Cheng Li <wuchengli@chromium.org>
Commit-Queue: Hung-yu Wu <hywu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487381}
[modify] https://crrev.com/33b93bbe4854966fa0c40745fba90fda24543521/media/gpu/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/33b93bbe4854966fa0c40745fba90fda24543521/ui/aura/env.cc
[modify] https://crrev.com/33b93bbe4854966fa0c40745fba90fda24543521/ui/compositor/test/test_suite.cc
[modify] https://crrev.com/33b93bbe4854966fa0c40745fba90fda24543521/ui/ozone/demo/ozone_demo.cc
[modify] https://crrev.com/33b93bbe4854966fa0c40745fba90fda24543521/ui/ozone/public/ozone_platform.cc
[modify] https://crrev.com/33b93bbe4854966fa0c40745fba90fda24543521/ui/ozone/public/ozone_platform.h

Blocking: -671355
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 19 2017

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

commit d0cd2b3c0a15b53fa998cc2000a3fb8fab1d16a8
Author: kylechar <kylechar@chromium.org>
Date: Tue Sep 19 00:30:16 2017

Use OzonePlatform::InitParams everywhere.

Remove the deprecated function that uses default InitParams. Add the
params in the remaining callsites and set |single_process| correctly for
tests. video_decode_accelerator_unittest.cc sets |single_process| to
false, as the test runs with Ozone DRM on CrOS devices.

Bug:  620934 ,  700142 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Iaac92d25eba7c9e052c250e84fdef2f46ecea7ee
Reviewed-on: https://chromium-review.googlesource.com/667720
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502731}
[modify] https://crrev.com/d0cd2b3c0a15b53fa998cc2000a3fb8fab1d16a8/media/gpu/video_decode_accelerator_unittest.cc
[modify] https://crrev.com/d0cd2b3c0a15b53fa998cc2000a3fb8fab1d16a8/ui/compositor/test/test_suite.cc
[modify] https://crrev.com/d0cd2b3c0a15b53fa998cc2000a3fb8fab1d16a8/ui/ozone/demo/ozone_demo.cc
[modify] https://crrev.com/d0cd2b3c0a15b53fa998cc2000a3fb8fab1d16a8/ui/ozone/public/ozone_platform.cc
[modify] https://crrev.com/d0cd2b3c0a15b53fa998cc2000a3fb8fab1d16a8/ui/ozone/public/ozone_platform.h

Status: Fixed (was: Assigned)

Comment 9 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 10 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment