Make gpu black list work again on Linux

Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895
R=jam@chromium.org, kbr@chromium.org, tsepez@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox ./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/021319910e95dd56440948ccee3b951acdf7973f
Cr-Commit-Position: refs/heads/master@{#374969}

[Julien Isorce Samsung, 2015-12-23 16:47:03]

Description was changed from

==========
Make gpu black list work again

g_browser_process->gl_string_manager()->Initialize() is currently
called before GpuDataManagerImpl::GetInstance()->Initialize().
Problem is that gl_string_manager()->Initialize() calls
content::GpuDataManager::GetInstance()->SetGLStrings. Which will
be erased by the call to GpuDataManagerImpl::GetInstance()->Initialize().

Then GpuDataManagerImplPrivate::InitializeImpl fails to properly
setup the gpu black list using kGpuDriverBugListJson.

BUG=571895

R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
currently starts the gpu process whereas it should not because
GPU_FEATURE_TYPE_GPU_COMPOSITING is black listed by default
and because kGpuDriverBugListJson does not have any exception
for Mesa softpipe driver.

Review URL:
==========

to

==========
Make gpu black list work again

g_browser_process->gl_string_manager()->Initialize() is currently
called before GpuDataManagerImpl::GetInstance()->Initialize().
Problem is that gl_string_manager()->Initialize() calls
content::GpuDataManager::GetInstance()->SetGLStrings. Which will
be erased by the call to GpuDataManagerImpl::GetInstance()->Initialize().

Then GpuDataManagerImplPrivate::InitializeImpl fails to properly
setup the gpu black list using kGpuDriverBugListJson.

BUG=571895

R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
currently starts the gpu process whereas it should not because
GPU_FEATURE_TYPE_GPU_COMPOSITING is black listed by default
and because kGpuDriverBugListJson does not have any exception
for Mesa softpipe driver.

Review URL:
==========

[Julien Isorce Samsung, 2015-12-23 16:47:04]

j.isorce@samsung.com changed reviewers:
+ brettw@chromium.org

[Julien Isorce Samsung, 2015-12-23 16:47:45]

j.isorce@samsung.com changed reviewers:
+ thestig@chromium.org

[Julien Isorce Samsung, 2015-12-27 07:59:56]

j.isorce@samsung.com changed reviewers:
+ cwallez@chromium.org, hendrikw@chromium.org, jbauman@chromium.org,
sadrul@chromium.org

[Julien Isorce Samsung, 2016-01-05 15:32:25]

j.isorce@samsung.com changed reviewers:
+ sky@chromium.org

[Julien Isorce Samsung, 2016-01-05 15:34:52]

On 2016/01/05 15:32:25, j.isorce wrote:
> mailto:j.isorce@samsung.com changed reviewers:
> + mailto:sky@chromium.org

The problem still occurs with current master and I found it is quite an
important issue. (Diff between Patch Set 1 and 2 is just to remove DEPS and
chrome/VERSION changes that were submitted by mistake)
Please review, thx.

[Zhenyao Mo, 2016-01-05 18:04:45]

https://codereview.chromium.org/1547793004/diff/20001/chrome/browser/chrome_b...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/1547793004/diff/20001/chrome/browser/chrome_b...
chrome/browser/chrome_browser_main.cc:1140: // Alternatively we could ensure
that content::GpuDataManager::GetInstance()
No, we want to be explicit where and when Initialize() happens.

Otherwise who knows what kind of bugs may sneak into this mechanism.

Please remove this last "Alternatively" comment.

It's really sad the gl_string_manager initialization got moved to the wrong
place.

To prevent this from happening again, what we probably could do is to add a
initialized_ member in GpuDataManagerImpl and DCHECK in GetInstance() that it's
already initialized.

[Ken Russell (switch to Gerrit), 2016-01-05 19:32:06]

kbr@chromium.org changed reviewers:
+ jam@chromium.org

[Ken Russell (switch to Gerrit), 2016-01-05 19:32:07]

I'm not sure it's a good idea (or needed) to add more hooks to BrowserMainParts.

[piman, 2016-01-05 22:44:31]

On Tue, Jan 5, 2016 at 11:32 AM, <kbr@chromium.org> wrote:

> I'm not sure it's a good idea (or needed) to add more hooks to
> BrowserMainParts.
>

Agreed. Is there a way instead that we can make sure GpuDataManagerImpl is
initialized before PreCreateThreads is called?


>
> https://codereview.chromium.org/1547793004/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Julien Isorce Samsung, 2016-01-06 01:00:19]

Description was changed from

==========
Make gpu black list work again

g_browser_process->gl_string_manager()->Initialize() is currently
called before GpuDataManagerImpl::GetInstance()->Initialize().
Problem is that gl_string_manager()->Initialize() calls
content::GpuDataManager::GetInstance()->SetGLStrings. Which will
be erased by the call to GpuDataManagerImpl::GetInstance()->Initialize().

Then GpuDataManagerImplPrivate::InitializeImpl fails to properly
setup the gpu black list using kGpuDriverBugListJson.

BUG=571895

R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
currently starts the gpu process whereas it should not because
GPU_FEATURE_TYPE_GPU_COMPOSITING is black listed by default
and because kGpuDriverBugListJson does not have any exception
for Mesa softpipe driver.

Review URL:
==========

to

==========
Make gpu black list work again
    
Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

BUG=571895

R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
==========

[Julien Isorce Samsung, 2016-01-06 01:09:24]

Sorry my mistake it seems I got confused by my own comments after multiple
attempts.

So currently the order is correct. The problem is that
gl_string_manager()->Initialize() is called after starting the gpu process.
But if I move it to PreCreateThreads then the order is wrong.

The Patch Set 3 is the same (just updated the comment), but I completely
rewritten the CL description. It contains the explanation why I would like to
move gl_string_manager()->Initialize() (which requires a new hook)

So please read this new CL description. Thx.

[Ken Russell (switch to Gerrit), 2016-01-06 01:53:24]

Description was changed from

==========
Make gpu black list work again
    
Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

BUG=571895

R=kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
==========

to

==========
Make gpu black list work again
    
Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

BUG=571895

R=piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
==========

[Ken Russell (switch to Gerrit), 2016-01-06 01:53:35]

Description was changed from

==========
Make gpu black list work again
    
Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

BUG=571895

R=piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
==========

to

==========
Make gpu black list work again
    
Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

BUG=571895

R=piman@chromium.org, zmo@chromium.org, jam@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
==========

[Ken Russell (switch to Gerrit), 2016-01-06 01:55:06]

Thanks for finding and fixing this problem but I'd like to excuse myself from
this review. If it's absolutely, positively necessary to add a new hook then I'd
suggest also renaming PreCreateThreads to PreCreateThreadsBegin, because
PreCreateThreadsEnd doesn't match any of the naming conventions in
BrowserMainParts.

Suggesting jam@ as an alternate reviewer.

[Ken Russell (switch to Gerrit), 2016-01-06 01:56:00]

On 2016/01/06 01:55:06, Ken Russell wrote:
> Thanks for finding and fixing this problem but I'd like to excuse myself from
> this review. If it's absolutely, positively necessary to add a new hook then
I'd
> suggest also renaming PreCreateThreads to PreCreateThreadsBegin, because
> PreCreateThreadsEnd doesn't match any of the naming conventions in
> BrowserMainParts.
> 
> Suggesting jam@ as an alternate reviewer.

Also, it's critical that tests be added for this, because otherwise it's going
to regress again. Please think about how tests can be added for this.

[Julien Isorce Samsung, 2016-01-07 00:04:07]

On 2016/01/06 01:56:00, Ken Russell wrote:
> On 2016/01/06 01:55:06, Ken Russell wrote:
> > Thanks for finding and fixing this problem but I'd like to excuse myself
from
> > this review. If it's absolutely, positively necessary to add a new hook then
> I'd
> > suggest also renaming PreCreateThreads to PreCreateThreadsBegin, because
> > PreCreateThreadsEnd doesn't match any of the naming conventions in
> > BrowserMainParts.
> > 
> > Suggesting jam@ as an alternate reviewer.
> 
> Also, it's critical that tests be added for this, because otherwise it's going
> to regress again. Please think about how tests can be added for this.

Hi, no problem. Then looking at
"content/test/gpu/gpu_tests/gpu_test_expectations_unittest.py" and
"content/test/gpu/gpu_tests/gpu_process.py", I could maybe add a something like:
self.assertExpectationEquals('fail', page, gl_renderer='Gallium.*softpipe') and
use it in combination to "has_gpu_process_js". So that a machine runs the
hypothetic new test with Mesa softpipe driver, it will fail if a gpu process is
detected.
Should I put the new test in gpu_process.py ?

[Ken Russell (switch to Gerrit), 2016-01-07 00:54:26]

On 2016/01/07 00:04:07, j.isorce wrote:
> On 2016/01/06 01:56:00, Ken Russell wrote:
> > On 2016/01/06 01:55:06, Ken Russell wrote:
> > > Thanks for finding and fixing this problem but I'd like to excuse myself
> from
> > > this review. If it's absolutely, positively necessary to add a new hook
then
> > I'd
> > > suggest also renaming PreCreateThreads to PreCreateThreadsBegin, because
> > > PreCreateThreadsEnd doesn't match any of the naming conventions in
> > > BrowserMainParts.
> > > 
> > > Suggesting jam@ as an alternate reviewer.
> > 
> > Also, it's critical that tests be added for this, because otherwise it's
going
> > to regress again. Please think about how tests can be added for this.
> 
> Hi, no problem. Then looking at
> "content/test/gpu/gpu_tests/gpu_test_expectations_unittest.py" and
> "content/test/gpu/gpu_tests/gpu_process.py", I could maybe add a something
like:
> self.assertExpectationEquals('fail', page, gl_renderer='Gallium.*softpipe')
and
> use it in combination to "has_gpu_process_js". So that a machine runs the
> hypothetic new test with Mesa softpipe driver, it will fail if a gpu process
is
> detected.
> Should I put the new test in gpu_process.py ?

Looking at gpu_process.py I don't think it will cover this case sufficiently. It
looks to me like you need to be able to spoof the vendor and device PCI IDs, and
GL renderer and vendor strings, for testing purposes. Then you'd need to launch
Chrome and assert that a GPU process was or was not started for a particular
case.

If you add the ability to spoof the GPU info, and after doing so gpu_process.py
would be the best place to put the new test, please go ahead.

[brettw, 2016-01-07 19:04:03]

Removing myself as reviewer since I don't seem to need to be involved now.

[Julien Isorce Samsung, 2016-01-08 18:22:06]

On 2016/01/07 00:54:26, Ken Russell wrote:
> Looking at gpu_process.py I don't think it will cover this case sufficiently.
It
> looks to me like you need to be able to spoof the vendor and device PCI IDs,
and
> GL renderer and vendor strings, for testing purposes. Then you'd need to
launch
> Chrome and assert that a GPU process was or was not started for a particular
> case.
> 
> If you add the ability to spoof the GPU info, and after doing so
gpu_process.py
> would be the best place to put the new test, please go ahead.

I see 3 potential solutions:

1: Add a new "switches::kGLVendorString" and pass it to the cmd line through in
gpu_process.py.
Then in "GLStringManager::Initialize()" we could parse this switch if exist
instead of getting from "local_state->GetString(prefs::kGLVendorString);".

2: Find a generic way to set the local "prefs" GLStringManager::RegisterPrefs()
(see also BrowserProcessImpl::CreateLocalState()) from a python test.
Maybe it is already done/used in other tests than gpu ?

3: In telemetry there is "fake_gpu_info.FAKE_GPU_INFO /
FakeSystemInfo(system_info.SystemInfo)". It is not clear to me if it usable in
our case because I can see "fakes.FakePossibleBrowser()" in
webgl_conformance_expectations_unittest.py. I am not sure how fake it is. I
still need to run a browser.

I need your lights at this point to go further. Thx.

[Julien Isorce Samsung, 2016-01-11 17:14:33]

Hi, I addressed remarks. I also added the assert that zmo suggested.

[Zhenyao Mo, 2016-01-11 23:23:33]

https://codereview.chromium.org/1547793004/diff/60001/content/browser/gpu/gpu...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/1547793004/diff/60001/content/browser/gpu/gpu...
content/browser/gpu/gpu_data_manager_impl_private.cc:277: return finalized_;
You can't use finalized_.  It has a very different semantic meaning from
initialized.  You should define a new variable initialized_.

Besides, it's a not a good idea to define a function that's not used at all.  It
seems that the bug is not we use the GpuDataManager before it's initialized, so
this CL does not need to include this change.

[Ken Russell (switch to Gerrit), 2016-01-11 23:50:16]

On 2016/01/08 18:22:06, j.isorce wrote:
> On 2016/01/07 00:54:26, Ken Russell wrote:
> > Looking at gpu_process.py I don't think it will cover this case
sufficiently.
> It
> > looks to me like you need to be able to spoof the vendor and device PCI IDs,
> and
> > GL renderer and vendor strings, for testing purposes. Then you'd need to
> launch
> > Chrome and assert that a GPU process was or was not started for a particular
> > case.
> > 
> > If you add the ability to spoof the GPU info, and after doing so
> gpu_process.py
> > would be the best place to put the new test, please go ahead.
> 
> I see 3 potential solutions:
> 
> 1: Add a new "switches::kGLVendorString" and pass it to the cmd line through
in
> gpu_process.py.
> Then in "GLStringManager::Initialize()" we could parse this switch if exist
> instead of getting from "local_state->GetString(prefs::kGLVendorString);".

You'll need to be able to potentially override four pieces of information:

1. the vendor PCI id
2. the device PCI id
3. the GL_VENDOR string
4. the GL_RENDERER string

I would suggest you add four command line flags to
src/gpu/config/gpu_switches.cc. Perhaps something like

--gpu-testing-vendor-id (kGpuTestingVendorId)
--gpu-testing-device-id (kGpuTestingDeviceId)
--gpu-testing-gl-vendor (kGpuTestingGLVendor)
--gpu-testing-gl-renderer (kGpuTestingGLRenderer)

These should override the values from the GpuInfoCollector if they're present.

> 2: Find a generic way to set the local "prefs"
GLStringManager::RegisterPrefs()
> (see also BrowserProcessImpl::CreateLocalState()) from a python test.
> Maybe it is already done/used in other tests than gpu ?

See above.

> 3: In telemetry there is "fake_gpu_info.FAKE_GPU_INFO /
> FakeSystemInfo(system_info.SystemInfo)". It is not clear to me if it usable in
> our case because I can see "fakes.FakePossibleBrowser()" in
> webgl_conformance_expectations_unittest.py. I am not sure how fake it is. I
> still need to run a browser.

This isn't applicable. It only affects the behavior of the Python harness, not
the actual browser, so would test the wrong thing.

[Julien Isorce Samsung, 2016-01-12 17:51:51]

Description was changed from

==========
Make gpu black list work again
    
Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

BUG=571895

R=piman@chromium.org, zmo@chromium.org, jam@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
==========

to

==========
Make gpu black list work again
    
Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
 - Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
 - Added cmd line switches gpu-testing-vendor-id, gpu-testing-device-id,
   gpu-testing-gl-vendor, gpu-testing-gl-renderer
 - Added no_gpu_process unit test

BUG=571895

R=jam@chromium.org, kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py no_gpu_process
==========

[Julien Isorce Samsung, 2016-01-12 17:57:41]

On 2016/01/11 23:50:16, Ken Russell wrote:
> On 2016/01/08 18:22:06, j.isorce wrote:
> > On 2016/01/07 00:54:26, Ken Russell wrote:
> > > Looking at gpu_process.py I don't think it will cover this case
> sufficiently.
> > It
> > > looks to me like you need to be able to spoof the vendor and device PCI
IDs,
> > and
> > > GL renderer and vendor strings, for testing purposes. Then you'd need to
> > launch
> > > Chrome and assert that a GPU process was or was not started for a
particular
> > > case.
> > > 
> > > If you add the ability to spoof the GPU info, and after doing so
> > gpu_process.py
> > > would be the best place to put the new test, please go ahead.
> > 
> > I see 3 potential solutions:
> > 
> > 1: Add a new "switches::kGLVendorString" and pass it to the cmd line through
> in
> > gpu_process.py.
> > Then in "GLStringManager::Initialize()" we could parse this switch if exist
> > instead of getting from "local_state->GetString(prefs::kGLVendorString);".
> 
> You'll need to be able to potentially override four pieces of information:
> 
> 1. the vendor PCI id
> 2. the device PCI id
> 3. the GL_VENDOR string
> 4. the GL_RENDERER string
> 
> I would suggest you add four command line flags to
> src/gpu/config/gpu_switches.cc. Perhaps something like
> 
> --gpu-testing-vendor-id (kGpuTestingVendorId)
> --gpu-testing-device-id (kGpuTestingDeviceId)
> --gpu-testing-gl-vendor (kGpuTestingGLVendor)
> --gpu-testing-gl-renderer (kGpuTestingGLRenderer)
> 
> These should override the values from the GpuInfoCollector if they're present.
> 
Done, I also added kGpuTestingGLVersion because it is used to set
gpu-driver-vendor and gpu-driver-version

Also I think the issue is not as big as I thought. Indeed the gpu process is
started even if card is black listed (because gl_string_manager is initialized
too late), but the gpu process will not be used because when creating the gpu
channel there is gpu_message_filter.cc::CanUseGpuBrowserCompositor(). Indeed the
gpu info are up to date by this time.

But I think there are other issues, I remember in gpu main it checks for one gpu
switch and it is empty. I need to check.

[jbauman, 2016-01-12 23:56:22]

On 2016/01/12 17:57:41, j.isorce wrote:
> On 2016/01/11 23:50:16, Ken Russell wrote:
> > On 2016/01/08 18:22:06, j.isorce wrote:
> > > On 2016/01/07 00:54:26, Ken Russell wrote:
> > > > Looking at gpu_process.py I don't think it will cover this case
> > sufficiently.
> > > It
> > > > looks to me like you need to be able to spoof the vendor and device PCI
> IDs,
> > > and
> > > > GL renderer and vendor strings, for testing purposes. Then you'd need to
> > > launch
> > > > Chrome and assert that a GPU process was or was not started for a
> particular
> > > > case.
> > > > 
> > > > If you add the ability to spoof the GPU info, and after doing so
> > > gpu_process.py
> > > > would be the best place to put the new test, please go ahead.
> > > 
> > > I see 3 potential solutions:
> > > 
> > > 1: Add a new "switches::kGLVendorString" and pass it to the cmd line
through
> > in
> > > gpu_process.py.
> > > Then in "GLStringManager::Initialize()" we could parse this switch if
exist
> > > instead of getting from "local_state->GetString(prefs::kGLVendorString);".
> > 
> > You'll need to be able to potentially override four pieces of information:
> > 
> > 1. the vendor PCI id
> > 2. the device PCI id
> > 3. the GL_VENDOR string
> > 4. the GL_RENDERER string
> > 
> > I would suggest you add four command line flags to
> > src/gpu/config/gpu_switches.cc. Perhaps something like
> > 
> > --gpu-testing-vendor-id (kGpuTestingVendorId)
> > --gpu-testing-device-id (kGpuTestingDeviceId)
> > --gpu-testing-gl-vendor (kGpuTestingGLVendor)
> > --gpu-testing-gl-renderer (kGpuTestingGLRenderer)
> > 
> > These should override the values from the GpuInfoCollector if they're
present.
> > 
> Done, I also added kGpuTestingGLVersion because it is used to set
> gpu-driver-vendor and gpu-driver-version
> 
> Also I think the issue is not as big as I thought. Indeed the gpu process is
> started even if card is black listed (because gl_string_manager is initialized
> too late), but the gpu process will not be used because when creating the gpu
> channel there is gpu_message_filter.cc::CanUseGpuBrowserCompositor(). Indeed
the
> gpu info are up to date by this time.

Actually we just removed this because it was causing problems with SwiftShader.
> 
> But I think there are other issues, I remember in gpu main it checks for one
gpu
> switch and it is empty. I need to check.

Would it make more sense to initialize GLStringManager before
GpuDataManagerImplPrivate, have GpuDataManagerImplPrivate::SetGLStrings set some
instance variables, then have  GpuDataManagerImplPrivate::Initialize copy from
the variables into the GpuInfo?

Or alternatively have a GLStringProvider interface (with a GetGLStrings method)
in content/public and have GLStringManager implement that. Then add a
GpuDataManager::SetGLStringProvider would set a static variable.

Either way we would reduce the amount of time that the GpuDataManager is
incompletely initialized.

[Ken Russell (switch to Gerrit), 2016-01-13 00:27:30]

https://codereview.chromium.org/1547793004/diff/80001/content/test/gpu/gpu_te...
File content/test/gpu/gpu_tests/gpu_process.py (right):

https://codereview.chromium.org/1547793004/diff/80001/content/test/gpu/gpu_te...
content/test/gpu/gpu_tests/gpu_process.py:56: class
NoGpuProcessValidator(gpu_test_base.ValidatorBase):
You'll need to factor this differently. It has to run as part of the gpu_process
test, because otherwise we have to change the tests that our bots run. Look at
.../gpu_tests/pixel.py, .../page_sets/pixel_tests.py, and its use of
allow_mixed_story_states and multiple SharedPageStates in order to be able to
run pages with different browser command line arguments.

Please run the test locally and make sure it runs as expected:

src/content/test/gpu/run_gpu_test.py gpu_process --browser=debug
(or release, whichever you built locally)

and also submit a CQ dry run. After that's done and the test is working, please
resubmit for review.

[Julien Isorce Samsung, 2016-01-13 17:48:01]

On 2016/01/12 23:56:22, jbauman wrote:
> > Also I think the issue is not as big as I thought. Indeed the gpu process is
> > started even if card is black listed (because gl_string_manager is
initialized
> > too late), but the gpu process will not be used because when creating the
gpu
> > channel there is gpu_message_filter.cc::CanUseGpuBrowserCompositor(). Indeed
> the
> > gpu info are up to date by this time.
> 
> Actually we just removed this because it was causing problems with
SwiftShader.

Ah great, thx for pointing that.

> > 
> > But I think there are other issues, I remember in gpu main it checks for one
> gpu
> > switch and it is empty. I need to check.
> 
> Would it make more sense to initialize GLStringManager before
> GpuDataManagerImplPrivate, have GpuDataManagerImplPrivate::SetGLStrings set
some
> instance variables, then have  GpuDataManagerImplPrivate::Initialize copy from
> the variables into the GpuInfo?
> 
> Or alternatively have a GLStringProvider interface (with a GetGLStrings
method)
> in content/public and have GLStringManager implement that. Then add a
> GpuDataManager::SetGLStringProvider would set a static variable.
> 
> Either way we would reduce the amount of time that the GpuDataManager is
> incompletely initialized.

I have no tried your suggestion yet.

On 2016/01/13 00:27:30, Ken Russell wrote:
>
https://codereview.chromium.org/1547793004/diff/80001/content/test/gpu/gpu_te...
> content/test/gpu/gpu_tests/gpu_process.py:56: class
> NoGpuProcessValidator(gpu_test_base.ValidatorBase):
> You'll need to factor this differently. It has to run as part of the
gpu_process
> test, because otherwise we have to change the tests that our bots run. Look at
> .../gpu_tests/pixel.py, .../page_sets/pixel_tests.py, and its use of
> allow_mixed_story_states and multiple SharedPageStates in order to be able to
> run pages with different browser command line arguments.
> 
> Please run the test locally and make sure it runs as expected:
> 
> src/content/test/gpu/run_gpu_test.py gpu_process --browser=debug
> (or release, whichever you built locally)
> 
> and also submit a CQ dry run. After that's done and the test is working,
please
> resubmit for review.

Done.

But currently I think the test will still fail, what I need is really to find a
way to check if the gpu process has been created. So checking the gpu channel is
not really what we want. Indeed I noticed that If I put about:blank as url (same
with chrome://gpu), it does not try to create the gpu channel between render
process and gpu process. Maybe I am missing something.

[Julien Isorce Samsung, 2016-01-13 17:48:21]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-13 17:48:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/100001

[commit-bot: I haz the power, 2016-01-13 17:48:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-13 17:48:44]

Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are
accepted.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Zhenyao Mo, 2016-01-13 18:17:51]

On 2016/01/12 23:56:22, jbauman wrote:
> On 2016/01/12 17:57:41, j.isorce wrote:
> > On 2016/01/11 23:50:16, Ken Russell wrote:
> > > On 2016/01/08 18:22:06, j.isorce wrote:
> > > > On 2016/01/07 00:54:26, Ken Russell wrote:
> > > > > Looking at gpu_process.py I don't think it will cover this case
> > > sufficiently.
> > > > It
> > > > > looks to me like you need to be able to spoof the vendor and device
PCI
> > IDs,
> > > > and
> > > > > GL renderer and vendor strings, for testing purposes. Then you'd need
to
> > > > launch
> > > > > Chrome and assert that a GPU process was or was not started for a
> > particular
> > > > > case.
> > > > > 
> > > > > If you add the ability to spoof the GPU info, and after doing so
> > > > gpu_process.py
> > > > > would be the best place to put the new test, please go ahead.
> > > > 
> > > > I see 3 potential solutions:
> > > > 
> > > > 1: Add a new "switches::kGLVendorString" and pass it to the cmd line
> through
> > > in
> > > > gpu_process.py.
> > > > Then in "GLStringManager::Initialize()" we could parse this switch if
> exist
> > > > instead of getting from
"local_state->GetString(prefs::kGLVendorString);".
> > > 
> > > You'll need to be able to potentially override four pieces of information:
> > > 
> > > 1. the vendor PCI id
> > > 2. the device PCI id
> > > 3. the GL_VENDOR string
> > > 4. the GL_RENDERER string
> > > 
> > > I would suggest you add four command line flags to
> > > src/gpu/config/gpu_switches.cc. Perhaps something like
> > > 
> > > --gpu-testing-vendor-id (kGpuTestingVendorId)
> > > --gpu-testing-device-id (kGpuTestingDeviceId)
> > > --gpu-testing-gl-vendor (kGpuTestingGLVendor)
> > > --gpu-testing-gl-renderer (kGpuTestingGLRenderer)
> > > 
> > > These should override the values from the GpuInfoCollector if they're
> present.
> > > 
> > Done, I also added kGpuTestingGLVersion because it is used to set
> > gpu-driver-vendor and gpu-driver-version
> > 
> > Also I think the issue is not as big as I thought. Indeed the gpu process is
> > started even if card is black listed (because gl_string_manager is
initialized
> > too late), but the gpu process will not be used because when creating the
gpu
> > channel there is gpu_message_filter.cc::CanUseGpuBrowserCompositor(). Indeed
> the
> > gpu info are up to date by this time.
> 
> Actually we just removed this because it was causing problems with
SwiftShader.
> > 
> > But I think there are other issues, I remember in gpu main it checks for one
> gpu
> > switch and it is empty. I need to check.
> 
> Would it make more sense to initialize GLStringManager before
> GpuDataManagerImplPrivate, have GpuDataManagerImplPrivate::SetGLStrings set
some
> instance variables, then have  GpuDataManagerImplPrivate::Initialize copy from
> the variables into the GpuInfo?
> 
> Or alternatively have a GLStringProvider interface (with a GetGLStrings
method)
> in content/public and have GLStringManager implement that. Then add a
> GpuDataManager::SetGLStringProvider would set a static variable.
> 
> Either way we would reduce the amount of time that the GpuDataManager is
> incompletely initialized.

The GL strings are read from user data, which is initialized pretty late in the
startup time. (At least that's the case when I implemented these). 
GpuDataManager wants to initialize as early as possible.

[Ken Russell (switch to Gerrit), 2016-01-13 19:37:03]

> On 2016/01/13 00:27:30, Ken Russell wrote:
> >
>
https://codereview.chromium.org/1547793004/diff/80001/content/test/gpu/gpu_te...
> > content/test/gpu/gpu_tests/gpu_process.py:56: class
> > NoGpuProcessValidator(gpu_test_base.ValidatorBase):
> > You'll need to factor this differently. It has to run as part of the
> gpu_process
> > test, because otherwise we have to change the tests that our bots run. Look
at
> > .../gpu_tests/pixel.py, .../page_sets/pixel_tests.py, and its use of
> > allow_mixed_story_states and multiple SharedPageStates in order to be able
to
> > run pages with different browser command line arguments.
> > 
> > Please run the test locally and make sure it runs as expected:
> > 
> > src/content/test/gpu/run_gpu_test.py gpu_process --browser=debug
> > (or release, whichever you built locally)
> > 
> > and also submit a CQ dry run. After that's done and the test is working,
> please
> > resubmit for review.
> 
> Done.
> 
> But currently I think the test will still fail, what I need is really to find
a
> way to check if the gpu process has been created. So checking the gpu channel
is
> not really what we want. Indeed I noticed that If I put about:blank as url
(same
> with chrome://gpu), it does not try to create the gpu channel between render
> process and gpu process. Maybe I am missing something.

Please feel free to improve GpuBenchmarking::HasGpuProcess. Perhaps you need to
send an IPC to the browser process to ask whether it has launched a GPU process.

[Julien Isorce Samsung, 2016-01-14 23:54:02]

Description was changed from

==========
Make gpu black list work again
    
Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
 - Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
 - Added cmd line switches gpu-testing-vendor-id, gpu-testing-device-id,
   gpu-testing-gl-vendor, gpu-testing-gl-renderer
 - Added no_gpu_process unit test

BUG=571895

R=jam@chromium.org, kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py no_gpu_process
==========

to

==========
Make gpu black list work again

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added cmd line switches gpu-testing-vendor-id, gpu-testing-device-id,
gpu-testing-gl-vendor, gpu-testing-gl-renderer, gpu-testing-gl-version
- Added GpuHostMsg_IsGpuProcessInitialized IPC to improve
GpuBenchmarking::HasGpuProcess
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py

BUG=571895

R=jam@chromium.org, kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

[Julien Isorce Samsung, 2016-01-15 00:01:11]

On 2016/01/13 19:37:03, Ken Russell wrote:
> Please feel free to improve GpuBenchmarking::HasGpuProcess. Perhaps you need
to
> send an IPC to the browser process to ask whether it has launched a GPU
process.

Done I added GpuHostMsg_IsGpuProcessInitialized. I have tested this IPC from C++
but not from gpu_process.py test since for some reasons telemetry get stuck for
all test (before this CL) at the moment.
But I have good feeling the test will work now in the sense it succeeds with
this CL and would have failed if the bug was still present.
I cannot hit the try bot since it requires for me to have at least one L-G-T-M.
So please submit a CQ dry run for me. Thx.

[Ken Russell (switch to Gerrit), 2016-01-15 00:38:18]

The CQ bit was checked by kbr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-15 00:39:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/120001

[commit-bot: I haz the power, 2016-01-15 00:52:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-15 00:52:18]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Julien Isorce Samsung, 2016-01-15 13:13:08]

Description was changed from

==========
Make gpu black list work again

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally setup the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added cmd line switches gpu-testing-vendor-id, gpu-testing-device-id,
gpu-testing-gl-vendor, gpu-testing-gl-renderer, gpu-testing-gl-version
- Added GpuHostMsg_IsGpuProcessInitialized IPC to improve
GpuBenchmarking::HasGpuProcess
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py

BUG=571895

R=jam@chromium.org, kbr@chromium.org, piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

to

==========
Make gpu black list work again

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added command line switches "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.

BUG=571895

R=nferno@chromium.org, jam@chromium.org, kbr@chromium.org, nasko@chromium.org,
piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

[Julien Isorce Samsung, 2016-01-15 13:17:37]

I addressed remaining issues.
I tested locally and I confirm the new test 'GpuProcess.no_gpu_process' works as
expected. With this CL it succeeds, and it fails if you remove the fix from this
CL. So the test would have caught the regression.
Please submit another CQ dry run for me. Thx

[sadrul, 2016-01-15 17:37:01]

sadrul@chromium.org changed reviewers:
- sadrul@chromium.org

[sadrul, 2016-01-15 17:37:01]

Removing myself from reviewer list (there isn't anything here I own that isn't
already covered by multiple owners in the reviewer list)

[Zhenyao Mo, 2016-01-15 22:27:10]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-15 22:28:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/140001

[commit-bot: I haz the power, 2016-01-15 22:51:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-15 22:52:00]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

[Julien Isorce Samsung, 2016-01-18 18:12:39]

Description was changed from

==========
Make gpu black list work again

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added command line switches "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.

BUG=571895

R=nferno@chromium.org, jam@chromium.org, kbr@chromium.org, nasko@chromium.org,
piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

to

==========
Make gpu black list work again

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added command line switches "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.

BUG=571895

R=inferno@chromium.org, jam@chromium.org, kbr@chromium.org, nasko@chromium.org,
piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

[Julien Isorce Samsung, 2016-01-18 18:12:39]

j.isorce@samsung.com changed reviewers:
+ inferno@chromium.org

[Julien Isorce Samsung, 2016-01-18 18:16:04]

On 2016/01/15 22:52:00, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
>
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)

I fixed android bits. The mac and win bots seem to fail the new unit test (I
need to check whether this is due to missing testing switches or because there
is another bug). Please submit another CQ dry run for me. Thx

[Zhenyao Mo, 2016-01-19 19:27:25]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-19 19:27:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/160001

[commit-bot: I haz the power, 2016-01-19 20:20:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-19 20:20:59]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Julien Isorce Samsung, 2016-01-19 23:00:51]

