Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1208603002:
content: implement unittests backend for Ozone GBM
|
Created:
5 years, 6 months ago by dshwang Modified:
4 years, 8 months ago Reviewers:
vignatti (out of this project) CC:
chromium-reviews, darin-cc_chromium.org, jam Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptioncontent: implement unittests backend for Ozone GBM
GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently.
Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture.
These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU()
is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires
browser side initialization also.
Introduce OzonePlatform::InitializeForTest() interface to initialize everything
synchronously for unit tests.
TEST=gpu_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests
gl_unittests --gtest_filter=GLImage* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests
BUG=475633
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
Patch Set 1 #
Total comments: 2
Patch Set 2 : ozone: Implement new interface for test #Patch Set 3 : fix NavigationControllerTest failure #
Total comments: 2
Patch Set 4 : fix DCHECK(thread_checker_) in DrmDeviceManager::AddDrmDevice #Patch Set 5 : WIP #
Total comments: 2
Patch Set 6 : rebase to ToT #Patch Set 7 : force single process #
Total comments: 7
Patch Set 8 : ContentTestSuite initialize message loop #
Total comments: 2
Patch Set 9 : fix content_gl_tests #Patch Set 10 : fix Mac bots #
Total comments: 11
Patch Set 11 : fix content_gl_tests for Mac bots #Patch Set 12 : rebase to ToT #Patch Set 13 : exclude content_unittests as all GMBs tests are moved to gpu_unittests #
Messages
Total messages: 58 (14 generated)
dongseong.hwang@intel.com changed reviewers: + reveman@chromium.org, sky@chromium.org, spang@chromium.org, tiago.vignatti@intel.com
sky, could you review?
I don't think we want our unit tests to require some form of ipc. Please try to design ozone code so we can run the same set of unit tests as on other platforms without using threads and ipc.
https://codereview.chromium.org/1208603002/diff/1/content/test/content_test_s... File content/test/content_test_suite.cc (right): https://codereview.chromium.org/1208603002/diff/1/content/test/content_test_s... content/test/content_test_suite.cc:81: scoped_ptr<ui::OzoneGpuTestHelper> gpu_helper_; would be better if we had the full GPU service in place, so perf tests could ran just a like in a real setup.
On 2015/06/24 18:49:50, reveman wrote: > I don't think we want our unit tests to require some form of ipc. Please try to > design ozone code so we can run the same set of unit tests as on other platforms > without using threads and ipc. To do that, ozone gpu code must open drm fd for test. spang, can I add above code in ozone layer for only test? https://codereview.chromium.org/1208603002/diff/1/content/test/content_test_s... File content/test/content_test_suite.cc (right): https://codereview.chromium.org/1208603002/diff/1/content/test/content_test_s... content/test/content_test_suite.cc:81: scoped_ptr<ui::OzoneGpuTestHelper> gpu_helper_; On 2015/06/24 22:16:17, vignatti wrote: > would be better if we had the full GPU service in place, so perf tests could ran > just a like in a real setup. reveman, what do you think about vignatti's opinion?
On 2015/06/25 at 05:38:40, dongseong.hwang wrote: > On 2015/06/24 18:49:50, reveman wrote: > > I don't think we want our unit tests to require some form of ipc. Please try to > > design ozone code so we can run the same set of unit tests as on other platforms > > without using threads and ipc. > > To do that, ozone gpu code must open drm fd for test. > spang, can I add above code in ozone layer for only test? Why can't we test this in the same way as we test IOSurface and SurfaceTexture support? We have both unit tests and integration tests. GpuMemoryBufferImpl/Factory unit tests and ChildThreadImpl integration tests. > > https://codereview.chromium.org/1208603002/diff/1/content/test/content_test_s... > File content/test/content_test_suite.cc (right): > > https://codereview.chromium.org/1208603002/diff/1/content/test/content_test_s... > content/test/content_test_suite.cc:81: scoped_ptr<ui::OzoneGpuTestHelper> gpu_helper_; > On 2015/06/24 22:16:17, vignatti wrote: > > would be better if we had the full GPU service in place, so perf tests could ran > > just a like in a real setup. > > reveman, what do you think about vignatti's opinion? I think our perf tests should be unittest-like and test very specific parts. Telemetry is typically what we use for integration style performance test.
Patchset #2 (id:20001) has been deleted
On 2015/06/25 13:49:30, reveman wrote: > On 2015/06/25 at 05:38:40, dongseong.hwang wrote: > > On 2015/06/24 18:49:50, reveman wrote: > > > I don't think we want our unit tests to require some form of ipc. Please try > to > > > design ozone code so we can run the same set of unit tests as on other > platforms > > > without using threads and ipc. > > > > To do that, ozone gpu code must open drm fd for test. > > spang, can I add above code in ozone layer for only test? > > Why can't we test this in the same way as we test IOSurface and SurfaceTexture > support? We have both unit tests and integration tests. > GpuMemoryBufferImpl/Factory unit tests and ChildThreadImpl integration tests. In rough explanation, ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only gpu side ozone (OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests()). Ozone GBM requires browser side initialization also. I'm hiding ozone specific code inside ozone in 2nd patch set. spang, reveman, could you review again? > https://codereview.chromium.org/1208603002/diff/1/content/test/content_test_s... > > File content/test/content_test_suite.cc (right): > > > > > https://codereview.chromium.org/1208603002/diff/1/content/test/content_test_s... > > content/test/content_test_suite.cc:81: scoped_ptr<ui::OzoneGpuTestHelper> > gpu_helper_; > > On 2015/06/24 22:16:17, vignatti wrote: > > > would be better if we had the full GPU service in place, so perf tests could > ran > > > just a like in a real setup. > > > > reveman, what do you think about vignatti's opinion? > > I think our perf tests should be unittest-like and test very specific parts. > Telemetry is typically what we use for integration style performance test. Let's talk about it further in https://codereview.chromium.org/1187793006/
On 2015/06/25 at 15:49:25, dongseong.hwang wrote: > On 2015/06/25 13:49:30, reveman wrote: > > On 2015/06/25 at 05:38:40, dongseong.hwang wrote: > > > On 2015/06/24 18:49:50, reveman wrote: > > > > I don't think we want our unit tests to require some form of ipc. Please try > > to > > > > design ozone code so we can run the same set of unit tests as on other > > platforms > > > > without using threads and ipc. > > > > > > To do that, ozone gpu code must open drm fd for test. > > > spang, can I add above code in ozone layer for only test? > > > > Why can't we test this in the same way as we test IOSurface and SurfaceTexture > > support? We have both unit tests and integration tests. > > GpuMemoryBufferImpl/Factory unit tests and ChildThreadImpl integration tests. > > In rough explanation, ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only gpu side ozone (OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests()). Ozone GBM requires browser side initialization also. That ozone code would rely on chromium IPC internally sounds inappropriate. Can you point me to the code that does that?
On 2015/06/25 16:23:45, reveman wrote: > On 2015/06/25 at 15:49:25, dongseong.hwang wrote: > > On 2015/06/25 13:49:30, reveman wrote: > > > On 2015/06/25 at 05:38:40, dongseong.hwang wrote: > > > > On 2015/06/24 18:49:50, reveman wrote: > > > > > I don't think we want our unit tests to require some form of ipc. Please > try > > > to > > > > > design ozone code so we can run the same set of unit tests as on other > > > platforms > > > > > without using threads and ipc. > > > > > > > > To do that, ozone gpu code must open drm fd for test. > > > > spang, can I add above code in ozone layer for only test? > > > > > > Why can't we test this in the same way as we test IOSurface and > SurfaceTexture > > > support? We have both unit tests and integration tests. > > > GpuMemoryBufferImpl/Factory unit tests and ChildThreadImpl integration > tests. > > > > In rough explanation, ozone uses chromium IPC internally unlike IOSurface and > SurfaceTexture. These unittests initialize only gpu side ozone > (OzonePlatform::InitializeForGPU() is called in > gfx::GLSurface::InitializeOneOffForTests()). Ozone GBM requires browser side > initialization also. > > That ozone code would rely on chromium IPC internally sounds inappropriate. Can > you point me to the code that does that? for example, https://code.google.com/p/chromium/codesearch#chromium/src/ui/ozone/platform/...
On 2015/06/25 at 16:30:58, dongseong.hwang wrote: > On 2015/06/25 16:23:45, reveman wrote: > > On 2015/06/25 at 15:49:25, dongseong.hwang wrote: > > > On 2015/06/25 13:49:30, reveman wrote: > > > > On 2015/06/25 at 05:38:40, dongseong.hwang wrote: > > > > > On 2015/06/24 18:49:50, reveman wrote: > > > > > > I don't think we want our unit tests to require some form of ipc. Please > > try > > > > to > > > > > > design ozone code so we can run the same set of unit tests as on other > > > > platforms > > > > > > without using threads and ipc. > > > > > > > > > > To do that, ozone gpu code must open drm fd for test. > > > > > spang, can I add above code in ozone layer for only test? > > > > > > > > Why can't we test this in the same way as we test IOSurface and > > SurfaceTexture > > > > support? We have both unit tests and integration tests. > > > > GpuMemoryBufferImpl/Factory unit tests and ChildThreadImpl integration > > tests. > > > > > > In rough explanation, ozone uses chromium IPC internally unlike IOSurface and > > SurfaceTexture. These unittests initialize only gpu side ozone > > (OzonePlatform::InitializeForGPU() is called in > > gfx::GLSurface::InitializeOneOffForTests()). Ozone GBM requires browser side > > initialization also. > > > > That ozone code would rely on chromium IPC internally sounds inappropriate. Can > > you point me to the code that does that? > > for example, https://code.google.com/p/chromium/codesearch#chromium/src/ui/ozone/platform/... Ok, seems a bit messy that we need to spin up some fake gpu/browser threads for unit tests but if there's already a system in place for this then I don't have any objections. Btw, this doesn't seem to enable any unit tests as mentioned in the description. Are these tests just failing prior to this?
On 2015/06/25 16:54:15, reveman wrote: > On 2015/06/25 at 16:30:58, dongseong.hwang wrote: > > On 2015/06/25 16:23:45, reveman wrote: > > > On 2015/06/25 at 15:49:25, dongseong.hwang wrote: > > > > On 2015/06/25 13:49:30, reveman wrote: > > > > > On 2015/06/25 at 05:38:40, dongseong.hwang wrote: > > > > > > On 2015/06/24 18:49:50, reveman wrote: > > > > > > > I don't think we want our unit tests to require some form of ipc. > Please > > > try > > > > > to > > > > > > > design ozone code so we can run the same set of unit tests as on > other > > > > > platforms > > > > > > > without using threads and ipc. > > > > > > > > > > > > To do that, ozone gpu code must open drm fd for test. > > > > > > spang, can I add above code in ozone layer for only test? > > > > > > > > > > Why can't we test this in the same way as we test IOSurface and > > > SurfaceTexture > > > > > support? We have both unit tests and integration tests. > > > > > GpuMemoryBufferImpl/Factory unit tests and ChildThreadImpl integration > > > tests. > > > > > > > > In rough explanation, ozone uses chromium IPC internally unlike IOSurface > and > > > SurfaceTexture. These unittests initialize only gpu side ozone > > > (OzonePlatform::InitializeForGPU() is called in > > > gfx::GLSurface::InitializeOneOffForTests()). Ozone GBM requires browser side > > > initialization also. > > > > > > That ozone code would rely on chromium IPC internally sounds inappropriate. > Can > > > you point me to the code that does that? > > > > for example, > https://code.google.com/p/chromium/codesearch#chromium/src/ui/ozone/platform/... > > Ok, seems a bit messy that we need to spin up some fake gpu/browser threads for > unit tests but if there's already a system in place for this then I don't have > any objections. > > Btw, this doesn't seem to enable any unit tests as mentioned in the description. > Are these tests just failing prior to this? these tests failed due to crash before :)
I just tried Patch Set 2 and tests are still broken.
On 2015/06/25 18:47:04, vignatti wrote: > I just tried Patch Set 2 and tests are still broken. I tried again and it works well. could you re-check? If failed again, could you copy&paste the crash log?
On 2015/06/26 09:04:34, dshwang wrote: > On 2015/06/25 18:47:04, vignatti wrote: > > I just tried Patch Set 2 and tests are still broken. > > I tried again and it works well. could you re-check? If failed again, could you > copy&paste the crash log? LGTM. I tried it again and now it works fine. Thank you dshwang.
On 2015/06/29 20:53:08, vignatti wrote: > On 2015/06/26 09:04:34, dshwang wrote: > > On 2015/06/25 18:47:04, vignatti wrote: > > > I just tried Patch Set 2 and tests are still broken. > > > > I tried again and it works well. could you re-check? If failed again, could > you > > copy&paste the crash log? > > LGTM. I tried it again and now it works fine. Thank you dshwang. Thank you for verifying. sky, spang, could you review?
On 2015/06/30 12:04:25, dshwang wrote: > sky, spang, could you review? sky, spang, could you review?
On 2015/07/07 08:59:31, dshwang wrote: > On 2015/06/30 12:04:25, dshwang wrote: > > sky, spang, could you review? > > sky, spang, could you review? sky, spang, could you review again?
Sorry, I started reviewing and got sidetracked. Can we maybe just make a helper class that calls the right initializers? Like OzoneGpuTestHelper. https://codereview.chromium.org/1208603002/diff/60001/ui/ozone/platform/drm/o... File ui/ozone/platform/drm/ozone_platform_gbm.cc (right): https://codereview.chromium.org/1208603002/diff/60001/ui/ozone/platform/drm/o... ui/ozone/platform/drm/ozone_platform_gbm.cc:207: ui_thread_.reset(new base::Thread("test_ui_thread")); Creating a "UI thread" won't work because any calls into UI code made by content in the test will be on the main thread, not the thread created here. So all of those calls become races. https://codereview.chromium.org/1208603002/diff/60001/ui/ozone/public/ozone_p... File ui/ozone/public/ozone_platform.h (right): https://codereview.chromium.org/1208603002/diff/60001/ui/ozone/public/ozone_p... ui/ozone/public/ozone_platform.h:55: static void InitializeForTest(); I don't think we should have an "InitializeForTest". Different tests have different requirements. Testing is a higher level concern than belongs here.
On 2015/07/13 17:08:26, spang wrote: > Sorry, I started reviewing and got sidetracked. > > Can we maybe just make a helper class that calls the right initializers? Like > OzoneGpuTestHelper. Thank you for good feedback. let me investigate how to extend OzoneGpuTestHelper to meet this requirement. > https://codereview.chromium.org/1208603002/diff/60001/ui/ozone/platform/drm/o... > File ui/ozone/platform/drm/ozone_platform_gbm.cc (right): > > https://codereview.chromium.org/1208603002/diff/60001/ui/ozone/platform/drm/o... > ui/ozone/platform/drm/ozone_platform_gbm.cc:207: ui_thread_.reset(new > base::Thread("test_ui_thread")); > Creating a "UI thread" won't work because any calls into UI code made by content > in the test will be on the main thread, not the thread created here. So all of > those calls become races. > > https://codereview.chromium.org/1208603002/diff/60001/ui/ozone/public/ozone_p... > File ui/ozone/public/ozone_platform.h (right): > > https://codereview.chromium.org/1208603002/diff/60001/ui/ozone/public/ozone_p... > ui/ozone/public/ozone_platform.h:55: static void InitializeForTest(); > I don't think we should have an "InitializeForTest". Different tests have > different requirements. Testing is a higher level concern than belongs here.
this CL is WIP. https://codereview.chromium.org/1208603002/diff/100001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.cc (right): https://codereview.chromium.org/1208603002/diff/100001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.cc:134: message_loop.reset(new base::MessageLoopForUI); MessageLoopForUI creation is ugly, but OzonePlatform::InitializeForGPU() in gbm ozone requires message loop. This is called in this stack. At the time, message loop is not created. #2 0x00000114fa33 content::ContentTestSuite::Initialize() #3 0x00000118319b base::TestSuite::Run() #4 0x00000117ceef base::(anonymous namespace)::LaunchUnitTestsInternal() #5 0x00000117d416 base::LaunchUnitTests() Later, some unittests create message loop in ad-hoc manner. #1 0x7f3f2f6c15cd base::MessageLoop::MessageLoop() #2 0x00000114b255 content::TestBrowserThreadBundle::Init() #3 0x00000114d780 content::RenderViewHostTestHarness::SetUp() #4 0x00000093cba6 content::NavigatorTestWithBrowserSideNavigation::SetUp() #5 0x0000011ba3a9 testing::Test::Run() #6 0x0000011ba598 testing::TestInfo::Run() #7 0x0000011ba6ed testing::TestCase::Run() #8 0x0000011bbaed testing::internal::UnitTestImpl::RunAllTests() #9 0x0000011bbd15 testing::UnitTest::Run() #10 0x00000118323d base::TestSuite::Run() #11 0x00000117ceef base::(anonymous namespace)::LaunchUnitTestsInternal() #12 0x00000117d416 base::LaunchUnitTests() I think it's natural for ContentTestSuite to create message loop, but refactoring TestBrowserThreadBundle or RenderViewHostTestHarness supposed to be very huge change. vignatti had the similar issue before, https://codereview.chromium.org/1043233003/ spang, vignatti, WDYT?
https://codereview.chromium.org/1208603002/diff/100001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.cc (right): https://codereview.chromium.org/1208603002/diff/100001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.cc:134: message_loop.reset(new base::MessageLoopForUI); On 2015/07/15 16:02:41, dshwang wrote: > MessageLoopForUI creation is ugly, but OzonePlatform::InitializeForGPU() in gbm > ozone requires message loop. > > This is called in this stack. At the time, message loop is not created. > > #2 0x00000114fa33 content::ContentTestSuite::Initialize() > #3 0x00000118319b base::TestSuite::Run() > #4 0x00000117ceef base::(anonymous namespace)::LaunchUnitTestsInternal() > #5 0x00000117d416 base::LaunchUnitTests() > > Later, some unittests create message loop in ad-hoc manner. > #1 0x7f3f2f6c15cd base::MessageLoop::MessageLoop() > #2 0x00000114b255 content::TestBrowserThreadBundle::Init() > #3 0x00000114d780 content::RenderViewHostTestHarness::SetUp() > #4 0x00000093cba6 content::NavigatorTestWithBrowserSideNavigation::SetUp() > #5 0x0000011ba3a9 testing::Test::Run() > #6 0x0000011ba598 testing::TestInfo::Run() > #7 0x0000011ba6ed testing::TestCase::Run() > #8 0x0000011bbaed testing::internal::UnitTestImpl::RunAllTests() > #9 0x0000011bbd15 testing::UnitTest::Run() > #10 0x00000118323d base::TestSuite::Run() > #11 0x00000117ceef base::(anonymous namespace)::LaunchUnitTestsInternal() > #12 0x00000117d416 base::LaunchUnitTests() > > I think it's natural for ContentTestSuite to create message loop, but > refactoring TestBrowserThreadBundle or RenderViewHostTestHarness supposed to be > very huge change. > > vignatti had the similar issue before, > https://codereview.chromium.org/1043233003/ > spang, vignatti, WDYT? another solution is to move `gfx::GLSurface::InitializeOneOffForTests()` from ContentTestSuite to each test such as TestBrowserThreadBundle.
spang, could you review it again? It makes it possible to run GpuMemoryBuffer unittest as well as to add GLImageOzone test to gl_unittests.
Patchset #7 (id:140001) has been deleted
https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... File base/test/launcher/unit_test_launcher.cc (right): https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... base/test/launcher/unit_test_launcher.cc:197: force_single_process = true; I don't think this is worth losing parallelism over - we don't run base unit tests with gbm in the typical case. https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.cc (right): https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.cc:149: message_loop_.reset(new base::MessageLoopForUI); This is a library component. I'm very unconvinced about creating a message loop for the main thread here. The main thread message loop is the main program's problem, so it belongs in either the test or test harness.
Thanks for reviewing. I'm back from vacation, so delay to response. https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... File base/test/launcher/unit_test_launcher.cc (right): https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... base/test/launcher/unit_test_launcher.cc:197: force_single_process = true; On 2015/12/02 17:00:03, spang wrote: > I don't think this is worth losing parallelism over - we don't run base unit > tests with gbm in the typical case. I agree. do you have good idea? My idea is - this class provide some hook or interface which client tests can add additional command line. - OzoneInitializerForTest or content_test_suite add additional command line via the interface. It will bloat the code. https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.cc (right): https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.cc:149: message_loop_.reset(new base::MessageLoopForUI); On 2015/12/02 17:00:03, spang wrote: > This is a library component. I'm very unconvinced about creating a message loop > for the main thread here. The main thread message loop is the main program's > problem, so it belongs in either the test or test harness. There are pros and cons. If the loop should belong to test harness, gl_image_test_support and content_test_suite needs additional ozone specific code. // time to initialize #if defined(USE_OZONE) message_loop_.reset(new base::MessageLoopForUI); #endif // time to destory #if defined(USE_OZONE) message_loop_.reset(); #endif Worth to note is that all code also creates OzoneInitializerForTest at the time. So it's not bad that OzoneInitializerForTest creates the loop by itself.
On 2015/12/08 01:12:38, dshwang wrote: > Thanks for reviewing. I'm back from vacation, so delay to response. > > https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... > File base/test/launcher/unit_test_launcher.cc (right): > > https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... > base/test/launcher/unit_test_launcher.cc:197: force_single_process = true; > On 2015/12/02 17:00:03, spang wrote: > > I don't think this is worth losing parallelism over - we don't run base unit > > tests with gbm in the typical case. > > I agree. do you have good idea? > > My idea is > - this class provide some hook or interface which client tests can add > additional command line. > - OzoneInitializerForTest or content_test_suite add additional command line via > the interface. > > It will bloat the code. > > https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... > File ui/ozone/public/ozone_gpu_test_helper.cc (right): > > https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... > ui/ozone/public/ozone_gpu_test_helper.cc:149: message_loop_.reset(new > base::MessageLoopForUI); > On 2015/12/02 17:00:03, spang wrote: > > This is a library component. I'm very unconvinced about creating a message > loop > > for the main thread here. The main thread message loop is the main program's > > problem, so it belongs in either the test or test harness. > > There are pros and cons. > > If the loop should belong to test harness, gl_image_test_support and > content_test_suite needs additional ozone specific code. > > // time to initialize > #if defined(USE_OZONE) > message_loop_.reset(new base::MessageLoopForUI); > #endif > > // time to destory > #if defined(USE_OZONE) > message_loop_.reset(); > #endif > > Worth to note is that all code also creates OzoneInitializerForTest at the time. > So it's not bad that OzoneInitializerForTest creates the loop by itself. Why do you need the #if defined(USE_OZONE) #endif ? If creating a message loop works on ozone, it should work on other platforms too. The DRM platform depends heavily on the message loop. It's just not OK for tests to enter this code when the main thread doesn't have one. If those tests need to work with this platform, then they'll have to provide a message loop.
https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... File base/test/launcher/unit_test_launcher.cc (right): https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... base/test/launcher/unit_test_launcher.cc:197: force_single_process = true; On 2015/12/08 01:12:38, dshwang wrote: > On 2015/12/02 17:00:03, spang wrote: > > I don't think this is worth losing parallelism over - we don't run base unit > > tests with gbm in the typical case. > > I agree. do you have good idea? > > My idea is > - this class provide some hook or interface which client tests can add > additional command line. > - OzoneInitializerForTest or content_test_suite add additional command line via > the interface. > > It will bloat the code. What I really want is that content_unittests and gl_unittests has kSingleProcessTestsFlag command line on only ozone. Do you have good idea? https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.cc (right): https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.cc:149: message_loop_.reset(new base::MessageLoopForUI); > Why do you need the > > #if defined(USE_OZONE) > #endif > > ? > > If creating a message loop works on ozone, it should work on other platforms > too. > > The DRM platform depends heavily on the message loop. It's just not OK for tests > to enter this code when the main thread doesn't have one. If those tests need to > work with this platform, then they'll have to provide a message loop. At least, gl_image_test_support and content_test_suite don't need message_loop so they don't create it. Do you want to create message loop for all platforms, although only ozone really requires it?
On 2015/12/08 22:19:34, dshwang wrote: > https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... > File base/test/launcher/unit_test_launcher.cc (right): > > https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... > base/test/launcher/unit_test_launcher.cc:197: force_single_process = true; > On 2015/12/08 01:12:38, dshwang wrote: > > On 2015/12/02 17:00:03, spang wrote: > > > I don't think this is worth losing parallelism over - we don't run base unit > > > tests with gbm in the typical case. > > > > I agree. do you have good idea? > > > > My idea is > > - this class provide some hook or interface which client tests can add > > additional command line. > > - OzoneInitializerForTest or content_test_suite add additional command line > via > > the interface. > > > > It will bloat the code. > > What I really want is that content_unittests and gl_unittests has > kSingleProcessTestsFlag command line on only ozone. Do you have good idea? I haven't looked into it, but hurting the case we actually run on the waterfall in favor of something that you're running locally is not a good tradeoff. If there isn't a good way to do it for platform=drm only, then you can just pass it on the command line locally. > > https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... > File ui/ozone/public/ozone_gpu_test_helper.cc (right): > > https://codereview.chromium.org/1208603002/diff/160001/ui/ozone/public/ozone_... > ui/ozone/public/ozone_gpu_test_helper.cc:149: message_loop_.reset(new > base::MessageLoopForUI); > > > Why do you need the > > > > #if defined(USE_OZONE) > > #endif > > > > ? > > > > If creating a message loop works on ozone, it should work on other platforms > > too. > > > > The DRM platform depends heavily on the message loop. It's just not OK for > tests > > to enter this code when the main thread doesn't have one. If those tests need > to > > work with this platform, then they'll have to provide a message loop. > > At least, gl_image_test_support and content_test_suite don't need message_loop > so they don't create it. > > Do you want to create message loop for all platforms, although only ozone really > requires it? I don't see what the issue is with providing one on all platforms. Anyway, either provide a message loop, or don't run code that requires a message loop. It's not OK for components that require a message loop on the current (non-owned) thread to create their own message loop. If everyone did that it would be a maintenance disaster.
Your message is clear. Let me apply soon. Thank you :) https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... File base/test/launcher/unit_test_launcher.cc (right): https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... base/test/launcher/unit_test_launcher.cc:197: force_single_process = true; > I haven't looked into it, but hurting the case we actually run on the waterfall > in favor of something that you're running locally is not a good tradeoff. If > there isn't a good way to do it for platform=drm only, then you can just pass it > on the command line locally. Interesting. I assume that we will make test bots cover ozone-gbm. However, you imply you don't have any plan. Got it. Let's re-visit when we make test bots cover ozone-gbm.
On 2015/12/08 23:04:34, dshwang wrote: > Your message is clear. Let me apply soon. Thank you :) > > https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... > File base/test/launcher/unit_test_launcher.cc (right): > > https://codereview.chromium.org/1208603002/diff/160001/base/test/launcher/uni... > base/test/launcher/unit_test_launcher.cc:197: force_single_process = true; > > I haven't looked into it, but hurting the case we actually run on the > waterfall > > in favor of something that you're running locally is not a good tradeoff. If > > there isn't a good way to do it for platform=drm only, then you can just pass > it > > on the command line locally. > > Interesting. I assume that we will make test bots cover ozone-gbm. However, you > imply you don't have any plan. > > Got it. Let's re-visit when we make test bots cover ozone-gbm. Not on the chrome waterfall - those bots don't even run on Chrome OS! We do run tests with gbm on the CrOS waterfall via autotest. See for example https://chromium.googlesource.com/chromiumos/third_party/autotest/+/stabilize... It's possible to pass extra flags needed to serialize the test execution within the autotest. These tests are quite special, they are allowed to be hardware dependent unlike most unit tests, and we run them across all supported boards.
Description was changed from ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless BUG=475633 ========== to ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 ==========
spang, could you review again? Now ContentTestSuite creates message loop.
https://codereview.chromium.org/1208603002/diff/180001/content/test/content_t... File content/test/content_test_suite.cc (right): https://codereview.chromium.org/1208603002/diff/180001/content/test/content_t... content/test/content_test_suite.cc:92: ozone_ = ui::OzoneInitializerForTest::Create(); Can you just call ui::OzonePlatform::InitializeForUI here? I do not think we want a OzoneInitializerForTest. What this is initializing is the UI.
https://codereview.chromium.org/1208603002/diff/180001/content/test/content_t... File content/test/content_test_suite.cc (right): https://codereview.chromium.org/1208603002/diff/180001/content/test/content_t... content/test/content_test_suite.cc:92: ozone_ = ui::OzoneInitializerForTest::Create(); On 2015/12/14 17:51:37, spang wrote: > Can you just call ui::OzonePlatform::InitializeForUI here? > > I do not think we want a OzoneInitializerForTest. What this is initializing is > the UI. What OzoneInitializerForTest is doing is to initialize both UI and GPU and wait for all communications. Especially unittests must wait for UI opening drm card0 and GPU receiving the drm fd. We can replace it with OzonePlatform::InitializeForUI(); OzonePlatform::InitializeForGPU(); gpu_helper_.reset(new OzoneGpuTestHelper); if (!gpu_helper_->Initialize(base::ThreadTaskRunnerHandle::Get(), base::ThreadTaskRunnerHandle::Get())) { return false; } { base::MessageLoop::ScopedNestableTaskAllower allow( base::MessageLoop::current()); base::RunLoop().RunUntilIdle(); } OzoneInitializerForTest abstract these code.
Patchset #10 (id:220001) has been deleted
hi spang, could you review it again? gl_unittests requires it. https://codereview.chromium.org/1484473003/
https://codereview.chromium.org/1208603002/diff/260001/content/test/content_t... File content/test/content_test_suite.cc (right): https://codereview.chromium.org/1208603002/diff/260001/content/test/content_t... content/test/content_test_suite.cc:92: if (!base::MessageLoop::current()) is this conditional needed? https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.h (right): https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.h:48: class OZONE_EXPORT OzoneInitializerForTest { Do we really need this class? I feel like the code would be easier to understand if we just called OzonePlatform::InitializeForUI/GPU() directly from ContentTestSuite::Initialize(). https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.h:57: scoped_ptr<OzoneGpuTestHelper> gpu_helper_; What is this used for?
https://codereview.chromium.org/1208603002/diff/260001/content/common/gpu/cli... File content/common/gpu/client/gl_helper_unittest.cc (right): https://codereview.chromium.org/1208603002/diff/260001/content/common/gpu/cli... content/common/gpu/client/gl_helper_unittest.cc:1994: base::MessageLoopForUI message_loop; content_gl_tests creates a ContentTestSuite instance after creating MessageLoopForUI. https://codereview.chromium.org/1208603002/diff/260001/content/test/content_t... File content/test/content_test_suite.cc (right): https://codereview.chromium.org/1208603002/diff/260001/content/test/content_t... content/test/content_test_suite.cc:92: if (!base::MessageLoop::current()) On 2016/01/11 19:13:51, reveman wrote: > is this conditional needed? content_gl_tests calls it after creating MessageLoopForUI. https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.h (right): https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.h:48: class OZONE_EXPORT OzoneInitializerForTest { On 2016/01/11 19:13:51, reveman wrote: > Do we really need this class? I feel like the code would be easier to understand > if we just called OzonePlatform::InitializeForUI/GPU() directly from > ContentTestSuite::Initialize(). The first patch set was similar to what you say now. However, in https://codereview.chromium.org/1208603002/#msg3, you said "I don't think we want our unit tests to require some form of ipc. Please try to design ozone code so we can run the same set of unit tests as on other platforms without using threads and ipc." It's how OzoneInitializerForTest was introduced. In addition, gl_unittests needs similar code now; https://codereview.chromium.org/1484473003/diff/60001/ui/gl/test/run_all_unit... So this test helper should stay in somewhere ozone/. https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.h:57: scoped_ptr<OzoneGpuTestHelper> gpu_helper_; On 2016/01/11 19:13:51, reveman wrote: > What is this used for? This sets up message forwarding between the "gpu" and "ui" threads. Like ozone_demo, OzoneInitializerForTest initializes both OzonePlatform UI and OzonePlatform GPU in the same thread. OzoneGpuTestHelper helps to establish ipc channel for both UI and GPU.
spang, could you review again?
https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.h (right): https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.h:48: class OZONE_EXPORT OzoneInitializerForTest { On 2016/01/11 at 20:17:45, dshwang wrote: > On 2016/01/11 19:13:51, reveman wrote: > > Do we really need this class? I feel like the code would be easier to understand > > if we just called OzonePlatform::InitializeForUI/GPU() directly from > > ContentTestSuite::Initialize(). > > The first patch set was similar to what you say now. However, in https://codereview.chromium.org/1208603002/#msg3, you said > "I don't think we want our unit tests to require some form of ipc. Please try to > design ozone code so we can run the same set of unit tests as on other platforms > without using threads and ipc." > > It's how OzoneInitializerForTest was introduced. I didn't mean that we should hide the need for ipc and threads in an ozone class but rather that we should removed it. The gl_image unit tests are not supposed to exercise any IPC code. I don't know how hard it would be to fix this but that the current code rely on threads and ipc makes me concerned as it's not clear that we can reuse any of this code in a MUS world with Mojo IPC. > > In addition, gl_unittests needs similar code now; https://codereview.chromium.org/1484473003/diff/60001/ui/gl/test/run_all_unit... > > So this test helper should stay in somewhere ozone/. https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.h:57: scoped_ptr<OzoneGpuTestHelper> gpu_helper_; On 2016/01/11 at 20:17:45, dshwang wrote: > On 2016/01/11 19:13:51, reveman wrote: > > What is this used for? > > This sets up message forwarding between the "gpu" and "ui" threads. > Like ozone_demo, OzoneInitializerForTest initializes both OzonePlatform UI and OzonePlatform GPU in the same thread. OzoneGpuTestHelper helps to establish ipc channel for both UI and GPU. I guess this is a more general Ozone testing problem but I think it's a mistake that we need to spin up threads and use ipc to exercise this code.
https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... File ui/ozone/public/ozone_gpu_test_helper.h (right): https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.h:48: class OZONE_EXPORT OzoneInitializerForTest { On 2016/01/13 15:05:17, reveman wrote: > I didn't mean that we should hide the need for ipc and threads in an ozone class > but rather that we should removed it. The gl_image unit tests are not supposed > to exercise any IPC code. I don't know how hard it would be to fix this but that > the current code rely on threads and ipc makes me concerned as it's not clear > that we can reuse any of this code in a MUS world with Mojo IPC. I see that you want light-version ozone backend for test which don't bother ipc or thread. Current Ozone can not work like it. To test GpuMemoryBuffer, it needs both browser and gpu ozone backend. browser backend grabs gpu device and send it to gpu, which require ipc. browser backend is also needed to create NativePixmap which belong to GpuMemoryBuffer in the test. gpu backend creates actual drm buffer and gl image. In the same sense, gl_unittests needs both browser and gpu ozone backend because gl_unittests needs to create NativePixmap. To not use ipc and thread, we need to create OzonePlatform::InitializeForTest(), which each ozone backend (e.g. gbm, wayland) supports by itself. It might be too big for only test. reveman, spang, what do you think? https://codereview.chromium.org/1208603002/diff/260001/ui/ozone/public/ozone_... ui/ozone/public/ozone_gpu_test_helper.h:57: scoped_ptr<OzoneGpuTestHelper> gpu_helper_; On 2016/01/13 15:05:17, reveman wrote: > I guess this is a more general Ozone testing problem but I think it's a mistake > that we need to spin up threads and use ipc to exercise this code. As I explain above, it's accidental problem to reuse current ozone code. It we want to avoid ipc and thread, ozone should provide something like ContextProviderInProcess. Current Ozone provide only something like ContextProviderCommandBuffer.
My apologies, I haven't given this the attention it deserves. I think you are on the wrong track, and so here's my attempt to steer this work in a different direction. I think the premise that content/ can *unit test* itself in combination with platform/gbm is wrong. Because of that, you're running into various layering issues that are actually not worth solving. gbm can unit test itself. content can unit test itself. We already run unit tests for both components on the chrome buildbots. But when content unit tests itself, it *at most* uses a mock ozone platform called "headless". Ideally it would mock everything itself, and never call through to ozone, but if it doesn't mock its dependencies well enough then underneath ozone there's a silly little platform with no graphics or input. content's unit tests pass with the headless platform, are we are happy with that because those tests are intended for testing the code inside of content; they are not and should not be strongly opinionated about platform behaviors. I do not think we should care about supporting tests in content/ that call themselves unit tests, but depend on nontrivial behavior at the platform level, and which oversimplify the runtime environment of those platforms by imposing new threading requirements on them. The gbm platform implements a display server inside of chrome. Early on in the freon project, we made the decision to only support the GPU process path for graphics. Every other mode content/ can run in on other platforms is totally unsupported, even if we try to use them only from tests. So I think trying to say that these tests are unit tests, and will be single threaded, but also start up gbm's internal display server with was not designed to run that way is a non-starter. But that's not to say we cannot do any testing, just that the approach used by these unit tests is not the way to do it. If you want to write a test in content/ to run with gbm, you just have to satisfy our assumptions. Take video_decode_accelerator_unittest for example. Nevermind that it calls itself a "unit test" - it's a complex, multithreaded hardware test and it's designed to exercise Chrome OS hardware in addition to exercising the video plumbing. It's a test, and it satisfies our assumptions, so it's able to run with platform=gbm on Chrome OS hardware. So if you want to run tests with platform=gbm from content, please do the following: (1) make a new test program with its own main function (like video_decode_accelerator_unittest). Maybe share some code with VDAunittest like rendering_helper.cc (2) create all of the threads & message loops we require (which are just a couple of the threads content uses in the gpu process path) (3) create an autotest in CrOS to run that test on each chrome os device, and get that test added to the CrOS hwtest suite It's essential to do (3) because otherwise we can't run the test automatically on the waterfall. The buildbots don't have the dependencies necessary to run this code, only Chrome OS devices do. So it needs to use autotest, and should really have its own target.
On 2016/01/18 20:39:56, spang wrote: > My apologies, I haven't given this the attention it deserves. I think you are on > the wrong track, and so here's my attempt to steer this work in a different > direction. > > I think the premise that content/ can *unit test* itself in combination with > platform/gbm is wrong. Because of that, you're running into various layering > issues that are actually not worth solving. > > gbm can unit test itself. content can unit test itself. We already run unit > tests for both components on the chrome buildbots. > > But when content unit tests itself, it *at most* uses a mock ozone platform > called "headless". Ideally it would mock everything itself, and never call > through to ozone, but if it doesn't mock its dependencies well enough then > underneath ozone there's a silly little platform with no graphics or input. > content's unit tests pass with the headless platform, are we are happy with that > because those tests are intended for testing the code inside of content; they > are not and should not be strongly opinionated about platform behaviors. > > I do not think we should care about supporting tests in content/ that call > themselves unit tests, but depend on nontrivial behavior at the platform level, > and which oversimplify the runtime environment of those platforms by imposing > new threading requirements on them. The gbm platform implements a display server > inside of chrome. Early on in the freon project, we made the decision to only > support the GPU process path for graphics. Every other mode content/ can run in > on other platforms is totally unsupported, even if we try to use them only from > tests. > > So I think trying to say that these tests are unit tests, and will be single > threaded, but also start up gbm's internal display server with was not designed > to run that way is a non-starter. > > But that's not to say we cannot do any testing, just that the approach used by > these unit tests is not the way to do it. If you want to write a test in > content/ to run with gbm, you just have to satisfy our assumptions. > > Take video_decode_accelerator_unittest for example. Nevermind that it calls > itself a "unit test" - it's a complex, multithreaded hardware test and it's > designed to exercise Chrome OS hardware in addition to exercising the video > plumbing. It's a test, and it satisfies our assumptions, so it's able to run > with platform=gbm on Chrome OS hardware. > > So if you want to run tests with platform=gbm from content, please do the > following: > > (1) make a new test program with its own main function (like > video_decode_accelerator_unittest). Maybe share some code with VDAunittest like > rendering_helper.cc > (2) create all of the threads & message loops we require (which are just a > couple of the threads content uses in the gpu process path) > (3) create an autotest in CrOS to run that test on each chrome os device, and > get that test added to the CrOS hwtest suite > > It's essential to do (3) because otherwise we can't run the test automatically > on the waterfall. The buildbots don't have the dependencies necessary to run > this code, only Chrome OS devices do. So it needs to use autotest, and should > really have its own target. And to wrap things up, the problems that have caused this CL to drag on in review mostly just go away with this approach: - The GPU mutual exclusion issue. You can write test to serialize appropriately out of the box. While unit tests are expected to be able to race one another, this would be a hardware test and it's natural to serialize it. - Design damage to the content tests. There's no message loop issue, or weird threads that show up only under ifdef. No test helper. - Test-only interfaces that don't generalize. No OzonePlatform::InitializeForTest() because the test takes care to satisfy the production code's assumptions. - Runs on the waterfall. The test wouldn't even have been instantiated with gbm as implemented. It's hard to justify maintaining test code that doesn't run automatically. Sorry it took so long to give this feedback, the time this has spent in review is clearly unacceptable. It's unfair to you to work for so long without being set on a path to something that'll actually work.
Ok, GBM needs to make something like video_decode_accelerator_unittest, rather than reusing existing content unittests. The test logic would be very similar to what GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests cover, though. reveman, in the same sense, GBM can not reuse gl_unittests in ui/gl also. https://codereview.chromium.org/1576353007/ IMO, it's quite bad news for GBM.
You can still reuse any necessary code, it just needs a test harness that is capable of dealing with hardware. I don't think such a harness is really suitable for content_unittests; those tests should be hermetic. We shouldn't conflate end-to-end hardware tests with unit tests.
On 2016/01/19 20:07:21, spang wrote: > You can still reuse any necessary code, it just needs a test harness that is > capable of dealing with hardware. > > I don't think such a harness is really suitable for content_unittests; those > tests should be hermetic. We shouldn't conflate end-to-end hardware tests with > unit tests. Thank you for explaining. I agree. Let me add new gbm test reusing the code in GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests.
When VGEM is removed, it's fixed. so close it as WontFix
dongseong.hwang@intel.com changed reviewers: - sky@chromium.org
dongseong.hwang@intel.com changed reviewers: - reveman@chromium.org, spang@chromium.org
On 2016/03/30 10:32:17, dshwang wrote: > When VGEM is removed, it's fixed. so close it as WontFix I was wrong. It's still issue.
Description was changed from ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 ========== to ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Description was changed from ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests gpu_unittests --gtest_filter=GpuMemory* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Patchset #13 (id:300001) has been deleted
Patchset #10 (id:240001) has been deleted
Description was changed from ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests gpu_unittests --gtest_filter=GpuMemory* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests gpu_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests gl_unittests --gtest_filter=GLImage* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Patchset #12 (id:320001) has been deleted
Description was changed from ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=content_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests gpu_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests gl_unittests --gtest_filter=GLImage* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== content: implement unittests backend for Ozone GBM GpuMemoryBufferImplTests and GpuMemoryBufferFactoryTests on Ozone GBM crash currently. Ozone uses chromium IPC internally unlike IOSurface and SurfaceTexture. These unittests initialize only GPU side Ozone; OzonePlatform::InitializeForGPU() is called in gfx::GLSurface::InitializeOneOffForTests(). Ozone GBM requires browser side initialization also. Introduce OzonePlatform::InitializeForTest() interface to initialize everything synchronously for unit tests. TEST=gpu_unittests --gtest_filter=GpuMemoryBuffer* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests gl_unittests --gtest_filter=GLImage* --ozone-platform=gbm --ozone-use-surfaceless --single-process-tests BUG=475633 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