On 2016/01/19 20:20:59, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

* linux error: "Failed steps failed nacl_integration (with patch) failed
nacl_integration (without patch) failed nacl_integration"
-> It seems to fail with and without this CL.

* mac / win:
-> Failure: GPU process detected
-> [  FAILED  ] GpuProcess.no_gpu_process (739 ms)
But indeed the log shows the testing switches (ex: --gpu-testing-gl-renderer)
have been ignored:
  GPU device 0: VENDOR = 0x8086 (Intel), DEVICE = 0xa2e
  gl_renderer         : Intel Iris OpenGL Engine
  gl_vendor           : Intel Inc.
  gl_version          : 2.1 INTEL-10.6.33

If the test succeeds I can read in the log: (ex:
linux_chromium_rel_ng/builds/168038)
  GPU device 0: VENDOR = 0x10de (Nvidia), DEVICE = 0xde1
  gl_renderer         : softpipe
  gl_vendor           : VMware
  gl_version          : "2.1 Mesa 10.1"

Which makes senses since I only uses these testing switches in
"GLStringManager::Initialize() -> #if defined(OS_LINUX)" and
"gpu_info_collector_linux.cc" :)

I'll update the CL to use these testing switches on non-linux platforms.

[Julien Isorce Samsung, 2016-01-20 01:19:31]

On 2016/01/19 23:00:51, j.isorce wrote:
> I'll update the CL to use these testing switches on non-linux platforms.

Done. Please submit another CQ dry run for me. Thx

[Julien Isorce Samsung, 2016-01-20 01:22:44]

On 2016/01/13 19:37:03, Ken Russell wrote:
> Please feel free to improve GpuBenchmarking::HasGpuProcess. Perhaps you need
to
> send an IPC to the browser process to ask whether it has launched a GPU
process.

What do you think of the IPC I added ? See "Also" in the updated CL description.

[aarya, 2016-01-20 01:35:37]

aarya@google.com changed reviewers:
+ tsepez@chromium.org
- inferno@chromium.org

[aarya, 2016-01-20 01:35:52]

On 2016/01/20 01:22:44, j.isorce wrote:
> On 2016/01/13 19:37:03, Ken Russell wrote:
> > Please feel free to improve GpuBenchmarking::HasGpuProcess. Perhaps you need
> to
> > send an IPC to the browser process to ask whether it has launched a GPU
> process.
> 
> What do you think of the IPC I added ? See "Also" in the updated CL
description.

Adding Tom for ipc review.

[Julien Isorce Samsung, 2016-01-20 10:22:09]

On 2016/01/20 01:35:52, aarya wrote:
> On 2016/01/20 01:22:44, j.isorce wrote:
> > On 2016/01/13 19:37:03, Ken Russell wrote:
> > > Please feel free to improve GpuBenchmarking::HasGpuProcess. Perhaps you
need
> > to
> > > send an IPC to the browser process to ask whether it has launched a GPU
> > process.
> > 
> > What do you think of the IPC I added ? See "Also" in the updated CL
> description.
> 
> Adding Tom for ipc review.

I re-upload the CL just to rebase against latest master and to check the unit
test still works as expected, and it does. Please submit another CQ dry run for
me. Thx

[Zhenyao Mo, 2016-01-20 18:50:14]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-20 18:51:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/200001

[commit-bot: I haz the power, 2016-01-20 19:28:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-20 19:28:48]

Dry run: Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Tom Sepez, 2016-01-20 19:52:47]

Messages LGTM

[Julien Isorce Samsung, 2016-01-20 23:09:08]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-20 23:09:37]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/220001

[commit-bot: I haz the power, 2016-01-20 23:41:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-20 23:41:48]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Julien Isorce Samsung, 2016-01-21 00:17:37]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-21 00:20:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/240001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/240001

[commit-bot: I haz the power, 2016-01-21 00:47:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-21 00:47:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Julien Isorce Samsung, 2016-01-21 11:25:04]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-21 11:25:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/260001

[commit-bot: I haz the power, 2016-01-21 11:34:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-21 11:34:33]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Julien Isorce Samsung, 2016-01-21 17:32:08]

On 2016/01/21 11:34:33, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

So for mac and win, I going through this CL a bit blindly :)

* win_chromium_rel_ng:
-----
[ RUN      ] GpuProcess.no_gpu_process
ERROR:browser_gpu_channel_host_factory.cc(145)] Failed to create channel.
GPU device 0: VENDOR = 0x5333, DEVICE = 0x8811
Failure: GPU process detected
[  FAILED  ] GpuProcess.no_gpu_process
-----

So it means that
"GpuDataManagerImpl::GetInstance()->CanUseGpuBrowserCompositor()" line 1217 from
browser_main_loop.cc returned true because this is the only place on windows
where BrowserGpuChannelHostFactory is used.
Looking at GpuDataManagerImplPrivate::CanUseGpuBrowserCompositor(), it cannot be
ShouldUseWarp() because I do not see --use-gl=angle (Though I can see
gl_renderer: ANGLE).
So it means that for some reasons "id": 34 from software_rendering_list_json.cc
is not loaded in GpuDataManagerImplPrivate::blacklisted_features_.
Which means GpuDataManagerImplPrivate::SetGLStrings got early return. Which
makes sense since I do not set gl_vendor, renderer and version on win. I'll
address that.

* mac_chromium_rel_ng:
gl_vendor, renderer, version are not those I have put in the .py test so I guess
it went to the early return line 480 in GpuDataManagerImplPrivate::SetGLStrings.
"If GPUInfo already got GL strings, do nothing.  This is for the rare situation
where GPU process collected GL strings before this call". Though I would expect
the gpu process to have not started before so I wonder where it gets those from.

[Julien Isorce Samsung, 2016-01-21 17:34:46]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-21 17:34:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/280001

[commit-bot: I haz the power, 2016-01-21 17:43:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-21 17:43:40]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Julien Isorce Samsung, 2016-01-22 00:43:31]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-22 00:43:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/300001

[Julien Isorce Samsung, 2016-01-22 00:48:54]

Description was changed from

==========
Make gpu black list work again

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added command line switches "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.

BUG=571895

R=inferno@chromium.org, jam@chromium.org, kbr@chromium.org, nasko@chromium.org,
piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

to

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.
(These gl infos can also be used when using chromecast and if not android,
see TODO in this CL around CastBrowserMainParts::PreMainMessageLoopRun)

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added cmd line switches "gpu-testing-os-version", "gpu-testing-vendor-id",
"gpu-testing-device-id", "gpu-testing-gl-vendor", "gpu-testing-gl-renderer",
"gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.

BUG=571895

R=inferno@chromium.org, jam@chromium.org, kbr@chromium.org, nasko@chromium.org,
piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

[commit-bot: I haz the power, 2016-01-22 01:40:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-22 01:40:57]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julien Isorce Samsung, 2016-01-24 20:51:31]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-24 20:51:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/320001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/320001

[commit-bot: I haz the power, 2016-01-24 22:13:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-24 22:13:23]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julien Isorce Samsung, 2016-01-24 23:48:15]

So I built chromium on Win and the test succeeds:

---
C:\workspace\chromium\src>python content\test\gpu\run_gpu_test.py gpu_process
--browser=release --show-stdout
(WARNING) 2016-01-25 01:32:55,239
desktop_browser_finder.FindAllAvailableBrowsers:147  Chrome build location is
not specified. Browser will be run without Flash.
C:\workspace\chromium\src\tools\telemetry\telemetry\internal\results\results_options.py:144:
UserWarning: Class BuildbotOutputFormatter is deprecated. It will no long
16. Please remove it or switch to an alternative before that time. Chart JSON is
a supported alternative. See https://goo.gl/8daFav .

  sys.stdout, trace_tag=options.output_trace_tag))
(WARNING) 2016-01-25 01:32:55,239
desktop_browser_finder.FindAllAvailableBrowsers:147  Chrome build location is
not specified. Browser will be run without Flash.
[ RUN      ] GpuProcess.canvas2d
[       OK ] GpuProcess.canvas2d (1397 ms)
[ RUN      ] GpuProcess.css3d
[       OK ] GpuProcess.css3d (1122 ms)
[ RUN      ] GpuProcess.webgl
[       OK ] GpuProcess.webgl (1176 ms)
[ RUN      ] GpuProcess.video
[       OK ] GpuProcess.video (1137 ms)
[ RUN      ] GpuProcess.gpu_info_complete
[       OK ] GpuProcess.gpu_info_complete (1015 ms)
(WARNING) 2016-01-25 01:33:01,362
desktop_browser_finder.FindAllAvailableBrowsers:147  Chrome build location is
not specified. Browser will be run without Flash.
[ RUN      ] GpuProcess.no_gpu_process
[       OK ] GpuProcess.no_gpu_process (698 ms)
(WARNING) 2016-01-25 01:33:02,220
desktop_browser_finder.FindAllAvailableBrowsers:147  Chrome build location is
not specified. Browser will be run without Flash.
[ RUN      ] GpuProcess.software_gpu_process
[       OK ] GpuProcess.software_gpu_process (734 ms)
[  PASSED  ] 7 tests.

Pages:
[GpuProcess.canvas2d,GpuProcess.video,GpuProcess.css3d,GpuProcess.webgl,GpuProcess.gpu_info_complete,GpuProcess.no_gpu_process,GpuProcess.software_gpu_process]
RESULT telemetry_page_measurement_results: num_failed= 0 count
RESULT telemetry_page_measurement_results: num_errored= 0 count

View result at file://C:\workspace\chromium\src\content\test\gpu\results.html
---

I tried with default GYP options and then with those used by the try bot: python
build\gyp_chromium.py -D component=shared_library -D archive_gpu_tests=1 -D
chromium_win_pch=0 -D dcheck_always_on=1 -D fastbuild=1 -D
ffmpeg_branding=Chrome -D proprietary_codecs=1 -D target_arch=ia32 -D
test_isolation_mode=prepare
And it is a Win 7 SP1 64-bit, similar to the try bot win_chromium_rel_ng.

So I do not understand why it fails on win_chromium_rel_ng whereas it succeeds
on a local Win I just setup. So I suspect the test environment to be different.

On this page https://www.chromium.org/developers/testing/gpu-testing I read "All
of the GPU tests running on the bots can be run locally from a Chromium build.
However, figuring out the exact command line to use can be a little tricky. The
Release GPU bots all run their tests via isolates" but then I do not follow how
to reproduce this test locally. Also it seems that for later attempt I could
just use: "git cl try -b win_chromium_rel_ng"

Any idea ?

[Zhenyao Mo, 2016-01-25 17:46:28]

On 2016/01/24 23:48:15, j.isorce wrote:
> So I built chromium on Win and the test succeeds:
> 
> ---
> C:\workspace\chromium\src>python content\test\gpu\run_gpu_test.py gpu_process
> --browser=release --show-stdout
> (WARNING) 2016-01-25 01:32:55,239
> desktop_browser_finder.FindAllAvailableBrowsers:147  Chrome build location is
> not specified. Browser will be run without Flash.
>
C:\workspace\chromium\src\tools\telemetry\telemetry\internal\results\results_options.py:144:
> UserWarning: Class BuildbotOutputFormatter is deprecated. It will no long
> 16. Please remove it or switch to an alternative before that time. Chart JSON
is
> a supported alternative. See https://goo.gl/8daFav .
> 
>   sys.stdout, trace_tag=options.output_trace_tag))
> (WARNING) 2016-01-25 01:32:55,239
> desktop_browser_finder.FindAllAvailableBrowsers:147  Chrome build location is
> not specified. Browser will be run without Flash.
> [ RUN      ] GpuProcess.canvas2d
> [       OK ] GpuProcess.canvas2d (1397 ms)
> [ RUN      ] GpuProcess.css3d
> [       OK ] GpuProcess.css3d (1122 ms)
> [ RUN      ] GpuProcess.webgl
> [       OK ] GpuProcess.webgl (1176 ms)
> [ RUN      ] GpuProcess.video
> [       OK ] GpuProcess.video (1137 ms)
> [ RUN      ] GpuProcess.gpu_info_complete
> [       OK ] GpuProcess.gpu_info_complete (1015 ms)
> (WARNING) 2016-01-25 01:33:01,362
> desktop_browser_finder.FindAllAvailableBrowsers:147  Chrome build location is
> not specified. Browser will be run without Flash.
> [ RUN      ] GpuProcess.no_gpu_process
> [       OK ] GpuProcess.no_gpu_process (698 ms)
> (WARNING) 2016-01-25 01:33:02,220
> desktop_browser_finder.FindAllAvailableBrowsers:147  Chrome build location is
> not specified. Browser will be run without Flash.
> [ RUN      ] GpuProcess.software_gpu_process
> [       OK ] GpuProcess.software_gpu_process (734 ms)
> [  PASSED  ] 7 tests.
> 
> Pages:
>
[GpuProcess.canvas2d,GpuProcess.video,GpuProcess.css3d,GpuProcess.webgl,GpuProcess.gpu_info_complete,GpuProcess.no_gpu_process,GpuProcess.software_gpu_process]
> RESULT telemetry_page_measurement_results: num_failed= 0 count
> RESULT telemetry_page_measurement_results: num_errored= 0 count
> 
> View result at file://C:\workspace\chromium\src\content\test\gpu\results.html
> ---
> 
> I tried with default GYP options and then with those used by the try bot:
python
> build\gyp_chromium.py -D component=shared_library -D archive_gpu_tests=1 -D
> chromium_win_pch=0 -D dcheck_always_on=1 -D fastbuild=1 -D
> ffmpeg_branding=Chrome -D proprietary_codecs=1 -D target_arch=ia32 -D
> test_isolation_mode=prepare
> And it is a Win 7 SP1 64-bit, similar to the try bot win_chromium_rel_ng.
> 
> So I do not understand why it fails on win_chromium_rel_ng whereas it succeeds
> on a local Win I just setup. So I suspect the test environment to be
different.
> 
> On this page https://www.chromium.org/developers/testing/gpu-testing I read
"All
> of the GPU tests running on the bots can be run locally from a Chromium build.
> However, figuring out the exact command line to use can be a little tricky.
The
> Release GPU bots all run their tests via isolates" but then I do not follow
how
> to reproduce this test locally. Also it seems that for later attempt I could
> just use: "git cl try -b win_chromium_rel_ng"
> 
> Any idea ?

It's more likely a racing situation, not a build difference.

[Julien Isorce Samsung, 2016-01-26 00:39:01]

The CQ bit was checked by j.isorce@samsung.com

[Julien Isorce Samsung, 2016-01-26 00:39:01]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/1547793004/#ps340001
(title: "Add a temporary trace around GpuDataManagerImplPrivate")

[commit-bot: I haz the power, 2016-01-26 00:40:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/340001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/340001

[Julien Isorce Samsung, 2016-01-26 00:41:52]

The CQ bit was unchecked by j.isorce@samsung.com

[Julien Isorce Samsung, 2016-01-26 00:42:00]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-26 00:42:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/340001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/340001

[Julien Isorce Samsung, 2016-01-26 00:51:30]

On 2016/01/25 17:46:28, Zhenyao Mo wrote:
> It's more likely a racing situation, not a build difference.

All right so I added a trace in GpuDataManagerPrivate that should tell if its
"GpuAccessAllowed" method is called before "initialize()".
Indeed the only place where a gpu process is launched is from
GpuProcessHost::Get which always checks for
gpu_data_manager->GpuAccessAllowed();
Do you see any other scenario ?

[commit-bot: I haz the power, 2016-01-26 01:37:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-26 01:37:16]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Julien Isorce Samsung, 2016-01-26 09:09:16]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-26 09:09:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/360001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/360001

[commit-bot: I haz the power, 2016-01-26 10:17:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-26 10:17:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Zhenyao Mo, 2016-01-26 19:27:16]

On 2016/01/26 00:51:30, j.isorce wrote:
> On 2016/01/25 17:46:28, Zhenyao Mo wrote:
> > It's more likely a racing situation, not a build difference.
> 
> All right so I added a trace in GpuDataManagerPrivate that should tell if its
> "GpuAccessAllowed" method is called before "initialize()".
> Indeed the only place where a gpu process is launched is from
> GpuProcessHost::Get which always checks for
> gpu_data_manager->GpuAccessAllowed();
> Do you see any other scenario ?

No, this is the only scenario, but you should debug into there and see if
gpu_data_manager->GpuAccessAllowed() returns true instead. Dig further and find
out why.

[Julien Isorce Samsung, 2016-01-27 00:46:10]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-27 00:48:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/380001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/380001

[commit-bot: I haz the power, 2016-01-27 01:47:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-27 01:47:18]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julien Isorce Samsung, 2016-01-27 09:48:00]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-27 09:49:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/400001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/400001

[commit-bot: I haz the power, 2016-01-27 10:46:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-27 10:46:29]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julien Isorce Samsung, 2016-01-27 18:30:10]

On 2016/01/26 19:27:16, Zhenyao Mo wrote:
> On 2016/01/26 00:51:30, j.isorce wrote:
> > On 2016/01/25 17:46:28, Zhenyao Mo wrote:
> > > It's more likely a racing situation, not a build difference.
> > 
> > All right so I added a trace in GpuDataManagerPrivate that should tell if
its
> > "GpuAccessAllowed" method is called before "initialize()".
> > Indeed the only place where a gpu process is launched is from
> > GpuProcessHost::Get which always checks for
> > gpu_data_manager->GpuAccessAllowed();
> > Do you see any other scenario ?
> 
> No, this is the only scenario, but you should debug into there and see if
> gpu_data_manager->GpuAccessAllowed() returns true instead. Dig further and
find
> out why.

Youpi! It now works on the "win_chromium_rel_ng" bot.
The problem was there is 2 gpu device on that machine and no one if active from
gpu::GpuInfo point of view.
Indeed it hits this code in GpuControlList::GpuControlListEntry::Contains:

      case kMultiGpuCategoryActive:
        if (gpu_info.gpu.active || gpu_info.secondary_gpus.empty()) // A
          candidates.push_back(gpu_info.gpu);
        for (size_t ii = 0; ii < gpu_info.secondary_gpus.size(); ++ii) {
          if (gpu_info.secondary_gpus[ii].active)
            candidates.push_back(gpu_info.secondary_gpus[ii]);
        }
and candidates is empty for the "id: 34" in kSoftwareRenderingListJson because
no multi_gpu_category is specified to it uses the default initial one which is
kMultiGpuCategoryActive (see GpuControlListEntry::GpuControlListEntry() :
multi_gpu_category_(kMultiGpuCategoryActive))

See diff between patch set 21 and 20.

On "linux_chromium_rel_ng" bit it worked before because I suspect the machine
only have one gpu device. So second condition of A above is true.

On "mac_chromium_rel_ng" it still does not work, I do not know why. I'll dig
even further. But make kind of sense because in gpu_info_collector_mac.mm there
is always a call to gpu.active = true. So for mac the issue is something else.

[Julien Isorce Samsung, 2016-01-28 07:47:40]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-28 07:48:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/420001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/420001

[Julien Isorce Samsung, 2016-01-28 08:44:14]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-28 08:44:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/440001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/440001

[commit-bot: I haz the power, 2016-01-28 09:38:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-28 09:38:18]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julien Isorce Samsung, 2016-01-28 14:40:50]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-01-28 14:41:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/460001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/460001

[commit-bot: I haz the power, 2016-01-28 16:23:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-28 16:23:33]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2016-01-28 16:28:12]

On 2016/01/28 16:23:33, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

:) Please review the whole CL. Thx

[Zhenyao Mo, 2016-01-29 22:40:35]

https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_control...
File gpu/config/gpu_control_list.cc (right):

https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_control...
gpu/config/gpu_control_list.cc:1436: base::CommandLine* command_line =
base::CommandLine::ForCurrentProcess();
I disagree with this method of hacking all over the places for testing purpose,
including this and the ones in GpuInfoCollector.

Instead, you should only process these test commandline switches in
gpu_data_manager_impl_private.cc, and rewrite GPUInfo if there are testing
values, and use the testing os_version for MakeDecisions() call. That's a much
less intrusive and easier to maintain way.

https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_info_co...
File gpu/config/gpu_info_collector.cc (right):

https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_info_co...
gpu/config/gpu_info_collector.cc:277: DCHECK(device_id);
Set them to 0 by default.

[brettw, 2016-01-29 23:26:20]

brettw@chromium.org changed reviewers:
- brettw@chromium.org

[Julien Isorce Samsung, 2016-02-01 08:57:20]

On 2016/01/29 22:40:35, Zhenyao Mo wrote:
>
https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_control...
> File gpu/config/gpu_control_list.cc (right):
> 
>
https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_control...
> gpu/config/gpu_control_list.cc:1436: base::CommandLine* command_line =
> base::CommandLine::ForCurrentProcess();
> I disagree with this method of hacking all over the places for testing
purpose,
> including this and the ones in GpuInfoCollector.
> 
> Instead, you should only process these test commandline switches in
> gpu_data_manager_impl_private.cc, and rewrite GPUInfo if there are testing
> values, and use the testing os_version for MakeDecisions() call. That's a much
> less intrusive and easier to maintain way.
> 
>
https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_info_co...
> File gpu/config/gpu_info_collector.cc (right):
> 
>
https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_info_co...
> gpu/config/gpu_info_collector.cc:277: DCHECK(device_id);
> Set them to 0 by default.

I addressed the remarks. Also about the new "PreCreateThreadsEnd" I wonder if I
should create a "CreateThreads" instead. It would be called at the beginning of
BrowserMainLoop::CreateThreads (see BrowserMainLoop::CreateStartupTasks). That
would avoid to rename "PreCreateThreads" to "PreCreateThreadsBegin" everywhere
in order to minimize this CL. What do you think ?

[Julien Isorce Samsung, 2016-02-01 08:57:40]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-01 08:58:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/480001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/480001

[commit-bot: I haz the power, 2016-02-01 10:06:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-01 10:06:39]

Dry run: This issue passed the CQ dry run.

[Zhenyao Mo, 2016-02-01 20:19:37]

gpu side LGTM with some minor fixes

https://codereview.chromium.org/1547793004/diff/480001/chrome/browser/gpu/gl_...
File chrome/browser/gpu/gl_string_manager.cc (right):

https://codereview.chromium.org/1547793004/diff/480001/chrome/browser/gpu/gl_...
chrome/browser/gpu/gl_string_manager.cc:45: if
(command_line->HasSwitch(switches::kGpuTestingGLVendor))
nit: with multi-line body, you should use {}

Here and below.

https://codereview.chromium.org/1547793004/diff/480001/content/browser/gpu/gp...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/1547793004/diff/480001/content/browser/gpu/gp...
content/browser/gpu/gpu_data_manager_impl_private.cc:531: if
(command_line->HasSwitch(switches::kGpuTestingVendorId)) {
It should always be the case that we either specify both vendor/device or we
specify either.  So can you change the code to that? Also, you need to clear
gpu_info.secondary_gpus to avoid unexpected testing behavior.

[Zhenyao Mo, 2016-02-01 20:20:59]

On 2016/02/01 08:57:20, j.isorce wrote:
> On 2016/01/29 22:40:35, Zhenyao Mo wrote:
> >
>
https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_control...
> > File gpu/config/gpu_control_list.cc (right):
> > 
> >
>
https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_control...
> > gpu/config/gpu_control_list.cc:1436: base::CommandLine* command_line =
> > base::CommandLine::ForCurrentProcess();
> > I disagree with this method of hacking all over the places for testing
> purpose,
> > including this and the ones in GpuInfoCollector.
> > 
> > Instead, you should only process these test commandline switches in
> > gpu_data_manager_impl_private.cc, and rewrite GPUInfo if there are testing
> > values, and use the testing os_version for MakeDecisions() call. That's a
much
> > less intrusive and easier to maintain way.
> > 
> >
>
https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_info_co...
> > File gpu/config/gpu_info_collector.cc (right):
> > 
> >
>
https://codereview.chromium.org/1547793004/diff/460001/gpu/config/gpu_info_co...
> > gpu/config/gpu_info_collector.cc:277: DCHECK(device_id);
> > Set them to 0 by default.
> 
> I addressed the remarks. Also about the new "PreCreateThreadsEnd" I wonder if
I
> should create a "CreateThreads" instead. It would be called at the beginning
of
> BrowserMainLoop::CreateThreads (see BrowserMainLoop::CreateStartupTasks). That
> would avoid to rename "PreCreateThreads" to "PreCreateThreadsBegin" everywhere
> in order to minimize this CL. What do you think ?

I am not very familiar with its design, so I will defer to others to guide you
in this. Maybe @jam?

[Zhenyao Mo, 2016-02-01 20:25:39]

not lgtm

https://codereview.chromium.org/1547793004/diff/480001/content/browser/gpu/gp...
File content/browser/gpu/gpu_data_manager_impl_private.cc (right):

https://codereview.chromium.org/1547793004/diff/480001/content/browser/gpu/gp...
content/browser/gpu/gpu_data_manager_impl_private.cc:484:
gpu_info.context_info_state = gpu::CollectDriverInfoGL(&gpu_info);
Sorry that I didn't realize this in the first place.

This is actually very wrong. It will cause IsCompleteGpuInfoAvailable() to
return true, and therefore, it prevents us from collect full GPU info.

[Julien Isorce Samsung, 2016-02-02 00:57:02]

On 2016/02/01 20:25:39, Zhenyao Mo wrote:
> not lgtm
>
>
https://codereview.chromium.org/1547793004/diff/480001/content/browser/gpu/gp...
> content/browser/gpu/gpu_data_manager_impl_private.cc:484:
> gpu_info.context_info_state = gpu::CollectDriverInfoGL(&gpu_info);
> Sorry that I didn't realize this in the first place.
> 
> This is actually very wrong. It will cause IsCompleteGpuInfoAvailable() to
> return true, and therefore, it prevents us from collect full GPU info.

All right, then instead what about:

+  // Let IsEssentialGpuInfoAvailable returns true if all features are disabled.
+  if (blacklisted_features_.size() == gpu::NUMBER_OF_GPU_FEATURE_TYPES)
+    gpu_info.context_info_state = gpu::kCollectInfoSuccess;

at the end of GpuDataManagerImplPrivate::SetGLStrings ?

Indeed SetGLStrings is most likely to be only called on Linux. And because the
function GpuDataManagerImplPrivate::GpuAccessAllowed returns true even if
blacklisted_features_.size() == gpu::NUMBER_OF_GPU_FEATURE_TYPES on Linux, then
SystemInfoHandler::GetInfo triggers RequestCompleteGpuInfoIfNeeded which one
launch the gpu process. The only way I found to avoid this was to make
IsEssentialGpuInfoAvailable() return true.

[Zhenyao Mo, 2016-02-02 03:54:46]

On 2016/02/02 00:57:02, j.isorce wrote:
> On 2016/02/01 20:25:39, Zhenyao Mo wrote:
> > not lgtm
> >
> >
>
https://codereview.chromium.org/1547793004/diff/480001/content/browser/gpu/gp...
> > content/browser/gpu/gpu_data_manager_impl_private.cc:484:
> > gpu_info.context_info_state = gpu::CollectDriverInfoGL(&gpu_info);
> > Sorry that I didn't realize this in the first place.
> > 
> > This is actually very wrong. It will cause IsCompleteGpuInfoAvailable() to
> > return true, and therefore, it prevents us from collect full GPU info.
> 
> All right, then instead what about:
> 
> +  // Let IsEssentialGpuInfoAvailable returns true if all features are
disabled.
> +  if (blacklisted_features_.size() == gpu::NUMBER_OF_GPU_FEATURE_TYPES)
> +    gpu_info.context_info_state = gpu::kCollectInfoSuccess;
> 
> at the end of GpuDataManagerImplPrivate::SetGLStrings ?
> 
> Indeed SetGLStrings is most likely to be only called on Linux. And because the
> function GpuDataManagerImplPrivate::GpuAccessAllowed returns true even if
> blacklisted_features_.size() == gpu::NUMBER_OF_GPU_FEATURE_TYPES on Linux,
then
> SystemInfoHandler::GetInfo triggers RequestCompleteGpuInfoIfNeeded which one
> launch the gpu process. The only way I found to avoid this was to make
> IsEssentialGpuInfoAvailable() return true.

You can add a new commandline switch to prevent full gpu info collection in the
test.

It's better not to fake any state.

[Julien Isorce Samsung, 2016-02-02 11:31:00]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-02 11:31:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/500001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/500001

[commit-bot: I haz the power, 2016-02-02 11:31:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-02 11:31:23]

Dry run: A disapproval has been posted by following reviewers: zmo@chromium.org.
Please make sure to get positive review before another CQ attempt.

[Julien Isorce Samsung, 2016-02-02 11:37:40]

On 2016/02/02 11:31:23, commit-bot: I haz the power wrote:
> Dry run: A disapproval has been posted by following reviewers:
mailto:zmo@chromium.org.
> Please make sure to get positive review before another CQ attempt.

I addressed remarks and added new switch kGpuTestingNoCompleteInfoCollection
upon zmo suggestion. Also I confirm that the new test
'GpuProcess.no_gpu_process' still works as
expected. I.e., with this CL it succeeds, and it fails if you remove the fix
from this CL. So the test still would have caught the regression.
Please submit another CQ dry run for me. Thx.

[Zhenyao Mo, 2016-02-02 19:02:49]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-02 19:07:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/500001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/500001

[commit-bot: I haz the power, 2016-02-02 21:18:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-02 21:19:01]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julien Isorce Samsung, 2016-02-02 21:52:56]

On 2016/02/02 21:19:01, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

This is because from last patch set it now requires to set both testing vendor
and device id in order to be taken into account. Since I only added a testing
vendor id in the macosx case it now fails. So I just need to add a device id
like on other platforms (even if the black list id I am targeting only specify
the vendor id)

[Zhenyao Mo, 2016-02-02 22:22:34]

mac bot is still failing the test

https://codereview.chromium.org/1547793004/diff/500001/content/browser/devtoo...
File content/browser/devtools/protocol/system_info_handler.cc (right):

https://codereview.chromium.org/1547793004/diff/500001/content/browser/devtoo...
content/browser/devtools/protocol/system_info_handler.cc:157: for (int feature =
0; feature < gpu::NUMBER_OF_GPU_FEATURE_TYPES; ++feature) {
I don't think this for-loop logic is necessary.  It's too tuned to the test.
Also, you don't need to limit the commandline switch to linux.  Semantically
when it's passed it, it should be honored everywhere.

Also, you should also honor this commandline switch in
gpu_data_manager_impl_private.cc.  Even it's not making a difference for this
CL's test, it's better we make it always semantically correct, so no confusion
and surprise in the future.

[Julien Isorce Samsung, 2016-02-03 01:00:08]

On 2016/02/02 22:22:34, Zhenyao Mo wrote:
> mac bot is still failing the test
> 
>
https://codereview.chromium.org/1547793004/diff/500001/content/browser/devtoo...
> File content/browser/devtools/protocol/system_info_handler.cc (right):
> 
>
https://codereview.chromium.org/1547793004/diff/500001/content/browser/devtoo...
> content/browser/devtools/protocol/system_info_handler.cc:157: for (int feature
=
> 0; feature < gpu::NUMBER_OF_GPU_FEATURE_TYPES; ++feature) {
> I don't think this for-loop logic is necessary.  It's too tuned to the test.
> Also, you don't need to limit the commandline switch to linux.  Semantically
> when it's passed it, it should be honored everywhere.
> 
> Also, you should also honor this commandline switch in
> gpu_data_manager_impl_private.cc.  Even it's not making a difference for this
> CL's test, it's better we make it always semantically correct, so no confusion
> and surprise in the future.

I addressed your remarks and added a testing device id for OS X case in the py
test.
Please submit another CQ dry run for me. Thx.

[Zhenyao Mo, 2016-02-04 19:53:11]

The CQ bit was checked by zmo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-02-04 19:54:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/520001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/520001

[commit-bot: I haz the power, 2016-02-04 21:49:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-04 21:49:45]

Dry run: This issue passed the CQ dry run.

[Zhenyao Mo, 2016-02-04 22:37:44]

gpu side lgtm

Thanks for your persistence.

[Julien Isorce Samsung, 2016-02-04 23:10:02]

On 2016/02/04 22:37:44, Zhenyao Mo wrote:
> gpu side lgtm
> 
> Thanks for your persistence.

Thx for your advices throughout this review.

On 2016/02/01 20:20:59, Zhenyao Mo wrote:
> On 2016/02/01 08:57:20, j.isorce wrote:
> > Also about the new "PreCreateThreadsEnd" I wonder if I should create a
> > "CreateThreads" instead. It would be called at the beginning of
> > BrowserMainLoop::CreateThreads (see BrowserMainLoop::CreateStartupTasks).
That
> > would avoid to rename existing "PreCreateThreads" to "PreCreateThreadsBegin"
> > everywhere in order to minimize this CL. What do you think ?
> 
> I am not very familiar with its design, so I will defer to others to guide you
> in this. Maybe @jam?

Hi jam, I need you review. Thx.

[Julien Isorce Samsung, 2016-02-08 10:35:40]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-08 10:36:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/540001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/540001

[commit-bot: I haz the power, 2016-02-08 12:22:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-08 12:22:54]

Dry run: This issue passed the CQ dry run.

[jam, 2016-02-08 17:16:16]

> Unfortunatelly it is not possible to just move gl_string_manager->Initialize
> to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
> GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
> BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
> And according to some comments around, it has to be after.

Can you expand on this some more? i.e. what exactly is stopping moving
gl_string_manager()->Initialize()  to PreCreateThreads? can that be fixed?

[Julien Isorce Samsung, 2016-02-08 17:38:30]

On 2016/02/08 17:16:16, jam wrote:
> > Unfortunatelly it is not possible to just move gl_string_manager->Initialize
> > to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
> > GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
> > BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
> > And according to some comments around, it has to be after.
> 
> Can you expand on this some more? i.e. what exactly is stopping moving
> gl_string_manager()->Initialize()  to PreCreateThreads? can that be fixed?

Because GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads(). And
according to existing comment
https://chromium.googlesource.com/chromium/src.git/+/master/content/browser/b...
it has to remains like this.
And we should not call gl_string_manager()->Initialize() before
GpuDataManagerImpl::GetInstance()->Initialize(), (because
gl_string_manager()->Initialize() calls
GpuDataManagerImpl::GetInstance()->SetGLStrings(..) so
GpuDataManagerImpl::GetInstance()->Initialize() has to be done before. )
That's actually the bug that this CL fixes.

The solution I found is to add a new hook.

[Julien Isorce Samsung, 2016-02-08 19:45:33]

On 2016/02/08 17:38:30, j.isorce wrote:
> On 2016/02/08 17:16:16, jam wrote:
> > > Unfortunatelly it is not possible to just move
gl_string_manager->Initialize
> > > to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads.
Indeed
> > > GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
> > > BrowserMainLoop::PreCreateThreads(), i.e. after
parts_->PreCreateThreads().
> > > And according to some comments around, it has to be after.
> > 
> > Can you expand on this some more? i.e. what exactly is stopping moving
> > gl_string_manager()->Initialize()  to PreCreateThreads? can that be fixed?
> 
> Because GpuDataManagerImpl::GetInstance()->Initialize() is called at the end
of
> BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And
> according to existing comment
>
https://chromium.googlesource.com/chromium/src.git/+/master/content/browser/b...
> it has to remains like this.
> And we should not call gl_string_manager()->Initialize() before
> GpuDataManagerImpl::GetInstance()->Initialize(), (because
> gl_string_manager()->Initialize() calls
> GpuDataManagerImpl::GetInstance()->SetGLStrings(..) so
> GpuDataManagerImpl::GetInstance()->Initialize() has to be done before. )
This is correct.

> That's actually the bug that this CL fixes.
Just this last sentence is wrong.

So currently the order is correct. The problem is that
gl_string_manager()->Initialize() is called after starting the gpu process.
But if I move it to PreCreateThreads then the order is wrong.

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

> 
> The solution I found is to add a new hook.

[jam, 2016-02-08 21:11:14]

On 2016/02/08 17:38:30, j.isorce wrote:
> On 2016/02/08 17:16:16, jam wrote:
> > > Unfortunatelly it is not possible to just move
gl_string_manager->Initialize
> > > to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads.
Indeed
> > > GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
> > > BrowserMainLoop::PreCreateThreads(), i.e. after
parts_->PreCreateThreads().
> > > And according to some comments around, it has to be after.
> > 
> > Can you expand on this some more? i.e. what exactly is stopping moving
> > gl_string_manager()->Initialize()  to PreCreateThreads? can that be fixed?
> 
> Because GpuDataManagerImpl::GetInstance()->Initialize() is called at the end
of
> BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And
> according to existing comment
>
https://chromium.googlesource.com/chromium/src.git/+/master/content/browser/b...
> it has to remains like this.
> And we should not call gl_string_manager()->Initialize() before
> GpuDataManagerImpl::GetInstance()->Initialize(), (because
> gl_string_manager()->Initialize() calls
> GpuDataManagerImpl::GetInstance()->SetGLStrings(..) so
> GpuDataManagerImpl::GetInstance()->Initialize() has to be done before. )
> That's actually the bug that this CL fixes.
> 
> The solution I found is to add a new hook.

Can we change GpuDataManagerImpl::GetInstance()->SetGLStrings so that it just
caches the strings locally, and then we can call it from
ChromeBrowserMainParts::PostMainMessageLoopStart()? Then when
GpuDataManagerImpl::GetInstance()->Initialize() runs later it can check the
strings. that way we don't need to change content/public.

[Julien Isorce Samsung, 2016-02-09 11:51:42]

Description was changed from

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.
(These gl infos can also be used when using chromecast and if not android,
see TODO in this CL around CastBrowserMainParts::PreMainMessageLoopRun)

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added cmd line switches "gpu-testing-os-version", "gpu-testing-vendor-id",
"gpu-testing-device-id", "gpu-testing-gl-vendor", "gpu-testing-gl-renderer",
"gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.

BUG=571895

R=inferno@chromium.org, jam@chromium.org, kbr@chromium.org, nasko@chromium.org,
piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

to

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.
(These gl infos can also be used when using chromecast and if not android,
see TODO in this CL around CastBrowserMainParts::PreMainMessageLoopRun)

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added cmd line switches "gpu-testing-os-version", "gpu-testing-vendor-id",
"gpu-testing-device-id", "gpu-testing-gl-vendor", "gpu-testing-gl-renderer",
"gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.

BUG=571895

R=inferno@chromium.org, jam@chromium.org, kbr@chromium.org, nasko@chromium.org,
piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Julien Isorce Samsung, 2016-02-09 12:09:34]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-09 12:09:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/580001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/580001

[commit-bot: I haz the power, 2016-02-09 12:40:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-09 12:40:09]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Julien Isorce Samsung, 2016-02-09 16:21:18]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-09 16:21:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/600001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/600001

[Julien Isorce Samsung, 2016-02-09 17:49:13]

On 2016/02/08 21:11:14, jam wrote:
> On 2016/02/08 17:38:30, j.isorce wrote:
> > On 2016/02/08 17:16:16, jam wrote:
> > > > Unfortunatelly it is not possible to just move
> gl_string_manager->Initialize
> > > > to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads.
> Indeed
> > > > GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
> > > > BrowserMainLoop::PreCreateThreads(), i.e. after
> parts_->PreCreateThreads().
> > > > And according to some comments around, it has to be after.
> > > 
> > > Can you expand on this some more? i.e. what exactly is stopping moving
> > > gl_string_manager()->Initialize()  to PreCreateThreads? can that be fixed?
> > 
> > Because GpuDataManagerImpl::GetInstance()->Initialize() is called at the end
> of
> > BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
> And
> > according to existing comment
> >
>
https://chromium.googlesource.com/chromium/src.git/+/master/content/browser/b...
> > it has to remains like this.
> > And we should not call gl_string_manager()->Initialize() before
> > GpuDataManagerImpl::GetInstance()->Initialize(), (because
> > gl_string_manager()->Initialize() calls
> > GpuDataManagerImpl::GetInstance()->SetGLStrings(..) so
> > GpuDataManagerImpl::GetInstance()->Initialize() has to be done before. )
> > That's actually the bug that this CL fixes.
> > 
> > The solution I found is to add a new hook.
> 
> Can we change GpuDataManagerImpl::GetInstance()->SetGLStrings so that it just
> caches the strings locally, and then we can call it from
> ChromeBrowserMainParts::PostMainMessageLoopStart()? Then when
> GpuDataManagerImpl::GetInstance()->Initialize() runs later it can check the
> strings. that way we don't need to change content/public.

Not possible in PostMainMessageLoopStart because g_browser_process is null at
this point but it is doable in PreCreateThreads since the later is called before
the gpu process is launched.
So in the 2 latest "Patch Set" that I pushed, I applied what you suggested, i.e
allow to call SetGLStrings before Initialize. I tried to do it in a generic way.
Indeed it does not concern only SetGLStrings but DisableHardwareAcceleration
too.

[commit-bot: I haz the power, 2016-02-09 18:30:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-09 18:31:00]

Dry run: This issue passed the CQ dry run.

[jam, 2016-02-10 18:32:33]

https://codereview.chromium.org/1547793004/diff/600001/chromecast/browser/cas...
File chromecast/browser/cast_browser_main_parts.cc (right):

https://codereview.chromium.org/1547793004/diff/600001/chromecast/browser/cas...
chromecast/browser/cast_browser_main_parts.cc:347: // launching the gpu process
since these infos are used to decide whether or
why is this a todo instead of actually doing it?

https://codereview.chromium.org/1547793004/diff/600001/content/browser/render...
File content/browser/renderer_host/gpu_message_filter.cc (right):

https://codereview.chromium.org/1547793004/diff/600001/content/browser/render...
content/browser/renderer_host/gpu_message_filter.cc:105: bool hasGpuProcess =
handles.size() > 0;
nit: has_gpu_process per style guide

https://codereview.chromium.org/1547793004/diff/600001/content/renderer/rende...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/1547793004/diff/600001/content/renderer/rende...
content/renderer/render_thread_impl.cc:1918: bool hasGpuProcess = false;
no reason to add this in RenderThreadImpl just for sending an IPC. just send
this IPC in content/renderer/gpu/gpu_benchmarking_extension.cc

[Zhenyao Mo, 2016-02-10 23:53:46]

GPU side lgtm again with the variable name fixed.

https://codereview.chromium.org/1547793004/diff/600001/content/browser/gpu/gp...
File content/browser/gpu/gpu_data_manager_impl_private.h (right):

https://codereview.chromium.org/1547793004/diff/600001/content/browser/gpu/gp...
content/browser/gpu/gpu_data_manager_impl_private.h:269: bool is_initialized;
is_initialized_ ("_" in the end)

[Julien Isorce Samsung, 2016-02-11 01:04:18]

Description was changed from

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.
(These gl infos can also be used when using chromecast and if not android,
see TODO in this CL around CastBrowserMainParts::PreMainMessageLoopRun)

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not possible to just move gl_string_manager->Initialize
to ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

So the only solution was to add a new hook PreCreateThreadsEnd().

Also:
- Renamed PreCreateThreads() to PreCreateThreadsEndBegin().
- Added cmd line switches "gpu-testing-os-version", "gpu-testing-vendor-id",
"gpu-testing-device-id", "gpu-testing-gl-vendor", "gpu-testing-gl-renderer",
"gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.

BUG=571895

R=inferno@chromium.org, jam@chromium.org, kbr@chromium.org, nasko@chromium.org,
piman@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium@rog, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

[Julien Isorce Samsung, 2016-02-11 01:08:11]

Description was changed from

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium@rog, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
==========

to

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium@rog, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Julien Isorce Samsung, 2016-02-11 01:10:27]

The CQ bit was checked by j.isorce@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-02-11 01:12:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/620001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/620001

[Julien Isorce Samsung, 2016-02-11 01:22:54]

Description was changed from

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium@rog, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-02-11 02:34:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-11 02:34:45]

Dry run: This issue passed the CQ dry run.

[jam, 2016-02-11 16:49:17]

lgtm

[Julien Isorce Samsung, 2016-02-11 17:01:43]

The CQ bit was checked by j.isorce@samsung.com

[Julien Isorce Samsung, 2016-02-11 17:01:43]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
zmo@chromium.org
Link to the patchset: https://codereview.chromium.org/1547793004/#ps620001
(title: "Execute todo, send IPC directly from gpu_benchmarking_extension.cc and
fixes variable names.")

[commit-bot: I haz the power, 2016-02-11 17:02:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1547793004/620001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1547793004/620001

[commit-bot: I haz the power, 2016-02-11 20:20:18]

Description was changed from

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-02-11 20:20:20]

Committed patchset #32 (id:620001)

[commit-bot: I haz the power, 2016-02-16 22:37:33]

Description was changed from

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Make gpu black list work again on Linux

Only OS_LINUX (see src/build/build_config.h) can rely on gl_renderer,
gl_vendor and gl_version to decide if it can start the GPU Process
based on the black list kSoftwareRenderingListJson.
Indeed GLStringManager::Initialize() is only effective #if defined(OS_LINUX).
On other platforms the triplet (os version, vendor id, device id) is enough
to make this decision.

Currently when using Mesa softpipe gl driver, chrome starts the gpu process
whereas it should not because GPU_FEATURE_TYPE_GPU_COMPOSITING is black
listed by default and because kGpuDriverBugListJson does not have any exception
for this driver.

The root cause is that gl_string_manager()->Initialize() is called after
starting the gpu process. Indeed it is called from PreMainMessageLoopRun
whereas the gpu process is started from BrowserThreadsStarted. See
BrowserMainLoop::CreateStartupTasks().

It is important to call gl_string_manager()->Initialize() before
starting the gpu process because it internally configures the gpu black list
using kGpuDriverBugListJson.

Unfortunatelly it is not as simple as moving gl_string_manager->Initialize
from ChromeBrowserMainParts::PreMainMessageLoopRun to PreCreateThreads. Indeed
GpuDataManagerImpl::GetInstance()->Initialize() is called at the end of
BrowserMainLoop::PreCreateThreads(), i.e. after parts_->PreCreateThreads().
And according to some comments around, it has to be after.

A solution would have been to add a new hook PreCreateThreadsEnd(). Since
it would have required to change content/public API. And it is not worth it
just to solve this bug. So the selected solution was to allow calling
GpuDataManager::SetGLStrings before Initialize.

Also:
- Added commandd line switches "gpu-no-complete-info-collection",
"gpu-testing-os-version", "gpu-testing-vendor-id", "gpu-testing-device-id",
"gpu-testing-gl-vendor", "gpu-testing-gl-renderer", "gpu-testing-gl-version".
- Renamed GpuBenchmarking::HasGpuProcess() to GpuBenchmarking::HasGpuChannel().
Which return true if a channel exists between renderer process and gpu process.
- Re-implemented GpuBenchmarking::HasGpuProcess by sending a new sync IPC
GpuHostMsg_HasGpuProcess that replies true if a gpu process exists. Even if
the channel between browser process and gpu process is still establishing.
- Uses of the new GpuBenchmarking::HasGpuChannel in gpu_process.py instead of
previous GpuBenchmarking::HasGpuProcess.
- Added GpuProcess.no_gpu_process unit test to gpu_process_tests.py.
- Added GpuProcess.software_gpu_process unit test to gpu_process_tests.py.
- Added GpuDataManagerImplPrivateTest_SetGLStringsDefered to content_unittests.

BUG=571895

R=jam@chromium.org, kbr@chromium.org, tsepez@chromium.org, zmo@chromium.org

TEST= GALLIUM_DRIVER=softpipe LIBGL_ALWAYS_SOFTWARE=1 chrome
CHROME_DEVEL_SANDBOX=out/Release/chrome_sandbox
./content/test/gpu/run_gpu_test.py gpu_process
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/021319910e95dd56440948ccee3b951acdf7973f
Cr-Commit-Position: refs/heads/master@{#374969}
==========

[commit-bot: I haz the power, 2016-02-16 22:37:36]

Patchset 32 (id:??) landed as
https://crrev.com/021319910e95dd56440948ccee3b951acdf7973f
Cr-Commit-Position: refs/heads/master@{#374969}